Add a switch to allow running in a pruned state #4481

rdponticelli · 2014-07-07T18:45:51Z

To avoid stalling the downloads of other peers, we disconnect those
who ask us for a block we don't have anymore.

petertodd · 2014-07-08T04:25:17Z

I think this is acceptable so long as you add a "-allowpruned" option that disables the "we have all blocks" check on startup and unsets the NODE_NETWORK service bit. (meaning we act like a SPV client)

You could argue with that you might as well not disconnect peers that ask for blocks you don't have, as you aren't advertising that you have any blocks at all. That might be useful for some types of local usage, and probably does no harm.

laanwj · 2014-07-08T07:18:50Z

Agree with @petertodd here. If you don't run a full node, you should not have it advertise NODE_NETWORK. The P2P protocol at this point does not support advertising availability of only ranges of blocks, and disconnecting anyone that happens to request a block that's missing seems very crude.

Also: -allowpruned should imply -disablewallet, as the wallet currently relies on being able to rescan the blocks on disk, for example on importkey or importwallet.

petertodd · 2014-07-08T08:23:52Z

@laanwj Good point. You could make pruning just disable some wallet functionality, but disabling all wallet functionality is simpler for now - we might not have the wallet in the future anyway.

rdponticelli · 2014-07-09T00:01:42Z

Thanks for the feedback. Pull updated, addressing the input.

laanwj · 2014-07-09T06:20:26Z

I like the idea of an UTXO-only, non-archival node.

Excluding the wallet, we still need to test the rest of the RPC API for robustness against missing block data. How does this interact with gettransaction (w/ possible txindex), for example?
Ideally this would be accompanied with an option to not write blocks to disk at all. I think the current expectation is that some external program does the 'pruning'?

rdponticelli · 2014-07-10T01:12:26Z

@ashleyholman: I guess we should try and test fully what happens unsetting the flag, but that looks like the sensible thing to do.

gmaxwell · 2014-07-10T13:28:40Z

It should also check that so much hasn't been removed that it can't survive realistic reorganizations... a bunch of nodes that just refuse to reorg would be fairly harmful to the network (not to mention their users!). I'd suggest the check go back 288 blocks from tip since thats our default for the extensive checkblocks.

This also needs at least a modest testplan to check that none of the exposed RPCs will crash, and that any that would crash are disabled. It should also be made incompatible with txindex.

gmaxwell · 2014-07-10T13:33:55Z

We may also want to leave this out of usage until it's had a fair amount of testing (e.g. in the first release with it)... esp I'd worry that there may be additional network triggerable misbehavior from places where this violates assumptions.

gmaxwell · 2014-07-10T16:21:10Z

So after giving this more though, instead of allowpruned— why not just go all the way and have a pruned=1 ... with the logic that if our validated height is >100k and a blockfiles highest block is <tip-288, just delete the block file? This would allow you to sync up without much free disk space (the first 119k blocks are small and fit in the first block file).

gmaxwell · 2014-07-10T20:44:04Z

(Sipa pointed out to me that the rational for the 100k limit was not obvious. The reason I proposed that was because without it if someone gave you a 400 block reorg during the very early initial block download you would not recover. The 100k limit gets the difficulty to a non-completely-trivial level, and is also few enough that its still in a single block file, so there would be no peak usage increase. Better to have a total-work check, but that would probably need headers first in order to be easy to implement).

rdponticelli · 2014-07-11T11:53:50Z

So, there are several possible modes of operation after we allow pruning. @laanwj's utxosonly, @gmaxwell's autoprune are both interesting, and there could be more...

Should I modify this patch to allow setting them as options, or would that add too much unneeded complexity?

gmaxwell · 2014-07-11T12:43:14Z

UTXO only cannot be done without substantial work because the node would be unable to reorganize.

rdponticelli · 2014-07-11T12:59:16Z

Yeah, obviously autoprune is a way lower hanging fruit.

Maybe after #4468 utxoonly can be possible, but yes, at a significantly cost.

laanwj · 2014-07-11T13:00:08Z

Right, I did not think about reorganizations. A node that would fall over on even a small reorganization would be pointless. And storing a few blocks isn't a problem.

@gmaxwell's idea about keeping only the recent N>288 blocks when height>100k sounds great, and is how auto-pruning should work.

gmaxwell · 2014-07-11T13:17:31Z

WRT UTXO-only, we'd also need to add code to hash the undo files, and add network code to fetch them (and make the undo datastructure normative along the way)... Or, just don't delete the undo files but this is unattractive since there is currently about 2gb of undo data.

So I think for now we should just do the autopruned which disables the wallet and txindex and keeps enough blocks that reorg failure is very unlikely. Then we can think about the above steps with the undo data, along with keeping the ability to remain acting as a node-network.

