devtools, utxo-snapshot: Fix block height out of range in script #30690
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Handle the Block height out of range error gracefully by checking if the node has synchronized to or beyond the required block height, otherwise without this validation the node would keep the network disabled if the user selected that option. Provide a user-friendly message if the block height is out of range and exit the script cleanly.
Contributor
|
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers. Code CoverageFor detailed information about the code coverage, see the test coverage report. ReviewsSee the guideline for information on the review process.
If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update. ConflictsReviewers, this pull request conflicts with the following ones:
If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first. |
fjahr
reviewed
Aug 25, 2024
Contributor
fjahr
left a comment
fjahr
left a comment
There was a problem hiding this comment.
tACK 5b4f340
Reviewed the code and did some light testing.
It's unclear when #29553 will get merged and since we may see some people come in and try out assumeutxo for the first time when v28 is released so I think it's ok to keep improving the script until then.
This could have also been fixed in a simpler way by moving the PIVOT_BLOCKHASH= line below the two trap lines but I guess it's nice to give users an understandable error message so I am fine with the approach.
|
|
||
| # Check current block height to ensure the node has synchronized past the required block | ||
| CURRENT_BLOCK_HEIGHT=$(${BITCOIN_CLI_CALL} getblockcount) | ||
| PIVOT_BLOCK_HEIGHT=$(( GENERATE_AT_HEIGHT + 1 )) |
Contributor
There was a problem hiding this comment.
nit: Could reuse that below in the PIVOT_BLOCKHASH= line but only if you have to retouch.
Member
|
ACK 5b4f340 |
PastaPastaPasta
pushed a commit
to PastaPastaPasta/dash
that referenced
this pull request
Oct 27, 2024
… range in script 5b4f340 devtools, utxo-snapshot: Fix block height out of range (pablomartin4btc) Pull request description: <details> <summary>Fixing a <a href="https://github.com/bitcoin/bitcoin/pull/28553#pullrequestreview-2251032570">bug</a> in <code>utxo_snapshot.sh</code>.</summary> ``` /contrib/devtools/utxo_snapshot.sh 840000 snapshot2.dat ./src/bitcoin-cli -datadir=${AU_DATADIR} Do you want to disable network activity (setnetworkactive false) before running invalidateblock? (Y/n): Disabling network activity false error code: -8 error message: Block height out of range ``` And the user will see the following in the node and it would stay there if not reset: ``` 2024-08-21T14:44:13Z UpdateTip: new best=00000000000000afa0cd000a16e244f56032735d41acd32ac00337aceb2a5240 height=235382 version=0x00000002 log2_work=69.987697 tx=17492185 date='2013-05-09T23:54:32Z' progress=0.016219 cache=71.0MiB(571085txo) 2024-08-21T14:44:13Z UpdateTip: new best=0000000000000087c5e0b820afff496b95ba44ad64640c73b234d3261d3f99d2 height=235383 version=0x00000002 log2_work=69.987750 tx=17492341 date='2013-05-09T23:54:47Z' progress=0.016219 cache=71.0MiB(571291txo) 2024-08-21T14:44:13Z UpdateTip: new best=000000000000014a4b5fddf3c8abb6209247255ca9e8df786b271dd1b2ac82a6 height=235384 version=0x00000002 log2_work=69.987804 tx=17492344 date='2013-05-10T00:20:18Z' progress=0.016219 cache=71.0MiB(571297txo) 2024-08-21T14:44:13Z SetNetworkActive: false ``` </details> This is a "temporary" fix until bitcoin#29553 gets merged, which will remove the script entirely. Handle the "Block height out of range" error gracefully by checking if the node has synchronized to or beyond the required block height, otherwise without this validation the node would keep the network disabled if the user selected that option. <details> <summary>Provide a user-friendly message if the block height is out of range and exit the script cleanly.</summary> ``` /contrib/devtools/utxo_snapshot.sh 840000 snapshot2.dat ./src/bitcoin-cli -datadir=${AU_DATADIR} Error: The node has not yet synchronized to block height 840001. Please wait until the node has synchronized past this block height and try again. ``` </details> ACKs for top commit: achow101: ACK 5b4f340 fjahr: tACK 5b4f340 Tree-SHA512: 2b71286b627872d7cfdb367e29361afa3806a7ef9d65075b93892b735ff2ab729069e2f7259d30262909e73cef17fb7dca231615cc1863968cd042f4a2a4f901
PastaPastaPasta
added a commit
to dashpay/dash
that referenced
this pull request
Oct 27, 2024
b654479 Merge bitcoin#30705: test: Avoid intermittent block download timeout in p2p_ibd_stalling (merge-script) 745a819 Merge bitcoin#30690: devtools, utxo-snapshot: Fix block height out of range in script (Ava Chow) 01b570e Merge bitcoin#29999: guix: fix suggested fake date for openssl-1.1.1l (Ava Chow) 432f352 Merge bitcoin#30580: doc: Add note about distro's `g++-mingw-w64-x86-64-posix` version (merge-script) 1bd090e Merge bitcoin#30597: doc: Drop no longer needed workaround for WSL (merge-script) 8a12237 Merge bitcoin#30630: doc: Update ccache website link (merge-script) f66547f Merge bitcoin#30588: depends: fix ZMQ CMake getcachesize check (merge-script) ddaec96 Merge bitcoin#30565: depends: Fix `zeromq` build on OpenBSD (merge-script) e4e5605 Merge bitcoin#30552: test: fix constructor of msg_tx (merge-script) df3c239 Merge bitcoin#26950: cleanse: switch to SecureZeroMemory for Windows cross-compile (merge-script) 57945ce Merge bitcoin#30506: depends: Cleanup postprocess commands after switching to CMake (merge-script) e016ffa Merge bitcoin#29878: depends: build expat with CMake (merge-script) 62dcd43 Merge bitcoin#29880: depends: build FreeType with CMake (merge-script) 745addf Merge bitcoin#30245: net: Allow -proxy=[::1] on nodes with IPV6 lo only (Ava Chow) 4e144be Merge bitcoin-core/gui#795: Keep focus on "Hide" while ModalOverlay is visible (Hennadii Stepanov) 69c04b2 Merge bitcoin#30372: util: Use SteadyClock in RandAddSeedPerfmon (merge-script) ebed8af Merge bitcoin#30336: depends: update doc in Qt pwd patch (merge-script) 9793fb1 Merge bitcoin#30340: test: Added coverage to Block not found error using gettxoutsetinfo (Ava Chow) 479cb8b Merge bitcoin#30312: contrib: add R(UN)PATH check to ELF symbol-check (merge-script) ca83773 Merge bitcoin#30283: upnp: fix build with miniupnpc 2.2.8 (merge-script) 63e139d Merge bitcoin#30185: guix: show `*_FLAGS` variables in pre-build output (merge-script) 3be0d3e Merge bitcoin#30097: crypto: disable asan for sha256_sse4 with clang and -O0 (merge-script) 3070c3e Merge bitcoin#30078: depends: set AR & RANLIB for CMake (merge-script) Pull request description: ## Issue being fixed or feature implemented Trivial backports ## What was done? ## How Has This Been Tested? built locally ## Breaking Changes ## Checklist: _Go over all the following points, and put an `x` in all the boxes that apply._ - [ ] I have performed a self-review of my own code - [ ] I have commented my code, particularly in hard-to-understand areas - [ ] I have added or updated relevant unit/integration/functional/e2e tests - [ ] I have made corresponding changes to the documentation - [x] I have assigned this pull request to a milestone _(for repository code-owners and collaborators only)_ ACKs for top commit: UdjinM6: utACK b654479 kwvg: utACK b654479 Tree-SHA512: 10b5af4e92c83fa9d6764b20bf066bba8e4c600402966fd5c1d6dad07b0549d8a42151a33f21e2f8263336c12a810a6f3fc2828d90bc98153e09c165d9e5b043
Fabcien
pushed a commit
to Bitcoin-ABC/bitcoin-abc
that referenced
this pull request
Mar 25, 2025
Summary: - Ensure that the snapshot height is higher than the pruned block height when the node is pruned. - Validate the correctness of the file path and check if the file already exists. - Make network activity disablement optional for the user. - Ensure the reconsiderblock command is triggered on exit, even in the case of user interruption (Ctrl-C). - Fix block height out of range - Fix `hash_serialized_2` -> `hash_serialized` This is a backport of [[bitcoin/bitcoin#28852 | core#28852]], [[bitcoin/bitcoin#30690 | core#30690]] Test Plan: Backup the datadir. Start bitcoind in a different terminal and monitor it while the script is running. ``` $ ./contrib/devtools/utxo_snapshot.sh 888000 ../utxo.dat ./build/src/bitcoin-cli Do you want to disable network activity (setnetworkactive false) before running invalidateblock? (Y/n): Y Disabling network activity false Rewinding chain back to height 888000 (by invalidating 00000000000000001e72f6310cc606c4a47d71d5168f55b6f96b6a6f61f9f616); this may take a while Generating UTXO snapshot... { "coins_written": 38780572, "base_hash": "00000000000000002b218d995a292c34bc4c0244bb4bbdad18f3a97e88ccb567", "base_height": 888000, "path": "/data0/ecashd/../utxo.dat", "txoutset_hash": "50493f6218661a189654dbad816821a656b519454190c63daf376610e4fa0a7e", "nchaintx": 299158458 } Restoring chain to original height; this may take a while Restoring network activity true ``` Reviewers: #bitcoin_abc, Fabien Reviewed By: #bitcoin_abc, Fabien Differential Revision: https://reviews.bitcoinabc.org/D17855
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
5 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixing a bug in
utxo_snapshot.sh.And the user will see the following in the node and it would stay there if not reset:
This is a "temporary" fix until #29553 gets merged, which will remove the script entirely.
Handle the "Block height out of range" error gracefully by checking if the node has synchronized to or beyond the required block height, otherwise without this validation the node would keep the network disabled if the user selected that option.
Provide a user-friendly message if the block height is out of range and exit the script cleanly.