From 1f72b42b81e01979919f7cbb8d507fa5a88807ed Mon Sep 17 00:00:00 2001 From: Greg Pfeil Date: Mon, 20 Mar 2023 11:24:50 -0600 Subject: [PATCH] Additional z_sendmany test cases Also improve error messages. --- qa/rpc-tests/wallet_z_sendmany.py | 31 +++++++++++++++++++++++++ src/wallet/asyncrpcoperation_common.cpp | 12 +++++++++- src/wallet/wallet_tx_builder.cpp | 10 ++++++-- src/wallet/wallet_tx_builder.h | 10 +++++++- 4 files changed, 59 insertions(+), 4 deletions(-) diff --git a/qa/rpc-tests/wallet_z_sendmany.py b/qa/rpc-tests/wallet_z_sendmany.py index 4c66affe9..6c710a8a3 100755 --- a/qa/rpc-tests/wallet_z_sendmany.py +++ b/qa/rpc-tests/wallet_z_sendmany.py @@ -339,6 +339,10 @@ class WalletZSendmanyTest(BitcoinTestFramework): opid = self.nodes[1].z_sendmany(n1ua0, recipients, 1, 0, policy) wait_and_assert_operationid_status(self.nodes[1], opid, 'failed', msg) + # If we try to send just a bit less than we have, it will fail, complaining about dust + opid = self.nodes[1].z_sendmany(n1ua0, [{"address":n0ua0, "amount":3.9999999}], 1, 0, 'AllowLinkingAccountAddresses') + wait_and_assert_operationid_status(self.nodes[1], opid, 'failed', 'Insufficient funds: have 4.00, need 0.00000044 more to avoid creating invalid change output 0.0000001 (dust threshold is 0.00000054).') + # Once we provide a sufficiently-weak policy, the transaction succeeds. opid = self.nodes[1].z_sendmany(n1ua0, recipients, 1, 0, 'AllowLinkingAccountAddresses') wait_and_assert_operationid_status(self.nodes[1], opid) @@ -359,6 +363,20 @@ class WalletZSendmanyTest(BitcoinTestFramework): assert_equal(self.nodes[1].z_getbalance(n1ua0), 1) assert_equal(self.nodes[1].z_getbalance(n1ua1), 1) + # + # Test Orchard-only UA before NU5 + # + + n0orchard_only = self.nodes[0].z_getaddressforaccount(n0account0, ["orchard"])['address'] + recipients = [{"address":n0orchard_only, "amount":1}] + for (policy, msg) in [ + ('FullPrivacy', 'Could not send to a shielded receiver of a unified address without spending funds from a different pool, which would reveal transaction amounts. THIS MAY AFFECT YOUR PRIVACY. Resubmit with the `privacyPolicy` parameter set to `AllowRevealedAmounts` or weaker if you wish to allow this transaction to proceed anyway.'), + ('AllowRevealedAmounts', 'This transaction would send to a transparent receiver of a unified address, which is not enabled by default because it will publicly reveal transaction recipients and amounts. THIS MAY AFFECT YOUR PRIVACY. Resubmit with the `privacyPolicy` parameter set to `AllowRevealedRecipients` or weaker if you wish to allow this transaction to proceed anyway.'), + ('AllowRevealedRecipients', 'Could not send to an Orchard-only receiver, despite a lax privacy policy. Either there are insufficent non-Sprout funds, or NU5 has not been activated yet.'), + ]: + opid = self.nodes[1].z_sendmany(n1ua0, recipients, 1, 0, policy) + wait_and_assert_operationid_status(self.nodes[1], opid, 'failed', msg) + # # Test NoPrivacy policy # @@ -392,6 +410,19 @@ class WalletZSendmanyTest(BitcoinTestFramework): self.nodes[1].generate(10) self.sync_all() + # + # Test Orchard-only UA with insufficient non-Sprout funds + # + + recipients = [{"address":n0orchard_only, "amount":100}] + for (policy, msg) in [ + ('FullPrivacy', 'Could not send to a shielded receiver of a unified address without spending funds from a different pool, which would reveal transaction amounts. THIS MAY AFFECT YOUR PRIVACY. Resubmit with the `privacyPolicy` parameter set to `AllowRevealedAmounts` or weaker if you wish to allow this transaction to proceed anyway.'), + ('AllowRevealedAmounts', 'This transaction would send to a transparent receiver of a unified address, which is not enabled by default because it will publicly reveal transaction recipients and amounts. THIS MAY AFFECT YOUR PRIVACY. Resubmit with the `privacyPolicy` parameter set to `AllowRevealedRecipients` or weaker if you wish to allow this transaction to proceed anyway.'), + ('AllowRevealedRecipients', 'Could not send to an Orchard-only receiver, despite a lax privacy policy. Either there are insufficent non-Sprout funds, or NU5 has not been activated yet.'), + ]: + opid = self.nodes[1].z_sendmany(n1ua0, recipients, 1, 0, policy) + wait_and_assert_operationid_status(self.nodes[1], opid, 'failed', msg) + # # Test AllowRevealedAmounts policy # diff --git a/src/wallet/asyncrpcoperation_common.cpp b/src/wallet/asyncrpcoperation_common.cpp index 3b8d955ec..21bd665b3 100644 --- a/src/wallet/asyncrpcoperation_common.cpp +++ b/src/wallet/asyncrpcoperation_common.cpp @@ -139,14 +139,24 @@ void ThrowInputSelectionError( FormatMoney(err.available - err.required))); }, [](const ExcessOrchardActionsError& err) { + std::string side; + switch (err.side) { + case ActionSide::Input: + side = "inputs"; + case ActionSide::Output: + side = "outputs"; + case ActionSide::Both: + side = "actions"; + }; throw JSONRPCError( RPC_INVALID_PARAMETER, strprintf( - "Attempting to spend %u Orchard notes would exceed the current limit " + "Including %u Orchard %s would exceed the current limit " "of %u notes, which exists to prevent memory exhaustion. Restart with " "`-orchardactionlimit=N` where N >= %u to allow the wallet to attempt " "to construct this transaction.", err.orchardNotes, + side, err.maxNotes, err.orchardNotes)); } diff --git a/src/wallet/wallet_tx_builder.cpp b/src/wallet/wallet_tx_builder.cpp index 9c78c1921..ffc7c8dac 100644 --- a/src/wallet/wallet_tx_builder.cpp +++ b/src/wallet/wallet_tx_builder.cpp @@ -307,7 +307,10 @@ InputSelectionResult WalletTxBuilder::ResolveInputsAndPayments( auto resolved = Payments(resolvedPayments); if (orchardOutputs > this->maxOrchardActions) { - return ExcessOrchardActionsError(orchardOutputs, this->maxOrchardActions); + return ExcessOrchardActionsError( + ActionSide::Output, + orchardOutputs, + this->maxOrchardActions); } // Set the dust threshold so that we can select enough inputs to avoid @@ -343,7 +346,10 @@ InputSelectionResult WalletTxBuilder::ResolveInputsAndPayments( } if (spendableMut.orchardNoteMetadata.size() > this->maxOrchardActions) { - return ExcessOrchardActionsError(spendableMut.orchardNoteMetadata.size(), this->maxOrchardActions); + return ExcessOrchardActionsError( + ActionSide::Input, + spendableMut.orchardNoteMetadata.size(), + this->maxOrchardActions); } return InputSelection(resolved, anchorHeight); diff --git a/src/wallet/wallet_tx_builder.h b/src/wallet/wallet_tx_builder.h index 5aa095091..6bd39b710 100644 --- a/src/wallet/wallet_tx_builder.h +++ b/src/wallet/wallet_tx_builder.h @@ -260,12 +260,20 @@ public: available(available), required(required) { } }; +enum ActionSide { + Input, + Output, + Both, +}; + class ExcessOrchardActionsError { public: + ActionSide side; uint32_t orchardNotes; uint32_t maxNotes; - ExcessOrchardActionsError(uint32_t orchardNotes, uint32_t maxNotes): orchardNotes(orchardNotes), maxNotes(maxNotes) { } + ExcessOrchardActionsError(ActionSide side, uint32_t orchardNotes, uint32_t maxNotes): + side(side), orchardNotes(orchardNotes), maxNotes(maxNotes) { } }; typedef std::variant<