util,test: fix roundtrip asymmetries in parser and serializer code#127
Draft
util,test: fix roundtrip asymmetries in parser and serializer code#127
Conversation
Define `uint160::FromHex`, `uint256::FromHex`, and `uint256::FromUserHex` in `uint256.cpp` and keep only declarations in `uint256.h`. Drop heavy `util/strencodings.h` and `util/string.h` includes from `uint256.h` by using a local consteval hex helper for literal construction and non-inline parsing helpers in the `.cpp` file. Add direct includes in `musig.cpp` and `outputtype.cpp` for symbols that were previously provided transitively via `uint256.h`.
Drop the heavy chainparams dependency from `key_io.h` and include it directly in the implementation and test files that use `Params()` or `SelectParams()`.
Move the SetupServerArgs implementation out of `init.cpp` into a dedicated translation unit and add it to the build. This keeps `init.cpp` smaller and isolates server argument registration code in one place.
Make ParseHDKeypath() accept h as a hardened suffix so it can parse the default output of WriteHDKeypath() again, and cover both ' and h in the wallet test and fuzz target. Add roundtrip checks for the text and serialization paths that were most likely to hide asymmetries, including network permissions, net addresses, scripts, transactions, PSBTs, money strings, and compressed txout/coin deserialization. Reject out-of-range values in AmountCompression::Unser() instead of materializing invalid CAmounts, and add a minimal unit reproducer plus focused deserialize fuzz assertions for TxOutCompression, Coin, CTxUndo, and CBlockUndo.
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
The PR bundles four logically independent changes under one title. The actual roundtrip fix (commit 4) is the only one that matches the PR title; the other three are preparatory refactors. The description should make that clear without repeating what the commit messages already say.
Several parser/serializer pairs in the codebase had asymmetries that prevented a successful roundtrip:
ParseHDKeypath()rejected thehhardened suffix thatWriteHDKeypath()can produce,AmountCompression::Unser()could materializeCAmountvalues aboveMAX_MONEYwithout error, and fuzz targets for PSBTs, transactions, scripts, net addresses, net permissions, and money strings exercised only one direction of the parse/serialize cycle. The three preparatory commits reduce transitive include weight by movinguint256hex parsing out of the header, droppingchainparams.hfromkey_io.h, and extractingSetupServerArgsinto its own translation unit.The fix accepts
has a hardened-path suffix inParseHDKeypath()alongside the existing', adds a bounds check inAmountCompression::Unser()that throws on values exceedingMAX_MONEY, and extends fuzz targets and unit tests to assert that serializing a successfully parsed value and parsing it again yields an identical result forPartiallySignedTransaction,CTxOut/TxOutCompression,Coin,CTxUndo,CBlockUndo,CNetAddr/CService/CSubNet,NetWhitebindPermissions/NetWhitelistPermissions, HD keypaths, and money strings.