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)

Author: stickies-v

src/node/chainstatemanager_args.cpp

@@ -39,7 +39,13 @@ util::Result<void> 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};
 

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);

test/functional/feature_assumevalid.py

@@ -139,8 +139,8 @@ def run_test(self):
             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])