Page MenuHomePhabricator

rpc: include_unsafe option for fundrawtransaction
ClosedPublic

Authored by PiRK on Aug 24 2023, 09:25.

Details

Reviewers
Fabien
Group Reviewers
Restricted Project
Commits
rABC95c9e10a1da4: rpc: include_unsafe option for fundrawtransaction
Summary

Allow RPC users to opt-in to unsafe (unconfirmed received from an external key) inputs when funding a raw transaction.

This is a backport of core#21359
Depends on D14402

Test Plan

ninja all check-all

Diff Detail

Repository
rABC Bitcoin ABC
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

PiRK requested review of this revision.Aug 24 2023, 09:25

optimize call to any(...) as per linter advice

PiRK planned changes to this revision.Aug 24 2023, 09:32

need to standardize some doxygen argnames

Tail of the build log:

[368/538] Building CXX object src/test/CMakeFiles/testutil.dir/util/blockindex.cpp.o
[369/538] Building CXX object src/test/CMakeFiles/testutil.dir/util/str.cpp.o
[370/538] Building CXX object src/CMakeFiles/server.dir/torcontrol.cpp.o
[371/538] Building CXX object src/test/CMakeFiles/testutil.dir/util/logging.cpp.o
[372/538] Building CXX object src/CMakeFiles/server.dir/rpc/rawtransaction.cpp.o
[373/538] Building CXX object src/CMakeFiles/server.dir/rpc/blockchain.cpp.o
[374/538] Building CXX object src/test/CMakeFiles/testutil.dir/util/blockfilter.cpp.o
[375/538] Building CXX object src/wallet/CMakeFiles/wallet.dir/context.cpp.o
[376/538] Building CXX object src/test/CMakeFiles/testutil.dir/util/transaction_utils.cpp.o
[377/538] Building CXX object src/CMakeFiles/bitcoin-tx.dir/bitcoin-tx.cpp.o
[378/538] Linking CXX executable src/bitcoin-tx
[379/538] Building CXX object src/wallet/CMakeFiles/wallet.dir/coincontrol.cpp.o
[380/538] Building CXX object src/CMakeFiles/server.dir/wallet/init.cpp.o
[381/538] Building CXX object src/test/CMakeFiles/testutil.dir/util/net.cpp.o
[382/538] Building CXX object src/test/CMakeFiles/testutil.dir/util/validation.cpp.o
[383/538] Building CXX object src/test/CMakeFiles/testutil.dir/util/mining.cpp.o
[384/538] Building CXX object src/wallet/CMakeFiles/wallet.dir/db.cpp.o
[385/538] Building CXX object src/wallet/CMakeFiles/wallet.dir/coinselection.cpp.o
[386/538] Building CXX object src/wallet/CMakeFiles/wallet.dir/bdb.cpp.o
[387/538] Building CXX object src/wallet/CMakeFiles/wallet.dir/crypter.cpp.o
[388/538] Building CXX object src/test/CMakeFiles/testutil.dir/util/wallet.cpp.o
[389/538] Building CXX object src/test/CMakeFiles/testutil.dir/util/setup_common.cpp.o
[390/538] Building CXX object src/wallet/CMakeFiles/wallet.dir/rpcwallet.cpp.o
FAILED: src/wallet/CMakeFiles/wallet.dir/rpcwallet.cpp.o 
/usr/bin/cmake -E __run_co_compile --launcher=/usr/bin/ccache --tidy="/usr/bin/clang-tidy-12;-warnings-as-errors=*;--extra-arg-before=--driver-mode=g++" --source=../../src/wallet/rpcwallet.cpp -- /usr/bin/clang++ -DBOOST_ALL_NO_LIB -DBUILD_BITCOIN_INTERNAL -DENABLE_AVX2 -DENABLE_SHANI -DENABLE_SSE41 -DHAVE_BUILD_INFO -DHAVE_CONFIG_H -DHAVE_CONSENSUS_LIB -DLEVELDB_ATOMIC_PRESENT -DLEVELDB_PLATFORM_POSIX -DOS_LINUX -I../../src/. -Isrc -I../../src/univalue/include -Isrc/crypto/.. -I../../src/secp256k1/include -I../../src/leveldb/include -isystem /usr/include/jemalloc -isystem /usr/include/miniupnpc -g -O2 -fPIC -fvisibility=hidden -fstack-protector-all -Wstack-protector -fcf-protection=full -fstack-clash-protection -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=2 -Wall -Wextra -Wformat -Wgnu -Wvla -Wcast-align -Wunused-parameter -Wmissing-braces -Wthread-safety -Wrange-loop-analysis -Wredundant-decls -Wunreachable-code-loop-increment -Wsign-compare -Wconditional-uninitialized -Wdocumentation -Wformat-security -Wredundant-move -Woverloaded-virtual -Wshadow -Wshadow-field -Wno-unused-parameter -Wno-implicit-fallthrough -Wno-psabi -pthread -std=gnu++17 -MD -MT src/wallet/CMakeFiles/wallet.dir/rpcwallet.cpp.o -MF src/wallet/CMakeFiles/wallet.dir/rpcwallet.cpp.o.d -o src/wallet/CMakeFiles/wallet.dir/rpcwallet.cpp.o -c ../../src/wallet/rpcwallet.cpp
/work/abc-ci-builds/build-clang-tidy/../../src/wallet/rpcwallet.cpp:3468:57: error: argument name 'default' in comment does not match parameter name 'fallback' [bugprone-argument-comment,-warnings-as-errors]
                 {"include_unsafe", RPCArg::Type::BOOL, /*default=*/"false",
                                                        ^
../../src/./rpc/util.h:198:68: note: 'fallback' declared here
    RPCArg(const std::string name, const Type type, const Fallback fallback,
                                                                   ^
3440 warnings generated.
Suppressed 3438 warnings (3438 in non-user code).
Use -header-filter=.* to display errors from all non-system headers. Use -system-headers to display errors from system headers as well.
1 warning treated as error
[391/538] Building CXX object src/wallet/CMakeFiles/wallet-tool.dir/wallettool.cpp.o
[392/538] Building CXX object src/wallet/CMakeFiles/wallet.dir/fees.cpp.o
[393/538] Building CXX object src/CMakeFiles/server.dir/validation.cpp.o
[394/538] Building CXX object src/wallet/CMakeFiles/wallet.dir/load.cpp.o
[395/538] Building CXX object src/wallet/CMakeFiles/wallet.dir/transaction.cpp.o
[396/538] Building CXX object src/wallet/CMakeFiles/wallet.dir/rpc/signmessage.cpp.o
[397/538] Building CXX object src/wallet/CMakeFiles/wallet.dir/receive.cpp.o
[398/538] Building CXX object src/wallet/CMakeFiles/wallet.dir/sqlite.cpp.o
[399/538] Building CXX object src/wallet/CMakeFiles/wallet.dir/rpc/util.cpp.o
[400/538] Building CXX object src/seeder/CMakeFiles/seeder-base.dir/dns.cpp.o
[401/538] Building CXX object src/wallet/CMakeFiles/wallet.dir/rpc/encrypt.cpp.o
[402/538] Building CXX object src/wallet/CMakeFiles/wallet.dir/salvage.cpp.o
[403/538] Building CXX object src/seeder/CMakeFiles/seeder-base.dir/options.cpp.o
[404/538] Building CXX object src/seeder/CMakeFiles/seeder-base.dir/db.cpp.o
[405/538] Building CXX object src/wallet/CMakeFiles/wallet.dir/walletutil.cpp.o
[406/538] Building CXX object src/wallet/CMakeFiles/wallet.dir/interfaces.cpp.o
[407/538] Building CXX object src/seeder/CMakeFiles/bitcoin-seeder.dir/main.cpp.o
[408/538] Building CXX object src/wallet/CMakeFiles/wallet.dir/spend.cpp.o
[409/538] Building CXX object src/seeder/CMakeFiles/seeder-base.dir/bitcoin.cpp.o
[410/538] Building CXX object src/wallet/CMakeFiles/wallet.dir/scriptpubkeyman.cpp.o
[411/538] Building CXX object src/wallet/CMakeFiles/wallet.dir/rpc/backup.cpp.o
[412/538] Building CXX object src/wallet/CMakeFiles/wallet.dir/walletdb.cpp.o
[413/538] Building CXX object src/wallet/CMakeFiles/wallet.dir/wallet.cpp.o
ninja: build stopped: cannot make progress due to previous errors.
Build build-clang-tidy failed with exit code 1

Tail of the build log:

[368/538] Building CXX object src/test/CMakeFiles/testutil.dir/util/str.cpp.o
[369/538] Building CXX object src/CMakeFiles/server.dir/txorphanage.cpp.o
[370/538] Building CXX object src/CMakeFiles/server.dir/torcontrol.cpp.o
[371/538] Building CXX object src/CMakeFiles/server.dir/rpc/rawtransaction.cpp.o
[372/538] Building CXX object src/CMakeFiles/server.dir/txmempool.cpp.o
[373/538] Building CXX object src/CMakeFiles/server.dir/rpc/blockchain.cpp.o
[374/538] Building CXX object src/test/CMakeFiles/testutil.dir/util/blockfilter.cpp.o
[375/538] Building CXX object src/wallet/CMakeFiles/wallet.dir/context.cpp.o
[376/538] Building CXX object src/test/CMakeFiles/testutil.dir/util/transaction_utils.cpp.o
[377/538] Building CXX object src/CMakeFiles/bitcoin-tx.dir/bitcoin-tx.cpp.o
[378/538] Linking CXX executable src/bitcoin-tx
[379/538] Building CXX object src/test/CMakeFiles/testutil.dir/util/net.cpp.o
[380/538] Building CXX object src/wallet/CMakeFiles/wallet.dir/coincontrol.cpp.o
[381/538] Building CXX object src/CMakeFiles/server.dir/wallet/init.cpp.o
[382/538] Building CXX object src/test/CMakeFiles/testutil.dir/util/validation.cpp.o
[383/538] Building CXX object src/test/CMakeFiles/testutil.dir/util/mining.cpp.o
[384/538] Building CXX object src/wallet/CMakeFiles/wallet.dir/crypter.cpp.o
[385/538] Building CXX object src/wallet/CMakeFiles/wallet.dir/db.cpp.o
[386/538] Building CXX object src/wallet/CMakeFiles/wallet.dir/coinselection.cpp.o
[387/538] Building CXX object src/wallet/CMakeFiles/wallet.dir/bdb.cpp.o
[388/538] Building CXX object src/test/CMakeFiles/testutil.dir/util/wallet.cpp.o
[389/538] Building CXX object src/test/CMakeFiles/testutil.dir/util/setup_common.cpp.o
[390/538] Building CXX object src/wallet/CMakeFiles/wallet.dir/rpcwallet.cpp.o
FAILED: src/wallet/CMakeFiles/wallet.dir/rpcwallet.cpp.o 
/usr/bin/cmake -E __run_co_compile --launcher=/usr/bin/ccache --tidy="/usr/bin/clang-tidy-12;-warnings-as-errors=*;--extra-arg-before=--driver-mode=g++" --source=../../src/wallet/rpcwallet.cpp -- /usr/bin/clang++ -DBOOST_ALL_NO_LIB -DBUILD_BITCOIN_INTERNAL -DENABLE_AVX2 -DENABLE_SHANI -DENABLE_SSE41 -DHAVE_BUILD_INFO -DHAVE_CONFIG_H -DHAVE_CONSENSUS_LIB -DLEVELDB_ATOMIC_PRESENT -DLEVELDB_PLATFORM_POSIX -DOS_LINUX -I../../src/. -Isrc -I../../src/univalue/include -Isrc/crypto/.. -I../../src/secp256k1/include -I../../src/leveldb/include -isystem /usr/include/jemalloc -isystem /usr/include/miniupnpc -g -O2 -fPIC -fvisibility=hidden -fstack-protector-all -Wstack-protector -fcf-protection=full -fstack-clash-protection -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=2 -Wall -Wextra -Wformat -Wgnu -Wvla -Wcast-align -Wunused-parameter -Wmissing-braces -Wthread-safety -Wrange-loop-analysis -Wredundant-decls -Wunreachable-code-loop-increment -Wsign-compare -Wconditional-uninitialized -Wdocumentation -Wformat-security -Wredundant-move -Woverloaded-virtual -Wshadow -Wshadow-field -Wno-unused-parameter -Wno-implicit-fallthrough -Wno-psabi -pthread -std=gnu++17 -MD -MT src/wallet/CMakeFiles/wallet.dir/rpcwallet.cpp.o -MF src/wallet/CMakeFiles/wallet.dir/rpcwallet.cpp.o.d -o src/wallet/CMakeFiles/wallet.dir/rpcwallet.cpp.o -c ../../src/wallet/rpcwallet.cpp
/work/abc-ci-builds/build-clang-tidy/../../src/wallet/rpcwallet.cpp:3468:57: error: argument name 'default' in comment does not match parameter name 'fallback' [bugprone-argument-comment,-warnings-as-errors]
                 {"include_unsafe", RPCArg::Type::BOOL, /*default=*/"false",
                                                        ^
../../src/./rpc/util.h:198:68: note: 'fallback' declared here
    RPCArg(const std::string name, const Type type, const Fallback fallback,
                                                                   ^
3440 warnings generated.
Suppressed 3438 warnings (3438 in non-user code).
Use -header-filter=.* to display errors from all non-system headers. Use -system-headers to display errors from system headers as well.
1 warning treated as error
[391/538] Building CXX object src/wallet/CMakeFiles/wallet-tool.dir/wallettool.cpp.o
[392/538] Building CXX object src/wallet/CMakeFiles/wallet.dir/fees.cpp.o
[393/538] Building CXX object src/wallet/CMakeFiles/wallet.dir/transaction.cpp.o
[394/538] Building CXX object src/wallet/CMakeFiles/wallet.dir/load.cpp.o
[395/538] Building CXX object src/wallet/CMakeFiles/wallet.dir/receive.cpp.o
[396/538] Building CXX object src/CMakeFiles/server.dir/validation.cpp.o
[397/538] Building CXX object src/wallet/CMakeFiles/wallet.dir/rpc/signmessage.cpp.o
[398/538] Building CXX object src/wallet/CMakeFiles/wallet.dir/rpc/util.cpp.o
[399/538] Building CXX object src/wallet/CMakeFiles/wallet.dir/sqlite.cpp.o
[400/538] Building CXX object src/wallet/CMakeFiles/wallet.dir/walletutil.cpp.o
[401/538] Building CXX object src/wallet/CMakeFiles/wallet.dir/rpc/encrypt.cpp.o
[402/538] Building CXX object src/seeder/CMakeFiles/seeder-base.dir/dns.cpp.o
[403/538] Building CXX object src/seeder/CMakeFiles/seeder-base.dir/db.cpp.o
[404/538] Building CXX object src/wallet/CMakeFiles/wallet.dir/salvage.cpp.o
[405/538] Building CXX object src/seeder/CMakeFiles/seeder-base.dir/options.cpp.o
[406/538] Building CXX object src/wallet/CMakeFiles/wallet.dir/interfaces.cpp.o
[407/538] Building CXX object src/wallet/CMakeFiles/wallet.dir/spend.cpp.o
[408/538] Building CXX object src/seeder/CMakeFiles/bitcoin-seeder.dir/main.cpp.o
[409/538] Building CXX object src/seeder/CMakeFiles/seeder-base.dir/bitcoin.cpp.o
[410/538] Building CXX object src/wallet/CMakeFiles/wallet.dir/scriptpubkeyman.cpp.o
[411/538] Building CXX object src/wallet/CMakeFiles/wallet.dir/rpc/backup.cpp.o
[412/538] Building CXX object src/wallet/CMakeFiles/wallet.dir/walletdb.cpp.o
[413/538] Building CXX object src/wallet/CMakeFiles/wallet.dir/wallet.cpp.o
ninja: build stopped: cannot make progress due to previous errors.
Build build-clang-tidy failed with exit code 1

standardize parameter name comments

src/wallet/rpcwallet.cpp
3468 ↗(On Diff #41917)

https://github.com/bitcoin/bitcoin/pull/21679 will fix all the /* default */ comments (which should really be `/*fallback=*/)

Fabien requested changes to this revision.Aug 24 2023, 12:27
Fabien added a subscriber: Fabien.
Fabien added inline comments.
doc/release-notes.md
9 ↗(On Diff #41917)

Please add the next sentence too:
Note that the resulting transaction may become invalid if one of the unsafe inputs disappears.

src/wallet/rpcwallet.cpp
3470 ↗(On Diff #41917)

dito please include the warning

4268 ↗(On Diff #41917)

dito

4733 ↗(On Diff #41917)

dito

src/wallet/spend.cpp
587 ↗(On Diff #41917)

Please also add the comment above. Core version of this function is much more readable due to our lack of comments

This revision now requires changes to proceed.Aug 24 2023, 12:27

add warnings and comments

Tail of the build log:

wallet_keypool.py                         | ○ Skipped | 0 s
wallet_keypool_topup.py                   | ○ Skipped | 0 s
wallet_keypool_topup.py --descriptors     | ○ Skipped | 0 s
wallet_labels.py                          | ○ Skipped | 0 s
wallet_labels.py --descriptors            | ○ Skipped | 0 s
wallet_listreceivedby.py                  | ○ Skipped | 0 s
wallet_listsinceblock.py                  | ○ Skipped | 0 s
wallet_listsinceblock.py --descriptors    | ○ Skipped | 0 s
wallet_listtransactions.py                | ○ Skipped | 0 s
wallet_listtransactions.py --descriptors  | ○ Skipped | 0 s
wallet_multiwallet.py                     | ○ Skipped | 0 s
wallet_multiwallet.py --usecli            | ○ Skipped | 0 s
wallet_reorgsrestore.py                   | ○ Skipped | 0 s
wallet_resendwallettransactions.py        | ○ Skipped | 0 s
wallet_send.py                            | ○ Skipped | 0 s
wallet_startup.py                         | ○ Skipped | 0 s
wallet_timelock.py                        | ○ Skipped | 0 s
wallet_txn_clone.py                       | ○ Skipped | 0 s
wallet_txn_clone.py --mineblock           | ○ Skipped | 0 s
wallet_txn_doublespend.py                 | ○ Skipped | 0 s
wallet_txn_doublespend.py --mineblock     | ○ Skipped | 0 s
wallet_watchonly.py                       | ○ Skipped | 0 s
wallet_watchonly.py --usecli              | ○ Skipped | 0 s

ALL                                       | ✓ Passed  | 808 s (accumulated) 
Runtime: 162 s

[31/447] Test Bitcoin RPC authentication...
...
----------------------------------------------------------------------
Ran 3 tests in 0.004s

OK
[32/447] cd /work/contrib/devtools/chainparams && /usr/bin/python3.9 ./test_make_chainparams.py
.....
----------------------------------------------------------------------
Ran 5 tests in 0.001s

OK
[192/447] Running seeder test suite
PASSED: seeder test suite
[194/447] avalanche: testing processor_tests
FAILED: src/avalanche/test/CMakeFiles/check-avalanche-processor_tests 
cd /work/abc-ci-builds/build-without-wallet/src/avalanche/test && /usr/bin/cmake -E make_directory /work/abc-ci-builds/build-without-wallet/test/junit && /usr/bin/cmake -E make_directory /work/abc-ci-builds/build-without-wallet/test/log && /usr/bin/cmake -E env /work/cmake/utils/log-and-print-on-failure.sh /work/abc-ci-builds/build-without-wallet/test/log/avalanche-processor_tests.log /work/abc-ci-builds/build-without-wallet/src/avalanche/test/test-avalanche --run_test=processor_tests --logger=HRF,message:JUNIT,message,avalanche-processor_tests.xml --catch_system_errors=no
Segmentation fault (core dumped)
[199/447] Running pow test suite
PASSED: pow test suite
[206/447] Running bitcoin-qt test suite
PASSED: bitcoin-qt test suite
[208/447] Building CXX object src/test/CMakeFiles/test_bitcoin.dir/script_tests.cpp.o
In file included from /usr/include/boost/test/unit_test.hpp:19,
                 from ../../src/test/script_tests.cpp:30:
../../src/test/script_tests.cpp: In member function ‘void script_tests::script_build::test_method()’:
../../src/test/script_tests.cpp:540:22: note: variable tracking size limit exceeded with ‘-fvar-tracking-assignments’, retrying without
  540 | BOOST_AUTO_TEST_CASE(script_build) {
      |                      ^~~~~~~~~~~~
[444/447] Running bitcoin test suite
PASSED: bitcoin test suite
ninja: build stopped: cannot make progress due to previous errors.
Build build-without-wallet failed with exit code 1
This revision is now accepted and ready to land.Aug 24 2023, 15:39
This revision was landed with ongoing or failed builds.Aug 24 2023, 15:53
This revision was automatically updated to reflect the committed changes.