Add simple light-client mode (RPC only) #10794

jonasschnelli · 2017-07-11T15:01:30Z

This adds a simple light client mode (RPC only, no wallet support).

With this PR, It is possible to disable auto-request-blocks by passing in -autorequestblocks=0.
In that mode, one can request out-of-band blocks by calling requestblock add ["<blockhash>", ...].
Those blocks will then be requested/downloaded and can be loaded via getblock (and they will also be passed through ZMQ).

This allows a very simple light-client mode ideally when you already have a validated peer in your trusted network.

This is also a reviewable step towards light client mode for the wallet (which will ultimately allow process separation o the wallet).

jonasschnelli · 2017-07-11T15:02:43Z

Contains overhauled parts from #9483

ryanofsky · 2017-07-11T17:03:13Z

I'd like to help out here, and I've spent literally days reviewing previous iterations of this change (#9076 #9483 #9171), but I can't figure out if this is going anywhere and if providing more review now is a good use of time.

As I've mentioned previously, I don't think the auxiliary block download class design is great, because it duplicates functionality from the networking layer in a wholly different part of the code, and gives the wallet too much control & responsibility over low level p2p details in the case where it's used to let the wallet code prioritize which blocks to download. I've tried to describe what I think would be better alternatives in my previous review comments, like #9483 (comment).

@jonasschnelli, assuming you don't want to take my previous suggestions, and do want to stick with the current design, I think it would be helpful if you could reach out to some other reviewers and get some concept acks for your current approach. It might help to put together some kind of design document, or at least a detailed comment to the top of auxiliaryblockrequest.h that lays out how the class works and what your future plans for it are in as much detail as possible, so someone can understand how it's intended to be used without having to wade through all the PRs.

jonasschnelli · 2017-07-11T18:36:31Z

@ryanofsky:
Thank you very much for the reviews in #9483. I implemented almost all of your suggesting and some of them where really great.
However, I think the current design of having a dedicated class (CauxiliaryBlockRequest) makes sense, because...

An out-of-band (auxiliary) block request in an object, you could have multiple in parallel (assume multiwallet, etc.), in future, we may want to serialise them to disk, etc. Therefore I think it should be capsulated in its own class.
I think the wallet needs control. For a light-client mode, the wallet is the one that tells the node what it needs. More or less, wallet tells node: "I need block A to E, give them to me as fast as possible", then node may call back "I have block A already ready (on disk), rest will follow soon", etc. Wallet then may say: "Okay, thats enough, you can stop at B already", etc.
Having the implementation in a designated file makes code overview simpler. It can result in having slightly more lines of code but it should worth it. Having another main.cpp for everything that is network related should be avoided now, early enough. I don't think we have a lot of duplicated code, or can you point out what would be more compact if we would implement it directly in net_processing.cpp?
Last but not least, it allows to have a patch-set the requires less rebasing. And this is an important one to me. Eventually, this will be something that "runs along" with the master branch until there has been reasonable amount of testing before it gets eventually merged.

jonasschnelli · 2017-07-12T15:47:29Z

Followed yesterdays discussion we had in #bitcoin-core-dev. Removed the CAuxiliaryBlockRequests and added a std::map blocksToDownloadFirst.
Such manually added, priority block downloads will not trigger ActivateBestChain.

This PR now also adds a new signal BlockProcessed().

The scope of this PR is not to make the block download interface flexible for multiple "users" (like the validation and eventually the light-client wallet).

jonasschnelli · 2017-07-20T18:11:11Z

fixed @ryanofsky points.
This does pass travis now. Thanks in advance for reviews...

ryanofsky

Change seems pretty straightforward. Main thing missing is a test for the new RPC call.

It would be useful to get concept ACKs from @gmaxwell and @sipa to make sure concerns from the earlier design discussion (https://botbot.me/freenode/bitcoin-core-dev/msg/88437543/) are addressed now.

I don't have a very clear idea of how this functionality is going to be used, and what future plans for it are, so I personally wouldn't mind seeing a list of what the next steps & future prs might look like, or knowing some applications that could use the new option & RPC. But probably these things are clearer to others.

jonasschnelli · 2017-07-20T19:58:12Z

Thanks @ryanofsky. Agree about required conceptual ACKs from other devs.
The long term goal was sketched in #9483, basically a light client mode for Bitcoin-Core that would allow node/wallet separation.

ryanofsky

utACK 1794a11037150b0d2c6ba2eb9f06e310f51f7c1c. Change looks great: code & test are straightforward and commit history is well thought out.

I still can't figure out if new RPC is actually supposed to be useful for something other than debugging/testing. Seems fine if it isn't, though, because it's a simple wrapper around the functions for downloading blocks of order, which obviously are useful for SPV.

ryanofsky · 2017-07-24T14:16:00Z

src/net_processing.cpp

In commit "Add priority block request queue"

Should update comment for new return value

ryanofsky · 2017-07-24T14:21:28Z

src/net_processing.cpp

In commit "Add priority block request queue"

Could be const auto

ryanofsky · 2017-07-24T14:32:37Z

src/rpc/blockchain.cpp

In commit "Add requestblocks - a simple way to priorize block downloads"

Maybe drop part of comment in parentheses, seems to imply value will be true for priority block requests.

ryanofsky · 2017-07-24T14:36:42Z

src/rpc/blockchain.cpp

In commit "[RPC] Add requestblocks - a simple way to priorize block downloads"

Copy is redundant, get_array just returns reference to *this.

ryanofsky · 2017-07-24T14:37:45Z

src/rpc/blockchain.cpp

In commit "[RPC] Add requestblocks - a simple way to priorize block downloads"

Can never happen, get_array call above would have thrown if the hash_Uarray / params[1] value was not an array

ryanofsky · 2017-07-24T14:38:28Z

src/rpc/blockchain.cpp

In commit "[RPC] Add requestblocks - a simple way to priorize block downloads"

Probably should write mi->second

jonasschnelli · 2017-07-25T08:50:17Z

Overhauled and fixed @ryanofsky points (mostly nits), also removed the new (unused) signal.

ryanofsky

utACK 9723672ced8b48eeee4847e6c0e0ed39833c7eaf. Changes since last review were dropping a commit that added a BlockProcessed signal, changing order of other commits, changing brace styles, and making various suggested tweaks.

ryanofsky · 2017-07-25T15:09:46Z

src/rpc/blockchain.cpp

In commit "[RPC] Add requestblocks - a simple way to priorize block downloads"

Don't really need this check, because the get_array call below will trigger it's own error about the value not being an array. But if you prefer this error, you should change the check to request.params[1].isNull() to avoid making a distinction between a null and a missing value (#10281, #10783).

jonasschnelli · 2017-07-26T14:04:52Z

Overhauled first commit to make sure we request blocks in order:
5927f7f#diff-eff7adeaec73a769788bb78858815c91R483

jonasschnelli · 2017-07-27T08:33:19Z

Overhauled once again. Added the processing logic to ensure blocks are processed in order (makes it much simpler to process by "the other side").
Added a new main signal for the processing (reusing BlockChecked seems wrong).
Priority requests are now pushed through signal

This is now tested on my SPV branch and could be the first reviewable step towards light clients / wallet process separation.

…e (pure light-client mode)

jonasschnelli · 2018-04-18T08:15:11Z

Rebased

Sjors · 2018-08-02T18:55:56Z

I rebased it here: https://github.com/Sjors/bitcoin/commits/2018/08/spv-rpc (I may have broken the functional test)

@ryanofsky said:

I still can't figure out if new RPC is actually supposed to be useful for something other than debugging/testing

The bigger picture for me is #9483, the ability for users to get started while IBD happens in the background.

However, a use case I have in mind for requestblocks add is ElementsProject/lightning#1297, in particular the ability for a lightning client to re-download historic blocks from a pruned node. This actually works now. Do they get pruned again at some point?

-autorequestblocks is useful when running on a phone (iOs: #11720, Android: ABCore). A native application could use the RPC and only allow fetching blocks when on wifi. Being able to toggle automatic download via RPC would be nice, but can wait.

Being able to fetch blocks by height range (or up to a certain height) instead of hashes would be nice, but that's something that can be built on top of this.

I noticed there's no log entry when you requestblocks add.

Can you explain when priorityRequest is true?

Did some light testing and couldn't find problems, but more people should try :-)

Sjors · 2018-08-02T19:01:28Z

test/functional/rpc_requestblocks.py

+        assert_equal(block['validated'], False)
+
+        #prevblock must not be available
+        assert_raises_rpc_error(-1, "Block not found on disk", self.nodes[1].getblock, block['previousblockhash'])


Maybe do a prune here and then refetch the old block?

practicalswift · 2018-09-02T18:26:42Z

src/rpc/blockchain.cpp

+        return NullUniValue;
+    }
+    else {
+        throw JSONRPCError(RPC_INVALID_PARAMETER, "Unkown action");


Typo found by codespell: Unkown

DrahtBot · 2018-12-03T16:54:20Z

There hasn't been much activity lately and the patch still needs rebase, so I am closing this for now. Please let me know when you want to continue working on this, so the pull request can be re-opened.

jonasschnelli added P2P Validation labels Jul 11, 2017

jonasschnelli mentioned this pull request Jul 11, 2017

Introduce auxiliary block requests #9171

Closed

ryanofsky mentioned this pull request Jul 11, 2017

Complete hybrid full block SPV mode #9483

Closed

bitcoin deleted a comment Jul 11, 2017

jonasschnelli force-pushed the 2017/07/spv branch 2 times, most recently from 7b8973f to b2364c6 Compare July 12, 2017 15:41

jonasschnelli force-pushed the 2017/07/spv branch 5 times, most recently from cd6f21b to 7a7be30 Compare July 19, 2017 14:27

jonasschnelli force-pushed the 2017/07/spv branch from 7a7be30 to dfc3bc8 Compare July 20, 2017 14:17

ryanofsky reviewed Jul 20, 2017

View reviewed changes

jonasschnelli force-pushed the 2017/07/spv branch from dfc3bc8 to 1794a11 Compare July 21, 2017 13:18

ryanofsky reviewed Jul 24, 2017

View reviewed changes

jonasschnelli force-pushed the 2017/07/spv branch from 1794a11 to 9723672 Compare July 25, 2017 08:49

ryanofsky reviewed Jul 25, 2017

View reviewed changes

jonasschnelli force-pushed the 2017/07/spv branch from 9723672 to 5544da6 Compare July 26, 2017 13:58

jonasschnelli force-pushed the 2017/07/spv branch from 5544da6 to c42ebeb Compare July 27, 2017 08:30

jonasschnelli force-pushed the 2017/07/spv branch from c42ebeb to 02e14b0 Compare July 27, 2017 08:36

jonasschnelli added 8 commits April 18, 2018 09:57

Add priority block request queue

9946ff7

Add new ProcessPriorityRequest main signal

f2b165f

Don't ActivateBestChain when processing priority block request

c2e6e77

Process priority requests

321b1f7

Optionally allow to skip the toFarAway check in AcceptBlock

65cf118

Add fAutoRequestBlocks to disabled/enable the verification progress

b9ebf85

[RPC] Add requestblocks - a simple way to priorize block downloads

5263167

Add -autorequestblocks which then allows to run in non-validation mod…

7045820

…e (pure light-client mode)

jonasschnelli force-pushed the 2017/07/spv branch from 47a25e2 to d18dce1 Compare April 18, 2018 08:14

jonasschnelli force-pushed the 2017/07/spv branch from d18dce1 to 0cc99c1 Compare April 18, 2018 08:17

jonasschnelli added 2 commits April 18, 2018 10:58

[QA] add requestblocks test

2326318

Pass priority requests through ZMQ notifications

77561f7

jonasschnelli force-pushed the 2017/07/spv branch from 0cc99c1 to 77561f7 Compare April 18, 2018 08:58

maflcko added the Needs rebase label Jun 6, 2018

Sjors mentioned this pull request Aug 2, 2018

Pruned bitcoind support ElementsProject/lightning#1297

Closed

3 tasks

Sjors reviewed Aug 2, 2018

View reviewed changes

This was referenced Aug 8, 2018

Getrawtransaction working partially without -txindex is confusing #3220

Closed

RFC: keepheight RPC for pruned nodes #13943

Closed

practicalswift reviewed Sep 2, 2018

View reviewed changes

DrahtBot added the Up for grabs label Dec 3, 2018

DrahtBot closed this Dec 3, 2018

oscarguindzberg mentioned this pull request Dec 12, 2018

Feasibility for replacing Bitcoinj with Bitcoin Core SPV bisq-network/bisq#1062

Closed

laanwj removed the Needs rebase label Oct 24, 2019

leshik mentioned this pull request Jun 7, 2020

Provide partial history for pruned nodes using scantxoutset bwt-dev/bwt#27

Open

mahnunchik mentioned this pull request Mar 26, 2021

Pruned node: (Re)download of pruned block #19312

Closed

maflcko mentioned this pull request Jul 29, 2021

rpc: getblockfrompeer #20295

Merged

bitcoin locked as resolved and limited conversation to collaborators Dec 16, 2021

Add simple light-client mode (RPC only) #10794

Add simple light-client mode (RPC only) #10794

Uh oh!

Conversation

jonasschnelli commented Jul 11, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jonasschnelli commented Jul 11, 2017

Uh oh!

ryanofsky commented Jul 11, 2017

Uh oh!

jonasschnelli commented Jul 11, 2017

Uh oh!

jonasschnelli commented Jul 12, 2017

Uh oh!

jonasschnelli commented Jul 20, 2017

Uh oh!

ryanofsky left a comment

Choose a reason for hiding this comment

Uh oh!

jonasschnelli commented Jul 20, 2017

Uh oh!

ryanofsky left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jonasschnelli commented Jul 25, 2017

Uh oh!

ryanofsky left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jonasschnelli commented Jul 26, 2017

Uh oh!

jonasschnelli commented Jul 27, 2017

Uh oh!

jonasschnelli commented Apr 18, 2018

Uh oh!

Sjors commented Aug 2, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

DrahtBot commented Dec 3, 2018

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

10 participants

jonasschnelli commented Jul 11, 2017 •

edited

Loading

Sjors commented Aug 2, 2018 •

edited

Loading