Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fetch block from a peer if we don't have it #7240

Merged

Conversation

nepet
Copy link
Collaborator

@nepet nepet commented Apr 19, 2024

This PR adds a path in the bcli plugin that utilizes getblockfrompeer to try to fetch a block from a peer in case that we don't know about the block. This is something that happens when core lightning tries to fetch a block from bitcoind that has already been pruned by bitcoind.

getblockfrompeer has been introduced in v23.0.0. The call asks a given peer for a block that the local node does not know about. Even though getblockfrompeer does not make any guarantees about whether the block will be fetched (or pruned again right away), the success probabilities are really high if we just try long enough.

Here is a brief description about what happens now if core lightning asks for a block
a) get blockhash for height x
b) try to get block from bitcoind:

  • found the block: perfect! return rawblock
  • did not find the block: iterate through the peers and ask them for the block, jump back to b) after asking a peer.

resolves #7201

nepet added 6 commits April 19, 2024 15:23
This allows us to intercept getblock on a non 0 exit when the block was
not found. We fill this with something meaningull in future commits.

Signed-off-by: Peter Neuroth <[email protected]>
We move the `geblock` call into a separate function. This allows us to
call if from various places in the future.

Signed-off-by: Peter Neuroth <[email protected]>
We introduce the peers array that contains the known set of connected
peers for future reference in case that we couldn't find the block we
are looking for.

Signed-off-by: Peter Neuroth <[email protected]>
In the case that we have peers left in the peers array and that we could
not find the block yet, we ask the next peer for the desired block.

Signed-off-by: Peter Neuroth <[email protected]>
`getblockfrompeer` was introduced in v23.0.0, we want to skip this path
if the version of bitcoind used is below that.

Signed-off-by: Peter Neuroth <[email protected]>
Changelog-Changed: Plugins: bcli: Add a path that tries to fetch blocks
from a peer if we can not find them locally.

Signed-off-by: Peter Neuroth <[email protected]>
Copy link
Collaborator

@vincenzopalazzo vincenzopalazzo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGMT, we should add also some information regarding the bitcoin core version supported.

@endothermicdev endothermicdev added this to the v24.05 milestone Apr 20, 2024
Copy link
Contributor

@rustyrussell rustyrussell left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor comments only, looks good!

plugins/bcli.c Outdated Show resolved Hide resolved
plugins/bcli.c Show resolved Hide resolved
@rustyrussell
Copy link
Contributor

I would also really like some tests! This can be done using our inbuilt mocks for bitcoind; you can give a made-up blockhash, let it fail (since there are no peers), but also can fake up some peers and check it does indeed iterate through them.

@nepet
Copy link
Collaborator Author

nepet commented Apr 22, 2024

@rustyrussell I am a bit confused about the tests. There are already a few tests using a mocked bitcoind that test the peer iteration in tests/test_misc.py added by 64c3b1b. Did I miss something?

nepet added 2 commits April 22, 2024 17:49
Using `getblockfrompeer` requires a version equal or above 23.0.0

Signed-off-by: Peter Neuroth <[email protected]>
It is simpler to just iterate through the peerlist backwards.

Signed-off-by: Peter Neuroth <[email protected]>
@rustyrussell
Copy link
Contributor

@rustyrussell I am a bit confused about the tests. There are already a few tests using a mocked bitcoind that test the peer iteration in tests/test_misc.py added by 64c3b1b. Did I miss something?

Hmm, how did I miss that commit? Sorry, these look good!

@rustyrussell rustyrussell merged commit 9ae5c04 into ElementsProject:master Apr 23, 2024
34 of 35 checks passed
@bubelov bubelov mentioned this pull request May 7, 2024
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.

Download pruned blocks (like lnd)
4 participants