Merge pull request #6377 from sellout/wallet_tx_builder/z_sendmany

Extract common transaction logic from z_sendmany

doc/release-notes.md

@@ -21,6 +21,9 @@ RPC Changes
     also available for testnet and regtest nodes, in which case it does not
     return end-of-service halt information (as testnet and regtest nodes do not
     have an end-of-service halt feature.)
+- Several `z_sendmany` failures have moved from synchronous to asynchronous, so
+  while you should already be checking the async operation status, there are now
+  more cases that may trigger failure at that stage.
 
 [Deprecations](https://zcash.github.io/zcash/user/deprecation.html)
 --------------

qa/rpc-tests/mempool_nu_activation.py

@@ -34,6 +34,7 @@ class MempoolUpgradeActivationTest(BitcoinTestFramework):
             "-debug=mempool",
             "-blockmaxsize=4000",
             '-allowdeprecated=getnewaddress',
+            '-allowdeprecated=legacy_privacy',
             '-allowdeprecated=z_getnewaddress',
             '-allowdeprecated=z_getbalance',
             nuparams(BLOSSOM_BRANCH_ID, 200),

qa/rpc-tests/remove_sprout_shielding.py

@@ -21,7 +21,7 @@ from test_framework.util import (
 import logging
 
 HAS_CANOPY = [
-    '-nurejectoldversions=false', 
+    '-nurejectoldversions=false',
     '-anchorconfirmations=1',
     nuparams(BLOSSOM_BRANCH_ID, 205),
     nuparams(HEARTWOOD_BRANCH_ID, 210),
@@ -88,11 +88,9 @@ class RemoveSproutShieldingTest (BitcoinTestFramework):
 
         # Create taddr -> Sprout z_sendmany transaction on node 0. Should fail
         sprout_addr = self.nodes[1].z_getnewaddress('sprout')
-        assert_raises_message(
-            JSONRPCException,
-            "Sending funds into the Sprout value pool is not supported by z_sendmany",
-            self.nodes[0].z_sendmany,
-            taddr_0, [{"address": sprout_addr, "amount": 1}])
+        recipients = [{"address": sprout_addr, "amount": Decimal('1')}]
+        myopid = self.nodes[0].z_sendmany(taddr_0, recipients, 1, 0, 'AllowRevealedSenders')
+        wait_and_assert_operationid_status(self.nodes[0], myopid, "failed", "Sending funds into the Sprout pool is no longer supported.")
         print("taddr -> Sprout z_sendmany tx rejected at Canopy activation on node 0")
 
         # Create z_mergetoaddress [taddr, Sprout] -> Sprout transaction on node 0. Should fail

qa/rpc-tests/wallet_sapling.py

@@ -4,7 +4,6 @@
 # file COPYING or https://www.opensource.org/licenses/mit-license.php .
 
 from test_framework.test_framework import BitcoinTestFramework
-from test_framework.authproxy import JSONRPCException
 from test_framework.util import (
     assert_equal,
     get_coinbase_address,
@@ -170,16 +169,13 @@ class WalletSaplingTest(BitcoinTestFramework):
         # Make sure we get a useful error when trying to send to both sprout and sapling
         node4_sproutaddr = self.nodes[3].z_getnewaddress('sprout')
         node4_saplingaddr = self.nodes[3].z_getnewaddress('sapling')
-        try:
-            self.nodes[1].z_sendmany(
-                taddr1,
-                [{'address': node4_sproutaddr, 'amount': Decimal('2.5')},
-                 {'address': node4_saplingaddr, 'amount': Decimal('2.5') - DEFAULT_FEE}],
-                1, DEFAULT_FEE, 'AllowRevealedSenders'
-            )
-            raise AssertionError("Should have thrown an exception")
-        except JSONRPCException as e:
-            assert_equal("Sending funds into the Sprout value pool is not supported by z_sendmany", e.error['message'])
+        myopid = self.nodes[1].z_sendmany(
+            taddr1,
+            [{'address': node4_sproutaddr, 'amount': Decimal('2.5')},
+             {'address': node4_saplingaddr, 'amount': Decimal('2.5') - DEFAULT_FEE}],
+            1, DEFAULT_FEE, 'AllowRevealedSenders'
+        )
+        wait_and_assert_operationid_status(self.nodes[1], myopid, "failed", "Sending funds into the Sprout pool is no longer supported.")
 
 if __name__ == '__main__':
     WalletSaplingTest().main()

qa/rpc-tests/wallet_sendmany_any_taddr.py

@@ -112,7 +112,7 @@ class WalletSendManyAnyTaddr(BitcoinTestFramework):
             'ANY_TADDR',
             [{'address': recipient, 'amount': 20}],
             1, DEFAULT_FEE, 'AllowRevealedSenders')
-        wait_and_assert_operationid_status(self.nodes[3], myopid, "failed", "Insufficient funds: have 14.99998, need 20.00001; note that coinbase outputs will not be selected if you specify ANY_TADDR or if any transparent recipients are included.")
+        wait_and_assert_operationid_status(self.nodes[3], myopid, "failed", "Insufficient funds: have 14.99998, need 20.00001; note that coinbase outputs will not be selected if you specify ANY_TADDR, any transparent recipients are included, or if the `privacyPolicy` parameter is not set to `AllowRevealedSenders` or weaker.")
 
         # Create an expired transaction on node 3.
         self.split_network()
@@ -144,7 +144,7 @@ class WalletSendManyAnyTaddr(BitcoinTestFramework):
                 'ANY_TADDR',
                 [{'address': recipient, 'amount': 13}],
                 1, DEFAULT_FEE, 'AllowRevealedSenders'),
-            "failed", "Insufficient funds: have 0.00, need 13.00001; note that coinbase outputs will not be selected if you specify ANY_TADDR or if any transparent recipients are included.")
+            "failed", "Insufficient funds: have 0.00, need 13.00001; note that coinbase outputs will not be selected if you specify ANY_TADDR, any transparent recipients are included, or if the `privacyPolicy` parameter is not set to `AllowRevealedSenders` or weaker.")
 
 if __name__ == '__main__':
     WalletSendManyAnyTaddr().main()

qa/rpc-tests/wallet_shieldingcoinbase.py

@@ -38,6 +38,7 @@ class WalletShieldingCoinbaseTest (BitcoinTestFramework):
             '-regtestshieldcoinbase',
             '-debug=zrpcunsafe',
             '-allowdeprecated=getnewaddress',
+            '-allowdeprecated=legacy_privacy',
             '-allowdeprecated=z_getnewaddress',
             '-allowdeprecated=z_getbalance',
             '-allowdeprecated=z_gettotalbalance',
@@ -232,7 +233,7 @@ class WalletShieldingCoinbaseTest (BitcoinTestFramework):
         amount = Decimal('10.0') - DEFAULT_FEE - Decimal('0.00000001')    # this leaves change at 1 zatoshi less than dust threshold
         recipients.append({"address":self.nodes[0].getnewaddress(), "amount":amount })
         myopid = self.nodes[0].z_sendmany(mytaddr, recipients, 1)
-        wait_and_assert_operationid_status(self.nodes[0], myopid, "failed", "Insufficient funds: have 10.00, need 0.00000053 more to avoid creating invalid change output 0.00000001 (dust threshold is 0.00000054); note that coinbase outputs will not be selected if you specify ANY_TADDR or if any transparent recipients are included.")
+        wait_and_assert_operationid_status(self.nodes[0], myopid, "failed", "Insufficient funds: have 10.00, need 0.00000053 more to avoid creating invalid change output 0.00000001 (dust threshold is 0.00000054); note that coinbase outputs will not be selected if you specify ANY_TADDR, any transparent recipients are included, or if the `privacyPolicy` parameter is not set to `AllowRevealedSenders` or weaker.")
 
         # Send will fail because send amount is too big, even when including coinbase utxos
         errorString = ""
@@ -246,9 +247,9 @@ class WalletShieldingCoinbaseTest (BitcoinTestFramework):
         recipients = []
         recipients.append({"address":self.nodes[1].getnewaddress(), "amount":Decimal('10000.0')})
         myopid = self.nodes[0].z_sendmany(mytaddr, recipients, 1)
-        wait_and_assert_operationid_status(self.nodes[0], myopid, "failed", "Insufficient funds: have 10.00, need 10000.00001; note that coinbase outputs will not be selected if you specify ANY_TADDR or if any transparent recipients are included.")
+        wait_and_assert_operationid_status(self.nodes[0], myopid, "failed", "Insufficient funds: have 10.00, need 10000.00001; note that coinbase outputs will not be selected if you specify ANY_TADDR, any transparent recipients are included, or if the `privacyPolicy` parameter is not set to `AllowRevealedSenders` or weaker.")
         myopid = self.nodes[0].z_sendmany(myzaddr, recipients, 1, DEFAULT_FEE, 'AllowRevealedRecipients')
-        wait_and_assert_operationid_status(self.nodes[0], myopid, "failed", "Insufficient funds: have 9.99998, need 10000.00001; note that coinbase outputs will not be selected if you specify ANY_TADDR or if any transparent recipients are included.")
+        wait_and_assert_operationid_status(self.nodes[0], myopid, "failed", "Insufficient funds: have 9.99998, need 10000.00001; note that coinbase outputs will not be selected if you specify ANY_TADDR, any transparent recipients are included, or if the `privacyPolicy` parameter is not set to `AllowRevealedSenders` or weaker.")
 
         # Send will fail because of insufficient funds unless sender uses coinbase utxos
         try:

qa/rpc-tests/wallet_z_sendmany.py

@@ -183,9 +183,10 @@ class WalletZSendmanyTest(BitcoinTestFramework):
         # If we attempt to spend with the default privacy policy, z_sendmany
         # fails because it needs to spend transparent coins in a transaction
         # involving a Unified Address.
-        revealed_senders_msg = 'This transaction requires selecting transparent coins, which is not enabled by default because it will publicly reveal transaction senders and amounts. THIS MAY AFFECT YOUR PRIVACY. Resubmit with the `privacyPolicy` parameter set to `AllowRevealedSenders` or weaker if you wish to allow this transaction to proceed anyway.'
+        unified_address_msg = '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.'
+        revealed_senders_msg = 'Insufficient funds: have 0.00, need 10.00; note that coinbase outputs will not be selected if you specify ANY_TADDR, any transparent recipients are included, or if the `privacyPolicy` parameter is not set to `AllowRevealedSenders` or weaker.'
         opid = self.nodes[2].z_sendmany(source, recipients, 1, 0)
