chain: PrunedBlockDispatcher bugfix. #903
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
ziggie1984
force-pushed
the
prune-node-deadlock-blocksync
branch
2 times, most recently
from
f13e5df to
3441fd7
Compare
Roasbeef
reviewed
Jan 24, 2024
Roasbeef
reviewed
Jan 25, 2024
ziggie1984
force-pushed
the
prune-node-deadlock-blocksync
branch
3 times, most recently
from
2397f1f to
1ef6e6c
Compare
C-Otto
reviewed
Jan 25, 2024
C-Otto
reviewed
Jan 25, 2024
ziggie1984
force-pushed
the
prune-node-deadlock-blocksync
branch
2 times, most recently
from
842064b to
577dd4a
Compare
ziggie1984
commented
Jan 25, 2024
ziggie1984
force-pushed
the
prune-node-deadlock-blocksync
branch
from
577dd4a to
b104af5
Compare
yyforyongyu
reviewed
Jan 25, 2024
There was a problem hiding this comment.
Thanks for the PR! Left a comment about how we add the cancelChan.
For future concerns, although GetBlock may ask for the same block, we should treat each request independently. We can use something like a requestID to handle sending back the corresponding response, to make sure one request doesn't interfere with others. On top of that we can think about optimizations such as caching a previous success block response for future requests. Then ofc there's the new RPC getblockfrompeer we can use for pruned nodes.
|
Note that caching is currently done in lnd: |
|
Refined the peer selection logic, to not waste time connecting to inbound peers and as well sort by |
ziggie1984
force-pushed
the
prune-node-deadlock-blocksync
branch
from
f4361d8 to
5d82d84
Compare
Roasbeef
requested changes
Jan 25, 2024
ziggie1984
force-pushed
the
prune-node-deadlock-blocksync
branch
from
5d82d84 to
c5fe5d3
Compare
ziggie1984
force-pushed
the
prune-node-deadlock-blocksync
branch
from
c5fe5d3 to
e55c3f7
Compare
In case we fail the request via the neutriono block peer fetcher we make sure we fail all dependant `GetBlock` calls and remove this block request completely so that a new request for this same block hash can be made.
We discard inbound peers because we don't know their listen port and sort the peers by pingtime to select the fastest first.
ziggie1984
force-pushed
the
prune-node-deadlock-blocksync
branch
from
e55c3f7 to
987fbac
Compare
buck54321
pushed a commit
to buck54321/btcwallet
that referenced
this pull request
Apr 21, 2024
…locksync Plus lightninglabs/neutrino#273 Author: Elle Mouton <[email protected]> Date: Mon May 22 15:53:49 2023 +0200 chain: PrunedBlockDispatcher bugfix.
buck54321
pushed a commit
to buck54321/btcwallet
that referenced
this pull request
Apr 21, 2024
…locksync Plus lightninglabs/neutrino#273 Author: Elle Mouton <[email protected]> Date: Mon May 22 15:53:49 2023 +0200 chain: PrunedBlockDispatcher bugfix.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This addresses #lightningnetwork/lnd#8250.
(Still need to update the unittests).
So this does not address the related time issue for now which is acutally the case why the requests where failing in the first place.
I would add the new timing logic and introduce the
getblockfrompeercommand for bitcoind >24 (iirc) in a followup PR.