refactor: remove deprecated TxidFromString() in favour of transaction_identifier::FromHex()
Since https://github.com/bitcoin/bitcoin/pull/30482/commits/fab6ddbee64e50d5e2f499aebca35b5911896ec4, TxidFromString()
has been deprecated because it is less robust than the transaction_identifier::FromHex()
introduced in the same PR. Specifically, it tries to recover from length-mismatches, recover from untrimmed whitespace, 0x-prefix and garbage at the end, instead of simply requiring exactly 64 hex-only characters.
In this PR, TxidFromString
is removed completely to clean up the code and prevent further unsafe usage. Unit and fuzz test coverage on uint256::FromHex()
and functions that wrap it is increased.
Note: TxidFromSring
allowed users to prefix strings with "0x", this is no longer allowed for transaction_identifier::FromHex()
, so a helper function for input validation may prove helpful in the future (this overlaps with the uint256::uint256S()
vs uint256::FromHex()
future cleanup). It is not relevant to this PR, though, besides the fact that this unused (except for in tests) functionality is removed.
The only users of TxidFromString
are:
-
test
, where it is straightforward to drop in the newFromHex()
methods without much further concern -
qt
coincontrol. There is no need for input validation here, but txids are not guaranteed to be 64 characters. This is already handled by the existing code, so again, usingFromHex()
here seems quite straightforward.
Addresses @maflcko
's suggestion: https://github.com/bitcoin/bitcoin/pull/30482#discussion_r1691826934
Also removes WtxidFromString()
, which is a test-only helper function.
Testing GUI changes
To test the GUI coincontrol affected lines, regtest
is probably the easiest way to quickly get some test coins, you can use e.g.
alias cli="./src/bitcoin-cli -regtest"
cli createwallet "coincontrol"
# generate 10 spendable outputs on 1 address
cli generatetoaddress 10 $(cli -rpcwallet=coincontrol getnewaddress)
# generate 10 spendable outputs on another address
cli generatetoaddress 10 $(cli -rpcwallet=coincontrol getnewaddress)
# make previous outputs spendable
cli generatetoaddress 100 $(cli -rpcwallet=coincontrol getnewaddress)