Skip to content

Sample pull request. Allows specification of BTCD parameters.#7

Closed
mkl- wants to merge 3 commits into
lightningnetwork:masterfrom
mkl-:master
Closed

Sample pull request. Allows specification of BTCD parameters.#7
mkl- wants to merge 3 commits into
lightningnetwork:masterfrom
mkl-:master

Conversation

@mkl-
Copy link
Copy Markdown
Contributor

@mkl- mkl- commented Feb 1, 2016

  1. Allows specification BTCD parameters. If not specified read them from btcd.conf file
  2. Allows specification network parameters

@Roasbeef
Copy link
Copy Markdown
Member

Roasbeef commented Feb 3, 2016

Thanks for the PR @mkl-!

I've been meaning to set up proper config parsing, and a separate config file for lnd but haven't yet got around to it yet :)

However, importing the entire config struct from btcd is unnecessary. The config for lnwallet only shares a handful of parameters with btcd's config.

Additionally, although lnwallet currently depends on a btcd full-node, in the near future we'll be integrating our in-progress spv library, giving users an option as to which wallet they'd like to use.

The added print statements are unnecessary as lnd will be getting a proper logging system in a future commit.

Finally, although we currently have the active Bitcoin network set to testnet, we've been conducting testing on a different testnet, testnetL which has the features necessary to proper implement the full version of Lightning. Much of the scripting functionality used in lnwallet will be rejected by any network other than testnetL.

@Roasbeef
Copy link
Copy Markdown
Member

Roasbeef commented Feb 5, 2016

But hey, how about we salvage this PR into something merge-able?

Here's how I recommend you proceed, if you'd still like to contribute 😎.

  • Remove all the unnecessary fields in the new config file you've created. Instead you should simply embed the Config struct from lnwallet into the new config file you've created for the daemon.
  • Set up the logging infrastructure for all packages. I had planned to use btclog for logging purposes accross all pacakges. btclog allows one to easily set up logging for subsystems within the larger system. The packages which don't require their own subsystem logger include: lnstate, lnwire, shachain, and elkrem. Here's an example from btcwallet setting up btclog.

You can either re-brand this PR with the above changes, or close this PR, instead creating a new PR implementing the above. It's up to you 🏂.

@mkl-
Copy link
Copy Markdown
Contributor Author

mkl- commented Feb 23, 2016

I've rewritten configuration using approach similar to btcd: command line options + configuration file.
#8

Could you please explain what is testnetL and how to use it?
It seems that certificates are hardcoded into the code so I had disabled TLS when worked with lnd.

@Roasbeef
Copy link
Copy Markdown
Member

Great! I'll close this PR so we can move along with #8.

So it turns out we've decided to abandon testnetL. Instead, we're now using segnet for testing purposes since it has the malleability fixes we need for LN, and this way we don't have to maintain our own testnet ;)

@Roasbeef Roasbeef closed this Feb 27, 2016
@anton48 anton48 mentioned this pull request Mar 20, 2018
@beussdelanight beussdelanight mentioned this pull request Apr 8, 2018
@xiaofo09 xiaofo09 mentioned this pull request Apr 15, 2018
bjarnemagnussen added a commit to LN-Zap/lnd that referenced this pull request Jun 11, 2021
ellemouton added a commit that referenced this pull request Aug 15, 2025
Check that the node ann doesnt contain more than 1 DNS addr.
This will ensure that we now start rejecting new node announcements
with multiple DNS addrs since this check is called in the gossiper
before persisting a node ann to our local graph.

It also validates the DNS fields according to BOLT #7 specs.
ellemouton added a commit that referenced this pull request Sep 3, 2025
Check that the node ann doesnt contain more than 1 DNS addr.
This will ensure that we now start rejecting new node announcements
with multiple DNS addrs since this check is called in the gossiper
before persisting a node ann to our local graph.

It also validates the DNS fields according to BOLT #7 specs.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants