Conversation
🟢 PR Severity: LOW
🟢 Low (1 files)
AnalysisThis PR only modifies To override, add a |
Lrifton92
left a comment
There was a problem hiding this comment.
Reviewed at af77f47.
The tradeoff described here matches the code: with neutrino.validatechannels=false (the default, lncfg/neutrino.go:21), initNeutrinoBackend sets Routing.AssumeChannelValid = true (config_builder.go:1798), and the gossiper then skips validateFundingTransaction (discovery/gossiper.go:2857), so ChannelPoint and Capacity are never set on the edge (gossiper.go:2944-2949).
Three things.
The suggested flag does not parse. lnd builds its parsers with flags.NewParser(&cfg, flags.Default) (config.go:948, :965), and flags.Default does not include AllowBoolValues, so go-flags rejects an argument on a bool option: --neutrino.validatechannels=true fails with "bool flag neutrino.validatechannels' cannot have an argument". The command-line form is --neutrino.validatechannels; neutrino.validatechannels=trueis only valid inlnd.conf`. See inline.
"does not validate every advertised channel" reads as partial validation. With the default, no announced channel is checked against the chain. Also, the fields are not absent in RPC responses: marshalDBEdge still sets chan_point and capacity (rpcserver.go:6338, :6343), so DescribeGraph/GetChanInfo return a zero outpoint (0000...0000:0) and capacity: 0, and SubscribeChannelGraph does the same in ChannelEdgeUpdate (rpcserver.go:6937-6943). Naming the RPCs and the zero values would make this easier to recognise for someone hitting #11166.
Enabling it later may not fix existing edges. Announcements for edges already in the graph return early at gossiper.go:2730 (IsKnownEdge), before any validation, so channels learned while the option was off appear to keep the zero values. If that is right, it is worth a sentence, since the natural reaction to this paragraph is to flip the option on an existing node.
Minor, optional: in this mode closed channels are not pruned on spend but via zombie pruning, which also treats channels with both policies disabled as closed (graph/builder.go:210-222, :620-623), so the graph can lag on closures. That is part of the same tradeoff.
| By default, Neutrino does not validate every advertised channel against its | ||
| funding transaction. This makes graph sync much faster and reduces bandwidth, | ||
| but channels learned this way may not have funding outpoints or capacities | ||
| available in graph RPC responses. Set `--neutrino.validatechannels=true` when | ||
| your application requires those channels to be validated against the chain. | ||
| This makes graph sync slower because the blocks containing the channel funding | ||
| transactions must be downloaded. |
There was a problem hiding this comment.
Proposed wording:
| By default, Neutrino does not validate every advertised channel against its | |
| funding transaction. This makes graph sync much faster and reduces bandwidth, | |
| but channels learned this way may not have funding outpoints or capacities | |
| available in graph RPC responses. Set `--neutrino.validatechannels=true` when | |
| your application requires those channels to be validated against the chain. | |
| This makes graph sync slower because the blocks containing the channel funding | |
| transactions must be downloaded. | |
| By default, Neutrino does not validate announced channels against their | |
| funding transactions. This makes graph sync much faster and reduces bandwidth, | |
| but channels learned this way have no funding outpoint or capacity: graph RPCs | |
| such as `DescribeGraph`, `GetChanInfo` and `SubscribeChannelGraph` report a | |
| zero `chan_point` and a `capacity` of 0 for them. Start `lnd` with | |
| `--neutrino.validatechannels` (or set `neutrino.validatechannels=true` in | |
| `lnd.conf`) when your application requires channels to be validated against | |
| the chain. This makes graph sync slower because the blocks containing the | |
| channel funding transactions must be downloaded. |
--neutrino.validatechannels=true is rejected by the command-line parser since flags.Default does not allow values on bool options.
|
Thanks, updated the wording. The CLI example now uses the bare --neutrino.validatechannels boolean flag, while neutrino.validatechannels=true is kept for lnd.conf. I also made the observable RPC behavior explicit (chan_point/capacity are zero rather than absent) and added a note that enabling validation later doesn't retroactively revalidate edges already present in the graph. |
|
Hi @vbrekher, thanks for taking the time to put this together. As laid out in our guidelines for new contributors, PRs from first-time contributors are not prioritized for review. Rather than leave this open indefinitely, we are going to close it for now. Since #11166 already covers this, we'll handle it from that issue; if a fix is warranted a maintainer will pick it up. The way to get future PRs reviewed promptly is to build some history first: PR reviews and issue triage are where that usually starts. Once there's a track record, new code from you becomes much easier to prioritize. |
Document the default Neutrino graph-validation tradeoff and why funding outpoints/capacities may be missing (#11166).