-
Notifications
You must be signed in to change notification settings - Fork 37.7k
Add two hour buffer to manual pruning #9778
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
utACK 051fc42521dd5417f34ed27a727ad045a9823764
I think it should probably not be hard to add a test for this in:
bitcoin/qa/rpc-tests/pruning.py
Line 229 in 1e92e04
def manual_test(self, node_number, use_timestamp): |
Though pruning.py is kind of a pain to run.
src/rpc/blockchain.cpp
Outdated
@@ -820,7 +820,8 @@ UniValue pruneblockchain(const JSONRPCRequest& request) | |||
throw runtime_error( | |||
"pruneblockchain\n" | |||
"\nArguments:\n" | |||
"1. \"height\" (numeric, required) The block height to prune up to. May be set to a discrete height, or to a unix timestamp to prune based on block time.\n" | |||
"1. \"height\" (numeric, required) The block height to prune up to. May be set to a discrete height, or a unix timestamp\n" | |||
" to prune based on block time. Timestamp based pruning automatically adds a 2 hour buffer.\n" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd maybe make the comment more literal and say timestamp pruning will only prune blocks that are at least 2 hours older than the provided timestamp (instead of that timestamp pruning automatically adds a 2 hour buffer).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
code review utACK 051fc42521dd5417f34ed27a727ad045a9823764
Maybe 7200 could be made a constant to be reused in #9761 ? (and the normal rescan?) |
ok slightly modified the help text The 7200 number was added simultaneously in 3 separate PR's, but I agree it should be cleaned up to a constant in a later PR. |
Actually my personal preference would have been to leave the details of the exact buffering mechanism used off the help text. It seems too dependent on the exact implementation choice, and there is nothing magical about the 2 hour number. But at this point I think we are bike shedding. |
utACK you could also leave it out of the description as far as I care, it's an implementation detail. |
I think it's fine to mention it in the description. The two hours could be important in edge cases e.g. determining what blocks to keep with manual pruning. |
utACK 91fb506 |
91fb506 Add two hour buffer to manual pruning (Alex Morcos)
This caused |
91fb506 Add two hour buffer to manual pruning (Alex Morcos)
91fb506 Add two hour buffer to manual pruning (Alex Morcos)
91fb506 Add two hour buffer to manual pruning (Alex Morcos)
@gmaxwell, this is meant to address #9761 (comment)
Should probably go in 0.14 with the rest of the grace period / importmulti changes