Page MenuHomePhabricator

Fix a bug in ParseMoney that prevents from parsing large numbers
ClosedPublic

Authored by Fabien on Apr 14 2022, 10:07.

Details

Summary

With the currency unit change, large numbers are failed to parse due to the 63 bits overflow prevention not being adjusted to account for the new decimal number. This diff fixes the issue, update the tests that was not checking for the correct error/not using the correct value and make sure we can parse the max money. The related avalanche processor test is expanded a bit for better coverage.

Test Plan
ninja all check-all

Diff Detail

Event Timeline

Fabien requested review of this revision.Apr 14 2022, 10:07

Tail of the build log:

[332/517] Building CXX object src/CMakeFiles/common.dir/rpc/util.cpp.o
[333/517] Building CXX object src/CMakeFiles/bitcoinconsensus.dir/pubkey.cpp.o
[334/517] Building CXX object src/CMakeFiles/bitcoinconsensus.dir/consensus/tx_check.cpp.o
[335/517] Building CXX object src/CMakeFiles/script.dir/script/standard.cpp.o
[336/517] Building CXX object src/CMakeFiles/script.dir/script/sign.cpp.o
[337/517] Building CXX object src/CMakeFiles/bitcoin-cli.dir/bitcoin-cli.cpp.o
[338/517] Building CXX object src/test/CMakeFiles/testutil.dir/util/str.cpp.o
[339/517] Building C object src/secp256k1/CMakeFiles/internal-bench.dir/src/bench_internal.c.o
[340/517] Building C object src/secp256k1/CMakeFiles/ecmult-bench.dir/src/bench_ecmult.c.o
[341/517] Building CXX object src/CMakeFiles/script.dir/script/signingprovider.cpp.o
[342/517] Building C object src/secp256k1/CMakeFiles/secp256k1.dir/src/secp256k1.c.o
[343/517] Linking C static library src/secp256k1/libsecp256k1.a
[344/517] Linking C executable src/secp256k1/ecmult-bench
[345/517] Building CXX object src/test/CMakeFiles/testutil.dir/util/logging.cpp.o
[346/517] Linking C executable src/secp256k1/internal-bench
[347/517] Linking C executable src/secp256k1/sign-bench
[348/517] Linking C executable src/secp256k1/verify-bench
[349/517] Linking C executable src/secp256k1/recover-bench
[350/517] Building CXX object src/CMakeFiles/script.dir/script/descriptor.cpp.o
[351/517] Installing component secp256k1
-- Install configuration: "Debug"
-- Install component: "secp256k1"
-- Installing: /results/artifacts/lib/libsecp256k1.a
-- Installing: /results/artifacts/include/secp256k1.h
-- Installing: /results/artifacts/include/secp256k1_preallocated.h
-- Installing: /results/artifacts/include/secp256k1_recovery.h
-- Installing: /results/artifacts/include/secp256k1_schnorr.h
[352/517] Building CXX object src/wallet/CMakeFiles/wallet.dir/context.cpp.o
[353/517] Building CXX object src/CMakeFiles/bitcoin-wallet.dir/bitcoin-wallet.cpp.o
[354/517] Building CXX object src/test/CMakeFiles/testutil.dir/util/blockfilter.cpp.o
[355/517] Building CXX object src/test/CMakeFiles/testutil.dir/util/transaction_utils.cpp.o
[356/517] Building CXX object src/CMakeFiles/bitcoin-tx.dir/bitcoin-tx.cpp.o
[357/517] Building CXX object src/wallet/CMakeFiles/wallet.dir/db.cpp.o
[358/517] Building CXX object src/wallet/CMakeFiles/wallet.dir/coinselection.cpp.o
[359/517] Building CXX object src/test/CMakeFiles/testutil.dir/util/mining.cpp.o
[360/517] Building CXX object src/wallet/CMakeFiles/wallet.dir/bdb.cpp.o
[361/517] Building CXX object src/test/CMakeFiles/testutil.dir/util/net.cpp.o
[362/517] Building CXX object src/wallet/CMakeFiles/wallet.dir/coincontrol.cpp.o
[363/517] Building CXX object src/wallet/CMakeFiles/wallet.dir/crypter.cpp.o
[364/517] Building CXX object src/test/CMakeFiles/testutil.dir/util/wallet.cpp.o
[365/517] Building CXX object src/wallet/CMakeFiles/wallet.dir/sqlite.cpp.o
[366/517] Building CXX object src/test/CMakeFiles/testutil.dir/util/setup_common.cpp.o
[367/517] Building CXX object src/seeder/CMakeFiles/seeder-base.dir/dns.cpp.o
[368/517] Building CXX object src/wallet/CMakeFiles/wallet-tool.dir/wallettool.cpp.o
[369/517] Building CXX object src/wallet/CMakeFiles/wallet.dir/walletutil.cpp.o
[370/517] Building CXX object src/seeder/CMakeFiles/seeder-base.dir/db.cpp.o
[371/517] Building CXX object src/wallet/CMakeFiles/wallet.dir/fees.cpp.o
[372/517] Building CXX object src/wallet/CMakeFiles/wallet.dir/load.cpp.o
[373/517] Building CXX object src/seeder/CMakeFiles/seeder-base.dir/options.cpp.o
[374/517] Building CXX object src/wallet/CMakeFiles/wallet.dir/__/interfaces/wallet.cpp.o
[375/517] Building CXX object src/seeder/CMakeFiles/seeder-base.dir/bitcoin.cpp.o
[376/517] Building CXX object src/wallet/CMakeFiles/wallet.dir/salvage.cpp.o
[377/517] Building CXX object src/seeder/CMakeFiles/bitcoin-seeder.dir/main.cpp.o
[378/517] Building CXX object src/wallet/CMakeFiles/wallet.dir/scriptpubkeyman.cpp.o
[379/517] Building CXX object src/wallet/CMakeFiles/wallet.dir/rpcdump.cpp.o
[380/517] Building CXX object src/wallet/CMakeFiles/wallet.dir/walletdb.cpp.o
[381/517] Building CXX object src/wallet/CMakeFiles/wallet.dir/rpcwallet.cpp.o
[382/517] Building CXX object src/wallet/CMakeFiles/wallet.dir/wallet.cpp.o
ninja: build stopped: cannot make progress due to previous errors.
Build build-debug failed with exit code 1

