From 6819e5a329c3bf38e47a07434e2a3c0031f808d0 Mon Sep 17 00:00:00 2001 From: stickies-v Date: Fri, 26 Jul 2024 15:10:25 +0100 Subject: [PATCH] node: use uint256::FromUserHex for -assumevalid parsing Removes dependency on unsafe and deprecated uint256S. This makes parsing more strict, by returning an error when the input contains non-hex characters, or when it contains more than 64 hex digits. Also make feature_assumevalid.py more robust by using CBlock.hash which is guaranteed to be 64 characters long, as opposed to the variable-length hex(CBlock.sha256) --- src/node/chainstatemanager_args.cpp | 8 +++++++- src/test/validation_chainstatemanager_tests.cpp | 3 +++ test/functional/feature_assumevalid.py | 4 ++-- 3 files changed, 12 insertions(+), 3 deletions(-) diff --git a/src/node/chainstatemanager_args.cpp b/src/node/chainstatemanager_args.cpp index 129d51c4046..b86d0b29913 100644 --- a/src/node/chainstatemanager_args.cpp +++ b/src/node/chainstatemanager_args.cpp @@ -39,7 +39,13 @@ util::Result ApplyArgsManOptions(const ArgsManager& args, ChainstateManage } } - if (auto value{args.GetArg("-assumevalid")}) opts.assumed_valid_block = uint256S(*value); + if (auto value{args.GetArg("-assumevalid")}) { + if (auto block_hash{uint256::FromUserHex(*value)}) { + opts.assumed_valid_block = *block_hash; + } else { + return util::Error{strprintf(Untranslated("Invalid assumevalid block hash specified (%s), must be up to %d hex digits (or 0 to disable)"), *value, uint256::size() * 2)}; + } + } if (auto value{args.GetIntArg("-maxtipage")}) opts.max_tip_age = std::chrono::seconds{*value}; diff --git a/src/test/validation_chainstatemanager_tests.cpp b/src/test/validation_chainstatemanager_tests.cpp index 052f5cd1a7b..6d1ef044899 100644 --- a/src/test/validation_chainstatemanager_tests.cpp +++ b/src/test/validation_chainstatemanager_tests.cpp @@ -814,6 +814,9 @@ BOOST_FIXTURE_TEST_CASE(chainstatemanager_args, BasicTestingSetup) const std::string cmd{"-assumevalid=0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef"}; BOOST_CHECK_EQUAL(get_valid_opts({cmd.c_str()}).assumed_valid_block.value().ToString(), cmd.substr(13, cmd.size())); + BOOST_CHECK(!get_opts({"-assumevalid=xyz"})); // invalid hex characters + BOOST_CHECK(!get_opts({"-assumevalid=01234567890123456789012345678901234567890123456789012345678901234"})); // > 64 hex chars + // test -minimumchainwork BOOST_CHECK(!get_valid_opts({}).minimum_chain_work.has_value()); BOOST_CHECK_EQUAL(get_valid_opts({"-minimumchainwork=0"}).minimum_chain_work.value().GetCompact(), 0U); diff --git a/test/functional/feature_assumevalid.py b/test/functional/feature_assumevalid.py index 03a9f666b2a..32ad3ad271e 100755 --- a/test/functional/feature_assumevalid.py +++ b/test/functional/feature_assumevalid.py @@ -139,8 +139,8 @@ class AssumeValidTest(BitcoinTestFramework): height += 1 # Start node1 and node2 with assumevalid so they accept a block with a bad signature. - self.start_node(1, extra_args=["-assumevalid=" + hex(block102.sha256)]) - self.start_node(2, extra_args=["-assumevalid=" + hex(block102.sha256)]) + self.start_node(1, extra_args=["-assumevalid=" + block102.hash]) + self.start_node(2, extra_args=["-assumevalid=" + block102.hash]) p2p0 = self.nodes[0].add_p2p_connection(BaseNode()) p2p0.send_header_for_blocks(self.blocks[0:2000])