bugfix: throw an error if an invalid parameter is passed to getnetworkhashps RPC #28554
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
|
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. ConflictsNo conflicts as of last run. |
057b3e0 to
6074de6
Compare
|
reACK 6074de6 |
maflcko
approved these changes
Oct 2, 2023
786ba64 to
1fd3715
Compare
luke-jr
approved these changes
Oct 2, 2023
|
reACK 1fd3715 |
|
lgtm ACK 1fd3715 |
1fd3715 to
ec54efd
Compare
|
Upon further inspection I realized that the "lookup" nblocks parameter also had insufficient validation so I've added checks to ensure that it's either a positive number or -1. |
maflcko
reviewed
Oct 3, 2023
maflcko
reviewed
Oct 3, 2023
7198be5 to
565ad11
Compare
This was referenced Oct 3, 2023
luke-jr
reviewed
Oct 3, 2023
565ad11 to
435ff29
Compare
|
Not sure what's going on with the windows build; it failed on a wallet address test with a socket timeout. Seems unrelated to my PR. Looks like it's a known issue. #28509 |
Sjors
approved these changes
Oct 10, 2023
|
Any further concerns? |
luke-jr
pushed a commit
to bitcoinknots/bitcoin
that referenced
this pull request
Oct 19, 2023
…oint Github-Pull: bitcoin#28554 Rebased-From: 435ff29
435ff29 to
9ac114e
Compare
|
Thanks for rebasing. re-utACK 9ac114e |
|
reACK 9ac114e |
|
ACK 9ac114e |
glozow
pushed a commit
to glozow/bitcoin
that referenced
this pull request
Apr 17, 2024
…oint Github-Pull: bitcoin#28554 Rebased-From: 9ac114e
glozow
added a commit
that referenced
this pull request
May 24, 2024
aa7e876 [doc] add draft release notes for 26.2rc1 (glozow) 21d9aaa p2p, bugfix: detect addnode cjdns peers in GetAddedNodeInfo() (Jon Atack) ec5ce2f windeploy: Renew certificate (Ava Chow) 96d0e81 rpc: Reword SighashFromStr error message (MarcoFalke) 6685aff rpc: move UniValue in blockToJSON (willcl-ark) 7f45e00 depends: Fix build of Qt for 32-bit platforms (laanwj) f9b76ba ci: Pull in qtbase5-dev instead of seperate low-level libraries (laanwj) c587753 doc: Suggest installing dev packages for debian/ubuntu qt5 build (laanwj) 7ecdb08 ci: Bump s390x to ubuntu:24.04 (MarcoFalke) d9ef6cf sign: don't assume we are parsing a sane Miniscript (Antoine Poinsot) e4859c8 depends: fix mingw-w64 Qt DEBUG=1 build (fanquake) bb46b90 Fix #29767, set m_synced = true after Commit() (nanlour) bf5b6fc Throw error if invalid parameters passed to getnetworkhashps RPC endpoint (Jameson Lopp) a81a922 [rpc, bugfix] Enforce maximum value for setmocktime (dergoegge) d39ea51 Change Luke Dashjr seed to dashjr-list-of-p2p-nodes.us (Luke Dashjr) c21bbcc [doc] archive 26.1 release notes (glozow) Pull request description: Archives 26.1 release notes and adds draft release notes for 26.2rc1 Also backports: - #29691 - #29869 - #28554 - #29747 - #29853 - #29856 - #29764 - #29776 - #29985 - #30094 - #29870 - #30149 - #30085 ACKs for top commit: stickies-v: re-ACK aa7e876, only changes are fixing commit msg and transifex reference willcl-ark: ACK aa7e876 Tree-SHA512: b81ba6092640de696d782114cdf43e7ed1d63ea0a3231cade30653c2743d87700e0f852a1b1fcc42ae313b2d4f004e6026ddbad87d58c2fde0a660e90026ed98
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
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.
When writing some scripts that iterated over many blocks to generate hashrate estimates I realized that my script was going out of range of the current chain tip height but was not encountering any errors.
I believe that passing an invalid block height to this function but receiving the hashrate estimate for the chain tip instead should be considered unexpected behavior.