rpc: [wallet] Use unsigned type for tx version in sendall #34135

1. 
   maflcko commented at 10:10 am on December 22, 2025: member

   It is confusing to parse the unsigned tx version as a signed type. Also, it makes it harder to use the integer sanitizer.

   Can be tested via:

   • Build with the flags -DCMAKE_C_COMPILER='clang' -DCMAKE_CXX_COMPILER='clang++' -DSANITIZERS=undefined,integer,float-divide-by-zero
   • Set the existing suppressions: export UBSAN_OPTIONS="suppressions=$(pwd)/test/sanitizer_suppressions/ubsan:print_stacktrace=1:halt_on_error=0:report_error_type=1"
   • Start the RPC server, e.g. ./bld-cmake/bin/bitcoin-qt -datadir=/tmp -regtest -server
   • Call the sendall RPC, e.g. ./bld-cmake/bin/bitcoin-cli -datadir=/tmp -regtest -named sendall '["bcrt1qlrt3xps4wxpfcjmljrayr2ualczmnfvd4vzdq3"]' fee_rate=1.234 version=-1

   Before:

   0src/wallet/rpc/spend.cpp:1470:42: runtime error: implicit conversion from type 'int' of value -1 (32-bit, signed) to type 'uint32_t' (aka 'unsigned int') changed the value to 4294967295 (32-bit, unsigned)
   1
   2Invalid parameter, version out of range(1~3)
   

   After:

   0JSON integer out of range
   

2. rpc: [wallet] Use unsigned type for tx version in sendall fafbc70d48
3. DrahtBot added the label RPC/REST/ZMQ on Dec 22, 2025
4. 
   DrahtBot commented at 10:11 am on December 22, 2025: contributor

   The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

   Code Coverage & Benchmarks

   For details see: https://corecheck.dev/bitcoin/bitcoin/pulls/34135.

   Reviews

   See the guideline for information on the review process.

   Type	Reviewers
   ACK	rkrux, bensig, theStack, achow101

   If your review is incorrectly listed, please copy-paste <!–meta-tag:bot-skip–> into the comment that the bot should ignore.