-        wait_and_assert_operationid_status(self.nodes[2], opid, 'failed', revealed_senders_msg)
+        wait_and_assert_operationid_status(self.nodes[2], opid, 'failed', unified_address_msg)
 
         # We can't create a transaction with an unknown privacy policy.
         assert_raises_message(
@@ -196,13 +197,13 @@ class WalletZSendmanyTest(BitcoinTestFramework):
 
         # If we set any policy that does not include AllowRevealedSenders,
         # z_sendmany also fails.
-        for policy in [
-            'FullPrivacy',
-            'AllowRevealedAmounts',
-            'AllowRevealedRecipients',
+        for (policy, msg) in [
+            ('FullPrivacy', unified_address_msg),
+            ('AllowRevealedAmounts', revealed_senders_msg),
+            ('AllowRevealedRecipients', revealed_senders_msg),
         ]:
             opid = self.nodes[2].z_sendmany(source, recipients, 1, 0, policy)
-            wait_and_assert_operationid_status(self.nodes[2], opid, 'failed', revealed_senders_msg)
+            wait_and_assert_operationid_status(self.nodes[2], opid, 'failed', msg)
 
         # By setting the correct policy, we can create the transaction.
         opid = self.nodes[2].z_sendmany(source, recipients, 1, 0, 'AllowRevealedSenders')
@@ -321,20 +322,26 @@ class WalletZSendmanyTest(BitcoinTestFramework):
 
         # If we try to send 3 ZEC from n1ua0, it will fail with too-few funds.
         recipients = [{"address":n0ua0, "amount":3}]
-        linked_addrs_msg = 'Insufficient funds: have 2.00, need 3.00. (This transaction may require selecting transparent coins that were sent to multiple Unified Addresses, which is not enabled by default because it would create a public link between the transparent receivers of these addresses. THIS MAY AFFECT YOUR PRIVACY. Resubmit with the `privacyPolicy` parameter set to `AllowLinkingAccountAddresses` or weaker if you wish to allow this transaction to proceed anyway.)'
+        linked_addrs_with_coinbase_note_msg = 'Insufficient funds: have 2.00, need 3.00; note that coinbase outputs will not be selected if you specify ANY_TADDR, any transparent recipients are included, or if the `privacyPolicy` parameter is not set to `AllowRevealedSenders` or weaker. (This transaction may require selecting transparent coins that were sent to multiple Unified Addresses, which is not enabled by default because it would create a public link between the transparent receivers of these addresses. THIS MAY AFFECT YOUR PRIVACY. Resubmit with the `privacyPolicy` parameter set to `AllowLinkingAccountAddresses` or weaker if you wish to allow this transaction to proceed anyway.)'
+        linked_addrs_without_coinbase_note_msg = 'Insufficient funds: have 2.00, need 3.00. (This transaction may require selecting transparent coins that were sent to multiple Unified Addresses, which is not enabled by default because it would create a public link between the transparent receivers of these addresses. THIS MAY AFFECT YOUR PRIVACY. Resubmit with the `privacyPolicy` parameter set to `AllowLinkingAccountAddresses` or weaker if you wish to allow this transaction to proceed anyway.)'
+        revealed_amounts_msg = '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.'
         opid = self.nodes[1].z_sendmany(n1ua0, recipients, 1, 0)
-        wait_and_assert_operationid_status(self.nodes[1], opid, 'failed', linked_addrs_msg)
+        wait_and_assert_operationid_status(self.nodes[1], opid, 'failed', revealed_amounts_msg)
 
         # If we try it again with any policy that is too strong, it also fails.
-        for policy in [
-            'FullPrivacy',
-            'AllowRevealedAmounts',
-            'AllowRevealedRecipients',
-            'AllowRevealedSenders',
-            'AllowFullyTransparent',
+        for (policy, msg) in [
+            ('FullPrivacy', revealed_amounts_msg),
+            ('AllowRevealedAmounts', linked_addrs_with_coinbase_note_msg),
+            ('AllowRevealedRecipients', linked_addrs_with_coinbase_note_msg),
+            ('AllowRevealedSenders', linked_addrs_without_coinbase_note_msg),
+            ('AllowFullyTransparent', linked_addrs_without_coinbase_note_msg),
         ]:
             opid = self.nodes[1].z_sendmany(n1ua0, recipients, 1, 0, policy)
-            wait_and_assert_operationid_status(self.nodes[1], opid, 'failed', linked_addrs_msg)
+            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')
@@ -356,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 insufficient non-Sprout funds (there is no transaction version that supports both Sprout and Orchard), 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
         #
@@ -363,8 +384,8 @@ class WalletZSendmanyTest(BitcoinTestFramework):
         # Send some legacy transparent funds to n1ua0, creating Sapling outputs.
         source = get_coinbase_address(self.nodes[2])
         recipients = [{"address":n1ua0, "amount":10}]
-        # This requires the AllowRevealedSenders policy...
-        opid = self.nodes[2].z_sendmany(source, recipients, 1, 0)
+        # This requires the AllowRevealedSenders policy, but we specify only AllowRevealedAmounts...
+        opid = self.nodes[2].z_sendmany(source, recipients, 1, 0, 'AllowRevealedAmounts')
         wait_and_assert_operationid_status(self.nodes[2], opid, 'failed', revealed_senders_msg)
         # ... which we can always override with the NoPrivacy policy.
         opid = self.nodes[2].z_sendmany(source, recipients, 1, 0, 'NoPrivacy')
@@ -389,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 insufficient non-Sprout funds (there is no transaction version that supports both Sprout and Orchard), 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
         #
@@ -402,7 +436,7 @@ class WalletZSendmanyTest(BitcoinTestFramework):
         recipients = [{"address":n0ua1, "amount": 6}]
 
         # Should fail under default and 'FullPrivacy' policies ...
-        revealed_amounts_msg = 'Sending from the Sapling shielded pool to the Orchard shielded pool is not enabled by default because it will publicly reveal the transaction amount. 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.'
+        revealed_amounts_msg = '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.'
         opid = self.nodes[1].z_sendmany(n1ua0, recipients, 1, 0)
         wait_and_assert_operationid_status(self.nodes[1], opid, 'failed', revealed_amounts_msg)
 

src/Makefile.am

@@ -327,6 +327,7 @@ BITCOIN_CORE_H = \
   wallet/asyncrpcoperation_saplingmigration.h \
   wallet/asyncrpcoperation_sendmany.h \
   wallet/asyncrpcoperation_shieldcoinbase.h \
+  wallet/wallet_tx_builder.h \
   wallet/crypter.h \
   wallet/db.h \
   wallet/memo.h \
@@ -418,6 +419,7 @@ libbitcoin_wallet_a_SOURCES = \
   wallet/asyncrpcoperation_saplingmigration.cpp \
   wallet/asyncrpcoperation_sendmany.cpp \
   wallet/asyncrpcoperation_shieldcoinbase.cpp \
+  wallet/wallet_tx_builder.cpp \
   wallet/crypter.cpp \
   wallet/db.cpp \
   wallet/orchard.cpp \

src/init.cpp

@@ -1889,7 +1889,11 @@ bool AppInit2(boost::thread_group& threadGroup, CScheduler& scheduler)
             if (!zaddr.has_value()) {
                 return InitError(_("-mineraddress is not a valid " PACKAGE_NAME " address."));
             }
-            auto ztxoSelector = pwalletMain->ZTXOSelectorForAddress(zaddr.value(), true, false);
+            auto ztxoSelector = pwalletMain->ZTXOSelectorForAddress(
+                    zaddr.value(),
+                    true,
+                    TransparentCoinbasePolicy::Allow,
+                    false);
             minerAddressInLocalWallet = ztxoSelector.has_value();
         }
         if (GetBoolArg("-minetolocalwallet", true) && !minerAddressInLocalWallet) {

src/wallet/asyncrpcoperation_common.cpp

@@ -3,6 +3,7 @@
 #include "core_io.h"
 #include "init.h"
 #include "rpc/protocol.h"
+#include "util/moneystr.h"
 
 extern UniValue signrawtransaction(const UniValue& params, bool fHelp);
 
@@ -40,3 +41,125 @@ std::pair<CTransaction, UniValue> SignSendRawTransaction(UniValue obj, std::opti
 
     return std::make_pair(tx, sendResult);
 }
+
+void ThrowInputSelectionError(
+        const InputSelectionError& err,
+        const ZTXOSelector& selector,
+        const TransactionStrategy& strategy)
+{
+    std::visit(match {
+        [](const AddressResolutionError& err) {
+            switch (err) {
+                case AddressResolutionError::SproutRecipientsNotSupported:
+                    throw JSONRPCError(
+                        RPC_INVALID_PARAMETER,
+                        "Sending funds into the Sprout pool is no longer supported.");
+                case AddressResolutionError::TransparentRecipientNotAllowed:
+                    throw JSONRPCError(
+                        RPC_INVALID_PARAMETER,
+                        "This transaction would have transparent recipients, 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.");
+                case AddressResolutionError::RevealingSaplingAmountNotAllowed:
+                    throw JSONRPCError(
+                        RPC_INVALID_PARAMETER,
+                        "Could not send to the Sapling shielded pool without spending non-Sapling "
+                        "funds, 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.");
+                case AddressResolutionError::CouldNotResolveReceiver:
+                    throw JSONRPCError(
+                        RPC_INVALID_PARAMETER,
+                        "Could not send to an Orchard-only receiver, despite a lax privacy policy. "
+                        "Either there are insufficient non-Sprout funds (there is no transaction "
+                        "version that supports both Sprout and Orchard), or NU5 has not been "
+                        "activated yet.");
+                case AddressResolutionError::TransparentReceiverNotAllowed:
+                    throw JSONRPCError(
+                        RPC_INVALID_PARAMETER,
+                        "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.");
+                case AddressResolutionError::RevealingReceiverAmountsNotAllowed:
+                    throw JSONRPCError(
+                        RPC_INVALID_PARAMETER,
+                        "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.");
+                default:
+                    assert(false);
+            }
+        },
+        [&](const InvalidFundsError& err) {
+            bool isFromUa = std::holds_alternative<libzcash::UnifiedAddress>(selector.GetPattern());
+            throw JSONRPCError(
+                RPC_INVALID_PARAMETER,
+                strprintf(
+                    "Insufficient funds: have %s, %s",
+                    FormatMoney(err.available),
+                    std::visit(match {
+                        [](const InsufficientFundsError& ife) {
+                            return strprintf("need %s", FormatMoney(ife.required));
+                        },
+                        [](const DustThresholdError& dte) {
+                            return strprintf(
+                                    "need %s more to avoid creating invalid change output %s (dust threshold is %s)",
+                                    FormatMoney(dte.dustThreshold - dte.changeAmount),
+                                    FormatMoney(dte.changeAmount),
+                                    FormatMoney(dte.dustThreshold));
+                        }
+                    },
+                    err.reason))
+                    + (selector.TransparentCoinbasePolicy() != TransparentCoinbasePolicy::Disallow
+                       ? "" :
+                       "; note that coinbase outputs will not be selected if you specify "
+                       "ANY_TADDR, any transparent recipients are included, or if the "
+                       "`privacyPolicy` parameter is not set to `AllowRevealedSenders` or weaker")
+                    + (!isFromUa || strategy.AllowLinkingAccountAddresses() ? "." :
+                       ". (This transaction may require selecting transparent coins that were sent "
+                       "to multiple Unified Addresses, which is not enabled by default because "
+                       "it would create a public link between the transparent receivers of these "
+                       "addresses. THIS MAY AFFECT YOUR PRIVACY. Resubmit with the `privacyPolicy` "
+                       "parameter set to `AllowLinkingAccountAddresses` or weaker if you wish to "
+                       "allow this transaction to proceed anyway.)"));
+        },
+        [](const ChangeNotAllowedError& err) {
+            throw JSONRPCError(
+                RPC_WALLET_ERROR,
+                strprintf(
+                    "When shielding coinbase funds, the wallet does not allow any change. "
+                    "The proposed transaction would result in %s in change.",
+                    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(
+                    "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));
+        }
+    }, err);
+}

src/wallet/asyncrpcoperation_common.h

@@ -11,6 +11,7 @@
 #include "rpc/protocol.h"
 #include "univalue.h"
 #include "wallet.h"
+#include "wallet/wallet_tx_builder.h"
 
 #include <optional>
 
@@ -63,4 +64,9 @@ UniValue SendTransaction(
  */
 std::pair<CTransaction, UniValue> SignSendRawTransaction(UniValue obj, std::optional<std::reference_wrapper<CReserveKey>> reservekey, bool testmode);
 
+void ThrowInputSelectionError(
+        const InputSelectionError& err,
+        const ZTXOSelector& selector,
+        const TransactionStrategy& strategy);
+
 #endif // ZCASH_WALLET_ASYNCRPCOPERATION_COMMON_H

src/wallet/asyncrpcoperation_sendmany.cpp

@@ -18,7 +18,6 @@
 #include "proof_verifier.h"
 #include "rpc/protocol.h"
 #include "rpc/server.h"
-#include "transaction_builder.h"
 #include "timedata.h"
 #include "util/system.h"
 #include "util/match.h"
@@ -29,7 +28,6 @@
 #include "util/time.h"
 #include "zcash/IncrementalMerkleTree.hpp"
 #include "miner.h"
-#include "wallet/paymentdisclosuredb.h"
 #include "wallet/wallet_tx_builder.h"
 
 #include <array>
@@ -45,9 +43,9 @@
 using namespace libzcash;
 
 AsyncRPCOperation_sendmany::AsyncRPCOperation_sendmany(
-        TransactionBuilder builder,
+        WalletTxBuilder builder,
         ZTXOSelector ztxoSelector,
-        std::vector<ResolvedPayment> recipients,
+        std::vector<Payment> recipients,
         int minDepth,
         unsigned int anchorDepth,
         TransactionStrategy strategy,
@@ -62,32 +60,6 @@ AsyncRPCOperation_sendmany::AsyncRPCOperation_sendmany(
     assert(!recipients_.empty());
     assert(ztxoSelector.RequireSpendingKeys());
 
-    sendFromAccount_ = pwalletMain->FindAccountForSelector(ztxoSelector_).value_or(ZCASH_LEGACY_ACCOUNT);
-
-    // Determine the target totals and recipient pools
-    for (const ResolvedPayment& recipient : recipients_) {
-        std::visit(match {
-            [&](const CKeyID& addr) {
-                txOutputAmounts_.t_outputs_total += recipient.amount;
-                recipientPools_.insert(OutputPool::Transparent);
-            },
-            [&](const CScriptID& addr) {
-                txOutputAmounts_.t_outputs_total += recipient.amount;
-                recipientPools_.insert(OutputPool::Transparent);
-            },
-            [&](const libzcash::SaplingPaymentAddress& addr) {
-                txOutputAmounts_.sapling_outputs_total += recipient.amount;
-                recipientPools_.insert(OutputPool::Sapling);
-            },
-            [&](const libzcash::OrchardRawAddress& addr) {
-                txOutputAmounts_.orchard_outputs_total += recipient.amount;
-                recipientPools_.insert(OutputPool::Orchard);
-                // No transaction allows sends from Sprout to Orchard.
-                assert(!ztxoSelector_.SelectsSprout());
-            }
-        }, recipient.address);
-    }
-
     // Log the context info i.e. the call parameters to z_sendmany
     if (LogAcceptCategory("zrpcunsafe")) {
         LogPrint("zrpcunsafe", "%s: z_sendmany initialized (params=%s)\n", getId(), contextInfo.write());
@@ -106,8 +78,6 @@ void AsyncRPCOperation_sendmany::main() {
     set_state(OperationStatus::EXECUTING);
     start_execution_clock();
 
-    bool success = false;
-
 #ifdef ENABLE_MINING
     GenerateBitcoins(false, 0, Params());
 #endif
@@ -164,606 +134,62 @@ void AsyncRPCOperation_sendmany::main() {
 // 2. #1360 Note selection is not optimal.
 // 3. #1277 Spendable notes are not locked, so an operation running in parallel
 //    could also try to use them.
-// 4. #1614 Anchors are chosen at the most recent block; this is unreliable and leaks
-//    information in case of rollback.
-// 5. #3615 There is no padding of inputs or outputs, which may leak information.
+// 4. #3615 There is no padding of inputs or outputs, which may leak information.
 //
-// At least 4. and 5. differ from the Rust transaction builder.
+// At least #4 differs from the Rust transaction builder.
 uint256 AsyncRPCOperation_sendmany::main_impl() {
-    CAmount sendAmount = (
-        txOutputAmounts_.orchard_outputs_total +
-        txOutputAmounts_.sapling_outputs_total +
-        txOutputAmounts_.t_outputs_total);
-    CAmount targetAmount = sendAmount + fee_;
+    auto spendable = builder_.FindAllSpendableInputs(ztxoSelector_, mindepth_);
 
-    builder_.SetFee(fee_);
-
-    // Allow transparent coinbase inputs if there are no transparent
-    // recipients.
-    bool allowTransparentCoinbase = !recipientPools_.count(OutputPool::Transparent);
-
-    // Set the dust threshold so that we can select enough inputs to avoid
-    // creating dust change amounts.
-    CAmount dustThreshold{DefaultDustThreshold()};
-
-    // Find spendable inputs, and select a minimal set of them that
-    // can supply the required target amount.
-    SpendableInputs spendable;
-    {
-        LOCK2(cs_main, pwalletMain->cs_wallet);
-        spendable = pwalletMain->FindSpendableInputs(ztxoSelector_, allowTransparentCoinbase, mindepth_, std::nullopt);
-    }
-    if (!spendable.LimitToAmount(targetAmount, dustThreshold, recipientPools_)) {
-        CAmount changeAmount{spendable.Total() - targetAmount};
-        std::string insufficientFundsMessage =
-            strprintf("Insufficient funds: have %s", FormatMoney(spendable.Total()));
-        if (changeAmount > 0 && changeAmount < dustThreshold) {
-            // TODO: we should provide the option for the caller to explicitly
-            // forego change (definitionally an amount below the dust amount)
-            // and send the extra to the recipient or the miner fee to avoid
-            // creating dust change, rather than prohibit them from sending
-            // entirely in this circumstance.
-            // (Daira disagrees, as this could leak information to the recipient
-            // or to an external viewing key holder.)
-            insufficientFundsMessage +=
-                strprintf(
-                    ", need %s more to avoid creating invalid change output %s (dust threshold is %s)",
-                    FormatMoney(dustThreshold - changeAmount),
-                    FormatMoney(changeAmount),
-                    FormatMoney(dustThreshold));
-        } else {
-            insufficientFundsMessage += strprintf(", need %s", FormatMoney(targetAmount));
-        }
-        bool isFromUa = std::holds_alternative<libzcash::UnifiedAddress>(ztxoSelector_.GetPattern());
-        throw JSONRPCError(
-                RPC_WALLET_INSUFFICIENT_FUNDS,
-                insufficientFundsMessage
-                + (allowTransparentCoinbase && ztxoSelector_.SelectsTransparentCoinbase() ? "." :
-                   "; note that coinbase outputs will not be selected if you specify "
-                   "ANY_TADDR or if any transparent recipients are included.")
-                + ((!isFromUa || strategy_.AllowLinkingAccountAddresses()) ? "" :
-                   " (This transaction may require selecting transparent coins that were sent "
-                   "to multiple Unified Addresses, which is not enabled by default because "
-                   "it would create a public link between the transparent receivers of these "
-                   "addresses. THIS MAY AFFECT YOUR PRIVACY. Resubmit with the `privacyPolicy` "
-                   "parameter set to `AllowLinkingAccountAddresses` or weaker if you wish to "
-                   "allow this transaction to proceed anyway.)"));
-    }
-
-    if (!(spendable.utxos.empty() || strategy_.AllowRevealedSenders())) {
-        throw JSONRPCError(
-            RPC_INVALID_PARAMETER,
-            "This transaction requires selecting transparent coins, which is "
-            "not enabled by default because it will publicly reveal transaction "
-            "senders and amounts. THIS MAY AFFECT YOUR PRIVACY. Resubmit "
-            "with the `privacyPolicy` parameter set to `AllowRevealedSenders` "
-            "or weaker if you wish to allow this transaction to proceed anyway.");
-    }
-
-    if (recipientPools_.count(OutputPool::Transparent) && !strategy_.AllowRevealedRecipients()) {
-        throw JSONRPCError(
-            RPC_INVALID_PARAMETER,
-            "This transaction would have transparent recipients, 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.");
-    }
-
-    if (!spendable.sproutNoteEntries.empty()) {
-        if (recipientPools_.count(OutputPool::Sapling) && !strategy_.AllowRevealedAmounts()) {
-            throw JSONRPCError(
-                RPC_INVALID_PARAMETER,
-                "Sending from the Sprout shielded pool to the Sapling "
-                "shielded pool is not enabled by default because it will "
-                "publicly reveal the transaction amount. 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.");
-        }
-    }
-
-    if (!spendable.saplingNoteEntries.empty()) {
-        if (recipientPools_.count(OutputPool::Orchard) && !strategy_.AllowRevealedAmounts()) {
-            throw JSONRPCError(
-                RPC_INVALID_PARAMETER,
-                "Sending from the Sapling shielded pool to the Orchard "
-                "shielded pool is not enabled by default because it will "
-                "publicly reveal the transaction amount. 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.");
-        }
-        // Sending from Sapling to transparent will be caught above in the
-        // AllowRevealedRecipients check; sending to Sprout is disallowed
-        // entirely.
-    }
-
-    if (!spendable.orchardNoteMetadata.empty()) {
-        if (recipientPools_.count(OutputPool::Sapling) && !strategy_.AllowRevealedAmounts()) {
-            throw JSONRPCError(
-                RPC_INVALID_PARAMETER,
-                "Sending from the Orchard shielded pool to the Sapling "
-                "shielded pool is not enabled by default because it will "
-                "publicly reveal the transaction amount. 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.");
-        }
-
-        // Sending from Orchard to transparent will be caught above in the
-        // AllowRevealedRecipients check; sending to Sprout is disallowed
-        // entirely.
-
-        if (spendable.orchardNoteMetadata.size() > nOrchardActionLimit) {
-            throw JSONRPCError(
-                RPC_INVALID_PARAMETER,
-                strprintf(
-                    "Attempting to spend %u Orchard notes 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.",
-                    spendable.orchardNoteMetadata.size(),
-                    nOrchardActionLimit,
-                    spendable.orchardNoteMetadata.size()));
-        }
-    }
-
-    spendable.LogInputs(getId());
-
-    CAmount t_inputs_total{0};
-    CAmount z_inputs_total{0};
-    for (const auto& t : spendable.utxos) {
-        t_inputs_total += t.Value();
-    }
-    for (const auto& t : spendable.sproutNoteEntries) {
-        z_inputs_total += t.note.value();
-    }
-    for (const auto& t : spendable.saplingNoteEntries) {
-        z_inputs_total += t.note.value();
-    }
-    for (const auto& t : spendable.orchardNoteMetadata) {
-        z_inputs_total += t.GetNoteValue();
-    }
-
-    if (z_inputs_total > 0 && mindepth_ == 0) {
-        throw JSONRPCError(
-                RPC_INVALID_PARAMETER,
-                "Minconf cannot be zero when sending from a shielded address");
-    }
-
-    // When spending transparent coinbase outputs, all inputs must be fully
-    // consumed, and they may only be sent to shielded recipients.
-    if (spendable.HasTransparentCoinbase()) {
-        if (t_inputs_total + z_inputs_total != targetAmount) {
-            throw JSONRPCError(
-                    RPC_WALLET_ERROR,
-                    strprintf(
-                        "When shielding coinbase funds, the wallet does not allow any change. "
-                        "The proposed transaction would result in %s in change.",
-                        FormatMoney(t_inputs_total - targetAmount)
-                        ));
-        }
-        if (txOutputAmounts_.t_outputs_total != 0) {
-            throw JSONRPCError(
-                    RPC_WALLET_ERROR,
-                    "Coinbase funds may only be sent to shielded recipients.");
-        }
-    }
-
-    LogPrint("zrpcunsafe", "%s: spending %s to send %s with fee %s\n",
-        getId(), FormatMoney(targetAmount), FormatMoney(sendAmount), FormatMoney(fee_));
-    LogPrint("zrpc", "%s: total transparent input: %s (to choose from)\n", getId(), FormatMoney(t_inputs_total));
-    LogPrint("zrpcunsafe", "%s: total shielded input: %s (to choose from)\n", getId(), FormatMoney(z_inputs_total));
-    LogPrint("zrpc", "%s: total transparent output: %s\n", getId(), FormatMoney(txOutputAmounts_.t_outputs_total));
-    LogPrint("zrpcunsafe", "%s: total shielded Sapling output: %s\n", getId(), FormatMoney(txOutputAmounts_.sapling_outputs_total));
-    LogPrint("zrpcunsafe", "%s: total shielded Orchard output: %s\n", getId(), FormatMoney(txOutputAmounts_.orchard_outputs_total));
-    LogPrint("zrpc", "%s: fee: %s\n", getId(), FormatMoney(fee_));
-
-    // Allow change to go to any pool for which we have recipients.
-    std::set<OutputPool> allowedChangeTypes = recipientPools_;
-
-    // We always allow shielded change when not sending from the legacy account.
-    if (sendFromAccount_ != ZCASH_LEGACY_ACCOUNT) {
-        allowedChangeTypes.insert(OutputPool::Sapling);
-    }
-
-    auto ovks = this->SelectOVKs(spendable);
-    auto allowChangeTypes = [&](const std::set<ReceiverType>& receiverTypes) {
-        for (ReceiverType rtype : receiverTypes) {
-            switch (rtype) {
-                case ReceiverType::P2PKH:
-                case ReceiverType::P2SH:
-                    if (!spendable.utxos.empty() || strategy_.AllowRevealedRecipients()) {
-                        allowedChangeTypes.insert(OutputPool::Transparent);
-                    }
-                    break;
-                case ReceiverType::Sapling:
-                    if (!spendable.saplingNoteEntries.empty() || strategy_.AllowRevealedAmounts()) {
-                        allowedChangeTypes.insert(OutputPool::Sapling);
-                    }
-                    break;
-                case ReceiverType::Orchard:
-                    if (builder_.SupportsOrchard() &&
-                            (!spendable.orchardNoteMetadata.empty() || strategy_.AllowRevealedAmounts())) {
-                        allowedChangeTypes.insert(OutputPool::Orchard);
-                    }
-                    break;
-            }
-        }
-    };
+    auto preparedTx = builder_.PrepareTransaction(
+            ztxoSelector_,
+            spendable,
+            recipients_,
+            chainActive,
+            strategy_,
+            fee_,
+            anchordepth_);
 
+    uint256 txid;
     std::visit(match {
-        [&](const CKeyID& keyId) {
-            allowedChangeTypes.insert(OutputPool::Transparent);
-            auto changeAddr = pwalletMain->GenerateChangeAddressForAccount(
-                    sendFromAccount_, allowedChangeTypes);
-            assert(changeAddr.has_value());
-            builder_.SendChangeTo(changeAddr.value(), ovks.first);
+        [&](const InputSelectionError& err) {
+            ThrowInputSelectionError(err, ztxoSelector_, strategy_);
         },
-        [&](const CScriptID& scriptId) {
-            allowedChangeTypes.insert(OutputPool::Transparent);
-            auto changeAddr = pwalletMain->GenerateChangeAddressForAccount(
-                    sendFromAccount_, allowedChangeTypes);
-            assert(changeAddr.has_value());
-            builder_.SendChangeTo(changeAddr.value(), ovks.first);
-        },
-        [&](const libzcash::SproutPaymentAddress& addr) {
-            // for Sprout, we return change to the originating address.
-            builder_.SendChangeToSprout(addr);
-        },
-        [&](const libzcash::SproutViewingKey& vk) {
-            // for Sprout, we return change to the originating address.
-            builder_.SendChangeToSprout(vk.address());
-        },
-        [&](const libzcash::SaplingPaymentAddress& addr) {
-            // for Sapling, if using a legacy address, return change to the
-            // originating address; otherwise return it to the Sapling internal
-            // address corresponding to the UFVK.
-            if (sendFromAccount_ == ZCASH_LEGACY_ACCOUNT) {
-                builder_.SendChangeTo(addr, ovks.first);
-            } else {
-                auto changeAddr = pwalletMain->GenerateChangeAddressForAccount(
-                        sendFromAccount_, allowedChangeTypes);
-                assert(changeAddr.has_value());
-                builder_.SendChangeTo(changeAddr.value(), ovks.first);
-            }
-        },
-        [&](const libzcash::SaplingExtendedFullViewingKey& fvk) {
-            // for Sapling, if using a legacy address, return change to the
-            // originating address; otherwise return it to the Sapling internal
-            // address corresponding to the UFVK.
-            if (sendFromAccount_ == ZCASH_LEGACY_ACCOUNT) {
-                builder_.SendChangeTo(fvk.DefaultAddress(), ovks.first);
-            } else {
-                auto changeAddr = pwalletMain->GenerateChangeAddressForAccount(
-                        sendFromAccount_, allowedChangeTypes);
-                assert(changeAddr.has_value());
-                builder_.SendChangeTo(changeAddr.value(), ovks.first);
-            }
-        },
-        [&](const libzcash::UnifiedAddress& ua) {
-            allowChangeTypes(ua.GetKnownReceiverTypes());
+        [&](const TransactionEffects& effects) {
+            const auto& spendable = effects.GetSpendable();
+            const auto& payments = effects.GetPayments();
+            spendable.LogInputs(getId());
 
-            auto zufvk = pwalletMain->GetUFVKForAddress(ua);
-            if (!zufvk.has_value()) {
-                throw JSONRPCError(
-                        RPC_WALLET_ERROR,
-                        "Could not determine full viewing key for unified address.");
-            }
+            LogPrint("zrpcunsafe", "%s: spending %s to send %s with fee %s\n", getId(),
+                FormatMoney(payments.Total()),
+                FormatMoney(spendable.Total()),
+                FormatMoney(effects.GetFee()));
+            LogPrint("zrpc", "%s: total transparent input: %s (to choose from)\n", getId(),
+                FormatMoney(spendable.GetTransparentTotal()));
+            LogPrint("zrpcunsafe", "%s: total shielded input: %s (to choose from)\n", getId(),
+                FormatMoney(spendable.GetSaplingTotal() + spendable.GetOrchardTotal()));
+            LogPrint("zrpc", "%s: total transparent output: %s\n", getId(),
+                FormatMoney(payments.GetTransparentTotal()));
+            LogPrint("zrpcunsafe", "%s: total shielded Sapling output: %s\n", getId(),
+                FormatMoney(payments.GetSaplingTotal()));
+            LogPrint("zrpcunsafe", "%s: total shielded Orchard output: %s\n", getId(),
+                FormatMoney(payments.GetOrchardTotal()));
+            LogPrint("zrpc", "%s: fee: %s\n", getId(), FormatMoney(effects.GetFee()));
 
-            auto changeAddr = zufvk.value().GetChangeAddress(allowedChangeTypes);
-            if (!changeAddr.has_value()) {
-                throw JSONRPCError(
-                        RPC_WALLET_ERROR,
-                        "Could not generate a change address from the inferred full viewing key.");
-            }
-            builder_.SendChangeTo(changeAddr.value(), ovks.first);
-        },
-        [&](const libzcash::UnifiedFullViewingKey& fvk) {
-            allowChangeTypes(fvk.GetKnownReceiverTypes());
-            auto zufvk = ZcashdUnifiedFullViewingKey::FromUnifiedFullViewingKey(Params(), fvk);
-            auto changeAddr = zufvk.GetChangeAddress(allowedChangeTypes);
-            if (!changeAddr.has_value()) {
-                throw JSONRPCError(
-                        RPC_WALLET_ERROR,
-                        "Could not generate a change address from the specified full viewing key.");
-            }
-            builder_.SendChangeTo(changeAddr.value(), ovks.first);
-        },
-        [&](const AccountZTXOPattern& acct) {
-            allowChangeTypes(acct.GetReceiverTypes());
-            auto changeAddr = pwalletMain->GenerateChangeAddressForAccount(
-                        acct.GetAccountId(),
-                        allowedChangeTypes);
+            auto buildResult = effects.ApproveAndBuild(
+                    Params().GetConsensus(),
+                    *pwalletMain,
+                    chainActive,
+                    strategy_);
+            auto tx = buildResult.GetTxOrThrow();
 
-            assert(changeAddr.has_value());
-            builder_.SendChangeTo(changeAddr.value(), ovks.first);
+            UniValue sendResult = SendTransaction(tx, payments.GetResolvedPayments(), std::nullopt, testmode);
+            set_result(sendResult);
+
+            txid = tx.GetHash();
         }
-    }, ztxoSelector_.GetPattern());
+    }, preparedTx);
 
-    // Track the total of notes that we've added to the builder. This
-    // shouldn't strictly be necessary, given `spendable.LimitToAmount`
-    CAmount sum = 0;
-
-    // Create Sapling outpoints
-    std::vector<SaplingOutPoint> saplingOutPoints;
-    std::vector<SaplingNote> saplingNotes;
-    std::vector<SaplingExtendedSpendingKey> saplingKeys;
-
-    for (const auto& t : spendable.saplingNoteEntries) {
-        saplingOutPoints.push_back(t.op);
-        saplingNotes.push_back(t.note);
-
-        libzcash::SaplingExtendedSpendingKey saplingKey;
-        assert(pwalletMain->GetSaplingExtendedSpendingKey(t.address, saplingKey));
-        saplingKeys.push_back(saplingKey);
-
-        sum += t.note.value();
-        if (sum >= targetAmount) {
-            break;
-        }
-    }
-
-    // Fetch Sapling anchor and witnesses, and Orchard Merkle paths.
-    uint256 anchor;
-    std::vector<std::optional<SaplingWitness>> witnesses;
-    std::vector<std::pair<libzcash::OrchardSpendingKey, orchard::SpendInfo>> orchardSpendInfo;
-    {
-        LOCK2(cs_main, pwalletMain->cs_wallet);
-        if (!pwalletMain->GetSaplingNoteWitnesses(saplingOutPoints, anchordepth_, witnesses, anchor)) {
-            // This error should not appear once we're nAnchorConfirmations blocks past
-            // Sapling activation.
-            throw JSONRPCError(RPC_WALLET_ERROR, "Insufficient Sapling witnesses.");
-        }
-        if (builder_.GetOrchardAnchor().has_value()) {
-            orchardSpendInfo = pwalletMain->GetOrchardSpendInfo(spendable.orchardNoteMetadata, builder_.GetOrchardAnchor().value());
-        }
-    }
-
-    // Add Orchard spends
-    for (size_t i = 0; i < orchardSpendInfo.size(); i++) {
-        auto spendInfo = std::move(orchardSpendInfo[i]);
-        if (!builder_.AddOrchardSpend(
-            std::move(spendInfo.first),
-            std::move(spendInfo.second)))
-        {
-            throw JSONRPCError(
-                RPC_WALLET_ERROR,
-                strprintf("Failed to add Orchard note to transaction (check %s for details)", GetDebugLogPath())
-            );
-        }
-    }
-
-    // Add Sapling spends
-    for (size_t i = 0; i < saplingNotes.size(); i++) {
-        if (!witnesses[i]) {
-            throw JSONRPCError(
-                    RPC_WALLET_ERROR,
-                    strprintf(
-                        "Missing witness for Sapling note at outpoint %s",
-                        spendable.saplingNoteEntries[i].op.ToString())
-                    );
-        }
-
-        builder_.AddSaplingSpend(saplingKeys[i].expsk, saplingNotes[i], anchor, witnesses[i].value());
-    }
-
-    // Add outputs
-    for (const auto& r : recipients_) {
-        std::visit(match {
-            [&](const CKeyID& keyId) {
-                builder_.AddTransparentOutput(keyId, r.amount);
-            },
-            [&](const CScriptID& scriptId) {
-                builder_.AddTransparentOutput(scriptId, r.amount);
-            },
-            [&](const libzcash::SaplingPaymentAddress& addr) {
-                builder_.AddSaplingOutput(
-                        ovks.second, addr, r.amount,
-                        r.memo.has_value() ? r.memo.value().ToBytes() : Memo::NoMemo().ToBytes());
-            },
-            [&](const libzcash::OrchardRawAddress& addr) {
-                builder_.AddOrchardOutput(
-                        ovks.second, addr, r.amount,
-                        r.memo.has_value() ? std::optional(r.memo.value().ToBytes()) : std::nullopt);
-            }
-        }, r.address);
-    }
-
-    // Add transparent utxos
-    for (const auto& out : spendable.utxos) {
-        const CTxOut& txOut = out.tx->vout[out.i];
-        builder_.AddTransparentInput(COutPoint(out.tx->GetHash(), out.i), txOut.scriptPubKey, txOut.nValue);
-
-        sum += txOut.nValue;
-        if (sum >= targetAmount) {
-            break;
-        }
-    }
-
-    // Find Sprout witnesses
-    // When spending notes, take a snapshot of note witnesses and anchors as the treestate will
-    // change upon arrival of new blocks which contain joinsplit transactions.  This is likely
-    // to happen as creating a chained joinsplit transaction can take longer than the block interval.
-    // So, we need to take locks on cs_main and pwalletMain->cs_wallet so that the witnesses aren't
-    // updated.
-    //
-    // TODO: these locks would ideally be shared for selection of Sapling anchors and witnesses
-    // as well.
-    std::vector<std::optional<SproutWitness>> vSproutWitnesses;
-    {
-        LOCK2(cs_main, pwalletMain->cs_wallet);
-        std::vector<JSOutPoint> vOutPoints;
-        for (const auto& t : spendable.sproutNoteEntries) {
-            vOutPoints.push_back(t.jsop);
-        }
-
-        // inputAnchor is not needed by builder_.AddSproutInput as it is for Sapling.
-        uint256 inputAnchor;
-        if (!pwalletMain->GetSproutNoteWitnesses(vOutPoints, anchordepth_, vSproutWitnesses, inputAnchor)) {
-            // This error should not appear once we're nAnchorConfirmations blocks past
-            // Sprout activation.
-            throw JSONRPCError(RPC_WALLET_ERROR, "Insufficient Sprout witnesses.");
-        }
-    }
-
-    // Add Sprout spends
-    for (int i = 0; i < spendable.sproutNoteEntries.size(); i++) {
-        const auto& t = spendable.sproutNoteEntries[i];
-        libzcash::SproutSpendingKey sk;
-        assert(pwalletMain->GetSproutSpendingKey(t.address, sk));
-
-        builder_.AddSproutInput(sk, t.note, vSproutWitnesses[i].value());
-
-        sum += t.note.value();
-        if (sum >= targetAmount) {
-            break;
-        }
-    }
-
-    // Build the transaction
-    auto buildResult = builder_.Build();
-    auto tx = buildResult.GetTxOrThrow();
-
-    UniValue sendResult = SendTransaction(tx, recipients_, std::nullopt, testmode);
-    set_result(sendResult);
-
-    return tx.GetHash();
-}
-
-std::pair<uint256, uint256> AsyncRPCOperation_sendmany::SelectOVKs(const SpendableInputs& spendable) const {
-    uint256 internalOVK;
-    uint256 externalOVK;
-    if (!spendable.orchardNoteMetadata.empty()) {
-        std::optional<OrchardFullViewingKey> fvk;
-        std::visit(match {
-            [&](const UnifiedAddress& addr) {
-                auto ufvk = pwalletMain->GetUFVKForAddress(addr);
-                // This is safe because spending key checks will have ensured that we
-                // have a UFVK corresponding to this address, and Orchard notes will
-                // not have been selected if the UFVK does not contain an Orchard key.
-                fvk = ufvk.value().GetOrchardKey().value();
-            },
-            [&](const UnifiedFullViewingKey& ufvk) {
-                // Orchard notes will not have been selected if the UFVK does not contain
-                // an Orchard key.
-                fvk = ufvk.GetOrchardKey().value();
-            },
-            [&](const AccountZTXOPattern& acct) {
-                // By definition, we have a UFVK for every known account.
-                auto ufvk = pwalletMain->GetUnifiedFullViewingKeyByAccount(acct.GetAccountId());
-                // Orchard notes will not have been selected if the UFVK does not contain
-                // an Orchard key.
-                fvk = ufvk.value().GetOrchardKey().value();
-            },
-            [&](const auto& other) {
-                throw std::runtime_error("SelectOVKs: Selector cannot select Orchard notes.");
-            }
-        }, this->ztxoSelector_.GetPattern());
-        assert(fvk.has_value());
-
-        internalOVK = fvk.value().ToInternalOutgoingViewingKey();
-        externalOVK = fvk.value().ToExternalOutgoingViewingKey();
-    } else if (!spendable.saplingNoteEntries.empty()) {
-        std::optional<SaplingDiversifiableFullViewingKey> dfvk;
-        std::visit(match {
-            [&](const libzcash::SaplingPaymentAddress& addr) {
-                libzcash::SaplingExtendedSpendingKey extsk;
-                assert(pwalletMain->GetSaplingExtendedSpendingKey(addr, extsk));
-                dfvk = extsk.ToXFVK();
-            },
-            [&](const UnifiedAddress& addr) {
-                auto ufvk = pwalletMain->GetUFVKForAddress(addr);
-                // This is safe because spending key checks will have ensured that we
-                // have a UFVK corresponding to this address, and Sapling notes will
-                // not have been selected if the UFVK does not contain a Sapling key.
-                dfvk = ufvk.value().GetSaplingKey().value();
-            },
-            [&](const UnifiedFullViewingKey& ufvk) {
-                // Sapling notes will not have been selected if the UFVK does not contain
-                // a Sapling key.
-                dfvk = ufvk.GetSaplingKey().value();
-            },
-            [&](const AccountZTXOPattern& acct) {
-                // By definition, we have a UFVK for every known account.
-                auto ufvk = pwalletMain->GetUnifiedFullViewingKeyByAccount(acct.GetAccountId());
-                // Sapling notes will not have been selected if the UFVK does not contain
-                // a Sapling key.
-                dfvk = ufvk.value().GetSaplingKey().value();
-            },
-            [&](const auto& other) {
-                throw std::runtime_error("SelectOVKs: Selector cannot select Sapling notes.");
-            }
-        }, this->ztxoSelector_.GetPattern());
-        assert(dfvk.has_value());
-
-        auto ovks = dfvk.value().GetOVKs();
-        internalOVK = ovks.first;
-        externalOVK = ovks.second;
-    } else if (!spendable.utxos.empty()) {
-        std::optional<transparent::AccountPubKey> tfvk;
-        std::visit(match {
-            [&](const CKeyID& keyId) {
-                tfvk = pwalletMain->GetLegacyAccountKey().ToAccountPubKey();
-            },
-            [&](const CScriptID& keyId) {
-                tfvk = pwalletMain->GetLegacyAccountKey().ToAccountPubKey();
-            },
-            [&](const UnifiedAddress& addr) {
-                // This is safe because spending key checks will have ensured that we
-                // have a UFVK corresponding to this address, and transparent UTXOs will
-                // not have been selected if the UFVK does not contain a transparent key.
-                auto ufvk = pwalletMain->GetUFVKForAddress(addr);
-                tfvk = ufvk.value().GetTransparentKey().value();
-            },
-            [&](const UnifiedFullViewingKey& ufvk) {
-                // Transparent UTXOs will not have been selected if the UFVK does not contain
-                // a transparent key.
-                tfvk = ufvk.GetTransparentKey().value();
-            },
-            [&](const AccountZTXOPattern& acct) {
-                if (acct.GetAccountId() == ZCASH_LEGACY_ACCOUNT) {
-                    tfvk = pwalletMain->GetLegacyAccountKey().ToAccountPubKey();
-                } else {
-                    // By definition, we have a UFVK for every known account.
-                    auto ufvk = pwalletMain->GetUnifiedFullViewingKeyByAccount(acct.GetAccountId()).value();
-                    // Transparent UTXOs will not have been selected if the UFVK does not contain
-                    // a transparent key.
-                    tfvk = ufvk.GetTransparentKey().value();
-                }
-            },
-            [&](const auto& other) {
-                throw std::runtime_error("SelectOVKs: Selector cannot select transparent UTXOs.");
-            }
-        }, this->ztxoSelector_.GetPattern());
-        assert(tfvk.has_value());
-
-        auto ovks = tfvk.value().GetOVKsForShielding();
-        internalOVK = ovks.first;
-        externalOVK = ovks.second;
-    } else if (!spendable.sproutNoteEntries.empty()) {
-        // use the legacy transparent account OVKs when sending from Sprout
-        auto tfvk = pwalletMain->GetLegacyAccountKey().ToAccountPubKey();
-        auto ovks = tfvk.GetOVKsForShielding();
-        internalOVK = ovks.first;
-        externalOVK = ovks.second;
-    } else {
-        // This should be unreachable; it is left in place as a guard to ensure
-        // that when new input types are added to SpendableInputs in the future
-        // that we do not accidentally return the all-zeros OVK.
-        throw std::runtime_error("No spendable inputs.");
-    }
-
-    return std::make_pair(internalOVK, externalOVK);
-}
-
-/**
- * Compute a dust threshold based upon a standard p2pkh txout.
- */
-CAmount AsyncRPCOperation_sendmany::DefaultDustThreshold() {
-    CKey secret{CKey::TestOnlyRandomKey(true)};
-    CScript scriptPubKey = GetScriptForDestination(secret.GetPubKey().GetID());
-    CTxOut txout(CAmount(1), scriptPubKey);
-    // TODO: use a local for minRelayTxFee rather than a global
-    return txout.GetDustThreshold(minRelayTxFee);
+    return txid;
 }
 
 /**

src/wallet/asyncrpcoperation_sendmany.h

@@ -26,24 +26,18 @@
 
 using namespace libzcash;
 
-class TxOutputAmounts {
-public:
-    CAmount t_outputs_total{0};
-    CAmount sapling_outputs_total{0};
-    CAmount orchard_outputs_total{0};
-};
-
 class AsyncRPCOperation_sendmany : public AsyncRPCOperation {
 public:
     AsyncRPCOperation_sendmany(
-        TransactionBuilder builder,
+        WalletTxBuilder builder,
         ZTXOSelector ztxoSelector,
-        std::vector<ResolvedPayment> recipients,
+        std::vector<Payment> recipients,
         int minDepth,
         unsigned int anchorDepth,
         TransactionStrategy strategy,
         CAmount fee = DEFAULT_FEE,
         UniValue contextInfo = NullUniValue);
+
     virtual ~AsyncRPCOperation_sendmany();
 
     // We don't want to be copied or moved around
@@ -61,29 +55,15 @@ public:
 private:
     friend class TEST_FRIEND_AsyncRPCOperation_sendmany;    // class for unit testing
 
-    TransactionBuilder builder_;
+    WalletTxBuilder builder_;
     ZTXOSelector ztxoSelector_;
-    std::vector<ResolvedPayment> recipients_;
+    std::vector<Payment> recipients_;
+    TransactionStrategy strategy_;
     int mindepth_{1};
     unsigned int anchordepth_{nAnchorConfirmations};
     CAmount fee_;
     UniValue contextinfo_;     // optional data to include in return value from getStatus()
 
-    bool isfromsprout_{false};
-    bool isfromsapling_{false};
-    TransactionStrategy strategy_;
-    AccountId sendFromAccount_;
-    std::set<OutputPool> recipientPools_;
-    TxOutputAmounts txOutputAmounts_;
-
-    /**
-     * Compute the internal and external OVKs to use in transaction construction, given
-     * the spendable inputs.
-     */
-    std::pair<uint256, uint256> SelectOVKs(const SpendableInputs& spendable) const;
-
-    static CAmount DefaultDustThreshold();
-
     uint256 main_impl();
 };
 

src/wallet/gtest/test_rpc_wallet.cpp

@@ -284,12 +284,18 @@ TEST(WalletRPCTests, RPCZsendmanyTaddrToSapling)
     pwalletMain->LoadWalletTx(wtx);
 
     // Context that z_sendmany requires
-    auto builder = TransactionBuilder(consensusParams, nextBlockHeight, std::nullopt, pwalletMain);
+    auto builder = WalletTxBuilder(Params(), *pwalletMain, minRelayTxFee);
     mtx = CreateNewContextualCMutableTransaction(consensusParams, nextBlockHeight, false);
 
-    auto selector = pwalletMain->ZTXOSelectorForAddress(taddr, true, false).value();
-    std::vector<ResolvedPayment> recipients = { ResolvedPayment(std::nullopt, pa, 1*COIN, Memo::FromHexOrThrow("ABCD")) };
-    TransactionStrategy strategy(PrivacyPolicy::AllowRevealedSenders);
+    // we need AllowFullyTransparent because the transaction will result
+    // in transparent change as a consequence of sending from a legacy taddr
+    TransactionStrategy strategy(PrivacyPolicy::AllowFullyTransparent);
+    auto selector = pwalletMain->ZTXOSelectorForAddress(
+            taddr,
+            true,
+            TransparentCoinbasePolicy::Disallow,
+            false).value();
+    std::vector<Payment> recipients = { Payment(pa, 1*COIN, Memo::FromHexOrThrow("ABCD")) };
     std::shared_ptr<AsyncRPCOperation> operation(new AsyncRPCOperation_sendmany(std::move(builder), selector, recipients, 0, 0, strategy));
     std::shared_ptr<AsyncRPCOperation_sendmany> ptr = std::dynamic_pointer_cast<AsyncRPCOperation_sendmany> (operation);
 

src/wallet/rpcwallet.cpp

@@ -3869,13 +3869,13 @@ UniValue z_getbalance(const UniValue& params, bool fHelp)
             nBalance = getBalanceZaddr(addr, std::nullopt, nMinDepth, INT_MAX, false);
         },
         [&](const libzcash::UnifiedAddress& addr) {
-            auto selector = pwalletMain->ZTXOSelectorForAddress(addr, true, false);
+            auto selector = pwalletMain->ZTXOSelectorForAddress(addr, true, TransparentCoinbasePolicy::Allow, false);
             if (!selector.has_value()) {
                 throw JSONRPCError(
                     RPC_INVALID_ADDRESS_OR_KEY,
                     "Unified address does not correspond to an account in the wallet");
             }
-            auto spendableInputs = pwalletMain->FindSpendableInputs(selector.value(), true, nMinDepth, std::nullopt);
+            auto spendableInputs = pwalletMain->FindSpendableInputs(selector.value(), nMinDepth, std::nullopt);
 
             for (const auto& t : spendableInputs.utxos) {
                 nBalance += t.Value();
@@ -3960,14 +3960,14 @@ UniValue z_getbalanceforviewingkey(const UniValue& params, bool fHelp)
     // FVKs make it possible to correctly determine balance without having the
     // spending key, so we permit that here.
     bool requireSpendingKey = std::holds_alternative<libzcash::SproutViewingKey>(fvk);
-    auto selector = pwalletMain->ZTXOSelectorForViewingKey(fvk, requireSpendingKey);
+    auto selector = pwalletMain->ZTXOSelectorForViewingKey(fvk, requireSpendingKey, TransparentCoinbasePolicy::Allow);
     if (!selector.has_value()) {
         throw JSONRPCError(
             RPC_INVALID_PARAMETER,
             "Error: the wallet does not recognize the specified viewing key.");
     }
 
-    auto spendableInputs = pwalletMain->FindSpendableInputs(selector.value(), true, minconf, asOfHeight);
+    auto spendableInputs = pwalletMain->FindSpendableInputs(selector.value(), minconf, asOfHeight);
 
     CAmount transparentBalance = 0;
     CAmount sproutBalance = 0;
@@ -4058,14 +4058,14 @@ UniValue z_getbalanceforaccount(const UniValue& params, bool fHelp)
     LOCK2(cs_main, pwalletMain->cs_wallet);
 
     // Get the receivers for this account.
-    auto selector = pwalletMain->ZTXOSelectorForAccount(account, false);
+    auto selector = pwalletMain->ZTXOSelectorForAccount(account, false, TransparentCoinbasePolicy::Allow);
     if (!selector.has_value()) {
         throw JSONRPCError(
             RPC_INVALID_PARAMETER,
             tfm::format("Error: account %d has not been generated by z_getnewaccount.", account));
     }
 
-    auto spendableInputs = pwalletMain->FindSpendableInputs(selector.value(), true, minconf, asOfHeight);
+    auto spendableInputs = pwalletMain->FindSpendableInputs(selector.value(), minconf, asOfHeight);
     // Accounts never contain Sprout notes.
     assert(spendableInputs.sproutNoteEntries.empty());
 
@@ -4637,7 +4637,7 @@ UniValue z_getoperationstatus_IMPL(const UniValue& params, bool fRemoveFinishedO
 
 size_t EstimateTxSize(
         const ZTXOSelector& ztxoSelector,
-        const std::vector<ResolvedPayment>& recipients,
+        const std::vector<Payment>& recipients,
         int nextBlockHeight) {
     CMutableTransaction mtx;
     mtx.fOverwintered = true;
@@ -4651,7 +4651,7 @@ size_t EstimateTxSize(
     size_t txsize = 0;
     size_t taddrRecipientCount = 0;
     size_t orchardRecipientCount = 0;
-    for (const ResolvedPayment& recipient : recipients) {
+    for (const Payment& recipient : recipients) {
         std::visit(match {
             [&](const CKeyID&) {
                 taddrRecipientCount += 1;
@@ -4667,15 +4667,17 @@ size_t EstimateTxSize(
                 jsdesc.proof = GrothProof();
                 mtx.vJoinSplit.push_back(jsdesc);
             },
-            [&](const libzcash::OrchardRawAddress& addr) {
-                if (fromSprout) {
-                    throw JSONRPCError(
-                        RPC_INVALID_PARAMETER,
-                        "Sending funds from a Sprout address to a Unified Address is not supported by z_sendmany");
+            [&](const libzcash::UnifiedAddress& addr) {
+                if (addr.GetOrchardReceiver().has_value()) {
+                    orchardRecipientCount += 1;
+                } else if (addr.GetSaplingReceiver().has_value()) {
+                    mtx.vShieldedOutput.push_back(OutputDescription());
+                } else if (addr.GetP2PKHReceiver().has_value()
+                           || addr.GetP2SHReceiver().has_value()) {
+                    taddrRecipientCount += 1;
                 }
-                orchardRecipientCount += 1;
             }
-        }, recipient.address);
+        }, recipient.GetAddress());
     }
 
     bool nu5Active = Params().GetConsensus().NetworkUpgradeActive(nextBlockHeight, Consensus::UPGRADE_NU5);
@@ -4798,65 +4800,18 @@ UniValue z_sendmany(const UniValue& params, bool fHelp)
         }
     }
 
-    bool involvesUnifiedAddress = false;
-    bool involvesOrchard = false;
-
-    // Check that the from address is valid.
-    // Unified address (UA) allowed here (#5185)
-    auto fromaddress = params[0].get_str();
-    ZTXOSelector ztxoSelector = [&]() {
-        if (fromaddress == "ANY_TADDR") {
-            return CWallet::LegacyTransparentZTXOSelector(true);
-        } else {
-            auto decoded = keyIO.DecodePaymentAddress(fromaddress);
-            if (!decoded.has_value()) {
-                throw JSONRPCError(
-                        RPC_INVALID_ADDRESS_OR_KEY,
-                        "Invalid from address: should be a taddr, zaddr, UA, or the string 'ANY_TADDR'.");
-            }
-
-            auto ztxoSelectorOpt = pwalletMain->ZTXOSelectorForAddress(
-                decoded.value(),
-                true,
-                // LegacyCompat does not include AllowLinkingAccountAddresses.
-                maybeStrategy.has_value() ? maybeStrategy.value().AllowLinkingAccountAddresses() : false);
-            if (!ztxoSelectorOpt.has_value()) {
-                throw JSONRPCError(
-                        RPC_INVALID_ADDRESS_OR_KEY,
-                        "Invalid from address, no payment source found for address.");
-            }
-
-            auto selectorAccount = pwalletMain->FindAccountForSelector(ztxoSelectorOpt.value());
-            std::visit(match {
-                [&](const libzcash::UnifiedAddress& ua) {
-                    if (!selectorAccount.has_value() || selectorAccount.value() == ZCASH_LEGACY_ACCOUNT) {
-                        throw JSONRPCError(
-                                RPC_INVALID_ADDRESS_OR_KEY,
-                                "Invalid from address, UA does not correspond to a known account.");
-                    }
-                    involvesUnifiedAddress = true;
-                    involvesOrchard = ua.GetOrchardReceiver().has_value();
-                },
-                [&](const auto& other) {
-                    if (selectorAccount.has_value() && selectorAccount.value() != ZCASH_LEGACY_ACCOUNT) {
-                        throw JSONRPCError(
-                                RPC_INVALID_ADDRESS_OR_KEY,
-                                "Invalid from address: is a bare receiver from a Unified Address in this wallet. Provide the UA as returned by z_getaddressforaccount instead.");
-                    }
-                }
-            }, decoded.value());
-
-            return ztxoSelectorOpt.value();
-        }
-    }();
-
     UniValue outputs = params[1].get_array();
     if (outputs.size() == 0) {
         throw JSONRPCError(RPC_INVALID_PARAMETER, "Invalid parameter, amounts array is empty.");
     }
 
-    std::set<RecipientAddress> recipientAddrs;
-    std::vector<ResolvedPayment> recipients;
+    bool involvesUnifiedAddress = false;
+    auto tcoinbasePolicy =
+        maybeStrategy.has_value() && maybeStrategy.value().AllowRevealedSenders()
+        ? TransparentCoinbasePolicy::Allow
+        : TransparentCoinbasePolicy::Disallow;
+    std::set<PaymentAddress> recipientAddrs;
+    std::vector<Payment> recipients;
     CAmount nTotalOut = 0;
     size_t nOrchardOutputs = 0;
     for (const UniValue& o : outputs.getValues()) {
@@ -4870,29 +4825,13 @@ UniValue z_sendmany(const UniValue& params, bool fHelp)
         }
 
         std::string addrStr = find_value(o, "address").get_str();
-        auto decoded = keyIO.DecodePaymentAddress(addrStr);
-        if (!decoded.has_value()) {
+        auto addr = keyIO.DecodePaymentAddress(addrStr);
+        if (!addr.has_value()) {
             throw JSONRPCError(
                     RPC_INVALID_PARAMETER,
                     std::string("Invalid parameter, unknown address format: ") + addrStr);
         }
 
-        std::optional<RecipientAddress> addr = std::visit(
-            SelectRecipientAddress(chainparams.GetConsensus(), nextBlockHeight),
-            decoded.value());
-        if (!addr.has_value()) {
-            bool toSprout = std::holds_alternative<libzcash::SproutPaymentAddress>(decoded.value());
-            if (toSprout) {
-                throw JSONRPCError(
-                    RPC_INVALID_PARAMETER,
-                    "Sending funds into the Sprout value pool is not supported by z_sendmany");
-            } else {
-                throw JSONRPCError(
-                    RPC_INVALID_PARAMETER,
-                    "Unified address contained only receiver types that are unrecognized or for which the required consensus feature is not yet active.");
-            }
-        }
-
         if (!recipientAddrs.insert(addr.value()).second) {
             throw JSONRPCError(RPC_INVALID_PARAMETER, string("Invalid parameter, duplicated recipient address: ") + addrStr);
         }
@@ -4901,7 +4840,7 @@ UniValue z_sendmany(const UniValue& params, bool fHelp)
         std::optional<Memo> memo;
         if (!memoValue.isNull()) {
             auto memoHex = memoValue.get_str();
-            if (!std::visit(libzcash::IsShieldedRecipient(), addr.value())) {
+            if (!std::visit(libzcash::HasShieldedRecipient(), addr.value())) {
                 throw JSONRPCError(
                         RPC_INVALID_PARAMETER,
                         "Invalid parameter, memos cannot be sent to transparent addresses.");
@@ -4934,36 +4873,89 @@ UniValue z_sendmany(const UniValue& params, bool fHelp)
             throw JSONRPCError(RPC_INVALID_PARAMETER, "Invalid parameter, amount must be positive");
         }
 
-        std::optional<libzcash::UnifiedAddress> ua = std::nullopt;
-        if (std::holds_alternative<libzcash::UnifiedAddress>(decoded.value())) {
-            ua = std::get<libzcash::UnifiedAddress>(decoded.value());
-            involvesUnifiedAddress = true;
-            involvesOrchard = involvesOrchard || ua.value().GetOrchardReceiver().has_value();
-        }
+        std::visit(match {
+            [&](const CKeyID &) {
+                tcoinbasePolicy = TransparentCoinbasePolicy::Disallow;
+            },
+            [&](const CScriptID &) {
+                tcoinbasePolicy = TransparentCoinbasePolicy::Disallow;
+            },
+            [&](const UnifiedAddress &ua) {
+                involvesUnifiedAddress = true;
+                auto preferredRecipient =
+                    ua.GetPreferredRecipientAddress(chainparams.GetConsensus(), nextBlockHeight);
+                if (preferredRecipient.has_value()) {
+                    std::visit(match {
+                        [&](const CKeyID &) {
+                            tcoinbasePolicy = TransparentCoinbasePolicy::Disallow;
+                        },
+                        [&](const CScriptID &) {
+                            tcoinbasePolicy = TransparentCoinbasePolicy::Disallow;
+                        },
+                        [](const auto &) { }
+                    }, preferredRecipient.value());
+                }
+            },
+            [](const auto &) { }
+        }, addr.value());
 
-        if (std::holds_alternative<libzcash::OrchardRawAddress>(addr.value())) {
-            nOrchardOutputs += 1;
-            if (nOrchardOutputs > nOrchardActionLimit) {
-                throw JSONRPCError(
-                    RPC_INVALID_PARAMETER,
-                    strprintf(
-                        "Attempting to create %u Orchard outputs 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.",
-                        nOrchardOutputs,
-                        nOrchardActionLimit,
-                        nOrchardOutputs));
-            }
-        }
-
-        recipients.push_back(ResolvedPayment(ua, addr.value(), nAmount, memo));
+        recipients.push_back(Payment(addr.value(), nAmount, memo));
         nTotalOut += nAmount;
     }
     if (recipients.empty()) {
         throw JSONRPCError(RPC_INVALID_PARAMETER, "No recipients");
     }
 
+    // Check that the from address is valid.
+    // Unified address (UA) allowed here (#5185)
+    auto fromaddress = params[0].get_str();
+    ZTXOSelector ztxoSelector = [&]() {
+        if (fromaddress == "ANY_TADDR") {
+            return CWallet::LegacyTransparentZTXOSelector(true, TransparentCoinbasePolicy::Disallow);
+        } else {
+            auto decoded = keyIO.DecodePaymentAddress(fromaddress);
+            if (!decoded.has_value()) {
+                throw JSONRPCError(
+                        RPC_INVALID_ADDRESS_OR_KEY,
+                        "Invalid from address: should be a taddr, zaddr, UA, or the string 'ANY_TADDR'.");
+            }
+
+            auto ztxoSelectorOpt = pwalletMain->ZTXOSelectorForAddress(
+                decoded.value(),
+                true,
+                tcoinbasePolicy,
+                // LegacyCompat does not include AllowLinkingAccountAddresses.
+                maybeStrategy.has_value() ? maybeStrategy.value().AllowLinkingAccountAddresses() : false);
+            if (!ztxoSelectorOpt.has_value()) {
+                throw JSONRPCError(
+                        RPC_INVALID_ADDRESS_OR_KEY,
+                        "Invalid from address, no payment source found for address.");
+            }
+
+            auto selectorAccount = pwalletMain->FindAccountForSelector(ztxoSelectorOpt.value());
+            bool unknownOrLegacy = !selectorAccount.has_value() || selectorAccount.value() == ZCASH_LEGACY_ACCOUNT;
+            std::visit(match {
+                [&](const libzcash::UnifiedAddress& ua) {
+                    if (unknownOrLegacy) {
+                        throw JSONRPCError(
+                                RPC_INVALID_ADDRESS_OR_KEY,
+                                "Invalid from address, UA does not correspond to a known account.");
+                    }
+                    involvesUnifiedAddress = true;
+                },
+                [&](const auto& other) {
+                    if (!unknownOrLegacy) {
+                        throw JSONRPCError(
+                                RPC_INVALID_ADDRESS_OR_KEY,
+                                "Invalid from address: is a bare receiver from a Unified Address in this wallet. Provide the UA as returned by z_getaddressforaccount instead.");
+                    }
+                }
+            }, decoded.value());
+
+            return ztxoSelectorOpt.value();
+        }
+    }();
+
     // Now that we've set involvesUnifiedAddress correctly, we can finish
     // evaluating the strategy.
     TransactionStrategy strategy = maybeStrategy.value_or(
@@ -5021,22 +5013,10 @@ UniValue z_sendmany(const UniValue& params, bool fHelp)
     o.pushKV("fee", std::stod(FormatMoney(nFee)));
     UniValue contextInfo = o;
 
-    std::optional<uint256> orchardAnchor;
-    auto nAnchorDepth = std::min((unsigned int) nMinDepth, nAnchorConfirmations);
-    if ((ztxoSelector.SelectsOrchard() || nOrchardOutputs > 0) && nAnchorDepth == 0) {
-        throw JSONRPCError(RPC_INVALID_PARAMETER, "Cannot select Orchard notes or send to Orchard recipients when minconf=0.");
-    }
-    if (!ztxoSelector.SelectsSprout() && (involvesOrchard || nPreferredTxVersion >= ZIP225_MIN_TX_VERSION) && nAnchorDepth > 0) {
-        auto orchardAnchorHeight = nextBlockHeight - nAnchorDepth;
-        if (chainparams.GetConsensus().NetworkUpgradeActive(orchardAnchorHeight, Consensus::UPGRADE_NU5)) {
-            auto anchorBlockIndex = chainActive[orchardAnchorHeight];
-            assert(anchorBlockIndex != nullptr);
-            orchardAnchor = anchorBlockIndex->hashFinalOrchardRoot;
-        }
-    }
-    TransactionBuilder builder(chainparams.GetConsensus(), nextBlockHeight, orchardAnchor, pwalletMain);
-
     // Create operation and add to global queue
+    auto nAnchorDepth = std::min((unsigned int) nMinDepth, nAnchorConfirmations);
+    WalletTxBuilder builder(Params(), *pwalletMain, minRelayTxFee);
+
     std::shared_ptr<AsyncRPCQueue> q = getAsyncRPCQueue();
     std::shared_ptr<AsyncRPCOperation> operation(
             new AsyncRPCOperation_sendmany(

src/wallet/test/rpc_wallet_tests.cpp

@@ -150,13 +150,13 @@ BOOST_AUTO_TEST_CASE(rpc_wallet)
     CTxDestination setaccountDemoAddress(CTxDestination(setaccountDemoPubkey.GetID()));
 
     /*********************************
-     *                  getbalance
+     *          getbalance
      *********************************/
     BOOST_CHECK_NO_THROW(CallRPC("getbalance"));
     BOOST_CHECK_THROW(CallRPC("getbalance " + keyIO.EncodeDestination(demoAddress)), runtime_error);
 
     /*********************************
-     * 			listunspent
+     *          listunspent
      *********************************/
     BOOST_CHECK_NO_THROW(CallRPC("listunspent"));
     BOOST_CHECK_THROW(CallRPC("listunspent string"), runtime_error);
@@ -167,7 +167,7 @@ BOOST_AUTO_TEST_CASE(rpc_wallet)
     BOOST_CHECK(r.get_array().empty());
 
     /*********************************
-     * 		listreceivedbyaddress
+     *          listreceivedbyaddress
      *********************************/
     BOOST_CHECK_NO_THROW(CallRPC("listreceivedbyaddress"));
     BOOST_CHECK_NO_THROW(CallRPC("listreceivedbyaddress 0"));
@@ -201,22 +201,22 @@ BOOST_AUTO_TEST_CASE(rpc_wallet)
     BOOST_CHECK_NO_THROW(CallRPC("listaddressgroupings"));
 
     /*********************************
-     * 		walletconfirmbackup
+     *          walletconfirmbackup
      *********************************/
     BOOST_CHECK_THROW(CallRPC(string("walletconfirmbackup \"badmnemonic\"")), runtime_error);
 
     /*********************************
-     * 		getrawchangeaddress
+     *          getrawchangeaddress
      *********************************/
     BOOST_CHECK_NO_THROW(CallRPC("getrawchangeaddress"));
 
     /*********************************
-     * 		getnewaddress
+     *          getnewaddress
      *********************************/
     BOOST_CHECK_NO_THROW(CallRPC("getnewaddress"));
 
     /*********************************
-     * 	signmessage + verifymessage
+     *          signmessage + verifymessage
      *********************************/
     BOOST_CHECK_NO_THROW(retValue = CallRPC("signmessage " + keyIO.EncodeDestination(demoAddress) + " mymessage"));
     BOOST_CHECK_THROW(CallRPC("signmessage"), runtime_error);
@@ -236,7 +236,7 @@ BOOST_AUTO_TEST_CASE(rpc_wallet)
     BOOST_CHECK(CallRPC("verifymessage " + keyIO.EncodeDestination(demoAddress) + " " + retValue.get_str() + " mymessage").get_bool() == true);
 
     /*********************************
-     * 		listaddresses
+     *          listaddresses
      *********************************/
     BOOST_CHECK_NO_THROW(retValue = CallRPC("listaddresses"));
     UniValue arr = retValue.get_array();
@@ -258,7 +258,7 @@ BOOST_AUTO_TEST_CASE(rpc_wallet)
     }
 
     /*********************************
-     * 	     fundrawtransaction
+     *          fundrawtransaction
      *********************************/
     BOOST_CHECK_THROW(CallRPC("fundrawtransaction 28z"), runtime_error);
     BOOST_CHECK_THROW(CallRPC("fundrawtransaction 01000000000180969800000000001976a91450ce0a4b0ee0ddeb633da85199728b940ac3fe9488ac00000000"), runtime_error);
@@ -802,7 +802,11 @@ void CheckHaveAddr(const std::optional<libzcash::PaymentAddress>& addr) {
     auto addr_of_type = std::get_if<ADDR_TYPE>(&(addr.value()));
     BOOST_ASSERT(addr_of_type != nullptr);
 
-    BOOST_CHECK(pwalletMain->ZTXOSelectorForAddress(*addr_of_type, true, false).has_value());
+    BOOST_CHECK(pwalletMain->ZTXOSelectorForAddress(
+                        *addr_of_type,
+                        true,
+                        TransparentCoinbasePolicy::Allow,
+                        false).has_value());
 }
 
 BOOST_AUTO_TEST_CASE(rpc_wallet_z_getnewaddress) {
@@ -1236,10 +1240,17 @@ BOOST_AUTO_TEST_CASE(rpc_z_sendmany_internals)
 
     // there are no utxos to spend
     {
-        auto selector = pwalletMain->ZTXOSelectorForAddress(taddr1, true, false).value();
-        TransactionBuilder builder(consensusParams, nHeight + 1, std::nullopt, pwalletMain);
-        std::vector<ResolvedPayment> recipients = { ResolvedPayment(std::nullopt, zaddr1, 100*COIN, Memo::FromHexOrThrow("DEADBEEF")) };
-        TransactionStrategy strategy;
+        auto selector = pwalletMain->ZTXOSelectorForAddress(
+                taddr1,
+                true,
+                // In the real transaction builder we use either Require or Disallow, but here we
+                // are checking that there are no UTXOs at all, so we allow either to be selected to
+                // confirm this.
+                TransparentCoinbasePolicy::Allow,
+                false).value();
+        WalletTxBuilder builder(Params(), *pwalletMain, minRelayTxFee);
+        std::vector<Payment> recipients = { Payment(zaddr1, 100*COIN, Memo::FromHexOrThrow("DEADBEEF")) };
+        TransactionStrategy strategy(PrivacyPolicy::AllowRevealedSenders);
         std::shared_ptr<AsyncRPCOperation> operation(new AsyncRPCOperation_sendmany(std::move(builder), selector, recipients, 1, 1, strategy));
         operation->main();
         BOOST_CHECK(operation->isFailed());
@@ -1249,9 +1260,13 @@ BOOST_AUTO_TEST_CASE(rpc_z_sendmany_internals)
 
     // there are no unspent notes to spend
     {
-        auto selector = pwalletMain->ZTXOSelectorForAddress(zaddr1, true, false).value();
-        TransactionBuilder builder(consensusParams, nHeight + 1, std::nullopt, pwalletMain);
-        std::vector<ResolvedPayment> recipients = { ResolvedPayment(std::nullopt, taddr1, 100*COIN, Memo::FromHexOrThrow("DEADBEEF")) };
+        auto selector = pwalletMain->ZTXOSelectorForAddress(
+                zaddr1,
+                true,
+                TransparentCoinbasePolicy::Disallow,
+                false).value();
+        WalletTxBuilder builder(Params(), *pwalletMain, minRelayTxFee);
+        std::vector<Payment> recipients = { Payment(taddr1, 100*COIN, Memo::FromHexOrThrow("DEADBEEF")) };
         TransactionStrategy strategy(PrivacyPolicy::AllowRevealedRecipients);
         std::shared_ptr<AsyncRPCOperation> operation(new AsyncRPCOperation_sendmany(std::move(builder), selector, recipients, 1, 1, strategy));
         operation->main();

src/wallet/wallet.cpp

@@ -1871,6 +1871,7 @@ void CWallet::SyncMetaData(pair<typename TxSpendMap<T>::iterator, typename TxSpe
 std::optional<ZTXOSelector> CWallet::ZTXOSelectorForAccount(
     libzcash::AccountId account,
     bool requireSpendingKey,
+    TransparentCoinbasePolicy transparentCoinbasePolicy,
     std::set<libzcash::ReceiverType> receiverTypes) const
 {
     if (mnemonicHDChain.has_value() &&
@@ -1878,7 +1879,10 @@ std::optional<ZTXOSelector> CWallet::ZTXOSelectorForAccount(
             std::make_pair(mnemonicHDChain.value().GetSeedFingerprint(), account)
         ) > 0)
     {
-        return ZTXOSelector(AccountZTXOPattern(account, receiverTypes), requireSpendingKey);
+        return ZTXOSelector(
+                AccountZTXOPattern(account, receiverTypes),
+                requireSpendingKey,
+                transparentCoinbasePolicy);
     } else {
         return std::nullopt;
     }
@@ -1887,6 +1891,7 @@ std::optional<ZTXOSelector> CWallet::ZTXOSelectorForAccount(
 std::optional<ZTXOSelector> CWallet::ZTXOSelectorForAddress(
         const libzcash::PaymentAddress& addr,
         bool requireSpendingKey,
+        TransparentCoinbasePolicy transparentCoinbasePolicy,
         bool allowAddressLinkability) const
 {
     auto self = this;
@@ -1928,14 +1933,12 @@ std::optional<ZTXOSelector> CWallet::ZTXOSelectorForAddress(
                         pattern = ua;
                     }
                 }
-            } else {
-                pattern = ua;
             }
         }
     }, addr);
 
     if (pattern.has_value()) {
-        return ZTXOSelector(pattern.value(), requireSpendingKey);
+        return ZTXOSelector(pattern.value(), requireSpendingKey, transparentCoinbasePolicy);
     } else {
         return std::nullopt;
     }
@@ -1943,7 +1946,8 @@ std::optional<ZTXOSelector> CWallet::ZTXOSelectorForAddress(
 
 std::optional<ZTXOSelector> CWallet::ZTXOSelectorForViewingKey(
         const libzcash::ViewingKey& vk,
-        bool requireSpendingKey) const
+        bool requireSpendingKey,
+        TransparentCoinbasePolicy transparentCoinbasePolicy) const
 {
     auto self = this;
     std::optional<ZTXOPattern> pattern = std::nullopt;
@@ -1970,16 +1974,17 @@ std::optional<ZTXOSelector> CWallet::ZTXOSelectorForViewingKey(
     }, vk);
 
     if (pattern.has_value()) {
-        return ZTXOSelector(pattern.value(), requireSpendingKey);
+        return ZTXOSelector(pattern.value(), requireSpendingKey, transparentCoinbasePolicy);
     } else {
         return std::nullopt;
     }
 }
 
-ZTXOSelector CWallet::LegacyTransparentZTXOSelector(bool requireSpendingKey) {
+ZTXOSelector CWallet::LegacyTransparentZTXOSelector(bool requireSpendingKey, TransparentCoinbasePolicy transparentCoinbasePolicy) {
     return ZTXOSelector(
             AccountZTXOPattern(ZCASH_LEGACY_ACCOUNT, {ReceiverType::P2PKH, ReceiverType::P2SH}),
-            requireSpendingKey);
+            requireSpendingKey,
+            transparentCoinbasePolicy);
 }
 
 std::optional<libzcash::AccountId> CWallet::FindAccountForSelector(const ZTXOSelector& selector) const {
@@ -2207,7 +2212,6 @@ std::optional<RecipientAddress> CWallet::GenerateChangeAddressForAccount(
 
 SpendableInputs CWallet::FindSpendableInputs(
         ZTXOSelector selector,
-        bool allowTransparentCoinbase,
         uint32_t minDepth,
         const std::optional<int>& asOfHeight) const {
     AssertLockHeld(cs_main);
@@ -2216,7 +2220,6 @@ SpendableInputs CWallet::FindSpendableInputs(
     KeyIO keyIO(Params());
 
     bool selectTransparent{selector.SelectsTransparent()};
-    bool selectTransparentCoinbase{selector.SelectsTransparentCoinbase()};
     bool selectSprout{selector.SelectsSprout()};
     bool selectSapling{selector.SelectsSapling()};
     bool selectOrchard{selector.SelectsOrchard()};
@@ -2230,10 +2233,18 @@ SpendableInputs CWallet::FindSpendableInputs(
         if (!CheckFinalTx(wtx)) continue;
         if (nDepth < 0 || nDepth < minDepth) continue;
 
-        if (selectTransparent &&
-            // skip transparent utxo selection if coinbase spend restrictions are not met
-            (!isCoinbase || (selectTransparentCoinbase && allowTransparentCoinbase && wtx.GetBlocksToMaturity(asOfHeight) <= 0))) {
-
+        if (selectTransparent && (
+            (
+                // Only select coinbase transparent utxos if spend restrictions are met.
+                isCoinbase &&
+                selector.transparentCoinbasePolicy != TransparentCoinbasePolicy::Disallow &&
+                wtx.GetBlocksToMaturity(asOfHeight) <= 0
+            ) || (
+                // Only select non-coinbase transparent utxos if we are allowed to.
+                !isCoinbase &&
+                selector.transparentCoinbasePolicy != TransparentCoinbasePolicy::Require
+            )
+        )) {
             for (int i = 0; i < wtx.vout.size(); i++) {
                 const auto& output = wtx.vout[i];
                 isminetype mine = IsMine(output);
@@ -7738,32 +7749,15 @@ bool ZTXOSelector::SelectsTransparent() const {
         [](const AccountZTXOPattern& acct) { return acct.IncludesP2PKH() || acct.IncludesP2SH(); }
     }, this->pattern);
 }
-bool ZTXOSelector::SelectsTransparentCoinbase() const {
-    return std::visit(match {
-        [](const CKeyID& keyId) { return true; },
-        [](const CScriptID& scriptId) { return true; },
-        [](const libzcash::SproutPaymentAddress& addr) { return false; },
-        [](const libzcash::SproutViewingKey& vk) { return false; },
-        [](const libzcash::SaplingPaymentAddress& addr) { return false; },
-        [](const libzcash::SaplingExtendedFullViewingKey& vk) { return false; },
-        [](const libzcash::UnifiedAddress& ua) {
-            return ua.GetP2PKHReceiver().has_value() || ua.GetP2SHReceiver().has_value();
-        },
-        [](const libzcash::UnifiedFullViewingKey& ufvk) { return ufvk.GetTransparentKey().has_value(); },
-        [](const AccountZTXOPattern& acct) {
-            return (acct.IncludesP2PKH() || acct.IncludesP2SH()) && acct.GetAccountId() != ZCASH_LEGACY_ACCOUNT;
-        }
-    }, this->pattern);
-}
 bool ZTXOSelector::SelectsSprout() const {
-    return std::visit(match {
+    return transparentCoinbasePolicy != TransparentCoinbasePolicy::Require && std::visit(match {
         [](const libzcash::SproutViewingKey& addr) { return true; },
         [](const libzcash::SproutPaymentAddress& extfvk) { return true; },
         [](const auto& addr) { return false; }
     }, this->pattern);
 }
 bool ZTXOSelector::SelectsSapling() const {
-    return std::visit(match {
+    return transparentCoinbasePolicy != TransparentCoinbasePolicy::Require && std::visit(match {
         [](const libzcash::SaplingPaymentAddress& addr) { return true; },
         [](const libzcash::SaplingExtendedSpendingKey& extfvk) { return true; },
         [](const libzcash::UnifiedAddress& ua) { return ua.GetSaplingReceiver().has_value(); },
@@ -7773,7 +7767,7 @@ bool ZTXOSelector::SelectsSapling() const {
     }, this->pattern);
 }
 bool ZTXOSelector::SelectsOrchard() const {
-    return std::visit(match {
+    return transparentCoinbasePolicy != TransparentCoinbasePolicy::Require && std::visit(match {
         [](const libzcash::UnifiedAddress& ua) { return ua.GetOrchardReceiver().has_value(); },
         [](const libzcash::UnifiedFullViewingKey& ufvk) { return ufvk.GetOrchardKey().has_value(); },
         [](const AccountZTXOPattern& acct) { return acct.IncludesOrchard(); },

src/wallet/wallet.h

@@ -904,13 +904,27 @@ typedef std::variant<
     libzcash::UnifiedFullViewingKey,
     AccountZTXOPattern> ZTXOPattern;
 
+/**
+ * For transactions, either `Disallow` or `Require` must be used, but `Allow` is generally used when
+ * calculating balances.
+ */
+enum class TransparentCoinbasePolicy {
+    Disallow, //!< Do not select transparent coinbase
+    Allow,    //!< Make transparent coinbase available to the selector
+    Require   //!< Only select transparent coinbase
+};
+
 class ZTXOSelector {
 private:
     ZTXOPattern pattern;
     bool requireSpendingKeys;
+    TransparentCoinbasePolicy transparentCoinbasePolicy;
 
-    ZTXOSelector(ZTXOPattern patternIn, bool requireSpendingKeysIn):
-        pattern(patternIn), requireSpendingKeys(requireSpendingKeysIn) {}
+    ZTXOSelector(ZTXOPattern patternIn, bool requireSpendingKeysIn, TransparentCoinbasePolicy transparentCoinbasePolicy):
+        pattern(patternIn), requireSpendingKeys(requireSpendingKeysIn), transparentCoinbasePolicy(transparentCoinbasePolicy) {
+        // We can’t require transparent coinbase unless we’re selecting transparent funds.
+        assert(SelectsTransparent() || transparentCoinbasePolicy != TransparentCoinbasePolicy::Require);
+}
 
     friend class CWallet;
 public:
@@ -922,8 +936,11 @@ public:
         return requireSpendingKeys;
     }
 
+    TransparentCoinbasePolicy TransparentCoinbasePolicy() const {
+        return transparentCoinbasePolicy;
+    }
+
     bool SelectsTransparent() const;
-    bool SelectsTransparentCoinbase() const;
     bool SelectsSprout() const;
     bool SelectsSapling() const;
     bool SelectsOrchard() const;
@@ -967,14 +984,14 @@ public:
      */
     CAmount Total() const {
         CAmount result = 0;
-        result += GetTransparentBalance();
-        result += GetSproutBalance();
-        result += GetSaplingBalance();
-        result += GetOrchardBalance();
+        result += GetTransparentTotal();
+        result += GetSproutTotal();
+        result += GetSaplingTotal();
+        result += GetOrchardTotal();
         return result;
     }
 
-    CAmount GetTransparentBalance() const {
+    CAmount GetTransparentTotal() const {
         CAmount result = 0;
         for (const auto& t : utxos) {
             result += t.Value();
@@ -982,7 +999,7 @@ public:
         return result;
     }
 
-    CAmount GetSproutBalance() const {
+    CAmount GetSproutTotal() const {
         CAmount result = 0;
         for (const auto& t : sproutNoteEntries) {
             result += t.note.value();
@@ -990,7 +1007,7 @@ public:
         return result;
     }
 
-    CAmount GetSaplingBalance() const {
+    CAmount GetSaplingTotal() const {
         CAmount result = 0;
         for (const auto& t : saplingNoteEntries) {
             result += t.note.value();
@@ -998,7 +1015,7 @@ public:
         return result;
     }
 
-    CAmount GetOrchardBalance() const {
+    CAmount GetOrchardTotal() const {
         CAmount result = 0;
         for (const auto& t : orchardNoteMetadata) {
             result += t.GetNoteValue();
@@ -1541,6 +1558,7 @@ public:
     std::optional<ZTXOSelector> ZTXOSelectorForAccount(
             libzcash::AccountId account,
             bool requireSpendingKey,
+            TransparentCoinbasePolicy transparentCoinbasePolicy,
             std::set<libzcash::ReceiverType> receiverTypes={}) const;
 
     /**
@@ -1552,6 +1570,7 @@ public:
     std::optional<ZTXOSelector> ZTXOSelectorForAddress(
             const libzcash::PaymentAddress& addr,
             bool requireSpendingKey,
+            TransparentCoinbasePolicy transparentCoinbasePolicy,
             bool allowAddressLinkability) const;
 
     /**
@@ -1562,13 +1581,14 @@ public:
      */
     std::optional<ZTXOSelector> ZTXOSelectorForViewingKey(
             const libzcash::ViewingKey& vk,
-            bool requireSpendingKey) const;
+            bool requireSpendingKey,
+            TransparentCoinbasePolicy transparentCoinbasePolicy) const;
 
     /**
      * Returns the ZTXO selector that will select UTXOs sent to legacy
      * transparent addresses managed by this wallet.
      */
-    static ZTXOSelector LegacyTransparentZTXOSelector(bool requireSpendingKey);
+    static ZTXOSelector LegacyTransparentZTXOSelector(bool requireSpendingKey, TransparentCoinbasePolicy transparentCoinbasePolicy);
 
     /**
      * Look up the account for a given selector. This resolves the account ID
@@ -1598,7 +1618,6 @@ public:
 
     SpendableInputs FindSpendableInputs(
             ZTXOSelector paymentSource,
-            bool allowTransparentCoinbase,
             uint32_t minDepth,
             const std::optional<int>& asOfHeight) const;
 

src/wallet/wallet_tx_builder.cpp

@@ -0,0 +1,630 @@
+// Copyright (c) 2022 The Zcash developers
+// Distributed under the MIT software license, see the accompanying
+// file COPYING or https://www.opensource.org/licenses/mit-license.php .
+
+#include "wallet/wallet_tx_builder.h"
+
+using namespace libzcash;
+
+int GetAnchorHeight(const CChain& chain, uint32_t anchorConfirmations)
+{
+    int nextBlockHeight = chain.Height() + 1;
+    return nextBlockHeight - anchorConfirmations;
+}
+
+PrepareTransactionResult WalletTxBuilder::PrepareTransaction(
+        const ZTXOSelector& selector,
+        SpendableInputs& spendable,
+        const std::vector<Payment>& payments,
+        const CChain& chain,
+        TransactionStrategy strategy,
+        CAmount fee,
+        uint32_t anchorConfirmations) const
+{
+    assert(fee < MAX_MONEY);
+
+    int anchorHeight = GetAnchorHeight(chain, anchorConfirmations);
+    auto selected = ResolveInputsAndPayments(selector, spendable, payments, chain, strategy, fee, anchorHeight);
+    if (std::holds_alternative<InputSelectionError>(selected)) {
+        return std::get<InputSelectionError>(selected);
+    }
+
+    auto resolvedSelection = std::get<InputSelection>(selected);
+    auto resolvedPayments = resolvedSelection.GetPayments();
+
+    // We do not set a change address if there is no change.
+    std::optional<ChangeAddress> changeAddr;
+    auto changeAmount = spendable.Total() - resolvedPayments.Total() - fee;
+    if (changeAmount > 0) {
+        // Determine the account we're sending from.
+        auto sendFromAccount = wallet.FindAccountForSelector(selector).value_or(ZCASH_LEGACY_ACCOUNT);
+
+        auto getAllowedChangePools = [&](const std::set<ReceiverType>& receiverTypes) {
+            std::set<OutputPool> result{resolvedPayments.GetRecipientPools()};
+            // We always allow shielded change when not sending from the legacy account.
+            if (sendFromAccount != ZCASH_LEGACY_ACCOUNT) {
+                result.insert(OutputPool::Sapling);
+            }
+            for (ReceiverType rtype : receiverTypes) {
+                switch (rtype) {
+                    case ReceiverType::P2PKH:
+                    case ReceiverType::P2SH:
+                        // TODO: This is the correct policy, but it’s a breaking change from
+                        //       previous behavior, so enable it separately. (#6409)
+                        // if (strategy.AllowRevealedRecipients()) {
+                        if (!spendable.utxos.empty() || strategy.AllowRevealedRecipients()) {
+                            result.insert(OutputPool::Transparent);
+                        }
+                        break;
+                    case ReceiverType::Sapling:
+                        if (!spendable.saplingNoteEntries.empty() || strategy.AllowRevealedAmounts()) {
+                            result.insert(OutputPool::Sapling);
+                        }
+                        break;
+                    case ReceiverType::Orchard:
+                        if (params.GetConsensus().NetworkUpgradeActive(anchorHeight, Consensus::UPGRADE_NU5)
+                            && (!spendable.orchardNoteMetadata.empty() || strategy.AllowRevealedAmounts())) {
+                            result.insert(OutputPool::Orchard);
+                        }
+                        break;
+                }
+            }
+            return result;
+        };
+
+        auto addChangePayment = [&](const std::optional<RecipientAddress>& sendTo) {
+            assert(sendTo.has_value());
+            resolvedPayments.AddPayment(
+                    ResolvedPayment(std::nullopt, sendTo.value(), changeAmount, std::nullopt, true));
+            return sendTo.value();
+        };
+
+        auto changeAddressForTransparentSelector = [&](const std::set<ReceiverType>& receiverTypes) {
+            return addChangePayment(
+                    pwalletMain->GenerateChangeAddressForAccount(
+                            sendFromAccount,
+                            getAllowedChangePools(receiverTypes)));
+        };
+
+        auto changeAddressForSaplingAddress = [&](const libzcash::SaplingPaymentAddress& addr) {
+            // for Sapling, if using a legacy address, return change to the
+            // originating address; otherwise return it to the Sapling internal
+            // address corresponding to the UFVK.
+            return addChangePayment(
+                    sendFromAccount == ZCASH_LEGACY_ACCOUNT
+                    ? addr
+                    : pwalletMain->GenerateChangeAddressForAccount(
+                            sendFromAccount,
+                            getAllowedChangePools({ReceiverType::Sapling})));
+        };
+
+        auto changeAddressForZUFVK = [&](
+                const ZcashdUnifiedFullViewingKey& zufvk,
+                const std::set<ReceiverType>& receiverTypes) {
+            return addChangePayment(zufvk.GetChangeAddress(getAllowedChangePools(receiverTypes)));
+        };
+
+        changeAddr = std::visit(match {
+            [&](const CKeyID&) -> ChangeAddress {
+                return changeAddressForTransparentSelector({ReceiverType::P2PKH});
+            },
+            [&](const CScriptID&) -> ChangeAddress {
+                return changeAddressForTransparentSelector({ReceiverType::P2SH});
+            },
+            [](const libzcash::SproutPaymentAddress& addr) -> ChangeAddress {
+                // for Sprout, we return change to the originating address using the tx builder.
+                return addr;
+            },
+            [](const libzcash::SproutViewingKey& vk) -> ChangeAddress {
+                // for Sprout, we return change to the originating address using the tx builder.
+                return vk.address();
+            },
+            [&](const libzcash::SaplingPaymentAddress& addr) -> ChangeAddress {
+                return changeAddressForSaplingAddress(addr);
+            },
+            [&](const libzcash::SaplingExtendedFullViewingKey& fvk) -> ChangeAddress {
+                return changeAddressForSaplingAddress(fvk.DefaultAddress());
+            },
+            [&](const libzcash::UnifiedAddress& ua) -> ChangeAddress {
+                auto zufvk = pwalletMain->GetUFVKForAddress(ua);
+                assert(zufvk.has_value());
+                return changeAddressForZUFVK(zufvk.value(), ua.GetKnownReceiverTypes());
+            },
+            [&](const libzcash::UnifiedFullViewingKey& fvk) -> ChangeAddress {
+                return changeAddressForZUFVK(
+                        ZcashdUnifiedFullViewingKey::FromUnifiedFullViewingKey(params, fvk),
+                        fvk.GetKnownReceiverTypes());
+            },
+            [&](const AccountZTXOPattern& acct) -> ChangeAddress {
+                return addChangePayment(
+                        pwalletMain->GenerateChangeAddressForAccount(
+                                acct.GetAccountId(),
+                                getAllowedChangePools(acct.GetReceiverTypes())));
+            }
+        }, selector.GetPattern());
+    }
+
+    auto ovks = SelectOVKs(selector, spendable);
+
+    return TransactionEffects(
+            anchorConfirmations,
+            spendable,
+            resolvedPayments,
+            changeAddr,
+            fee,
+            ovks.first,
+            ovks.second,
+            anchorHeight);
+}
+
+Payments InputSelection::GetPayments() const {
+    return this->payments;
+}
+
+CAmount WalletTxBuilder::DefaultDustThreshold() const {
+    CKey secret{CKey::TestOnlyRandomKey(true)};
+    CScript scriptPubKey = GetScriptForDestination(secret.GetPubKey().GetID());
+    CTxOut txout(CAmount(1), scriptPubKey);
+    return txout.GetDustThreshold(minRelayFee);
+}
+
+SpendableInputs WalletTxBuilder::FindAllSpendableInputs(
+        const ZTXOSelector& selector,
+        int32_t minDepth) const
+{
+    LOCK2(cs_main, wallet.cs_wallet);
+    return wallet.FindSpendableInputs(selector, minDepth, std::nullopt);
+}
+
+InputSelectionResult WalletTxBuilder::ResolveInputsAndPayments(
+        const ZTXOSelector& selector,
+        SpendableInputs& spendableMut,
+        const std::vector<Payment>& payments,
+        const CChain& chain,
+        TransactionStrategy strategy,
+        CAmount fee,
+        int anchorHeight) const
+{
+    LOCK2(cs_main, wallet.cs_wallet);
+
+    // Determine the target totals
+    CAmount sendAmount{0};
+    for (const auto& payment : payments) {
+        sendAmount += payment.GetAmount();
+    }
+    CAmount targetAmount = sendAmount + fee;
+
+    // This is a simple greedy algorithm to attempt to preserve requested
+    // transactional privacy while moving as much value to the most recent pool
+    // as possible.  This will also perform opportunistic shielding if the
+    // transaction strategy permits.
+
+    CAmount maxSaplingAvailable = spendableMut.GetSaplingTotal();
+    CAmount maxOrchardAvailable = spendableMut.GetOrchardTotal();
+    uint32_t orchardOutputs{0};
+
+    // we can only select Orchard addresses if there are sufficient non-Sprout
+    // funds to cover the total payments + fee.
+    bool canResolveOrchard =
+        params.GetConsensus().NetworkUpgradeActive(anchorHeight, Consensus::UPGRADE_NU5)
+        && spendableMut.Total() - spendableMut.GetSproutTotal() >= targetAmount;
+    std::vector<ResolvedPayment> resolvedPayments;
+    std::optional<AddressResolutionError> resolutionError;
+    for (const auto& payment : payments) {
+        std::visit(match {
+            [&](const CKeyID& p2pkh) {
+                if (strategy.AllowRevealedRecipients()) {
+                    resolvedPayments.emplace_back(
+                            std::nullopt, p2pkh, payment.GetAmount(), payment.GetMemo(), false);
+                } else {
+                    resolutionError = AddressResolutionError::TransparentRecipientNotAllowed;
+                }
+            },
+            [&](const CScriptID& p2sh) {
+                if (strategy.AllowRevealedRecipients()) {
+                    resolvedPayments.emplace_back(
+                            std::nullopt, p2sh, payment.GetAmount(), payment.GetMemo(), false);
+                } else {
+                    resolutionError = AddressResolutionError::TransparentRecipientNotAllowed;
+                }
+            },
+            [&](const SproutPaymentAddress&) {
+                resolutionError = AddressResolutionError::SproutRecipientsNotSupported;
+            },
+            [&](const SaplingPaymentAddress& addr) {
+                if (strategy.AllowRevealedAmounts() || payment.GetAmount() <= maxSaplingAvailable) {
+                    resolvedPayments.emplace_back(
+                            std::nullopt, addr, payment.GetAmount(), payment.GetMemo(), false);
+                    if (!strategy.AllowRevealedAmounts()) {
+                        maxSaplingAvailable -= payment.GetAmount();
+                    }
+                } else {
+                    resolutionError = AddressResolutionError::RevealingSaplingAmountNotAllowed;
+                }
+            },
+            [&](const UnifiedAddress& ua) {
+                if (canResolveOrchard
+                    && ua.GetOrchardReceiver().has_value()
+                    && (strategy.AllowRevealedAmounts() || payment.GetAmount() <= maxOrchardAvailable)
+                    ) {
+                    resolvedPayments.emplace_back(
+                        ua, ua.GetOrchardReceiver().value(), payment.GetAmount(), payment.GetMemo(), false);
+                    if (!strategy.AllowRevealedAmounts()) {
+                        maxOrchardAvailable -= payment.GetAmount();
+                    }
+                    orchardOutputs += 1;
+                } else if (ua.GetSaplingReceiver().has_value()
+                    && (strategy.AllowRevealedAmounts() || payment.GetAmount() <= maxSaplingAvailable)
+                    ) {
+                    resolvedPayments.emplace_back(
+                        ua, ua.GetSaplingReceiver().value(), payment.GetAmount(), payment.GetMemo(), false);
+                    if (!strategy.AllowRevealedAmounts()) {
+                        maxSaplingAvailable -= payment.GetAmount();
+                    }
+                } else {
+                    if (strategy.AllowRevealedRecipients()) {
+                        if (ua.GetP2SHReceiver().has_value()) {
+                            resolvedPayments.emplace_back(
+                                ua, ua.GetP2SHReceiver().value(), payment.GetAmount(), std::nullopt, false);
+                        } else if (ua.GetP2PKHReceiver().has_value()) {
+                            resolvedPayments.emplace_back(
+                                ua, ua.GetP2PKHReceiver().value(), payment.GetAmount(), std::nullopt, false);
+                        } else {
+                            // This should only occur when we have
+                            // • an Orchard-only UA,
+                            // • `AllowRevealedRecipients`, and
+                            // • can’t resolve Orchard (which means either insufficient non-Sprout
+                            //   funds or pre-NU5).
+                            resolutionError = AddressResolutionError::CouldNotResolveReceiver;
+                        }
+                    } else if (strategy.AllowRevealedAmounts()) {
+                        resolutionError = AddressResolutionError::TransparentReceiverNotAllowed;
+                    } else {
+                        resolutionError = AddressResolutionError::RevealingReceiverAmountsNotAllowed;
+                    }
+                }
+            }
+        }, payment.GetAddress());
+
+        if (resolutionError.has_value()) {
+            return resolutionError.value();
+        }
+    }
+    auto resolved = Payments(resolvedPayments);
+
+    if (orchardOutputs > this->maxOrchardActions) {
+        return ExcessOrchardActionsError(
+                ActionSide::Output,
+                orchardOutputs,
+                this->maxOrchardActions);
+    }
+
+    // Set the dust threshold so that we can select enough inputs to avoid
+    // creating dust change amounts.
+    CAmount dustThreshold{this->DefaultDustThreshold()};
+
+    // TODO: the set of recipient pools is not quite sufficient information here; we should
+    // probably perform note selection at the same time as we're performing resolved payment
+    // construction above.
+    if (!spendableMut.LimitToAmount(targetAmount, dustThreshold, resolved.GetRecipientPools())) {
+        CAmount changeAmount{spendableMut.Total() - targetAmount};
+        return InvalidFundsError(
+                spendableMut.Total(),
+                changeAmount > 0 && changeAmount < dustThreshold
+                // TODO: we should provide the option for the caller to explicitly
+                // forego change (definitionally an amount below the dust amount)
+                // and send the extra to the recipient or the miner fee to avoid
+                // creating dust change, rather than prohibit them from sending
+                // entirely in this circumstance.
+                // (Daira disagrees, as this could leak information to the recipient
+                // or publically in the fee.)
+                ? InvalidFundsReason(DustThresholdError(dustThreshold, changeAmount))
+                : InvalidFundsReason(InsufficientFundsError(targetAmount)));
+    }
+
+    // When spending transparent coinbase outputs, all inputs must be fully
+    // consumed, and they may only be sent to shielded recipients.
+    if (spendableMut.HasTransparentCoinbase()) {
+        if (spendableMut.Total() != targetAmount) {
+            return ChangeNotAllowedError(spendableMut.Total(), targetAmount);
+        } else if (resolved.HasTransparentRecipient()) {
+            return AddressResolutionError::TransparentRecipientNotAllowed;
+        }
+    }
+
+    if (spendableMut.orchardNoteMetadata.size() > this->maxOrchardActions) {
+        return ExcessOrchardActionsError(
+                ActionSide::Input,
+                spendableMut.orchardNoteMetadata.size(),
+                this->maxOrchardActions);
+    }
+
+    return InputSelection(resolved, anchorHeight);
+}
+
+std::pair<uint256, uint256>
+GetOVKsForUFVK(const UnifiedFullViewingKey& ufvk, const SpendableInputs& spendable)
+{
+    if (!spendable.orchardNoteMetadata.empty()) {
+        auto fvk = ufvk.GetOrchardKey();
+        // Orchard notes will not have been selected if the UFVK does not contain an Orchard key.
+        assert(fvk.has_value());
+        return std::make_pair(
+                fvk.value().ToInternalOutgoingViewingKey(),
+                fvk.value().ToExternalOutgoingViewingKey());
+    } else if (!spendable.saplingNoteEntries.empty()) {
+        auto dfvk = ufvk.GetSaplingKey();
+        // Sapling notes will not have been selected if the UFVK does not contain a Sapling key.
+        assert(dfvk.has_value());
+        return dfvk.value().GetOVKs();
+    } else if (!spendable.utxos.empty()) {
+        // Transparent UTXOs will not have been selected if the UFVK does not contain a transparent
+        // key.
+        auto tfvk = ufvk.GetTransparentKey();
+        assert(tfvk.has_value());
+        return tfvk.value().GetOVKsForShielding();
+    } else {
+        // This should be unreachable.
+        throw std::runtime_error("No spendable inputs.");
+    }
+}
+
+std::pair<uint256, uint256> WalletTxBuilder::SelectOVKs(
+        const ZTXOSelector& selector,
+        const SpendableInputs& spendable) const
+{
+    return std::visit(match {
+        [&](const CKeyID& keyId) {
+            return wallet.GetLegacyAccountKey().ToAccountPubKey().GetOVKsForShielding();
+        },
+        [&](const CScriptID& keyId) {
+            return wallet.GetLegacyAccountKey().ToAccountPubKey().GetOVKsForShielding();
+        },
+        [&](const libzcash::SproutPaymentAddress&) {
+            return wallet.GetLegacyAccountKey().ToAccountPubKey().GetOVKsForShielding();
+        },
+        [&](const libzcash::SproutViewingKey&) {
+            return wallet.GetLegacyAccountKey().ToAccountPubKey().GetOVKsForShielding();
+        },
+        [&](const libzcash::SaplingPaymentAddress& addr) {
+            libzcash::SaplingExtendedSpendingKey extsk;
+            assert(wallet.GetSaplingExtendedSpendingKey(addr, extsk));
+            return extsk.ToXFVK().GetOVKs();
+        },
+        [](const libzcash::SaplingExtendedFullViewingKey& sxfvk) {
+            return sxfvk.GetOVKs();
+        },
+        [&](const UnifiedAddress& ua) {
+            auto ufvk = wallet.GetUFVKForAddress(ua);
+            // This is safe because spending key checks will have ensured that we have a UFVK
+            // corresponding to this address.
+            assert(ufvk.has_value());
+            return GetOVKsForUFVK(ufvk.value().ToFullViewingKey(), spendable);
+        },
+        [&](const UnifiedFullViewingKey& ufvk) {
+            return GetOVKsForUFVK(ufvk, spendable);
+        },
+        [&](const AccountZTXOPattern& acct) {
+            if (acct.GetAccountId() == ZCASH_LEGACY_ACCOUNT) {
+                return wallet.GetLegacyAccountKey().ToAccountPubKey().GetOVKsForShielding();
+            } else {
+                auto ufvk = wallet.GetUnifiedFullViewingKeyByAccount(acct.GetAccountId());
+                // By definition, we have a UFVK for every known non-legacy account.
+                assert(ufvk.has_value());
+                return GetOVKsForUFVK(ufvk.value().ToFullViewingKey(), spendable);
+            }
+        },
+    }, selector.GetPattern());
+}
+
+PrivacyPolicy TransactionEffects::GetRequiredPrivacyPolicy() const
+{
+    if (!spendable.utxos.empty()) {
+        // TODO: Add a check for whether we need AllowLinkingAccountAddresses here. (#6467)
+        if (payments.HasTransparentRecipient()) {
+            // TODO: AllowFullyTransparent is the correct policy, but it’s a breaking change from
+            //       previous behavior, so enable it separately. (#6409)
+            // maxPrivacy = PrivacyPolicy::AllowFullyTransparent;
+            return PrivacyPolicy::AllowRevealedSenders;
+        } else {
+            return PrivacyPolicy::AllowRevealedSenders;
+        }
+    } else if (payments.HasTransparentRecipient()) {
+        return PrivacyPolicy::AllowRevealedRecipients;
+    } else if (!spendable.orchardNoteMetadata.empty() && payments.HasSaplingRecipient()
+               || !spendable.saplingNoteEntries.empty() && payments.HasOrchardRecipient()
+               || !spendable.sproutNoteEntries.empty() && payments.HasSaplingRecipient()) {
+        // TODO: This should only trigger when there is a non-zero valueBalance.
+        return PrivacyPolicy::AllowRevealedAmounts;
+    } else {
+        return PrivacyPolicy::FullPrivacy;
+    }
+}
+
+bool TransactionEffects::InvolvesOrchard() const
+{
+    return spendable.GetOrchardTotal() > 0 || payments.HasOrchardRecipient();
+}
+
+TransactionBuilderResult TransactionEffects::ApproveAndBuild(
+        const Consensus::Params& consensus,
+        const CWallet& wallet,
+        const CChain& chain,
+        const TransactionStrategy& strategy) const
+{
+    auto requiredPrivacy = this->GetRequiredPrivacyPolicy();
+    if (!strategy.IsCompatibleWith(requiredPrivacy)) {
+        return TransactionBuilderResult(strprintf(
+            "The specified privacy policy, %s, does not permit the creation of "
+            "the requested transaction. Select %s to allow this transaction "
+            "to be constructed.",
+            strategy.PolicyName(),
+            TransactionStrategy::ToString(requiredPrivacy)
+            + (requiredPrivacy == PrivacyPolicy::NoPrivacy ? "" : " or weaker")));
+    }
+
+    int nextBlockHeight = chain.Height() + 1;
+
+    // Allow Orchard recipients by setting an Orchard anchor.
+    std::optional<uint256> orchardAnchor;
+    if (spendable.sproutNoteEntries.empty()
+        && (InvolvesOrchard() || nPreferredTxVersion > ZIP225_MIN_TX_VERSION)
+        && this->anchorConfirmations > 0)
+    {
+        LOCK(cs_main);
+        auto anchorBlockIndex = chain[this->anchorHeight];
+        assert(anchorBlockIndex != nullptr);
+        orchardAnchor = anchorBlockIndex->hashFinalOrchardRoot;
+    }
+
+    auto builder = TransactionBuilder(consensus, nextBlockHeight, orchardAnchor, &wallet);
+    builder.SetFee(fee);
+
+    // Track the total of notes that we've added to the builder. This
+    // shouldn't strictly be necessary, given `spendable.LimitToAmount`
+    CAmount totalSpend = 0;
+
+    // Create Sapling outpoints
+    std::vector<SaplingOutPoint> saplingOutPoints;
+    std::vector<SaplingNote> saplingNotes;
+    std::vector<SaplingExtendedSpendingKey> saplingKeys;
+
+    for (const auto& t : spendable.saplingNoteEntries) {
+        saplingOutPoints.push_back(t.op);
+        saplingNotes.push_back(t.note);
+
+        libzcash::SaplingExtendedSpendingKey saplingKey;
+        assert(wallet.GetSaplingExtendedSpendingKey(t.address, saplingKey));
+        saplingKeys.push_back(saplingKey);
+
+        totalSpend += t.note.value();
+    }
+
+    // Fetch Sapling anchor and witnesses, and Orchard Merkle paths.
+    uint256 anchor;
+    std::vector<std::optional<SaplingWitness>> witnesses;
+    std::vector<std::pair<libzcash::OrchardSpendingKey, orchard::SpendInfo>> orchardSpendInfo;
+    {
+        LOCK(wallet.cs_wallet);
+        if (!wallet.GetSaplingNoteWitnesses(saplingOutPoints, anchorConfirmations, witnesses, anchor)) {
+            // This error should not appear once we're nAnchorConfirmations blocks past
+            // Sapling activation.
+            return TransactionBuilderResult("Insufficient Sapling witnesses.");
+        }
+        if (builder.GetOrchardAnchor().has_value()) {
+            orchardSpendInfo = wallet.GetOrchardSpendInfo(spendable.orchardNoteMetadata, builder.GetOrchardAnchor().value());
+        }
+    }
+
+    // Add Orchard spends
+    for (size_t i = 0; i < orchardSpendInfo.size(); i++) {
+        auto spendInfo = std::move(orchardSpendInfo[i]);
+        if (!builder.AddOrchardSpend(
+            std::move(spendInfo.first),
+            std::move(spendInfo.second)))
+        {
+            return TransactionBuilderResult(
+                strprintf("Failed to add Orchard note to transaction (check %s for details)", GetDebugLogPath())
+            );
+        } else {
+            totalSpend += spendInfo.second.Value();
+        }
+    }
+
+    // Add Sapling spends
+    for (size_t i = 0; i < saplingNotes.size(); i++) {
+        if (!witnesses[i]) {
+            return TransactionBuilderResult(strprintf(
+                "Missing witness for Sapling note at outpoint %s",
+                spendable.saplingNoteEntries[i].op.ToString()
+            ));
+        }
+
+        builder.AddSaplingSpend(saplingKeys[i].expsk, saplingNotes[i], anchor, witnesses[i].value());
+    }
+
+    // Add outputs
+    for (const auto& r : payments.GetResolvedPayments()) {
+        std::visit(match {
+            [&](const CKeyID& keyId) {
+                builder.AddTransparentOutput(keyId, r.amount);
+            },
+            [&](const CScriptID& scriptId) {
+                builder.AddTransparentOutput(scriptId, r.amount);
+            },
+            [&](const libzcash::SaplingPaymentAddress& addr) {
+                builder.AddSaplingOutput(
+                        r.isInternal ? internalOVK : externalOVK, addr, r.amount,
+                        r.memo.has_value() ? r.memo.value().ToBytes() : Memo::NoMemo().ToBytes());
+            },
+            [&](const libzcash::OrchardRawAddress& addr) {
+                builder.AddOrchardOutput(
+                        r.isInternal ? internalOVK : externalOVK, addr, r.amount,
+                        r.memo.has_value() ? std::optional(r.memo.value().ToBytes()) : std::nullopt);
+            }
+        }, r.address);
+    }
+
+    // Add transparent utxos
+    for (const auto& out : spendable.utxos) {
+        const CTxOut& txOut = out.tx->vout[out.i];
+        builder.AddTransparentInput(COutPoint(out.tx->GetHash(), out.i), txOut.scriptPubKey, txOut.nValue);
+
+        totalSpend += txOut.nValue;
+    }
+
+    // Find Sprout witnesses
+    // When spending notes, take a snapshot of note witnesses and anchors as the treestate will
+    // change upon arrival of new blocks which contain joinsplit transactions.  This is likely
+    // to happen as creating a chained joinsplit transaction can take longer than the block interval.
+    // So, we need to take locks on cs_main and wallet.cs_wallet so that the witnesses aren't
+    // updated.
+    //
+    // TODO: these locks would ideally be shared for selection of Sapling anchors and witnesses
+    // as well.
+    std::vector<std::optional<SproutWitness>> vSproutWitnesses;
+    {
+        LOCK2(cs_main, wallet.cs_wallet);
+        std::vector<JSOutPoint> vOutPoints;
+        for (const auto& t : spendable.sproutNoteEntries) {
+            vOutPoints.push_back(t.jsop);
+        }
+
+        // inputAnchor is not needed by builder.AddSproutInput as it is for Sapling.
+        uint256 inputAnchor;
+        if (!wallet.GetSproutNoteWitnesses(vOutPoints, anchorConfirmations, vSproutWitnesses, inputAnchor)) {
+            // This error should not appear once we're nAnchorConfirmations blocks past
+            // Sprout activation.
+            return TransactionBuilderResult("Insufficient Sprout witnesses.");
+        }
+    }
+
+    // Add Sprout spends
+    for (int i = 0; i < spendable.sproutNoteEntries.size(); i++) {
+        const auto& t = spendable.sproutNoteEntries[i];
+        libzcash::SproutSpendingKey sk;
+        assert(wallet.GetSproutSpendingKey(t.address, sk));
+
+        builder.AddSproutInput(sk, t.note, vSproutWitnesses[i].value());
+
+        totalSpend += t.note.value();
+    }
+
+    // TODO: We currently can’t store Sprout change in `Payments`, so we only validate the
+    //       spend/output balance in the case that `TransactionBuilder` doesn’t need to
+    //       (re)calculate the change. In future, we shouldn’t rely on `TransactionBuilder` ever
+    //       calculating change.
+    if (changeAddr.has_value()) {
+        std::visit(match {
+            [&](const SproutPaymentAddress& addr) {
+                builder.SendChangeToSprout(addr);
+            },
+            [&](const RecipientAddress&) {
+                assert(totalSpend == payments.Total() + fee);
+            }
+        }, changeAddr.value());
+    }
+
+    // Build the transaction
+    return builder.Build();
+}

src/wallet/wallet_tx_builder.h

@@ -5,24 +5,352 @@
 #ifndef ZCASH_WALLET_WALLET_TX_BUILDER_H
 #define ZCASH_WALLET_WALLET_TX_BUILDER_H
 
+#include "consensus/params.h"
+#include "transaction_builder.h"
 #include "wallet/memo.h"
+#include "wallet/wallet.h"
 
 using namespace libzcash;
 
+int GetAnchorHeight(const CChain& chain, int anchorConfirmations);
+
 /**
- * A payment that has been resolved to send to a specific
- * recipient address in a single pool.
+ * A payment that has been resolved to send to a specific recipient address in a single pool. This
+ * is an internal type that represents both user-requested payment addresses and generated
+ * (internal) payments (like change).
  */
 class ResolvedPayment : public RecipientMapping {
 public:
     CAmount amount;
     std::optional<Memo> memo;
+    bool isInternal;
 
     ResolvedPayment(
             std::optional<libzcash::UnifiedAddress> ua,
             libzcash::RecipientAddress address,
             CAmount amount,
-            std::optional<Memo> memo) :
-        RecipientMapping(ua, address), amount(amount), memo(memo) {}
+            std::optional<Memo> memo,
+            bool isInternal) :
+        RecipientMapping(ua, address), amount(amount), memo(memo), isInternal(isInternal) {}
 };
+
+/**
+ * A requested payment that has not yet been resolved to a
+ * specific recipient address.
+ */
+class Payment {
+private:
+    PaymentAddress address;
+    CAmount amount;
+    std::optional<Memo> memo;
+public:
+    Payment(
+            PaymentAddress address,
+            CAmount amount,
+            std::optional<Memo> memo) :
+        address(address), amount(amount), memo(memo) {}
+
+    const PaymentAddress& GetAddress() const {
+        return address;
+    }
+
+    CAmount GetAmount() const {
+        return amount;
+    }
+
+    const std::optional<Memo>& GetMemo() const {
+        return memo;
+    }
+};
+
+class Payments {
+private:
+    std::vector<ResolvedPayment> payments;
+    std::set<OutputPool> recipientPools;
+    CAmount t_outputs_total{0};
+    CAmount sapling_outputs_total{0};
+    CAmount orchard_outputs_total{0};
+public:
+    Payments(std::vector<ResolvedPayment> payments) {
+        for (const ResolvedPayment& payment : payments) {
+            AddPayment(payment);
+        }
+    }
+
+    void AddPayment(ResolvedPayment payment) {
+        std::visit(match {
+            [&](const CKeyID& addr) {
+                t_outputs_total += payment.amount;
+                recipientPools.insert(OutputPool::Transparent);
+            },
+            [&](const CScriptID& addr) {
+                t_outputs_total += payment.amount;
+                recipientPools.insert(OutputPool::Transparent);
+            },
+            [&](const libzcash::SaplingPaymentAddress& addr) {
+                sapling_outputs_total += payment.amount;
+                recipientPools.insert(OutputPool::Sapling);
+            },
+            [&](const libzcash::OrchardRawAddress& addr) {
+                orchard_outputs_total += payment.amount;
+                recipientPools.insert(OutputPool::Orchard);
+            }
+        }, payment.address);
+        payments.push_back(payment);
+    }
+
+    std::set<OutputPool> GetRecipientPools() const {
+        return recipientPools;
+    }
+
+    bool HasTransparentRecipient() const {
+        return recipientPools.count(OutputPool::Transparent) > 0;
+    }
+
+    bool HasSaplingRecipient() const {
+        return recipientPools.count(OutputPool::Sapling) > 0;
+    }
+
+    bool HasOrchardRecipient() const {
+        return recipientPools.count(OutputPool::Orchard) > 0;
+    }
+
+    const std::vector<ResolvedPayment>& GetResolvedPayments() const {
+        return payments;
+    }
+
+    CAmount GetTransparentTotal() const {
+        return t_outputs_total;
+    }
+
+    CAmount GetSaplingTotal() const {
+        return sapling_outputs_total;
+    }
+
+    CAmount GetOrchardTotal() const {
+        return orchard_outputs_total;
+    }
+
+    CAmount Total() const {
+        return
+            t_outputs_total +
+            sapling_outputs_total +
+            orchard_outputs_total;
+    }
+};
+
+typedef std::variant<
+    RecipientAddress,
+    SproutPaymentAddress> ChangeAddress;
+
+class TransactionEffects {
+private:
+    uint32_t anchorConfirmations{1};
+    SpendableInputs spendable;
+    Payments payments;
+    std::optional<ChangeAddress> changeAddr;
+    CAmount fee{0};
+    uint256 internalOVK;
+    uint256 externalOVK;
+    // TODO: This needs to be richer, like an `anchorBlock`, so the `TransactionEffects` can
+    //       be recalculated if the state of the chain has changed significantly between the time of
+    //       preparation and the time of approval.
+    int anchorHeight;
+
+public:
+    TransactionEffects(
+        uint32_t anchorConfirmations,
+        SpendableInputs spendable,
+        Payments payments,
+        std::optional<ChangeAddress> changeAddr,
+        CAmount fee,
+        uint256 internalOVK,
+        uint256 externalOVK,
+        int anchorHeight) :
+            anchorConfirmations(anchorConfirmations),
+            spendable(spendable),
+            payments(payments),
+            changeAddr(changeAddr),
+            fee(fee),
+            internalOVK(internalOVK),
+            externalOVK(externalOVK),
+            anchorHeight(anchorHeight) {}
+
+    /**
+     * Returns the strongest `PrivacyPolicy` that is compatible with the transaction’s effects.
+     */
+    PrivacyPolicy GetRequiredPrivacyPolicy() const;
+
+    const SpendableInputs& GetSpendable() const {
+        return spendable;
+    }
+
+    const Payments& GetPayments() const {
+        return payments;
+    }
+
+    CAmount GetFee() const {
+        return fee;
+    }
+
+    bool InvolvesOrchard() const;
+
+    TransactionBuilderResult ApproveAndBuild(
+            const Consensus::Params& consensus,
+            const CWallet& wallet,
+            const CChain& chain,
+            const TransactionStrategy& strategy) const;
+};
+
+enum class AddressResolutionError {
+    //! Zcashd no longer supports sending to Sprout.
+    SproutRecipientsNotSupported,
+    //! Requested `PrivacyPolicy` doesn’t include `AllowRevealedRecipients`
+    TransparentRecipientNotAllowed,
+    //! Requested `PrivacyPolicy` doesn’t include `AllowRevealedAmounts`, but we don’t have enough
+    //! Sapling funds to avoid revealing amounts
+    RevealingSaplingAmountNotAllowed,
+    //! Despite a lax `PrivacyPolicy`, other factors made it impossible to find a receiver for a
+    //! recipient UA
+    CouldNotResolveReceiver,
+    //! Requested `PrivacyPolicy` doesn’t include `AllowRevealedRecipients`, but we are trying to
+    //! pay a UA where we can only select a transparent receiver
+    TransparentReceiverNotAllowed,
+    //! Requested `PrivacyPolicy` doesn’t include `AllowRevealedAmounts`, but we are trying to pay a
+    //! UA where we don’t have enough funds in any single pool that it has a receiver for
+    RevealingReceiverAmountsNotAllowed,
+};
+
+class InsufficientFundsError {
+public:
+    CAmount required;
+
+    InsufficientFundsError(CAmount required):
+        required(required) { }
+};
+
+class DustThresholdError {
+public:
+    CAmount dustThreshold;
+    CAmount changeAmount;
+
+    DustThresholdError(CAmount dustThreshold, CAmount changeAmount):
+        dustThreshold(dustThreshold), changeAmount(changeAmount) { }
+};
+
+typedef std::variant<
+    InsufficientFundsError,
+    DustThresholdError> InvalidFundsReason;
+
+class InvalidFundsError {
+public:
+    CAmount available;
+    const InvalidFundsReason reason;
+
+    InvalidFundsError(CAmount available, const InvalidFundsReason reason):
+        available(available), reason(reason) { }
+};
+
+class ChangeNotAllowedError {
+public:
+    CAmount available;
+    CAmount required;
+
+    ChangeNotAllowedError(CAmount available, CAmount required):
+        available(available), required(required) { }
+};
+
+enum ActionSide {
+    Input,
+    Output,
+    Both,
+};
+
+class ExcessOrchardActionsError {
+public:
+    ActionSide side;
+    uint32_t orchardNotes;
+    uint32_t maxNotes;
+
+    ExcessOrchardActionsError(ActionSide side, uint32_t orchardNotes, uint32_t maxNotes):
+        side(side), orchardNotes(orchardNotes), maxNotes(maxNotes) { }
+};
+
+typedef std::variant<
+    AddressResolutionError,
+    InvalidFundsError,
+    ChangeNotAllowedError,
+    ExcessOrchardActionsError> InputSelectionError;
+
+class InputSelection {
+private:
+    Payments payments;
+    int orchardAnchorHeight;
+
+public:
+    InputSelection(Payments payments, int orchardAnchorHeight):
+        payments(payments), orchardAnchorHeight(orchardAnchorHeight) {}
+
+    Payments GetPayments() const;
+};
+
+typedef std::variant<
+    InputSelectionError,
+    InputSelection> InputSelectionResult;
+
+typedef std::variant<
+    InputSelectionError,
+    TransactionEffects> PrepareTransactionResult;
+
+class WalletTxBuilder {
+private:
+    const CChainParams& params;
+    const CWallet& wallet;
+    CFeeRate minRelayFee;
+    uint32_t maxOrchardActions;
+
+    /**
+     * Compute the default dust threshold
+     */
+    CAmount DefaultDustThreshold() const;
+
+    /**
+     * Select inputs sufficient to fulfill the specified requested payments,
+     * and choose unified address receivers based upon the available inputs
+     * and the requested transaction strategy.
+     */
+    InputSelectionResult ResolveInputsAndPayments(
+            const ZTXOSelector& selector,
+            SpendableInputs& spendable,
+            const std::vector<Payment>& payments,
+            const CChain& chain,
+            TransactionStrategy strategy,
+            CAmount fee,
+            int anchorHeight) const;
+    /**
+     * Compute the internal and external OVKs to use in transaction construction, given
+     * the spendable inputs.
+     */
+    std::pair<uint256, uint256> SelectOVKs(
+            const ZTXOSelector& selector,
+            const SpendableInputs& spendable) const;
+
+public:
+    WalletTxBuilder(const CChainParams& params, const CWallet& wallet, CFeeRate minRelayFee):
+        params(params), wallet(wallet), minRelayFee(minRelayFee), maxOrchardActions(nOrchardActionLimit) {}
+
+    SpendableInputs FindAllSpendableInputs(
+            const ZTXOSelector& selector,
+            int32_t minDepth) const;
+
+    PrepareTransactionResult PrepareTransaction(
+            const ZTXOSelector& selector,
+            SpendableInputs& spendable,
+            const std::vector<Payment>& payments,
+            const CChain& chain,
+            TransactionStrategy strategy,
+            CAmount fee,
+            uint32_t anchorConfirmations) const;
+};
+
 #endif

src/zcash/Address.hpp

@@ -270,13 +270,13 @@ typedef std::variant<
     SproutSpendingKey,
     SaplingExtendedSpendingKey> SpendingKey;
 
-class IsShieldedRecipient {
+class HasShieldedRecipient {
 public:
     bool operator()(const CKeyID& p2pkh) { return false; }
     bool operator()(const CScriptID& p2sh) { return false; }
     bool operator()(const SproutPaymentAddress& addr) { return true; }
     bool operator()(const SaplingPaymentAddress& addr) { return true; }
-    bool operator()(const OrchardRawAddress& addr) { return true; }
+    bool operator()(const UnifiedAddress& addr) { return true; }
 };
 
 class SelectRecipientAddress {