Tail of the build log:

[305/463] Building CXX object src/CMakeFiles/server.dir/policy/settings.cpp.o
[306/463] Building CXX object src/CMakeFiles/server.dir/pow/daa.cpp.o
[307/463] Building CXX object src/CMakeFiles/server.dir/node/context.cpp.o
[308/463] Building CXX object src/CMakeFiles/server.dir/noui.cpp.o
[309/463] Building CXX object src/CMakeFiles/server.dir/rpc/command.cpp.o
[310/463] Building CXX object src/CMakeFiles/server.dir/node/transaction.cpp.o
[311/463] Building CXX object src/CMakeFiles/server.dir/pow/eda.cpp.o
[312/463] Building CXX object src/CMakeFiles/server.dir/pow/pow.cpp.o
[313/463] Building CXX object src/CMakeFiles/server.dir/pow/grasberg.cpp.o
[314/463] Building CXX object src/CMakeFiles/server.dir/script/scriptcache.cpp.o
[315/463] Building CXX object src/CMakeFiles/server.dir/rpc/abc.cpp.o
[316/463] Building CXX object src/CMakeFiles/server.dir/shutdown.cpp.o
[317/463] Building CXX object src/CMakeFiles/server.dir/node/ui_interface.cpp.o
[318/463] Building CXX object src/CMakeFiles/server.dir/net.cpp.o
[319/463] Building CXX object src/CMakeFiles/server.dir/rest.cpp.o
[320/463] Building CXX object src/CMakeFiles/server.dir/timedata.cpp.o
[321/463] Building CXX object src/CMakeFiles/server.dir/rpc/misc.cpp.o
[322/463] Building CXX object src/CMakeFiles/server.dir/script/sigcache.cpp.o
[323/463] Building CXX object src/CMakeFiles/server.dir/rpc/server.cpp.o
[324/463] Building CXX object src/CMakeFiles/server.dir/versionbits.cpp.o
[325/463] Building CXX object src/CMakeFiles/server.dir/rpc/mining.cpp.o
[326/463] Building CXX object src/CMakeFiles/server.dir/rpc/avalanche.cpp.o
[327/463] Building C object src/secp256k1/CMakeFiles/sign-bench.dir/src/bench_sign.c.o
[328/463] Building C object src/secp256k1/CMakeFiles/verify-bench.dir/src/bench_verify.c.o
[329/463] Building C object src/secp256k1/CMakeFiles/recover-bench.dir/src/bench_recover.c.o
[330/463] Building CXX object src/CMakeFiles/server.dir/dummywallet.cpp.o
[331/463] Building CXX object src/CMakeFiles/server.dir/txdb.cpp.o
[332/463] Building CXX object src/CMakeFiles/server.dir/rpc/net.cpp.o
[333/463] Building C object src/secp256k1/CMakeFiles/internal-bench.dir/src/bench_internal.c.o
[334/463] Building C object src/secp256k1/CMakeFiles/ecmult-bench.dir/src/bench_ecmult.c.o
[335/463] Building CXX object src/CMakeFiles/server.dir/validationinterface.cpp.o
[336/463] Building C object src/secp256k1/CMakeFiles/secp256k1.dir/src/secp256k1.c.o
[337/463] Linking C static library src/secp256k1/libsecp256k1.a
[338/463] Linking C executable src/secp256k1/ecmult-bench
[339/463] Linking C executable src/secp256k1/internal-bench
[340/463] Building CXX object src/test/CMakeFiles/testutil.dir/util/str.cpp.o
[341/463] Linking C executable src/secp256k1/sign-bench
[342/463] Linking C executable src/secp256k1/verify-bench
[343/463] Linking C executable src/secp256k1/recover-bench
[344/463] Building CXX object src/test/CMakeFiles/testutil.dir/util/logging.cpp.o
[345/463] Building CXX object src/CMakeFiles/server.dir/torcontrol.cpp.o
[346/463] Building CXX object src/test/CMakeFiles/testutil.dir/util/blockfilter.cpp.o
[347/463] Building CXX object src/test/CMakeFiles/testutil.dir/util/wallet.cpp.o
[348/463] Building CXX object src/seeder/CMakeFiles/seeder-base.dir/dns.cpp.o
[349/463] Building CXX object src/CMakeFiles/server.dir/txmempool.cpp.o
[350/463] Building CXX object src/test/CMakeFiles/testutil.dir/util/transaction_utils.cpp.o
[351/463] Building CXX object src/CMakeFiles/server.dir/init.cpp.o
[352/463] Building CXX object src/test/CMakeFiles/testutil.dir/util/mining.cpp.o
[353/463] Building CXX object src/test/CMakeFiles/testutil.dir/util/net.cpp.o
[354/463] Building CXX object src/seeder/CMakeFiles/seeder-base.dir/db.cpp.o
[355/463] Building CXX object src/CMakeFiles/server.dir/rpc/rawtransaction.cpp.o
[356/463] Building CXX object src/seeder/CMakeFiles/seeder-base.dir/options.cpp.o
[357/463] Building CXX object src/seeder/CMakeFiles/bitcoin-seeder.dir/main.cpp.o
[358/463] Building CXX object src/CMakeFiles/server.dir/rpc/blockchain.cpp.o
[359/463] Building CXX object src/test/CMakeFiles/testutil.dir/util/setup_common.cpp.o
[360/463] Building CXX object src/CMakeFiles/server.dir/net_processing.cpp.o
[361/463] Building CXX object src/seeder/CMakeFiles/seeder-base.dir/bitcoin.cpp.o
[362/463] Building CXX object src/CMakeFiles/server.dir/validation.cpp.o
ninja: build stopped: cannot make progress due to previous errors.
Build build-without-wallet failed with exit code 1