5. maflcko added the label Wallet on Dec 22, 2025
6. 
   brunoerg commented at 8:15 pm on December 23, 2025: contributor

   Code lgtm. I just think that the current message “Invalid parameter, version out of range” is better then a simple “JSON integer out of range”.

   I followed the steps to reproduce the issue/fix and worked fine. Before calling sendall, we have to create or load a wallet, this isn’t in the steps btw.

   master:

    0/home/bruno/projects/bitcoin/src/wallet/rpc/spend.cpp:1470:42: runtime error: implicit conversion from type 'int' of value -1 (32-bit, signed) to type 'uint32_t' (aka 'unsigned int') changed the value to 4294967295 (32-bit, unsigned)
    1    [#0](/bitcoin-bitcoin/0/) 0x60f26e95ee11 in wallet::sendall()::$_0::operator()(RPCHelpMan const&, JSONRPCRequest const&) const /home/bruno/projects/bitcoin/build_34135/src/wallet/./wallet/rpc/spend.cpp:1470:42
    2    [#1](/bitcoin-bitcoin/1/) 0x60f26e95aa49 in UniValue std::__invoke_impl<UniValue, wallet::sendall()::$_0&, RPCHelpMan const&, JSONRPCRequest const&>(std::__invoke_other, wallet::sendall()::$_0&, RPCHelpMan const&, JSONRPCRequest const&) /usr/lib/gcc/x86_64-linux-gnu/13/../../../../include/c++/13/bits/invoke.h:61:14
    3    [#2](/bitcoin-bitcoin/2/) 0x60f26e95aa49 in std::enable_if<is_invocable_r_v<UniValue, wallet::sendall()::$_0&, RPCHelpMan const&, JSONRPCRequest const&>, UniValue>::type std::__invoke_r<UniValue, wallet::sendall()::$_0&, RPCHelpMan const&, JSONRPCRequest const&>(wallet::sendall()::$_0&, RPCHelpMan const&, JSONRPCRequest const&) /usr/lib/gcc/x86_64-linux-gnu/13/../../../../include/c++/13/bits/invoke.h:114:9
    4    [#3](/bitcoin-bitcoin/3/) 0x60f26e95aa49 in std::_Function_handler<UniValue (RPCHelpMan const&, JSONRPCRequest const&), wallet::sendall()::$_0>::_M_invoke(std::_Any_data const&, RPCHelpMan const&, JSONRPCRequest const&) /usr/lib/gcc/x86_64-linux-gnu/13/../../../../include/c++/13/bits/std_function.h:290:9
    5    [#4](/bitcoin-bitcoin/4/) 0x60f26eb32b61 in std::function<UniValue (RPCHelpMan const&, JSONRPCRequest const&)>::operator()(RPCHelpMan const&, JSONRPCRequest const&) const /usr/lib/gcc/x86_64-linux-gnu/13/../../../../include/c++/13/bits/std_function.h:591:9
    6    [#5](/bitcoin-bitcoin/5/) 0x60f26eb32b61 in RPCHelpMan::HandleRequest(JSONRPCRequest const&) const /home/bruno/projects/bitcoin/build_34135/src/./rpc/util.cpp:660:20
    7    [#6](/bitcoin-bitcoin/6/) 0x60f26e2d88b1 in CRPCCommand::CRPCCommand(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char>>, RPCHelpMan (*)())::'lambda'(JSONRPCRequest const&, UniValue&, bool)::operator()(JSONRPCRequest const&, UniValue&, bool) const /home/bruno/projects/bitcoin/build_34135/src/./rpc/server.h:61:91
    8    [#7](/bitcoin-bitcoin/7/) 0x60f26e6b01b3 in std::function<bool (JSONRPCRequest const&, UniValue&, bool)>::operator()(JSONRPCRequest const&, UniValue&, bool) const /usr/lib/gcc/x86_64-linux-gnu/13/../../../../include/c++/13/bits/std_function.h:591:9
    9    [#8](/bitcoin-bitcoin/8/) 0x60f26e6b01b3 in wallet::(anonymous namespace)::WalletLoaderImpl::registerRpcs()::'lambda'(JSONRPCRequest const&, UniValue&, bool)::operator()(JSONRPCRequest const&, UniValue&, bool) const /home/bruno/projects/bitcoin/build_34135/src/wallet/./wallet/interfaces.cpp:553:24
   10    [#9](/bitcoin-bitcoin/9/) 0x60f26e6b01b3 in bool std::__invoke_impl<bool, wallet::(anonymous namespace)::WalletLoaderImpl::registerRpcs()::'lambda'(JSONRPCRequest const&, UniValue&, bool)&, JSONRPCRequest const&, UniValue&, bool>(std::__invoke_other, wallet::(anonymous namespace)::WalletLoaderImpl::registerRpcs()::'lambda'(JSONRPCRequest const&, UniValue&, bool)&, JSONRPCRequest const&, UniValue&, bool&&) /usr/lib/gcc/x86_64-linux-gnu/13/../../../../include/c++/13/bits/invoke.h:61:14
   11    [#10](/bitcoin-bitcoin/10/) 0x60f26e6b01b3 in std::enable_if<is_invocable_r_v<bool, wallet::(anonymous namespace)::WalletLoaderImpl::registerRpcs()::'lambda'(JSONRPCRequest const&, UniValue&, bool)&, JSONRPCRequest const&, UniValue&, bool>, bool>::type std::__invoke_r<bool, wallet::(anonymous namespace)::WalletLoaderImpl::registerRpcs()::'lambda'(JSONRPCRequest const&, UniValue&, bool)&, JSONRPCRequest const&, UniValue&, bool>(wallet::(anonymous namespace)::WalletLoaderImpl::registerRpcs()::'lambda'(JSONRPCRequest const&, UniValue&, bool)&, JSONRPCRequest const&, UniValue&, bool&&) /usr/lib/gcc/x86_64-linux-gnu/13/../../../../include/c++/13/bits/invoke.h:114:9
   12    [#11](/bitcoin-bitcoin/11/) 0x60f26e6b01b3 in std::_Function_handler<bool (JSONRPCRequest const&, UniValue&, bool), wallet::(anonymous namespace)::WalletLoaderImpl::registerRpcs()::'lambda'(JSONRPCRequest const&, UniValue&, bool)>::_M_invoke(std::_Any_data const&, JSONRPCRequest const&, UniValue&, bool&&) /usr/lib/gcc/x86_64-linux-gnu/13/../../../../include/c++/13/bits/std_function.h:290:9
   13    [#12](/bitcoin-bitcoin/12/) 0x60f26e17a9a7 in std::function<bool (JSONRPCRequest const&, UniValue&, bool)>::operator()(JSONRPCRequest const&, UniValue&, bool) const /usr/lib/gcc/x86_64-linux-gnu/13/../../../../include/c++/13/bits/std_function.h:591:9
   14    [#13](/bitcoin-bitcoin/13/) 0x60f26e17a9a7 in node::(anonymous namespace)::RpcHandlerImpl::RpcHandlerImpl(CRPCCommand const&)::'lambda'(JSONRPCRequest const&, UniValue&, bool)::operator()(JSONRPCRequest const&, UniValue&, bool) const /home/bruno/projects/bitcoin/build_34135/src/./node/interfaces.cpp:512:24
   15    [#14](/bitcoin-bitcoin/14/) 0x60f26e17a9a7 in bool std::__invoke_impl<bool, node::(anonymous namespace)::RpcHandlerImpl::RpcHandlerImpl(CRPCCommand const&)::'lambda'(JSONRPCRequest const&, UniValue&, bool)&, JSONRPCRequest const&, UniValue&, bool>(std::__invoke_other, node::(anonymous namespace)::RpcHandlerImpl::RpcHandlerImpl(CRPCCommand const&)::'lambda'(JSONRPCRequest const&, UniValue&, bool)&, JSONRPCRequest const&, UniValue&, bool&&) /usr/lib/gcc/x86_64-linux-gnu/13/../../../../include/c++/13/bits/invoke.h:61:14
   16    [#15](/bitcoin-bitcoin/15/) 0x60f26e17a9a7 in std::enable_if<is_invocable_r_v<bool, node::(anonymous namespace)::RpcHandlerImpl::RpcHandlerImpl(CRPCCommand const&)::'lambda'(JSONRPCRequest const&, UniValue&, bool)&, JSONRPCRequest const&, UniValue&, bool>, bool>::type std::__invoke_r<bool, node::(anonymous namespace)::RpcHandlerImpl::RpcHandlerImpl(CRPCCommand const&)::'lambda'(JSONRPCRequest const&, UniValue&, bool)&, JSONRPCRequest const&, UniValue&, bool>(node::(anonymous namespace)::RpcHandlerImpl::RpcHandlerImpl(CRPCCommand const&)::'lambda'(JSONRPCRequest const&, UniValue&, bool)&, JSONRPCRequest const&, UniValue&, bool&&) /usr/lib/gcc/x86_64-linux-gnu/13/../../../../include/c++/13/bits/invoke.h:114:9
   17    [#16](/bitcoin-bitcoin/16/) 0x60f26e17a9a7 in std::_Function_handler<bool (JSONRPCRequest const&, UniValue&, bool), node::(anonymous namespace)::RpcHandlerImpl::RpcHandlerImpl(CRPCCommand const&)::'lambda'(JSONRPCRequest const&, UniValue&, bool)>::_M_invoke(std::_Any_data const&, JSONRPCRequest const&, UniValue&, bool&&) /usr/lib/gcc/x86_64-linux-gnu/13/../../../../include/c++/13/bits/std_function.h:290:9
   18    [#17](/bitcoin-bitcoin/17/) 0x60f26e460740 in std::function<bool (JSONRPCRequest const&, UniValue&, bool)>::operator()(JSONRPCRequest const&, UniValue&, bool) const /usr/lib/gcc/x86_64-linux-gnu/13/../../../../include/c++/13/bits/std_function.h:591:9
   19    [#18](/bitcoin-bitcoin/18/) 0x60f26e460740 in ExecuteCommand(CRPCCommand const&, JSONRPCRequest const&, UniValue&, bool) /home/bruno/projects/bitcoin/build_34135/src/./rpc/server.cpp:508:20
   20    [#19](/bitcoin-bitcoin/19/) 0x60f26e460740 in ExecuteCommands(std::vector<CRPCCommand const*, std::allocator<CRPCCommand const*>> const&, JSONRPCRequest const&, UniValue&) /home/bruno/projects/bitcoin/build_34135/src/./rpc/server.cpp:475:13
   21    [#20](/bitcoin-bitcoin/20/) 0x60f26e45f0c0 in CRPCTable::execute(JSONRPCRequest const&) const /home/bruno/projects/bitcoin/build_34135/src/./rpc/server.cpp:495:13
   22    [#21](/bitcoin-bitcoin/21/) 0x60f26e45e7d4 in JSONRPCExec(JSONRPCRequest const&, bool) /home/bruno/projects/bitcoin/build_34135/src/./rpc/server.cpp:351:31
   23    [#22](/bitcoin-bitcoin/22/) 0x60f26e640cff in HTTPReq_JSONRPC(std::any const&, HTTPRequest*) /home/bruno/projects/bitcoin/build_34135/src/./httprpc.cpp:165:21
   24    [#23](/bitcoin-bitcoin/23/) 0x60f26e654579 in std::function<bool (HTTPRequest*, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char>> const&)>::operator()(HTTPRequest*, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char>> const&) const /usr/lib/gcc/x86_64-linux-gnu/13/../../../../include/c++/13/bits/std_function.h:591:9
   25    [#24](/bitcoin-bitcoin/24/) 0x60f26e654579 in HTTPWorkItem::operator()() /home/bruno/projects/bitcoin/build_34135/src/./httpserver.cpp:62:9
   26    [#25](/bitcoin-bitcoin/25/) 0x60f26e658386 in WorkQueue<HTTPClosure>::Run() /home/bruno/projects/bitcoin/build_34135/src/./httpserver.cpp:117:13
   27    [#26](/bitcoin-bitcoin/26/) 0x60f26e64c5e9 in HTTPWorkQueueRun(WorkQueue<HTTPClosure>*, int) /home/bruno/projects/bitcoin/build_34135/src/./httpserver.cpp:419:12
   28    [#27](/bitcoin-bitcoin/27/) 0x730a73aecdb3 in execute_native_thread_routine /build/gcc-14-ig5ci0/gcc-14-14.2.0/build/x86_64-linux-gnu/libstdc++-v3/src/c++11/../../../../../src/libstdc++-v3/src/c++11/thread.cc:104:18
   29    [#28](/bitcoin-bitcoin/28/) 0x730a7369caa3 in start_thread nptl/pthread_create.c:447:8
   30    [#29](/bitcoin-bitcoin/29/) 0x730a73729c6b in clone3 misc/../sysdeps/unix/sysv/linux/x86_64/clone3.S:78
   31
   32SUMMARY: UndefinedBehaviorSanitizer: implicit-integer-sign-change /home/bruno/projects/bitcoin/src/wallet/rpc/spend.cpp:1470:42
   

   0bruno@bruno-MS-Challenger-B850M-PLUS:~/projects/bitcoin(master)$ ./build_34135/bin/bitcoin-cli -datadir=/tmp -regtest -named sendall '["bcrt1qlrt3xps4wxpfcjmljrayr2ualczmnfvd4vzdq3"]' fee_rate=1.234  version=-1
   1/usr/lib/gcc/x86_64-linux-gnu/13/../../../../include/c++/13/bits/basic_string.h:490:51: runtime error: unsigned integer overflow: 7 - 10 cannot be represented in type 'size_type' (aka 'unsigned long')
   2SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior /usr/lib/gcc/x86_64-linux-gnu/13/../../../../include/c++/13/bits/basic_string.h:490:51 
   3/usr/lib/gcc/x86_64-linux-gnu/13/../../../../include/c++/13/bits/basic_string.h:490:51: runtime error: unsigned integer overflow: 8 - 9 cannot be represented in type 'size_type' (aka 'unsigned long')
   4SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior /usr/lib/gcc/x86_64-linux-gnu/13/../../../../include/c++/13/bits/basic_string.h:490:51 
   5/usr/lib/gcc/x86_64-linux-gnu/13/../../../../include/c++/13/bits/string_view.tcc:124:25: runtime error: unsigned integer overflow: 0 - 1 cannot be represented in type 'size_type' (aka 'unsigned long')
   6SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior /usr/lib/gcc/x86_64-linux-gnu/13/../../../../include/c++/13/bits/string_view.tcc:124:25 
   7error code: -8
   8error message:
   9Invalid parameter, version out of range(1~3)
   

   ---

   This PR:

   0bruno@bruno-MS-Challenger-B850M-PLUS:~/projects/bitcoin(34135)$ ./build_34135/bin/bitcoin-cli -datadir=/tmp -regtest -named sendall '["bcrt1qlrt3xps4wxpfcjmljrayr2ualczmnfvd4vzdq3"]' fee_rate=1.234  version=-1
   1/usr/lib/gcc/x86_64-linux-gnu/13/../../../../include/c++/13/bits/basic_string.h:490:51: runtime error: unsigned integer overflow: 7 - 10 cannot be represented in type 'size_type' (aka 'unsigned long')
   2SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior /usr/lib/gcc/x86_64-linux-gnu/13/../../../../include/c++/13/bits/basic_string.h:490:51 
   3/usr/lib/gcc/x86_64-linux-gnu/13/../../../../include/c++/13/bits/basic_string.h:490:51: runtime error: unsigned integer overflow: 8 - 9 cannot be represented in type 'size_type' (aka 'unsigned long')
   4SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior /usr/lib/gcc/x86_64-linux-gnu/13/../../../../include/c++/13/bits/basic_string.h:490:51 
   5/usr/lib/gcc/x86_64-linux-gnu/13/../../../../include/c++/13/bits/string_view.tcc:124:25: runtime error: unsigned integer overflow: 0 - 1 cannot be represented in type 'size_type' (aka 'unsigned long')
   6SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior /usr/lib/gcc/x86_64-linux-gnu/13/../../../../include/c++/13/bits/string_view.tcc:124:25 
   7error code: -1
   8error message:
   9JSON integer out of range
   

7. achow101 requested review from achow101 on Dec 23, 2025
8. rkrux approved
9. 
   rkrux commented at 3:10 pm on December 24, 2025: contributor

   utACK fafbc70d48e1fb42c4ed3da609e5f30c4cc39418

   I just think that the current message “Invalid parameter, version out of range” is better then a simple “JSON integer out of range”.

   The previous error does read better but it displays the error for the implicitly converted value and not the actual value that was passed in the request, which is now correctly handled by this change, i.e. before ConstructTransaction is called where the version value range checks occur.

   The semantic correctness added by this change has convinced me to ack this PR, even though with some loss in error readability. One way I can think of that might improve the overall error to some extent is by the following diff:

    0diff --git a/src/univalue/include/univalue.h b/src/univalue/include/univalue.h
    1index bc71677bab..217ee11224 100644
    2--- a/src/univalue/include/univalue.h
    3+++ b/src/univalue/include/univalue.h
    4@@ -144,7 +144,7 @@ Int UniValue::getInt() const
    5     Int result;
    6     const auto [first_nonmatching, error_condition] = std::from_chars(val.data(), val.data() + val.size(), result);
    7     if (first_nonmatching != val.data() + val.size() || error_condition != std::errc{}) {
    8-        throw std::runtime_error("JSON integer out of range");
    9+        throw type_error("JSON integer out of range");
   10     }
   11     return result;
   12 }
   

   This gives the marginal benefit of having a more relatable error code (from -1 to -3, though its usefulness to the end user is arguable), even though the message remains same. I don’t feel strongly about this suggestion though; I haven’t run all the tests and only tested with CLI once.

   0bitcoin git: ✗ bitcoinclireg -named sendall '["bcrt1qlrt3xps4wxpfcjmljrayr2ualczmnfvd4vzdq3"]' fee_rate=1.234  version=-1
   1error code: -3
   2error message:
   3JSON integer out of range
   

10. 
    bensig commented at 5:42 am on January 5, 2026: contributor

    ACK fafbc70d48e1fb42c4ed3da609e5f30c4cc39418

    Using decltype(coin_control.m_version) instead of int correctly matches the target type and avoids the signed-to-unsigned conversion warning.

11. theStack approved
12. 
    theStack commented at 11:58 am on January 5, 2026: contributor

    ACK fafbc70d48e1fb42c4ed3da609e5f30c4cc39418

    Tested with the instructions in the PR description (also had to create a wallet first, as noted by brunoerg above). I agree that this is the semantically correct approach. There is indeed a slight loss in error readability, but I doubt that this would matter much in practice, considering that negative versions generally don’t make sense.

13. 
    maflcko commented at 11:09 am on January 6, 2026: member

    Yeah, the error message could be clearer, but this should be an edge case here and seems a general issue with getInt, so ideally it is done in a follow-up
14. 
    achow101 commented at 11:38 pm on January 6, 2026: member

    ACK fafbc70d48e1fb42c4ed3da609e5f30c4cc39418
15. achow101 merged this on Jan 6, 2026
16. achow101 closed this on Jan 6, 2026

    ---

Contributors
maflcko DrahtBot brunoerg rkrux bensig theStack achow101

Review Requested
achow101

Labels
Wallet RPC/REST/ZMQ