btcsuite / btcd

An alternative full node bitcoin implementation written in Go (golang)
https://github.com/btcsuite/btcd/blob/master/README.md
ISC License
6.11k stars 2.32k forks source link

main, wire, blockchain, indexers, ffldb: Add pruning #1971

Closed kcalvinalvin closed 10 months ago

kcalvinalvin commented 1 year ago

Adds pruning to btcd.

Change to ffldb:

1) Add PruneBlocks() to transaction interface. Prune blocks is a no-op when the total block file size is equal to or under the target size. If the total block files size is over the target size, oldest block files are deleted along with the block indexes that point to those block files. Returns the hashes of the deleted blocks.

Change to wire:

1) Add SFNodeNetworkLimited. 2) Add SFNodeNetworkLimited to default services.

Change to blockchain:

1) New pruneTarget param to indicate how much the block files should take up.l 2) Attempts to prune blocks, spend journals, and relevant indexes on every connectBlock

Change to indexers:

1) Added PruneBlock to Manager and implemented PruneBlock to every indexer.

Change to main:

1) Prune flag is defined. It limits the prune to 1536MiB as we need to guarantee 288 blocks to abide by the NODE_NETWORK_LIMITED rule set by BIP159. 2) --addrindex and --prune at the same time is not allowed. 3) --txindex and --prune at the same time is not allowed.

Fixes https://github.com/btcsuite/btcd/issues/1069

coveralls commented 1 year ago

Pull Request Test Coverage Report for Build 5936174185


Changes Missing Coverage Covered Lines Changed/Added Lines %
server.go 0 4 0.0%
blockchain/indexers/addrindex.go 0 8 0.0%
blockchain/indexers/cfindex.go 0 8 0.0%
blockchain/indexers/txindex.go 0 8 0.0%
blockchain/chainio.go 0 9 0.0%
rpcserver.go 0 9 0.0%
database/ffldb/blockio.go 25 35 71.43%
blockchain/chain.go 4 16 25.0%
database/ffldb/db.go 56 76 73.68%
config.go 0 21 0.0%
<!-- Total: 85 252 33.73% -->
Files with Coverage Reduction New Missed Lines %
rpcserver.go 4 0.26%
<!-- Total: 4 -->
Totals Coverage Status
Change from base Build 5872394378: -0.09%
Covered Lines: 26925
Relevant Lines: 48707

💛 - Coveralls
saubyk commented 1 year ago

cc @halseth in case you're interested in reviewing this pr. Thanks

kcalvinalvin commented 11 months ago
  • Should we still allow txindex since it allows you to fetch blocks beyond the prune horizon?

I'm not sure. We could allow for it and just return empty strings when you attempt to fetch an already pruned block.

  • Should we attempt to delete the old information for existing indexes, making them partially present, or just wipe the entire thing?

Refer to https://github.com/btcsuite/btcd/pull/1971#discussion_r1284037346

  • Should we continue to keep the compact filters as well, since they can be used to still serve light clients?

I'm indifferent either way. The user does have the option to drop the entire thing if they want. Though it may be nice to be able to only keep a portion of it?

kcalvinalvin commented 10 months ago

Made some additional changes:

1: Correctly return the prune status for getblockchaininfo 2: Got rid of pruning code for indexers. Compact filters are no longer pruned when the blocks are deleted. User either has all the cfindex data or none. 3: Force user to drop already existing addr and/or tx index when trying to enable pruning. 4: Error if user enables addr and/or tx index while pruned. 5: Error if user tries to enable cfindex if they started the ibd while pruned and cfindex disabled. 6: Force user to drop already indexed cfindex data when trying to disable cfindex while pruned.

kcalvinalvin commented 10 months ago

The last push just adds a more descriptive error message if the user tries to disable cfindex while the node is already pruned and has cfindex enabled.

kcalvinalvin commented 10 months ago

Sorry for yet another additional push. 3 things here.

1) The biggest thing: I realized that a lot of the code depends on cfg.Prune to tell if the node is pruned or not. But the code allowed the user to omit the flag even if the node had been previously pruned. This would've resulted in broadcasting to peers that the node isn't pruned and also would've given the wrong prune status for getblockchaininfo.

Instead of trying to see every case where I'd also have to call the database for the prune status, I added a commit that forces the user to prune if the database had been pruned previously like bitcoind. I don't think this really affects the UX and negates future bugs as well that can arise from not checking db for the prune status and just relying on cfg.Prune.

2) getblockchaininfo now depends on cfg.Prune != 0 3) Added additional message for the --prune flag on -h that tells the user about the default value of cfg.Prune == 0 and how that disables pruning.

Roasbeef commented 10 months ago

This would've resulted in broadcasting to peers that the node isn't pruned

Can you elaborate on this? Broadcasting as in the set of services bits we advertise, or something else? IIUC, those service bits will always give away if a node has been pruned or not.