wtogami · 2014-07-12T09:28:11Z

Shouldn't this also disable UPNP as the only reason it is used is to allow incoming connections to possibly sync from your node?

petertodd · 2014-07-12T09:58:46Z

I wouldn't bother, as we'll want to let nodes advertise useful services beyond the blockchain, e.g. tx relaying.

On 12 July 2014 10:28:41 BST, Warren Togami notifications@github.com wrote:

Shouldn't this also disable UPNP as the only reason it is used is to
allow incoming connections to possibly sync from your node?

Reply to this email directly or view it on GitHub:
#4481 (comment)

gmaxwell · 2014-07-12T12:32:28Z

We don't want to disable listening or UPNP because you may want to use the node with p2pool or the like, but we should probably suppress address messages when we're not node-network. I think that should just wait until another pull though.

petertodd · 2014-07-12T12:36:51Z

@gmaxwell Sounds good to me.

On 12 July 2014 13:32:58 GMT+01:00, Gregory Maxwell notifications@github.com wrote:

We don't want to disable listening or UPNP because you may want to use
the node with p2pool or the like, but we should probably suppress
address messages when we're not node-network. I think that should just
wait until another pull though.

Reply to this email directly or view it on GitHub:
#4481 (comment)

jgarzik · 2014-07-14T00:06:40Z

Looks mostly OK, modulo nits.

Do we want to be able to find pruned nodes? Seems like allocating a NODE_xxx service bit in this PR is appropriate.

gmaxwell · 2014-07-14T04:16:50Z

The bit ought to depend on willingness to at least relay the most recent N transactions from the tip. N=288 by definition sounds good to me, but if it's in the bit definition perhaps it needs more shed painting, which is why I didn't recommend this above (for this pull, e.g. we could do it in a separate one).

jgarzik · 2014-07-14T04:36:08Z

@gmaxwell To forestall shed painting, I observe that NODE_NETWORK behavior has varied over time; only it's general definition "a full node!" has remained static.

As such, it seems reasonable to add NODE_PRUNED or whatever we want to call the high level attribute being advertised. If it's only minor tweaks to find a good value for N, that seems like something that could be performed in-tree before the next release.

It's all about communication. A release note "NODE_PRUNED is experimental until 0.11" can further extend the life.

laanwj · 2014-07-14T06:48:47Z

What about adding a service bit NODE_UTXO? And then having full nodes advertise it as well? This has been proposed before and I think that was a good suggestion (by @maaku?). Pruned nodes would only advertise NODE_UTXO.

This would give NODE_NETWORK - starting from version X - the meaning of 'can serve blocks', and NODE_UTXO the meaning of 'tracks UTXO and can validate transactions'.

This way, service bits actually tell about provided services. I like 'negative' service bits like NODE_PRUNED considerably less.

sipa · 2014-07-14T12:03:28Z

I would move the meaning of block-servavility out of NODE_NETWORK, not the
complement. Reason: we don't really know in what ways the remainder of the
functionality will be split off in the future, while the ability to serve
blocks is well defined. Let NODE_NETWORK remain the kitchen sink, and give
new bits a well defined meaning.

As to the meaning itself, I have previously proposed to use 2 service bits,
the combination of which indicates one of 4 levels (e.g. 0,288,4032,all or
288,2016,52500,all if we want some mandatory serving). Discussion mostly
turned into using such a solution vs. adding a field to CAddress to
indicate this information with more granularity.

gmaxwell · 2014-07-14T12:37:32Z

I was trying to avoid the granularity rathole here. :) Can we please put the new flag in a subsequent pull to avoid delaying this based on that? The approach of removing node network on a pruning node is safe if far from perfect... and will suffice until the extra behavior is hammered out.

jgarzik · 2014-07-14T12:46:56Z

I see that shed painting was not forestalled, @gmaxwell :)

ACK

rdponticelli · 2014-07-14T22:46:56Z

Most nits has been addressed, and I added a branch in my repository for anybody willing to test the rejection instead of disconnecting case:

https://github.com/Criptomonedas/bitcoin/tree/prunedreject

It seems to work pretty fine, just a little more resource wasteful than disconnecting.

sipa · 2014-07-15T17:37:14Z

utACK

maaku · 2014-07-15T17:45:42Z

ut?

jgarzik · 2014-07-15T17:57:13Z

+1

maaku · 2014-07-15T21:40:31Z

You would want to serve not just the most recent blocks, but also a
range of past blocks. Otherwise it would be very difficult to perform an
initial block download in a scenario where most nodes are pruning.
Adding a field to CAddress seems the more compatible approach with this
requirement.

On 07/14/2014 08:03 AM, Pieter Wuille wrote:

