diff options
| author | Andrew Chow <github@achow101.com> | 2022-09-15 13:22:10 -0400 |
|---|---|---|
| committer | Andrew Chow <github@achow101.com> | 2022-09-15 13:26:22 -0400 |
| commit | a56876e6b9dab4e0080cb8d7e9d0b4dd117f79a8 (patch) | |
| tree | 8d633626733f547b787fe8d420c1a1ac2e3f4242 | |
| parent | 96f1b2d34fd5a5f0ab0628caa20df6d5cc989dfd (diff) | |
| parent | cc434cbf583ec8d1b0f3aa504417231fdc81f33a (diff) | |
Merge bitcoin/bitcoin#26024: wallet: fix sendall creates tx that fails tx-size check
cc434cbf583ec8d1b0f3aa504417231fdc81f33a wallet: fix sendall creates tx that fails tx-size check (kouloumos)
Pull request description:
Fixes https://github.com/bitcoin/bitcoin/issues/26011
The `sendall` RPC doesn't use `CreateTransactionInternal` as the rest of
the wallet RPCs. [This has already been discussed in the original PR](https://github.com/bitcoin/bitcoin/pull/24118#issuecomment-1029462114).
By not going through that path, it never checks the transaction's weight
against the maximum tx weight for transactions we're willing to relay.
https://github.com/bitcoin/bitcoin/blob/447f50e4aed9a8b1d80e1891cda85801aeb80b4e/src/wallet/spend.cpp#L1013-L1018
This PR adds a check for tx-size as well as test coverage for that case.
_Note: It seems that the test takes a bit of time on slower machines,
I'm not sure if dropping it might be for the better._
ACKs for top commit:
glozow:
re ACK cc434cb via range-diff. Changes were addressing https://github.com/bitcoin/bitcoin/pull/26024#discussion_r971325299 and https://github.com/bitcoin/bitcoin/pull/26024#discussion_r970651614.
achow101:
ACK cc434cbf583ec8d1b0f3aa504417231fdc81f33a
w0xlt:
reACK https://github.com/bitcoin/bitcoin/pull/26024/commits/cc434cbf583ec8d1b0f3aa504417231fdc81f33a
Tree-SHA512: 64a1d8f2c737b39f3ccee90689eda1dd9c1b65f11b2c7bc0ec8bfe72f0202ce90ab4033bb0ecfc6080af8c947575059588a00938fe48e7fd553f7fb5ee03b3cc
| -rw-r--r-- | src/wallet/rpc/spend.cpp | 5 | ||||
| -rwxr-xr-x | test/functional/wallet_sendall.py | 17 |
2 files changed, 22 insertions, 0 deletions
diff --git a/src/wallet/rpc/spend.cpp b/src/wallet/rpc/spend.cpp index 68e392790a..e38b13624c 100644 --- a/src/wallet/rpc/spend.cpp +++ b/src/wallet/rpc/spend.cpp @@ -1414,6 +1414,11 @@ RPCHelpMan sendall() } } + // If this transaction is too large, e.g. because the wallet has many UTXOs, it will be rejected by the node's mempool. + if (tx_size.weight > MAX_STANDARD_TX_WEIGHT) { + throw JSONRPCError(RPC_WALLET_ERROR, "Transaction too large."); + } + CAmount output_amounts_claimed{0}; for (const CTxOut& out : rawTx.vout) { output_amounts_claimed += out.nValue; diff --git a/test/functional/wallet_sendall.py b/test/functional/wallet_sendall.py index f24329f7b3..db4f32fe16 100755 --- a/test/functional/wallet_sendall.py +++ b/test/functional/wallet_sendall.py @@ -276,6 +276,20 @@ class SendallTest(BitcoinTestFramework): recipients=[self.remainder_target], fee_rate=100000) + # This tests needs to be the last one otherwise @cleanup will fail with "Transaction too large" error + def sendall_fails_with_transaction_too_large(self): + self.log.info("Test that sendall fails if resulting transaction is too large") + # create many inputs + outputs = {self.wallet.getnewaddress(): 0.000025 for _ in range(1600)} + self.def_wallet.sendmany(amounts=outputs) + self.generate(self.nodes[0], 1) + + assert_raises_rpc_error( + -4, + "Transaction too large.", + self.wallet.sendall, + recipients=[self.remainder_target]) + def run_test(self): self.nodes[0].createwallet("activewallet") self.wallet = self.nodes[0].get_wallet_rpc("activewallet") @@ -327,5 +341,8 @@ class SendallTest(BitcoinTestFramework): # Sendall fails when providing a fee that is too high self.sendall_fails_on_high_fee() + # Sendall fails when many inputs result to too large transaction + self.sendall_fails_with_transaction_too_large() + if __name__ == '__main__': SendallTest().main() |