Tail of the build log:

[332/517] Building C object src/secp256k1/CMakeFiles/recover-bench.dir/src/bench_recover.c.o
[333/517] Building CXX object src/CMakeFiles/script.dir/script/interpreter.cpp.o
[334/517] Building CXX object src/CMakeFiles/script.dir/script/sign.cpp.o
[335/517] Building C object src/secp256k1/CMakeFiles/ecmult-bench.dir/src/bench_ecmult.c.o
[336/517] Building CXX object src/CMakeFiles/common.dir/rpc/util.cpp.o
[337/517] Building CXX object src/CMakeFiles/bitcoin-wallet.dir/bitcoin-wallet.cpp.o
[338/517] Building CXX object src/test/CMakeFiles/testutil.dir/util/str.cpp.o
[339/517] Building CXX object src/CMakeFiles/script.dir/script/signingprovider.cpp.o
[340/517] Building C object src/secp256k1/CMakeFiles/internal-bench.dir/src/bench_internal.c.o
[341/517] Building C object src/secp256k1/CMakeFiles/secp256k1.dir/src/secp256k1.c.o
[342/517] Linking C static library src/secp256k1/libsecp256k1.a
[343/517] Linking C executable src/secp256k1/ecmult-bench
[344/517] Linking C executable src/secp256k1/internal-bench
[345/517] Linking C executable src/secp256k1/sign-bench
[346/517] Building CXX object src/test/CMakeFiles/testutil.dir/util/logging.cpp.o
[347/517] Linking C executable src/secp256k1/verify-bench
[348/517] Linking C executable src/secp256k1/recover-bench
[349/517] Installing component secp256k1
-- Install configuration: "RelWithDebInfo"
-- Install component: "secp256k1"
-- Installing: /results/artifacts/lib/libsecp256k1.a
-- Installing: /results/artifacts/include/secp256k1.h
-- Installing: /results/artifacts/include/secp256k1_preallocated.h
-- Installing: /results/artifacts/include/secp256k1_recovery.h
-- Installing: /results/artifacts/include/secp256k1_schnorr.h
[350/517] Building CXX object src/CMakeFiles/script.dir/script/descriptor.cpp.o
[351/517] Building CXX object src/test/CMakeFiles/testutil.dir/util/blockfilter.cpp.o
[352/517] Building CXX object src/CMakeFiles/bitcoin-cli.dir/bitcoin-cli.cpp.o
[353/517] Building CXX object src/wallet/CMakeFiles/wallet.dir/context.cpp.o
[354/517] Building CXX object src/test/CMakeFiles/testutil.dir/util/transaction_utils.cpp.o
[355/517] Building CXX object src/test/CMakeFiles/testutil.dir/util/mining.cpp.o
[356/517] Building CXX object src/test/CMakeFiles/testutil.dir/util/net.cpp.o
[357/517] Building CXX object src/wallet/CMakeFiles/wallet.dir/coincontrol.cpp.o
[358/517] Building CXX object src/wallet/CMakeFiles/wallet.dir/db.cpp.o
[359/517] Building CXX object src/CMakeFiles/bitcoin-tx.dir/bitcoin-tx.cpp.o
[360/517] Building CXX object src/CMakeFiles/server.dir/validation.cpp.o
[361/517] Building CXX object src/wallet/CMakeFiles/wallet.dir/crypter.cpp.o
[362/517] Building CXX object src/test/CMakeFiles/testutil.dir/util/wallet.cpp.o
[363/517] Building CXX object src/wallet/CMakeFiles/wallet.dir/coinselection.cpp.o
[364/517] Building CXX object src/wallet/CMakeFiles/wallet.dir/bdb.cpp.o
[365/517] Building CXX object src/test/CMakeFiles/testutil.dir/util/setup_common.cpp.o
[366/517] Building CXX object src/wallet/CMakeFiles/wallet.dir/sqlite.cpp.o
[367/517] Building CXX object src/wallet/CMakeFiles/wallet-tool.dir/wallettool.cpp.o
[368/517] Building CXX object src/wallet/CMakeFiles/wallet.dir/fees.cpp.o
[369/517] Building CXX object src/seeder/CMakeFiles/seeder-base.dir/dns.cpp.o
[370/517] Building CXX object src/seeder/CMakeFiles/seeder-base.dir/db.cpp.o
[371/517] Building CXX object src/wallet/CMakeFiles/wallet.dir/walletutil.cpp.o
[372/517] Building CXX object src/wallet/CMakeFiles/wallet.dir/load.cpp.o
[373/517] Building CXX object src/seeder/CMakeFiles/seeder-base.dir/options.cpp.o
[374/517] Building CXX object src/wallet/CMakeFiles/wallet.dir/salvage.cpp.o
[375/517] Building CXX object src/wallet/CMakeFiles/wallet.dir/__/interfaces/wallet.cpp.o
[376/517] Building CXX object src/seeder/CMakeFiles/seeder-base.dir/bitcoin.cpp.o
[377/517] Building CXX object src/seeder/CMakeFiles/bitcoin-seeder.dir/main.cpp.o
[378/517] Building CXX object src/wallet/CMakeFiles/wallet.dir/scriptpubkeyman.cpp.o
[379/517] Building CXX object src/wallet/CMakeFiles/wallet.dir/walletdb.cpp.o
[380/517] Building CXX object src/wallet/CMakeFiles/wallet.dir/rpcdump.cpp.o
[381/517] Building CXX object src/wallet/CMakeFiles/wallet.dir/wallet.cpp.o
[382/517] Building CXX object src/wallet/CMakeFiles/wallet.dir/rpcwallet.cpp.o
ninja: build stopped: cannot make progress due to previous errors.
Build build-diff failed with exit code 1
Fabien planned changes to this revision.Apr 14 2022, 11:24

Fix signedness, assert this subtraction is valid (and explain why), and prevent a pointless copy that should never have existed to begin with

This revision is now accepted and ready to land.Apr 14 2022, 15:38