I would move the meaning of block-servavility out of NODE_NETWORK, not the
complement. Reason: we don't really know in what ways the remainder of the
functionality will be split off in the future, while the ability to serve
blocks is well defined. Let NODE_NETWORK remain the kitchen sink, and give
new bits a well defined meaning.

As to the meaning itself, I have previously proposed to use 2 service
bits,
the combination of which indicates one of 4 levels (e.g. 0,288,4032,all or
288,2016,52500,all if we want some mandatory serving). Discussion mostly
turned into using such a solution vs. adding a field to CAddress to
indicate this information with more granularity.

—
Reply to this email directly or view it on GitHub
#4481 (comment).

gmaxwell · 2014-07-15T22:37:30Z

@maaku absolutely, but I think we should do that as part of a pull that turns back on network services bits. This pull just turns the node into a client, and the only reason to keep any blocks at all is otherwise a reorg leaves the node stuck and it needs a total (30gbyte) reinit, and while it's stuck it's forked and potentially following a losing chain.

Diapolo · 2014-07-16T13:48:26Z

src/init.cpp

 #ifdef ENABLE_WALLET
+    if (fAllowPruned) {
+        if (SoftSetBoolArg("-disablewallet", true))
+            LogPrintf("AppInit2 : parameter interaction: -allowpruned=1 -> setting -disablewallet=1\n");


Those checks and the message don't belong here but in Step 2: parameter interactions in init.

sipa · 2014-08-03T09:45:17Z

src/util.cpp

@@ -97,6 +97,7 @@ string strMiscWarning;
 bool fLogTimestamps = false;
 bool fLogIPs = false;
 volatile bool fReopenDebugLog = false;
+bool fPruned = false;


This really doesn't belong in util.

I'd say main, but apparently you need it inside net, which feels wrong. Feel like moving the pnodeLocalHost code to init?

What about passing an extra parameter to StartNode to allow setting the service bits from the caller in init? Would that be a correct solution? It would seem to be simpler and less invasive, wouldn't it?

Sounds good to me.

Agree it does not belong in util, but also a parameter to StartNode does not seem appropriate. See NODE_EXT_SERVICES in the following WIP branch at https://github.com/jgarzik/bitcoin/tree/2014_ext_services

sipa · 2014-08-03T18:25:35Z

@jgarzik Please, let's not make this dependent on how to expose this functionality to the network yet, that can come later. All we need for now is something that disables full node functionality when running pruned.

EDIT: Oh, you mean just modifying nLocalServices from init? That seems fine too.

jgarzik · 2014-08-03T18:45:50Z

@sipa Correct, that's all I meant. Local service configuration will ultimately be multi-step, and can be handled directly from init.

BitcoinPullTester · 2014-08-26T20:06:57Z

Automatic sanity-testing: PASSED, see http://jenkins.bluematt.me/pull-tester/p4481_d00a934cc05301d6a08f8d686428c4fd704ed76b/ for binaries and test log.
This test script verifies pulls every time they are updated. It, however, dies sometimes and fails to test properly. If you are waiting on a test, please check timestamps to verify that the test.log is moving at http://jenkins.bluematt.me/pull-tester/current/
Contact BlueMatt on freenode if something looks broken.

BitcoinPullTester · 2014-08-27T18:42:50Z

Automatic sanity-testing: PASSED, see http://jenkins.bluematt.me/pull-tester/p4481_eae33b1af4f866257a84bcb438a1768fff07a63e/ for binaries and test log.
This test script verifies pulls every time they are updated. It, however, dies sometimes and fails to test properly. If you are waiting on a test, please check timestamps to verify that the test.log is moving at http://jenkins.bluematt.me/pull-tester/current/
Contact BlueMatt on freenode if something looks broken.

sipa · 2014-08-27T18:46:33Z

Untested ACK.

sipa · 2014-08-27T18:48:34Z

There are a few changes I'd like to make, but let's do that post-merge:

Merge the block/undo data file handling; for now, it seems they will be deleted simultaneously anyway.
Make the pruning depth configurable.
Abstract the deletion code out of the startup routine, and into something that can run regularly.

EDIT: this is actually about #4701.

rdponticelli · 2014-08-28T14:17:58Z

Added incompatibility with -txindex, as requested.

BitcoinPullTester · 2014-08-28T14:28:45Z

Automatic sanity-testing: PASSED, see http://jenkins.bluematt.me/pull-tester/p4481_9045aa715c2a043af244d14170fc11b84f986dcb/ for binaries and test log.
This test script verifies pulls every time they are updated. It, however, dies sometimes and fails to test properly. If you are waiting on a test, please check timestamps to verify that the test.log is moving at http://jenkins.bluematt.me/pull-tester/current/
Contact BlueMatt on freenode if something looks broken.

The check is now simpler, somewhat more efficient, and covers more data, to be sure the node can reorganize.

These are the main functional changes on this state: * Do not allow running with a wallet or a txindex. * Check for data at startup is mandatory up to last 288 blocks. * NODE_NETWORK flag is unset. * Requests for pruned blocks from other peers is answered with "notfound" and they are disconnected, not to stall their IBD. * The range of blocks pruned is logged.

BitcoinPullTester · 2014-09-12T23:41:23Z

Automatic sanity-testing: PASSED, see http://jenkins.bluematt.me/pull-tester/p4481_efbf886cbe560950f432b5af78f63d4a57c81b51/ for binaries and test log.
This test script verifies pulls every time they are updated. It, however, dies sometimes and fails to test properly. If you are waiting on a test, please check timestamps to verify that the test.log is moving at http://jenkins.bluematt.me/pull-tester/current/
Contact BlueMatt on freenode if something looks broken.

rdponticelli mentioned this pull request Jul 12, 2014

Check for missing undo files at startup, making the check somewhat more ... #4515

Closed

Diapolo reviewed Jul 16, 2014
View reviewed changes

laanwj added the UTXO Db and Indexes label Jul 31, 2014

sipa reviewed Aug 3, 2014
View reviewed changes

rdponticelli mentioned this pull request Aug 14, 2014

Autoprune #4701

Closed

rdponticelli force-pushed the pruned branch 2 times, most recently from feaff36 to d00a934 Compare August 26, 2014 19:52

rdponticelli changed the title ~~Allow the node to run with history pruned.~~ Add a switch to allow running in a pruned state Aug 26, 2014

rdponticelli force-pushed the pruned branch from d00a934 to eae33b1 Compare August 27, 2014 18:26

rdponticelli force-pushed the pruned branch from eae33b1 to 9045aa7 Compare August 28, 2014 14:13

rdponticelli added 2 commits September 12, 2014 20:15

Include undo data to startup checks and refactor them onto a function.

709a984

The check is now simpler, somewhat more efficient, and covers more data, to be sure the node can reorganize.

rdponticelli force-pushed the pruned branch from 9045aa7 to efbf886 Compare September 12, 2014 23:27

rdponticelli closed this Sep 18, 2014

bitcoin locked as resolved and limited conversation to collaborators Sep 8, 2021

Add a switch to allow running in a pruned state #4481

Add a switch to allow running in a pruned state #4481

Conversation

rdponticelli commented Jul 7, 2014

petertodd commented Jul 8, 2014

laanwj commented Jul 8, 2014

petertodd commented Jul 8, 2014

rdponticelli commented Jul 9, 2014

laanwj commented Jul 9, 2014

rdponticelli commented Jul 10, 2014

gmaxwell commented Jul 10, 2014

gmaxwell commented Jul 10, 2014

gmaxwell commented Jul 10, 2014

gmaxwell commented Jul 10, 2014

rdponticelli commented Jul 11, 2014

gmaxwell commented Jul 11, 2014

rdponticelli commented Jul 11, 2014

laanwj commented Jul 11, 2014

gmaxwell commented Jul 11, 2014

wtogami commented Jul 12, 2014

petertodd commented Jul 12, 2014

gmaxwell commented Jul 12, 2014

petertodd commented Jul 12, 2014

jgarzik commented Jul 14, 2014

gmaxwell commented Jul 14, 2014

jgarzik commented Jul 14, 2014

laanwj commented Jul 14, 2014

sipa commented Jul 14, 2014

gmaxwell commented Jul 14, 2014

jgarzik commented Jul 14, 2014

rdponticelli commented Jul 14, 2014

sipa commented Jul 15, 2014

maaku commented Jul 15, 2014

jgarzik commented Jul 15, 2014

maaku commented Jul 15, 2014

gmaxwell commented Jul 15, 2014

Diapolo Jul 16, 2014

Choose a reason for hiding this comment

sipa Aug 3, 2014

Choose a reason for hiding this comment

rdponticelli Aug 3, 2014

Choose a reason for hiding this comment

sipa Aug 3, 2014

Choose a reason for hiding this comment

jgarzik Aug 3, 2014

Choose a reason for hiding this comment

sipa commented Aug 3, 2014

jgarzik commented Aug 3, 2014

BitcoinPullTester commented Aug 26, 2014

BitcoinPullTester commented Aug 27, 2014

sipa commented Aug 27, 2014

sipa commented Aug 27, 2014

rdponticelli commented Aug 28, 2014

BitcoinPullTester commented Aug 28, 2014

BitcoinPullTester commented Sep 12, 2014