Page MenuHomePhabricator
Differential D14403

rpc: include_unsafe option for fundrawtransaction
ClosedPublic
Actions

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

Event Timeline

PiRK created this revision.Aug 24 2023, 09:25
Herald added a reviewer: Restricted Project. · View Herald TranscriptAug 24 2023, 09:25
PiRK requested review of this revision.Aug 24 2023, 09:25
teamcity edited the summary of this revision. (Show Details)Aug 24 2023, 09:25
PiRK updated this revision to Diff 41916.Aug 24 2023, 09:28

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

Harbormaster completed remote builds in B24834: Diff 41915.Aug 24 2023, 09:35
teamcity added a comment.Aug 24 2023, 09:35
Build Bitcoin ABC Diffs / Diff Testing (build-clang-tidy) failed.

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
Harbormaster completed remote builds in B24835: Diff 41916.Aug 24 2023, 09:36
teamcity added a comment.Aug 24 2023, 09:36
Build Bitcoin ABC Diffs / Diff Testing (build-clang-tidy) failed.

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
PiRK updated this revision to Diff 41917.Aug 24 2023, 09:38

standardize parameter name comments

PiRK added inline comments.Aug 24 2023, 09:43
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=*/)

Harbormaster completed remote builds in B24836: Diff 41917.Aug 24 2023, 09:54
PiRK added a child revision: D14405: refactor: remove redundant fOnlySafe argument.Aug 24 2023, 11:59
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
PiRK updated this revision to Diff 41931.Aug 24 2023, 13:40

add warnings and comments

Harbormaster completed remote builds in B24845: Diff 41931.Aug 24 2023, 13:50
teamcity added a comment.Aug 24 2023, 13:50
Build Bitcoin ABC Diffs / Diff Testing (build-without-wallet) failed.

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
Fabien accepted this revision.Aug 24 2023, 15:39
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
Closed by commit rABC95c9e10a1da4: rpc: include_unsafe option for fundrawtransaction (authored by t-bast <bastuc@hotmail.fr>, committed by PiRK). · Explain Why
This revision was automatically updated to reflect the committed changes.
PiRK added a commit: rABC95c9e10a1da4: rpc: include_unsafe option for fundrawtransaction.

Revision Contents
Changeset List

PathSize
doc/
3 lines
src/
wallet/
2 lines
4 lines
30 lines
12 lines
test/
functional/
45 lines
19 lines
37 lines

Diff 41931

View Options

doc/release-notes.md

Loading...
View Options

src/wallet/coincontrol.h

Loading...
View Options

src/wallet/coinselection.h

Loading...
View Options

src/wallet/rpcwallet.cpp

Loading...
View Options

src/wallet/spend.cpp

Loading...
View Options

test/functional/rpc_fundrawtransaction.py

Loading...
View Options

test/functional/rpc_psbt.py

Loading...
View Options

test/functional/wallet_send.py

Loading...