Repository: zcash/zcash (language: rust)

Issue:
Implement ZIP 317 wallet fee logic
https://zips.z.cash/zip-0317#fee-calculation


Discussion / hints:
Also, we should make sure to look at #5984 in the process.
As part of this implementation, we will need to make it possible for each RPC method that currently takes a fee amount as an argument to take that argument in a fashion that means "compute the correct value for the transaction." Ideally we'd be able to specify RPC arguments via key/value pairs for this scenario, but we might also need to simply choose a sentinel value (like `-1`) that will be interpreted as an instruction to compute & use the correct conventional fee.
> Also, we should make sure to look at #5984 in the process.

`estimatefee` is confusing – the documentation (and Bitcoin usage, of course) specifies that it’s meant to estimate the fee you should use for a tx to be mined within `n` blocks. For us, regardless of ZIP 317, that question is nonsensical, since our fees aren’t size-based. The best we could do is some kind of weighted average based on the expected (or maybe historical) sizes of various logical actions. But that still doesn’t tell the user what fee they should expect to pay.

We could alternatively rename this to something like `averagefee` and return the average fee/kb we’ve seen without tying it to something the user should add to a transaction.

Or we could make `estimatefee` a sort of “dry run” operation that builds the transaction and returns the fee calculated in that process. When we split Prepare/Approve into a two-phase operation, that would subsume this usage.

But otherwise, I don’t see why we haven’t deprecated that call.

Fix patch:
diff --git a/src/wallet/asyncrpcoperation_common.cpp b/src/wallet/asyncrpcoperation_common.cpp
index f03dad03a0c..0bd6e60375e 100644
--- a/src/wallet/asyncrpcoperation_common.cpp
+++ b/src/wallet/asyncrpcoperation_common.cpp
@@ -4,6 +4,7 @@
 #include "init.h"
 #include "rpc/protocol.h"
 #include "util/moneystr.h"
+#include "zip317.h"
 
 extern UniValue signrawtransaction(const UniValue& params, bool fHelp);
 
@@ -48,7 +49,7 @@ void ThrowInputSelectionError(
         const TransactionStrategy& strategy)
 {
     examine(err, match {
-        [](const AddressResolutionError& err) {
+        [&](const AddressResolutionError& err) {
             switch (err) {
                 case AddressResolutionError::SproutRecipientsNotSupported:
                     throw JSONRPCError(
@@ -62,6 +63,14 @@ void ThrowInputSelectionError(
                         "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::TransparentChangeNotAllowed:
+                    throw JSONRPCError(
+                        RPC_INVALID_PARAMETER,
+                        "This transaction would have transparent change, which is not "
+                        "enabled by default because it will publicly reveal the change "
+                        "address 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,
@@ -73,10 +82,19 @@ void ThrowInputSelectionError(
                 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.");
+                        strprintf("Could not send to an Orchard-only receiver %s.",
+                                  strategy.AllowRevealedAmounts()
+                                  ? strprintf("despite a lax privacy policy, because %s",
+                                              selector.SelectsSprout()
+                                              ? "you are sending from the Sprout pool and there is "
+                                                "no transaction version that supports both Sprout "
+                                                "and Orchard"
+                                              : "NU5 has not been activated yet")
+                                  : "without spending non-Orchard 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::TransparentReceiverNotAllowed:
                     throw JSONRPCError(
                         RPC_INVALID_PARAMETER,
@@ -106,9 +124,20 @@ void ThrowInputSelectionError(
                     "Insufficient funds: have %s, %s",
                     FormatMoney(err.available),
                     examine(err.reason, match {
+                        [](const PhantomChangeError& qce) {
+                            return strprintf(
+                                    "need %s more to surpass the dust threshold and avoid being "
+                                    "forced to over-pay the fee. Alternatively, you could specify "
+                                    "a fee of %s to allow overpayment of the conventional fee and "
+                                    "have this transaction proceed.",
+                                    FormatMoney(qce.dustThreshold),
+                                    FormatMoney(qce.finalFee));
+                        },
                         [](const InsufficientFundsError& ife) {
                             return strprintf("need %s", FormatMoney(ife.required));
                         },
+                        // TODO: Add the fee here, so we can suggest specifying an explicit fee (see
+                        //       `PhantomChangeError`).
                         [](const DustThresholdError& dte) {
                             return strprintf(
                                     "need %s more to avoid creating invalid change output %s (dust threshold is %s)",
@@ -138,6 +167,18 @@ void ThrowInputSelectionError(
                     "The proposed transaction would result in %s in change.",
                     FormatMoney(err.available - err.required)));
         },
+        [](const AbsurdFeeError& err) {
+            throw JSONRPCError(
+                RPC_INVALID_PARAMETER,
+                strprintf(
+                    "Fee %s is greater than %d times the conventional fee for this tx (which is "
+                    "%s). There is no prioritisation benefit to a fee this large (see "
+                    "https://zips.z.cash/zip-0317#recommended-algorithm-for-block-template-construction), "
+                    "and it likely indicates a mistake in setting the fee.",
+                    FormatMoney(err.fixedFee),
+                    WEIGHT_RATIO_CAP,
+                    FormatMoney(err.conventionalFee)));
+        },
         [](const ExcessOrchardActionsError& err) {
             std::string side;
             switch (err.side) {
diff --git a/src/wallet/asyncrpcoperation_sendmany.cpp b/src/wallet/asyncrpcoperation_sendmany.cpp
index 01af895351b..904e841fa39 100644
--- a/src/wallet/asyncrpcoperation_sendmany.cpp
+++ b/src/wallet/asyncrpcoperation_sendmany.cpp
@@ -49,13 +49,13 @@ AsyncRPCOperation_sendmany::AsyncRPCOperation_sendmany(
         int minDepth,
         unsigned int anchorDepth,
         TransactionStrategy strategy,
-        CAmount fee,
+        std::optional<CAmount> fee,
         UniValue contextInfo) :
         builder_(std::move(builder)), ztxoSelector_(ztxoSelector), recipients_(recipients),
         mindepth_(minDepth), anchordepth_(anchorDepth), strategy_(strategy), fee_(fee),
         contextinfo_(contextInfo)
 {
-    assert(fee_ >= 0);
+    assert(!fee_.has_value() || fee_.value() >= 0);
     assert(mindepth_ >= 0);
     assert(!recipients_.empty());
     assert(ztxoSelector.RequireSpendingKeys());
@@ -84,7 +84,11 @@ void AsyncRPCOperation_sendmany::main() {
 
     std::optional<uint256> txid;
     try {
-        txid = main_impl(*pwalletMain);
+        txid = main_impl(*pwalletMain)
+            .map_error([&](const InputSelectionError& err) {
+                ThrowInputSelectionError(err, ztxoSelector_, strategy_);
+            })
+            .value();
     } catch (const UniValue& objError) {
         int code = find_value(objError, "code").get_int();
         std::string message = find_value(objError, "message").get_str();
@@ -137,7 +141,8 @@ void AsyncRPCOperation_sendmany::main() {
 // 4. #3615 There is no padding of inputs or outputs, which may leak information.
 //
 // At least #4 differs from the Rust transaction builder.
-uint256 AsyncRPCOperation_sendmany::main_impl(CWallet& wallet) {
+tl::expected<uint256, InputSelectionError>
+AsyncRPCOperation_sendmany::main_impl(CWallet& wallet) {
     auto spendable = builder_.FindAllSpendableInputs(wallet, ztxoSelector_, mindepth_);
 
     auto preparedTx = builder_.PrepareTransaction(
@@ -150,12 +155,8 @@ uint256 AsyncRPCOperation_sendmany::main_impl(CWallet& wallet) {
             fee_,
             anchordepth_);
 
-    uint256 txid;
-    examine(preparedTx, match {
-        [&](const InputSelectionError& err) {
-            ThrowInputSelectionError(err, ztxoSelector_, strategy_);
-        },
-        [&](const TransactionEffects& effects) {
+    return preparedTx
+        .map([&](const TransactionEffects& effects) {
             try {
                 const auto& spendable = effects.GetSpendable();
                 const auto& payments = effects.GetPayments();
@@ -187,16 +188,13 @@ uint256 AsyncRPCOperation_sendmany::main_impl(CWallet& wallet) {
                 UniValue sendResult = SendTransaction(tx, payments.GetResolvedPayments(), std::nullopt, testmode);
                 set_result(sendResult);
 
-                txid = tx.GetHash();
                 effects.UnlockSpendable(wallet);
+                return tx.GetHash();
             } catch (...) {
                 effects.UnlockSpendable(wallet);
                 throw;
             }
-        }
-    });
-
-    return txid;
+        });
 }
 
 /**
diff --git a/src/wallet/asyncrpcoperation_sendmany.h b/src/wallet/asyncrpcoperation_sendmany.h
index ec59470d2ac..1ccc9dfc68f 100644
--- a/src/wallet/asyncrpcoperation_sendmany.h
+++ b/src/wallet/asyncrpcoperation_sendmany.h
@@ -35,7 +35,7 @@ class AsyncRPCOperation_sendmany : public AsyncRPCOperation {
         int minDepth,
         unsigned int anchorDepth,
         TransactionStrategy strategy,
-        CAmount fee = DEFAULT_FEE,
+        std::optional<CAmount> fee,
         UniValue contextInfo = NullUniValue);
 
     virtual ~AsyncRPCOperation_sendmany();
@@ -61,10 +61,10 @@ class AsyncRPCOperation_sendmany : public AsyncRPCOperation {
     TransactionStrategy strategy_;
     int mindepth_{1};
     unsigned int anchordepth_{nAnchorConfirmations};
-    CAmount fee_;
+    std::optional<CAmount> fee_;
     UniValue contextinfo_;     // optional data to include in return value from getStatus()
 
-    uint256 main_impl(CWallet& wallet);
+    tl::expected<uint256, InputSelectionError> main_impl(CWallet& wallet);
 };
 
 // To test private methods, a friend class can act as a proxy
@@ -74,7 +74,7 @@ class TEST_FRIEND_AsyncRPCOperation_sendmany {
 
     TEST_FRIEND_AsyncRPCOperation_sendmany(std::shared_ptr<AsyncRPCOperation_sendmany> ptr) : delegate(ptr) {}
 
-    uint256 main_impl(CWallet& wallet) {
+    tl::expected<uint256, InputSelectionError> main_impl(CWallet& wallet) {
         return delegate->main_impl(wallet);
     }
 
diff --git a/src/wallet/rpcwallet.cpp b/src/wallet/rpcwallet.cpp
index ff4c4019025..560ab9ac4d3 100644
--- a/src/wallet/rpcwallet.cpp
+++ b/src/wallet/rpcwallet.cpp
@@ -4813,7 +4813,8 @@ UniValue z_sendmany(const UniValue& params, bool fHelp)
             "                           the output is being sent to a transparent address, it’s an error to include this field.\n"
             "    }, ... ]\n"
             "3. minconf               (numeric, optional, default=" + strprintf("%u", DEFAULT_NOTE_CONFIRMATIONS) + ") Only use funds confirmed at least this many times.\n"
-            "4. fee                   (numeric, optional, default=" + strprintf("%s", FormatMoney(DEFAULT_FEE)) + ") The fee amount to attach to this transaction.\n"
+            "4. fee                   (numeric, optional, default=-1) The fee amount in " + CURRENCY_UNIT + " to attach to this transaction. The default behavior\n"
+            "                         is to use a fee calculated according to ZIP 317.\n"
             "5. privacyPolicy         (string, optional, default=\"LegacyCompat\") Policy for what information leakage is acceptable.\n"
             "                         One of the following strings:\n"
             "                               - \"FullPrivacy\": Only allow fully-shielded transactions (involving a single shielded value pool).\n"
@@ -5027,32 +5028,9 @@ UniValue z_sendmany(const UniValue& params, bool fHelp)
     // Minimum confirmations
     int nMinDepth = parseMinconf(DEFAULT_NOTE_CONFIRMATIONS, params, 2, std::nullopt);
 
-    // Fee in Zatoshis, not currency format)
-    CAmount nFee = DEFAULT_FEE;
-    if (params.size() > 3) {
-        if (params[3].get_real() == 0.0) {
-            nFee = 0;
-        } else {
-            nFee = AmountFromValue( params[3] );
-        }
-
-        // Check that the user specified fee is not absurd.
-        // This allows amount=0 (and all amount < DEFAULT_FEE) transactions to use the default network fee
-        // or anything less than DEFAULT_FEE instead of being forced to use a custom fee and leak metadata
-        if (nTotalOut < DEFAULT_FEE) {
-            if (nFee > DEFAULT_FEE) {
-                throw JSONRPCError(
-                        RPC_INVALID_PARAMETER,
-                        strprintf("Small transaction amount %s has fee %s that is greater than the default fee %s", FormatMoney(nTotalOut), FormatMoney(nFee), FormatMoney(DEFAULT_FEE)));
-            }
-        } else {
-            // Check that the user specified fee is not absurd.
-            if (nFee > nTotalOut) {
-                throw JSONRPCError(
-                        RPC_INVALID_PARAMETER,
-                        strprintf("Fee %s is greater than the sum of outputs %s and also greater than the default fee", FormatMoney(nFee), FormatMoney(nTotalOut)));
-            }
-        }
+    std::optional<CAmount> nFee;
+    if (params.size() > 3 && params[3].get_real() != -1.0) {
+        nFee = AmountFromValue( params[3] );
     }
 
     // Use input parameters as the optional context info to be returned by z_getoperationstatus and z_getoperationresult.
@@ -5060,7 +5038,9 @@ UniValue z_sendmany(const UniValue& params, bool fHelp)
     o.pushKV("fromaddress", params[0]);
     o.pushKV("amounts", params[1]);
     o.pushKV("minconf", nMinDepth);
-    o.pushKV("fee", std::stod(FormatMoney(nFee)));
+    if (nFee.has_value()) {
+        o.pushKV("fee", ValueFromAmount(nFee.value()));
+    }
     UniValue contextInfo = o;
 
     // Create operation and add to global queue
diff --git a/src/wallet/wallet_tx_builder.cpp b/src/wallet/wallet_tx_builder.cpp
index 28605b137b5..1576d1ba2b9 100644
--- a/src/wallet/wallet_tx_builder.cpp
+++ b/src/wallet/wallet_tx_builder.cpp
@@ -3,6 +3,7 @@
 // file COPYING or https://www.opensource.org/licenses/mit-license.php .
 
 #include "wallet/wallet_tx_builder.h"
+#include "zip317.h"
 
 using namespace libzcash;
 
@@ -12,153 +13,178 @@ int GetAnchorHeight(const CChain& chain, uint32_t anchorConfirmations)
     return nextBlockHeight - anchorConfirmations;
 }
 
-PrepareTransactionResult WalletTxBuilder::PrepareTransaction(
+tl::expected<ChangeAddress, AddressResolutionError>
+WalletTxBuilder::GetChangeAddress(
+        CWallet& wallet,
+        const ZTXOSelector& selector,
+        SpendableInputs& spendable,
+        const Payments& resolvedPayments,
+        const TransactionStrategy& strategy,
+        bool afterNU5) const
+{
+    // 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:
+                    if (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 (afterNU5
+                        && (!spendable.orchardNoteMetadata.empty() || strategy.AllowRevealedAmounts())) {
+                        result.insert(OutputPool::Orchard);
+                    }
+                    break;
+            }
+        }
+        return result;
+    };
+
+    auto changeAddressForTransparentSelector = [&](const std::set<ReceiverType>& receiverTypes)
+        -> tl::expected<ChangeAddress, AddressResolutionError> {
+        auto addr = wallet.GenerateChangeAddressForAccount(
+                sendFromAccount,
+                getAllowedChangePools(receiverTypes));
+        if (addr.has_value()) {
+            return {addr.value()};
+        } else {
+            return tl::make_unexpected(AddressResolutionError::TransparentChangeNotAllowed);
+        }
+    };
+
+    auto changeAddressForSaplingAddress = [&](const libzcash::SaplingPaymentAddress& addr)
+        -> RecipientAddress {
+        // 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) {
+            return addr;
+        } else {
+            auto addr = wallet.GenerateChangeAddressForAccount(
+                    sendFromAccount,
+                    getAllowedChangePools({ReceiverType::Sapling}));
+            assert(addr.has_value());
+            return addr.value();
+        }
+    };
+
+    auto changeAddressForZUFVK = [&](
+            const ZcashdUnifiedFullViewingKey& zufvk,
+            const std::set<ReceiverType>& receiverTypes) {
+        auto addr = zufvk.GetChangeAddress(getAllowedChangePools(receiverTypes));
+        assert(addr.has_value());
+        return addr.value();
+    };
+
+    return examine(selector.GetPattern(), match {
+        [&](const CKeyID&) {
+            return changeAddressForTransparentSelector({ReceiverType::P2PKH});
+        },
+        [&](const CScriptID&) {
+            return changeAddressForTransparentSelector({ReceiverType::P2SH});
+        },
+        [](const libzcash::SproutPaymentAddress& addr)
+            -> tl::expected<ChangeAddress, AddressResolutionError> {
+            // for Sprout, we return change to the originating address using the tx builder.
+            return addr;
+        },
+        [](const libzcash::SproutViewingKey& vk)
+            -> tl::expected<ChangeAddress, AddressResolutionError> {
+            // for Sprout, we return change to the originating address using the tx builder.
+            return vk.address();
+        },
+        [&](const libzcash::SaplingPaymentAddress& addr)
+            -> tl::expected<ChangeAddress, AddressResolutionError> {
+            return changeAddressForSaplingAddress(addr);
+        },
+        [&](const libzcash::SaplingExtendedFullViewingKey& fvk)
+            -> tl::expected<ChangeAddress, AddressResolutionError> {
+            return changeAddressForSaplingAddress(fvk.DefaultAddress());
+        },
+        [&](const libzcash::UnifiedAddress& ua)
+            -> tl::expected<ChangeAddress, AddressResolutionError> {
+            auto zufvk = wallet.GetUFVKForAddress(ua);
+            assert(zufvk.has_value());
+            return changeAddressForZUFVK(zufvk.value(), ua.GetKnownReceiverTypes());
+        },
+        [&](const libzcash::UnifiedFullViewingKey& fvk)
+            -> tl::expected<ChangeAddress, AddressResolutionError> {
+            return changeAddressForZUFVK(
+                    ZcashdUnifiedFullViewingKey::FromUnifiedFullViewingKey(params, fvk),
+                    fvk.GetKnownReceiverTypes());
+        },
+        [&](const AccountZTXOPattern& acct) -> tl::expected<ChangeAddress, AddressResolutionError> {
+                auto addr = wallet.GenerateChangeAddressForAccount(
+                    acct.GetAccountId(),
+                    getAllowedChangePools(acct.GetReceiverTypes()));
+            assert(addr.has_value());
+            return addr.value();
+        }
+    });
+}
+
+tl::expected<TransactionEffects, InputSelectionError>
+WalletTxBuilder::PrepareTransaction(
         CWallet& wallet,
         const ZTXOSelector& selector,
         SpendableInputs& spendable,
         const std::vector<Payment>& payments,
         const CChain& chain,
         TransactionStrategy strategy,
-        CAmount fee,
+        std::optional<CAmount> fee,
         uint32_t anchorConfirmations) const
 {
     assert(fee < MAX_MONEY);
 
     int anchorHeight = GetAnchorHeight(chain, anchorConfirmations);
-    auto selected = ResolveInputsAndPayments(wallet, 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();
+    bool afterNU5 = params.GetConsensus().NetworkUpgradeActive(anchorHeight, Consensus::UPGRADE_NU5);
+    auto selected = ResolveInputsAndPayments(wallet, selector, spendable, payments, chain, strategy, fee, afterNU5);
+    return selected.map([&](const InputSelection& resolvedSelection) {
+        auto ovks = SelectOVKs(wallet, selector, spendable);
+
+        auto effects = TransactionEffects(
+                anchorConfirmations,
+                resolvedSelection.GetInputs(),
+                resolvedSelection.GetPayments(),
+                resolvedSelection.GetChangeAddress(),
+                resolvedSelection.GetFee(),
+                ovks.first,
+                ovks.second,
+                anchorHeight);
+        effects.LockSpendable(wallet);
+        return effects;
+    });
+}
 
-    // 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:
-                        if (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;
-        };
+const SpendableInputs& InputSelection::GetInputs() const {
+    return inputs;
+}
 
-        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(
-                    wallet.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
-                    : wallet.GenerateChangeAddressForAccount(
-                            sendFromAccount,
-                            getAllowedChangePools({ReceiverType::Sapling})));
-        };
-
-        auto changeAddressForZUFVK = [&](
-                const ZcashdUnifiedFullViewingKey& zufvk,
-                const std::set<ReceiverType>& receiverTypes) {
-            return addChangePayment(zufvk.GetChangeAddress(getAllowedChangePools(receiverTypes)));
-        };
-
-        changeAddr = examine(selector.GetPattern(), 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 = wallet.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(
-                        wallet.GenerateChangeAddressForAccount(
-                                acct.GetAccountId(),
-                                getAllowedChangePools(acct.GetReceiverTypes())));
-            }
-        });
-    }
+const Payments& InputSelection::GetPayments() const {
+    return payments;
+}
 
-    auto ovks = SelectOVKs(wallet, selector, spendable);
-
-    auto effects = TransactionEffects(
-            anchorConfirmations,
-            spendable,
-            resolvedPayments,
-            changeAddr,
-            fee,
-            ovks.first,
-            ovks.second,
-            anchorHeight);
-    effects.LockSpendable(wallet);
-    return effects;
+CAmount InputSelection::GetFee() const {
+    return fee;
 }
 
-Payments InputSelection::GetPayments() const {
-    return this->payments;
+const std::optional<ChangeAddress> InputSelection::GetChangeAddress() const {
+    return changeAddr;
 }
 
 CAmount WalletTxBuilder::DefaultDustThreshold() const {
@@ -177,25 +203,248 @@ SpendableInputs WalletTxBuilder::FindAllSpendableInputs(
     return wallet.FindSpendableInputs(selector, minDepth, std::nullopt);
 }
 
-InputSelectionResult WalletTxBuilder::ResolveInputsAndPayments(
-        const CWallet& wallet,
+static size_t PadCount(size_t n)
+{
+    return n == 1 ? 2 : n;
+}
+
+static CAmount
+CalcZIP317Fee(
+        const std::optional<SpendableInputs>& inputs,
+        const std::vector<ResolvedPayment>& payments,
+        const std::optional<ChangeAddress>& changeAddr)
+{
+    std::vector<CTxOut> vout{};
+    size_t sproutOutputCount{}, saplingOutputCount{}, orchardOutputCount{};
+    for (const auto& payment : payments) {
+        std::visit(match {
+            [&](const CKeyID& addr) {
+                vout.emplace_back(payment.amount, GetScriptForDestination(addr));
+            },
+            [&](const CScriptID& addr) {
+                vout.emplace_back(payment.amount, GetScriptForDestination(addr));
+            },
+            [&](const libzcash::SaplingPaymentAddress&) {
+                ++saplingOutputCount;
+            },
+            [&](const libzcash::OrchardRawAddress&) {
+                ++orchardOutputCount;
+            }
+        }, payment.address);
+    }
+
+    if (changeAddr.has_value()) {
+        examine(changeAddr.value(), match {
+            [&](const SproutPaymentAddress&) { ++sproutOutputCount; },
+            [&](const RecipientAddress& addr) {
+                examine(addr, match {
+                    [&](const CKeyID& taddr) {
+                        vout.emplace_back(0, GetScriptForDestination(taddr));
+                    },
+                    [&](const CScriptID taddr) {
+                        vout.emplace_back(0, GetScriptForDestination(taddr));
+                    },
+                    [&](const libzcash::SaplingPaymentAddress&) { ++saplingOutputCount; },
+                    [&](const libzcash::OrchardRawAddress&) { ++orchardOutputCount; }
+                });
+            }
+        });
+    }
+
+    std::vector<CTxIn> vin{};
+    size_t sproutInputCount = 0;
+    size_t saplingInputCount = 0;
+    size_t orchardInputCount = 0;
+    if (inputs.has_value()) {
+        for (const auto& utxo : inputs.value().utxos) {
+            vin.emplace_back(
+                    COutPoint(utxo.tx->GetHash(), utxo.i),
+                    utxo.tx->vout[utxo.i].scriptPubKey);
+        }
+        sproutInputCount = inputs.value().sproutNoteEntries.size();
+        saplingInputCount = inputs.value().saplingNoteEntries.size();
+        orchardInputCount = inputs.value().orchardNoteMetadata.size();
+    }
+    size_t logicalActionCount = CalculateLogicalActionCount(
+            vin,
+            vout,
+            std::max(sproutInputCount, sproutOutputCount),
+            saplingInputCount,
+            PadCount(saplingOutputCount),
+            PadCount(std::max(orchardInputCount, orchardOutputCount)));
+
+    return CalculateConventionalFee(logicalActionCount);
+}
+
+InvalidFundsError ReportInvalidFunds(
+        const SpendableInputs& spendable,
+        bool hasPhantomChange,
+        CAmount fee,
+        CAmount dustThreshold,
+        CAmount targetAmount,
+        CAmount changeAmount)
+{
+    return InvalidFundsError(
+            spendable.Total(),
+            hasPhantomChange
+            // TODO: NEED TESTS TO EXERCISE THIS
+            ? InvalidFundsReason(PhantomChangeError(fee, dustThreshold))
+            : (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 publicly in the fee.)
+               ? InvalidFundsReason(DustThresholdError(dustThreshold, changeAmount))
+               : InvalidFundsReason(InsufficientFundsError(targetAmount))));
+}
+
+static tl::expected<void, InputSelectionError>
+AddChangePayment(
+        const SpendableInputs& spendable,
+        Payments& resolvedPayments,
+        const ChangeAddress& changeAddr,
+        CAmount changeAmount,
+        CAmount targetAmount)
+{
+    assert(changeAmount > 0);
+
+    // When spending transparent coinbase outputs, all inputs must be fully consumed.
+    if (spendable.HasTransparentCoinbase()) {
+        return tl::make_unexpected(ChangeNotAllowedError(spendable.Total(), targetAmount));
+    }
+
+    examine(changeAddr, match {
+        // TODO: Once we can add Sprout change to `resolvedPayments`, we don’t need to pass
+        //       `changeAddr` around the rest of these functions.
+        [](const libzcash::SproutPaymentAddress&) {},
+        [](const libzcash::SproutViewingKey&) {},
+        [&](const auto& sendTo) {
+            resolvedPayments.AddPayment(
+                    ResolvedPayment(std::nullopt, sendTo, changeAmount, std::nullopt, true));
+        }
+    });
+
+    return {};
+}
+
+/// On the initial call, we haven’t yet selected inputs, so we assume the outputs dominate the
+/// actions.
+///
+/// 1. calc fee using only resolvedPayments to set a lower bound on the actual fee
+///    • this also needs to know which pool change is going to, so it can determine what the fee is
+///      with change _if_ there is change
+/// 2. iterate over LimitToAmount until the updated fee (now including spends) matches the expected
+///    fee
+tl::expected<
+    std::tuple<SpendableInputs, CAmount, std::optional<ChangeAddress>>,
+    InputSelectionError>
+WalletTxBuilder::IterateLimit(
+        CWallet& wallet,
+        const ZTXOSelector& selector,
+        const TransactionStrategy strategy,
+        CAmount sendAmount,
+        CAmount dustThreshold,
+        const SpendableInputs& spendable,
+        Payments& resolved,
+        bool afterNU5) const
+{
+    SpendableInputs spendableMut;
+
+    auto previousFee = MINIMUM_FEE;
+    auto updatedFee = CalcZIP317Fee(std::nullopt, resolved.GetResolvedPayments(), std::nullopt);
+    // This is used to increase the target amount just enough (generally by 0 or 1) to force
+    // selection of additional notes.
+    CAmount bumpTargetAmount{0};
+    std::optional<ChangeAddress> changeAddr;
+    CAmount changeAmount{0};
+    CAmount targetAmount{0};
+
+    do {
+        // NB: This makes a fresh copy so that we start from the full set of notes when we re-limit.
+        spendableMut = spendable;
+
+        targetAmount = sendAmount + updatedFee;
+
+        // 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.
+        bool foundSufficientFunds =
+            spendableMut.LimitToAmount(
+                    targetAmount + bumpTargetAmount,
+                    dustThreshold,
+                    resolved.GetRecipientPools());
+        changeAmount = spendableMut.Total() - targetAmount;
+        if (foundSufficientFunds) {
+            // Don’t want to generate a change address if we don’t need one (because it could be
+            // fresh) and once we generate it, hold onto it. But we still don’t have a guarantee
+            // that we won’t end up discarding it.
+            if (changeAmount > 0 && !changeAddr.has_value()) {
+                auto maybeChangeAddr = GetChangeAddress(
+                        wallet,
+                        selector,
+                        spendableMut,
+                        resolved,
+                        strategy,
+                        afterNU5);
+
+                if (maybeChangeAddr.has_value()) {
+                    changeAddr = maybeChangeAddr.value();
+                } else {
+                    return tl::make_unexpected(maybeChangeAddr.error());
+                }
+            }
+            previousFee = updatedFee;
+            updatedFee = CalcZIP317Fee(
+                    spendableMut,
+                    resolved.GetResolvedPayments(),
+                    changeAmount > 0 ? changeAddr : std::nullopt);
+        } else {
+            return tl::make_unexpected(
+                    ReportInvalidFunds(
+                            spendableMut,
+                            bumpTargetAmount != 0,
+                            previousFee,
+                            dustThreshold,
+                            targetAmount,
+                            changeAmount));
+        }
+        // This happens when we have exactly `MARGINAL_FEE` change, then add a change output that
+        // causes the conventional fee to consume that change, leaving us with no change, which then
+        // lowers the fee.
+        if (updatedFee < previousFee) {
+            // Bump the updated fee so that we don’t exit the loop, but should force us to take an
+            // extra note (or fail) in the next `LimitToAmount`.
+            bumpTargetAmount = 1;
+        }
+    } while (updatedFee != previousFee);
+
+    if (changeAmount > 0) {
+        assert(changeAddr.has_value());
+
+        auto changeRes =
+            AddChangePayment(spendableMut, resolved, changeAddr.value(), changeAmount, targetAmount);
+        if (!changeRes.has_value()) {
+            return tl::make_unexpected(changeRes.error());
+        }
+    }
+
+    return std::make_tuple(spendableMut, updatedFee, changeAddr);
+}
+
+tl::expected<InputSelection, InputSelectionError>
+WalletTxBuilder::ResolveInputsAndPayments(
+        CWallet& wallet,
         const ZTXOSelector& selector,
         SpendableInputs& spendableMut,
         const std::vector<Payment>& payments,
         const CChain& chain,
-        TransactionStrategy strategy,
-        CAmount fee,
-        int anchorHeight) const
+        const TransactionStrategy& strategy,
+        std::optional<CAmount> fee,
+        bool afterNU5) 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
@@ -205,11 +454,9 @@ InputSelectionResult WalletTxBuilder::ResolveInputsAndPayments(
     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;
+    // we can only select Orchard addresses if we’re not sending from Sprout, since there is no tx
+    // version where both Sprout and Orchard are valid.
+    bool canResolveOrchard = afterNU5 && !selector.SelectsSprout();
     std::vector<ResolvedPayment> resolvedPayments;
     std::optional<AddressResolutionError> resolutionError;
     for (const auto& payment : payments) {
@@ -289,59 +536,113 @@ InputSelectionResult WalletTxBuilder::ResolveInputsAndPayments(
         });
 
         if (resolutionError.has_value()) {
-            return resolutionError.value();
+            return tl::make_unexpected(resolutionError.value());
         }
     }
     auto resolved = Payments(resolvedPayments);
 
     if (orchardOutputs > this->maxOrchardActions) {
-        return ExcessOrchardActionsError(
-                ActionSide::Output,
-                orchardOutputs,
-                this->maxOrchardActions);
+        return tl::make_unexpected(
+                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 publicly in the fee.)
-                ? InvalidFundsReason(DustThresholdError(dustThreshold, changeAmount))
-                : InvalidFundsReason(InsufficientFundsError(targetAmount)));
+    // Determine the target totals
+    CAmount sendAmount{0};
+    for (const auto& payment : payments) {
+        sendAmount += payment.GetAmount();
     }
 
-    // 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;
+    CAmount finalFee;
+    CAmount targetAmount;
+    std::optional<ChangeAddress> changeAddr;
+    if (fee.has_value()) {
+        finalFee = fee.value();
+        targetAmount = sendAmount + finalFee;
+        // 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.
+        bool foundSufficientFunds = spendableMut.LimitToAmount(
+                targetAmount,
+                dustThreshold,
+                resolved.GetRecipientPools());
+        CAmount changeAmount{spendableMut.Total() - targetAmount};
+        if (!foundSufficientFunds) {
+            return tl::make_unexpected(
+                    ReportInvalidFunds(
+                            spendableMut,
+                            false,
+                            finalFee,
+                            dustThreshold,
+                            targetAmount,
+                            changeAmount));
         }
+        if (changeAmount > 0) {
+            auto maybeChangeAddr = GetChangeAddress(
+                    wallet,
+                    selector,
+                    spendableMut,
+                    resolved,
+                    strategy,
+                    afterNU5);
+
+            if (maybeChangeAddr.has_value()) {
+                changeAddr = maybeChangeAddr.value();
+            } else {
+                return tl::make_unexpected(maybeChangeAddr.error());
+            }
+
+            // TODO: This duplicates the check in the `else` branch of the containing `if`. Until we
+            //       can add Sprout change to `Payments` (#5660), we need to check this before
+            //       adding the change payment. We can remove this check and make the later one
+            //       unconditional once that’s fixed.
+            auto conventionalFee =
+                CalcZIP317Fee(spendableMut, resolved.GetResolvedPayments(), changeAddr);
+            if (finalFee > WEIGHT_RATIO_CAP * conventionalFee) {
+                return tl::make_unexpected(AbsurdFeeError(conventionalFee, finalFee));
+            }
+            auto changeRes =
+                AddChangePayment(spendableMut, resolved, changeAddr.value(), changeAmount, targetAmount);
+            if (!changeRes.has_value()) {
+                return tl::make_unexpected(changeRes.error());
+            }
+        } else {
+            auto conventionalFee =
+                CalcZIP317Fee(spendableMut, resolved.GetResolvedPayments(), std::nullopt);
+            if (finalFee > WEIGHT_RATIO_CAP * conventionalFee) {
+                return tl::make_unexpected(AbsurdFeeError(resolved.Total(), finalFee));
+            }
+        }
+    } else {
+        auto limitResult = IterateLimit(wallet, selector, strategy, sendAmount, dustThreshold, spendableMut, resolved, afterNU5);
+        if (limitResult.has_value()) {
+            std::tie(spendableMut, finalFee, changeAddr) = limitResult.value();
+            targetAmount = sendAmount + finalFee;
+        } else {
+            return tl::make_unexpected(limitResult.error());
+        }
+    }
+
+    // When spending transparent coinbase outputs they may only be sent to shielded recipients.
+    if (spendableMut.HasTransparentCoinbase() && resolved.HasTransparentRecipient()) {
+        return tl::make_unexpected(AddressResolutionError::TransparentRecipientNotAllowed);
     }
 
     if (spendableMut.orchardNoteMetadata.size() > this->maxOrchardActions) {
-        return ExcessOrchardActionsError(
-                ActionSide::Input,
-                spendableMut.orchardNoteMetadata.size(),
-                this->maxOrchardActions);
+        return tl::make_unexpected(
+                ExcessOrchardActionsError(
+                        ActionSide::Input,
+                        spendableMut.orchardNoteMetadata.size(),
+                        this->maxOrchardActions));
     }
 
-    return InputSelection(resolved, anchorHeight);
+    return InputSelection(spendableMut, resolved, finalFee, changeAddr);
 }
 
 std::pair<uint256, uint256>
@@ -625,7 +926,7 @@ TransactionBuilderResult TransactionEffects::ApproveAndBuild(
     // 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.
+    //       calculating change. (#5660)
     if (changeAddr.has_value()) {
         examine(changeAddr.value(), match {
             [&](const SproutPaymentAddress& addr) {
diff --git a/src/wallet/wallet_tx_builder.h b/src/wallet/wallet_tx_builder.h
index cb1b5441c07..d44b665bcef 100644
--- a/src/wallet/wallet_tx_builder.h
+++ b/src/wallet/wallet_tx_builder.h
@@ -225,6 +225,8 @@ enum class AddressResolutionError {
     SproutRecipientsNotSupported,
     //! Requested `PrivacyPolicy` doesn’t include `AllowRevealedRecipients`
     TransparentRecipientNotAllowed,
+    //! Requested `PrivacyPolicy` doesn’t include `AllowRevealedRecipients`
+    TransparentChangeNotAllowed,
     //! Requested `PrivacyPolicy` doesn’t include `AllowRevealedAmounts`, but we don’t have enough
     //! Sapling funds to avoid revealing amounts
     RevealingSaplingAmountNotAllowed,
@@ -239,6 +241,19 @@ enum class AddressResolutionError {
     RevealingReceiverAmountsNotAllowed,
 };
 
+/// Phantom change is change that appears to exist until we add the output for it, at which point it
+/// is consumed by the increase to the conventional fee. When we are at the limit of selectable
+/// notes, this makes it impossible to create the transaction without either creating a 0-valued
+/// output or overpaying the fee.
+class PhantomChangeError {
+public:
+    CAmount finalFee;
+    CAmount dustThreshold;
+
+    PhantomChangeError(CAmount finalFee, CAmount dustThreshold):
+        finalFee(finalFee), dustThreshold(dustThreshold) { }
+};
+
 class InsufficientFundsError {
 public:
     CAmount required;
@@ -257,6 +272,7 @@ class DustThresholdError {
 };
 
 typedef std::variant<
+    PhantomChangeError,
     InsufficientFundsError,
     DustThresholdError> InvalidFundsReason;
 
@@ -278,6 +294,17 @@ class ChangeNotAllowedError {
         available(available), required(required) { }
 };
 
+/// Error when a fee is higher than can be useful. This reduces the chance of accidentally
+/// overpaying with explicit fees.
+class AbsurdFeeError {
+public:
+    CAmount conventionalFee;
+    CAmount fixedFee;
+
+    AbsurdFeeError(CAmount conventionalFee, CAmount fixedFee):
+        conventionalFee(conventionalFee), fixedFee(fixedFee) { }
+};
+
 enum ActionSide {
     Input,
     Output,
@@ -298,28 +325,26 @@ typedef std::variant<
     AddressResolutionError,
     InvalidFundsError,
     ChangeNotAllowedError,
+    AbsurdFeeError,
     ExcessOrchardActionsError> InputSelectionError;
 
 class InputSelection {
 private:
+    SpendableInputs inputs;
     Payments payments;
-    int orchardAnchorHeight;
+    CAmount fee;
+    std::optional<ChangeAddress> changeAddr;
 
 public:
-    InputSelection(Payments payments, int orchardAnchorHeight):
-        payments(payments), orchardAnchorHeight(orchardAnchorHeight) {}
+    InputSelection(SpendableInputs inputs, Payments payments, CAmount fee, std::optional<ChangeAddress> changeAddr):
+        inputs(inputs), payments(payments), fee(fee), changeAddr(changeAddr) {}
 
-    Payments GetPayments() const;
+    const SpendableInputs& GetInputs() const;
+    const Payments& GetPayments() const;
+    CAmount GetFee() const;
+    const std::optional<ChangeAddress> GetChangeAddress() const;
 };
 
-typedef std::variant<
-    InputSelectionError,
-    InputSelection> InputSelectionResult;
-
-typedef std::variant<
-    InputSelectionError,
-    TransactionEffects> PrepareTransactionResult;
-
 class WalletTxBuilder {
 private:
     const CChainParams& params;
@@ -331,20 +356,43 @@ class WalletTxBuilder {
      */
     CAmount DefaultDustThreshold() const;
 
+    tl::expected<ChangeAddress, AddressResolutionError>
+    GetChangeAddress(
+            CWallet& wallet,
+            const ZTXOSelector& selector,
+            SpendableInputs& spendable,
+            const Payments& resolvedPayments,
+            const TransactionStrategy& strategy,
+            bool afterNU5) const;
+
+    tl::expected<
+        std::tuple<SpendableInputs, CAmount, std::optional<ChangeAddress>>,
+        InputSelectionError>
+    IterateLimit(
+            CWallet& wallet,
+            const ZTXOSelector& selector,
+            const TransactionStrategy strategy,
+            CAmount sendAmount,
+            CAmount dustThreshold,
+            const SpendableInputs& spendable,
+            Payments& resolved,
+            bool afterNU5) 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 CWallet& wallet,
+    tl::expected<InputSelection, InputSelectionError>
+    ResolveInputsAndPayments(
+            CWallet& wallet,
             const ZTXOSelector& selector,
             SpendableInputs& spendable,
             const std::vector<Payment>& payments,
             const CChain& chain,
-            TransactionStrategy strategy,
-            CAmount fee,
-            int anchorHeight) const;
+            const TransactionStrategy& strategy,
+            std::optional<CAmount> fee,
+            bool afterNU5) const;
     /**
      * Compute the internal and external OVKs to use in transaction construction, given
      * the spendable inputs.
@@ -363,14 +411,16 @@ class WalletTxBuilder {
             const ZTXOSelector& selector,
             int32_t minDepth) const;
 
-    PrepareTransactionResult PrepareTransaction(
+    tl::expected<TransactionEffects, InputSelectionError>
+    PrepareTransaction(
             CWallet& wallet,
             const ZTXOSelector& selector,
             SpendableInputs& spendable,
             const std::vector<Payment>& payments,
             const CChain& chain,
             TransactionStrategy strategy,
-            CAmount fee,
+            /// A fixed fee is used if provided, otherwise it is calculated based on ZIP 317.
+            std::optional<CAmount> fee,
             uint32_t anchorConfirmations) const;
 };
 
diff --git a/src/zip317.cpp b/src/zip317.cpp
index e0bd1c64096..b9b65e7c82c 100644
--- a/src/zip317.cpp
+++ b/src/zip317.cpp
@@ -14,6 +14,13 @@ CAmount CalculateConventionalFee(size_t logicalActionCount) {
     return MARGINAL_FEE * std::max(GRACE_ACTIONS, logicalActionCount);
 }
 
+template<typename T>
+static size_t GetTxIOFieldSize(const std::vector<T>& txIOs) {
+    auto size = GetSerializeSize(txIOs, SER_NETWORK, PROTOCOL_VERSION);
+    auto countSize = GetSizeOfCompactSize(txIOs.size());
+    return size - countSize;
+}
+
 size_t CalculateLogicalActionCount(
         const std::vector<CTxIn>& vin,
         const std::vector<CTxOut>& vout,
@@ -21,8 +28,8 @@ size_t CalculateLogicalActionCount(
         unsigned int saplingSpendCount,
         unsigned int saplingOutputCount,
         unsigned int orchardActionCount) {
-    const size_t tx_in_total_size = GetSerializeSize(vin, SER_NETWORK, PROTOCOL_VERSION);
-    const size_t tx_out_total_size = GetSerializeSize(vout, SER_NETWORK, PROTOCOL_VERSION);
+    const size_t tx_in_total_size = GetTxIOFieldSize(vin);
+    const size_t tx_out_total_size = GetTxIOFieldSize(vout);
 
     return std::max(ceil_div(tx_in_total_size, P2PKH_STANDARD_INPUT_SIZE),
                     ceil_div(tx_out_total_size, P2PKH_STANDARD_OUTPUT_SIZE)) +
diff --git a/src/zip317.h b/src/zip317.h
index 342b35e9930..10657368c07 100644
--- a/src/zip317.h
+++ b/src/zip317.h
@@ -23,6 +23,9 @@ static const int64_t WEIGHT_RATIO_SCALE = INT64_C(10000000000000000);
 static const int64_t WEIGHT_RATIO_CAP = 4;
 static const size_t BLOCK_UNPAID_ACTION_LIMIT = 50;
 
+/// This is the lowest the conventional fee can be in ZIP 317.
+static const CAmount MINIMUM_FEE = MARGINAL_FEE * GRACE_ACTIONS;
+
 /// Return the conventional fee for the given `logicalActionCount` calculated according to
 /// <https://zips.z.cash/zip-0317#fee-calculation>.
 CAmount CalculateConventionalFee(size_t logicalActionCount);


Test patch:
diff --git a/qa/rpc-tests/mempool_limit.py b/qa/rpc-tests/mempool_limit.py
index 9a3c2d5c2a1..466444006cd 100755
--- a/qa/rpc-tests/mempool_limit.py
+++ b/qa/rpc-tests/mempool_limit.py
@@ -13,6 +13,7 @@
     wait_and_assert_operationid_status,
     DEFAULT_FEE
 )
+from test_framework.zip317 import conventional_fee
 
 from decimal import Decimal
 from time import sleep
@@ -105,9 +106,9 @@ def run_test(self):
         print("Checking mempool size reset after block mined...")
         self.check_mempool_sizes(0)
         zaddr4 = self.nodes[0].z_getnewaddress('sapling')
-        opid4 = self.nodes[0].z_sendmany(zaddr1, [{"address": zaddr4, "amount": Decimal('10.0') - 2*DEFAULT_FEE}], 1)
+        opid4 = self.nodes[0].z_sendmany(zaddr1, [{"address": zaddr4, "amount": Decimal('10.0') - 2*conventional_fee(2)}], 1)
         wait_and_assert_operationid_status(self.nodes[0], opid4)
-        opid5 = self.nodes[0].z_sendmany(zaddr2, [{"address": zaddr4, "amount": Decimal('10.0') - 2*DEFAULT_FEE}], 1)
+        opid5 = self.nodes[0].z_sendmany(zaddr2, [{"address": zaddr4, "amount": Decimal('10.0') - 2*conventional_fee(2)}], 1)
         wait_and_assert_operationid_status(self.nodes[0], opid5)
         self.sync_all()
 
diff --git a/qa/rpc-tests/mempool_tx_expiry.py b/qa/rpc-tests/mempool_tx_expiry.py
index 526328768b5..a5d77ef9933 100755
--- a/qa/rpc-tests/mempool_tx_expiry.py
+++ b/qa/rpc-tests/mempool_tx_expiry.py
@@ -12,6 +12,7 @@
 from test_framework.util import assert_equal, \
     connect_nodes_bi, sync_blocks, start_nodes, \
     wait_and_assert_operationid_status, DEFAULT_FEE
+from test_framework.zip317 import conventional_fee
 
 from decimal import Decimal
 
@@ -93,7 +94,7 @@ def run_test(self):
 
         # Create transactions
         blockheight = self.nodes[0].getblockchaininfo()['blocks']
-        zsendamount = Decimal('1.0') - DEFAULT_FEE
+        zsendamount = Decimal('1.0') - conventional_fee(2)
         recipients = []
         recipients.append({"address": z_bob, "amount": zsendamount})
         myopid = self.nodes[0].z_sendmany(z_alice, recipients, 1)
diff --git a/qa/rpc-tests/test_framework/zip317.py b/qa/rpc-tests/test_framework/zip317.py
new file mode 100644
index 00000000000..dbe8f7e514d
--- /dev/null
+++ b/qa/rpc-tests/test_framework/zip317.py
@@ -0,0 +1,35 @@
+#!/usr/bin/env python3
+# Copyright (c) 2023 The Zcash developers
+# Distributed under the MIT software license, see the accompanying
+# file COPYING or https://www.opensource.org/licenses/mit-license.php .
+
+#
+# zip317.py
+#
+# Utilities for ZIP 317 conventional fee specification, as defined in https://zips.z.cash/zip-0317.
+#
+
+from test_framework.mininode import COIN
+from decimal import Decimal
+
+# The fee per logical action, in zatoshis. See https://zips.z.cash/zip-0317#fee-calculation.
+MARGINAL_FEE = 5000
+
+# The lower bound on the number of logical actions in a tx, for purposes of fee calculation. See
+# https://zips.z.cash/zip-0317#fee-calculation.
+GRACE_ACTIONS = 2
+
+# Limits the relative probability of picking a given transaction to be at most `WEIGHT_RATIO_CAP`
+# times greater than a transaction that pays exactly the conventional fee. See
+# https://zips.z.cash/zip-0317#recommended-algorithm-for-block-template-construction
+WEIGHT_RATIO_CAP = 4
+
+# The zcashd RPC sentinel value to indicate the conventional_fee when a positional argument is
+# required.
+ZIP_317_FEE = -1
+
+def conventional_fee_zats(logical_actions):
+    return MARGINAL_FEE * max(GRACE_ACTIONS, logical_actions)
+
+def conventional_fee(logical_actions):
+    return Decimal(conventional_fee_zats(logical_actions)) / COIN
diff --git a/qa/rpc-tests/wallet_listreceived.py b/qa/rpc-tests/wallet_listreceived.py
index 96eea5aa686..22fcbb63e21 100755
--- a/qa/rpc-tests/wallet_listreceived.py
+++ b/qa/rpc-tests/wallet_listreceived.py
@@ -13,10 +13,10 @@
     connect_nodes_bi,
     nuparams,
     DEFAULT_FEE,
-    DEFAULT_FEE_ZATS,
     NU5_BRANCH_ID,
 )
 from test_framework.util import wait_and_assert_operationid_status, start_nodes
+from test_framework.zip317 import conventional_fee, conventional_fee_zats
 from decimal import Decimal
 
 my_memo_str = 'c0ffee' # stay awake
@@ -157,8 +157,8 @@ def test_received_sapling(self, height):
         outputs = sorted(pt['outputs'], key=lambda x: x['valueZat'])
         assert_equal(outputs[0]['pool'], 'sapling')
         assert_equal(outputs[0]['address'], zaddr1)
-        assert_equal(outputs[0]['value'], Decimal('0.4') - DEFAULT_FEE)
-        assert_equal(outputs[0]['valueZat'], 40000000 - DEFAULT_FEE_ZATS)
+        assert_equal(outputs[0]['value'], Decimal('0.4') - conventional_fee(2))
+        assert_equal(outputs[0]['valueZat'], 40000000 - conventional_fee_zats(2))
         assert_equal(outputs[0]['output'], 1)
         assert_equal(outputs[0]['outgoing'], False)
         assert_equal(outputs[0]['memo'], no_memo)
@@ -178,8 +178,8 @@ def test_received_sapling(self, height):
         assert_equal(2, len(r), "zaddr1 Should have received 2 notes")
         r = sorted(r, key = lambda received: received['amount'])
         assert_equal(txid, r[0]['txid'])
-        assert_equal(Decimal('0.4')-DEFAULT_FEE, r[0]['amount'])
-        assert_equal(40000000-DEFAULT_FEE_ZATS, r[0]['amountZat'])
+        assert_equal(Decimal('0.4')-conventional_fee(2), r[0]['amount'])
+        assert_equal(40000000-conventional_fee_zats(2), r[0]['amountZat'])
         assert_equal(r[0]['change'], True, "Note valued at (0.4-"+str(DEFAULT_FEE)+") should be change")
         assert_equal(no_memo, r[0]['memo'])
 
@@ -382,8 +382,8 @@ def test_received_orchard(self, height):
 
         # Verify that we observe the change output
         assert_equal(outputs[2]['pool'], 'orchard')
-        assert_equal(outputs[2]['value'], Decimal('0.49999'))
-        assert_equal(outputs[2]['valueZat'], 49999000)
+        assert_equal(outputs[2]['value'], Decimal('0.5') - conventional_fee(3))
+        assert_equal(outputs[2]['valueZat'], 50000000 - conventional_fee_zats(3))
         assert_equal(outputs[2]['outgoing'], False)
         assert_equal(outputs[2]['walletInternal'], True)
         assert_equal(outputs[2]['memo'], no_memo)
diff --git a/qa/rpc-tests/wallet_nullifiers.py b/qa/rpc-tests/wallet_nullifiers.py
index 938945bc6eb..1a16df9939b 100755
--- a/qa/rpc-tests/wallet_nullifiers.py
+++ b/qa/rpc-tests/wallet_nullifiers.py
@@ -7,6 +7,7 @@
 from test_framework.util import assert_equal, assert_true, bitcoind_processes, \
     connect_nodes_bi, start_node, start_nodes, wait_and_assert_operationid_status, \
     get_coinbase_address, DEFAULT_FEE
+from test_framework.zip317 import conventional_fee
 
 from decimal import Decimal
 
@@ -92,7 +93,7 @@ def run_test (self):
 
         # check zaddr balance
         zsendmany2notevalue = Decimal('2.0')
-        zsendmanyfee = DEFAULT_FEE
+        zsendmanyfee = conventional_fee(2)
         zaddrremaining = zsendmanynotevalue - zsendmany2notevalue - zsendmanyfee
         assert_equal(self.nodes[3].z_getbalance(myzaddr3), zsendmany2notevalue)
         assert_equal(self.nodes[2].z_getbalance(myzaddr), zaddrremaining)
@@ -117,7 +118,7 @@ def run_test (self):
 
         # check zaddr balance
         zsendmany3notevalue = Decimal('1.0')
-        zaddrremaining2 = zaddrremaining - zsendmany3notevalue - zsendmanyfee
+        zaddrremaining2 = zaddrremaining - zsendmany3notevalue - DEFAULT_FEE
         assert_equal(self.nodes[1].z_getbalance(myzaddr), zaddrremaining2)
         assert_equal(self.nodes[2].z_getbalance(myzaddr), zaddrremaining2)
 
diff --git a/qa/rpc-tests/wallet_parsing_amounts.py b/qa/rpc-tests/wallet_parsing_amounts.py
index bdfdf42c3ec..ff792b9340b 100755
--- a/qa/rpc-tests/wallet_parsing_amounts.py
+++ b/qa/rpc-tests/wallet_parsing_amounts.py
@@ -70,19 +70,6 @@ def run_test(self):
             print(errorString)
             assert(False)
 
-        # This fee is larger than the default fee and since amount=0
-        # it should trigger error
-        fee         = Decimal('0.1')
-        recipients  = [ {"address": myzaddr, "amount": Decimal('0.0') } ]
-        minconf     = 1
-        errorString = ''
-
-        try:
-            myopid = self.nodes[0].z_sendmany(myzaddr, recipients, minconf, fee)
-        except JSONRPCException as e:
-            errorString = e.error['message']
-        assert('Small transaction amount' in errorString)
-
         # This fee is less than default and greater than amount, but still valid
         fee         = Decimal('0.0000001')
         recipients  = [ {"address": myzaddr, "amount": Decimal('0.00000001') } ]
diff --git a/qa/rpc-tests/wallet_shieldingcoinbase.py b/qa/rpc-tests/wallet_shieldingcoinbase.py
index 7e1ccc42d18..f3b9b15bee6 100755
--- a/qa/rpc-tests/wallet_shieldingcoinbase.py
+++ b/qa/rpc-tests/wallet_shieldingcoinbase.py
@@ -10,6 +10,7 @@
     start_nodes, connect_nodes_bi, wait_and_assert_operationid_status, \
     wait_and_assert_operationid_status_result, get_coinbase_address, \
     check_node_log, DEFAULT_FEE
+from test_framework.zip317 import conventional_fee, WEIGHT_RATIO_CAP, ZIP_317_FEE
 
 import sys
 import timeit
@@ -104,7 +105,7 @@ def run_test (self):
         recipients = []
         recipients.append({"address":myzaddr, "amount":Decimal('1.23456789')})
 
-        myopid = self.nodes[0].z_sendmany(mytaddr, recipients, 10, DEFAULT_FEE, 'AllowRevealedSenders')
+        myopid = self.nodes[0].z_sendmany(mytaddr, recipients, 10, DEFAULT_FEE, 'AllowFullyTransparent')
         error_result = wait_and_assert_operationid_status_result(
                 self.nodes[0],
                 myopid, "failed",
@@ -188,7 +189,7 @@ def run_test (self):
 
         # A custom fee of 0 is okay.  Here the node will send the note value back to itself.
         recipients = []
-        recipients.append({"address":myzaddr, "amount": Decimal('20.0') - DEFAULT_FEE})
+        recipients.append({"address":myzaddr, "amount": Decimal('20.0') - conventional_fee(2)})
         myopid = self.nodes[0].z_sendmany(myzaddr, recipients, 1, Decimal('0.0'))
         mytxid = wait_and_assert_operationid_status(self.nodes[0], myopid)
         self.sync_all()
@@ -230,7 +231,7 @@ def run_test (self):
         # UTXO selection in z_sendmany sorts in ascending order, so smallest utxos are consumed first.
         # At this point in time, unspent notes all have a value of 10.0.
         recipients = []
-        amount = Decimal('10.0') - DEFAULT_FEE - Decimal('0.00000001')    # this leaves change at 1 zatoshi less than dust threshold
+        amount = Decimal('10.0') - conventional_fee(2) - 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, any transparent recipients are included, or if the `privacyPolicy` parameter is not set to `AllowRevealedSenders` or weaker.")
@@ -247,7 +248,7 @@ def run_test (self):
         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, any transparent recipients are included, or if the `privacyPolicy` parameter is not set to `AllowRevealedSenders` or weaker.")
+        wait_and_assert_operationid_status(self.nodes[0], myopid, "failed", "Insufficient funds: have 10.00, need 10000.0001; 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, any transparent recipients are included, or if the `privacyPolicy` parameter is not set to `AllowRevealedSenders` or weaker.")
 
@@ -264,7 +265,7 @@ def run_test (self):
         # given the tx size, resulting in mempool rejection.
         errorString = ''
         recipients = []
-        num_t_recipients = 2500
+        num_t_recipients = 1998 # stay just under the absurdly-high-fee error
         amount_per_recipient = Decimal('0.00000546') # dust threshold
         # Note that regtest chainparams does not require standard tx, so setting the amount to be
         # less than the dust threshold, e.g. 0.00000001 will not result in mempool rejection.
@@ -284,7 +285,7 @@ def run_test (self):
         self.nodes[0].getinfo()
         # Issue #2263 Workaround END
 
-        myopid = self.nodes[0].z_sendmany(myzaddr, recipients, 1, DEFAULT_FEE, 'AllowRevealedRecipients')
+        myopid = self.nodes[0].z_sendmany(myzaddr, recipients, 1, ZIP_317_FEE, 'AllowRevealedRecipients')
         try:
             wait_and_assert_operationid_status(self.nodes[0], myopid)
         except JSONRPCException as e:
@@ -298,15 +299,18 @@ def run_test (self):
         self.nodes[1].generate(1)
         self.sync_all()
 
+        many_recipient_fee = conventional_fee(2+num_t_recipients) # 2+ for padded Sapling input/change
+
         # check balance
         node2balance = amount_per_recipient * num_t_recipients
-        saplingvalue -= node2balance + DEFAULT_FEE
+        saplingvalue -= node2balance + many_recipient_fee
         assert_equal(self.nodes[2].getbalance(), node2balance)
         check_value_pool(self.nodes[0], 'sapling', saplingvalue)
 
         # Send will fail because fee is negative
         try:
-            self.nodes[0].z_sendmany(myzaddr, recipients, 1, -1)
+            # NB: Using -2 as the fee because -1 is the sentinel for using the conventional fee.
+            self.nodes[0].z_sendmany(myzaddr, recipients, 1, -2)
         except JSONRPCException as e:
             errorString = e.error['message']
         assert_equal("Amount out of range" in errorString, True)
@@ -318,12 +322,9 @@ def run_test (self):
             errorString = e.error['message']
         assert_equal("Amount out of range" in errorString, True)
 
-        # Send will fail because fee is larger than sum of outputs
-        try:
-            self.nodes[0].z_sendmany(myzaddr, recipients, 1, (amount_per_recipient * num_t_recipients) + Decimal('0.00000001'))
-        except JSONRPCException as e:
-            errorString = e.error['message']
-        assert_equal("is greater than the sum of outputs" in errorString, True)
+        # Send will fail because fee is more than `WEIGHT_RATIO_CAP * conventional_fee`
+        myopid = self.nodes[0].z_sendmany(myzaddr, recipients, 1, (WEIGHT_RATIO_CAP * many_recipient_fee) + Decimal('0.00000001'))
+        wait_and_assert_operationid_status(self.nodes[0], myopid, 'failed', 'Fee 0.40000001 is greater than 4 times the conventional fee for this tx (which is 0.10). There is no prioritisation benefit to a fee this large (see https://zips.z.cash/zip-0317#recommended-algorithm-for-block-template-construction), and it likely indicates a mistake in setting the fee.')
 
         # Send will succeed because the balance of non-coinbase utxos is 10.0
         try:
diff --git a/qa/rpc-tests/wallet_treestate.py b/qa/rpc-tests/wallet_treestate.py
index e1afb60c59e..4fa1a245ccd 100755
--- a/qa/rpc-tests/wallet_treestate.py
+++ b/qa/rpc-tests/wallet_treestate.py
@@ -7,6 +7,7 @@
 from test_framework.util import assert_equal, initialize_chain_clean, \
     start_nodes, connect_nodes_bi, wait_and_assert_operationid_status, \
     get_coinbase_address, DEFAULT_FEE
+from test_framework.zip317 import conventional_fee
 
 import time
 from decimal import Decimal
@@ -79,7 +80,7 @@ def run_test (self):
         # the z_sendmany implementation because there are only two inputs per joinsplit.
         recipients = []
         recipients.append({"address": self.nodes[2].z_getnewaddress(), "amount": Decimal('18.0')})
-        recipients.append({"address": self.nodes[2].z_getnewaddress(), "amount": Decimal('12.0') - 4*DEFAULT_FEE})
+        recipients.append({"address": self.nodes[2].z_getnewaddress(), "amount": Decimal('12.0') - 4 * conventional_fee(3)})
         myopid = self.nodes[0].z_sendmany(myzaddr, recipients, 1)
 
         # Wait for Tx 2 to begin executing...
diff --git a/qa/rpc-tests/wallet_z_sendmany.py b/qa/rpc-tests/wallet_z_sendmany.py
index 10734465ead..7dc57757cab 100755
--- a/qa/rpc-tests/wallet_z_sendmany.py
+++ b/qa/rpc-tests/wallet_z_sendmany.py
@@ -22,6 +22,10 @@
 
 # Test wallet address behaviour across network upgrades
 class WalletZSendmanyTest(BitcoinTestFramework):
+    def __init__(self):
+        super().__init__()
+        self.cache_behavior = 'sprout'
+
     def setup_network(self, split=False):
         self.nodes = start_nodes(3, self.options.tmpdir, [[
             nuparams(NU5_BRANCH_ID, 238),
@@ -110,12 +114,13 @@ def run_test(self):
         # check balances
         zsendmanynotevalue = Decimal('7.0')
         zsendmanyfee = DEFAULT_FEE
-        node2utxobalance = Decimal('260.00000000') - zsendmanynotevalue - zsendmanyfee
+        node2sproutbalance = Decimal('50.00000000')
+        node2utxobalance = Decimal('210.00000000') - zsendmanynotevalue - zsendmanyfee
 
         # check shielded balance status with getwalletinfo
         wallet_info = self.nodes[2].getwalletinfo()
         assert_equal(Decimal(wallet_info["shielded_unconfirmed_balance"]), zsendmanynotevalue)
-        assert_equal(Decimal(wallet_info["shielded_balance"]), Decimal('0.0'))
+        assert_equal(Decimal(wallet_info["shielded_balance"]), node2sproutbalance)
 
         self.nodes[2].generate(10)
         self.sync_all()
@@ -130,13 +135,13 @@ def run_test(self):
         # check via z_gettotalbalance
         resp = self.nodes[2].z_gettotalbalance()
         assert_equal(Decimal(resp["transparent"]), node2utxobalance)
-        assert_equal(Decimal(resp["private"]), zbalance)
-        assert_equal(Decimal(resp["total"]), node2utxobalance + zbalance)
+        assert_equal(Decimal(resp["private"]), node2sproutbalance + zbalance)
+        assert_equal(Decimal(resp["total"]), node2utxobalance + node2sproutbalance + zbalance)
 
         # check confirmed shielded balance with getwalletinfo
         wallet_info = self.nodes[2].getwalletinfo()
         assert_equal(Decimal(wallet_info["shielded_unconfirmed_balance"]), Decimal('0.0'))
-        assert_equal(Decimal(wallet_info["shielded_balance"]), zsendmanynotevalue)
+        assert_equal(Decimal(wallet_info["shielded_balance"]), node2sproutbalance + zsendmanynotevalue)
 
         # there should be at least one Sapling output
         mytxdetails = self.nodes[2].getrawtransaction(mytxid, 1)
@@ -377,7 +382,7 @@ def run_test(self):
         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.'),
+            ('AllowRevealedRecipients', 'Could not send to an Orchard-only receiver despite a lax privacy policy, because 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)
@@ -416,17 +421,18 @@ def run_test(self):
         self.sync_all()
 
         #
-        # Test Orchard-only UA with insufficient non-Sprout funds
+        # Test sending Sprout funds to Orchard-only UA
         #
 
+        sproutAddr = self.nodes[2].listaddresses()[0]['sprout']['addresses'][0]
         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.'),
+            ('AllowRevealedRecipients', 'Could not send to an Orchard-only receiver despite a lax privacy policy, because you are sending from the Sprout pool and there is no transaction version that supports both Sprout and Orchard.'),
         ]:
-            opid = self.nodes[1].z_sendmany(n1ua0, recipients, 1, 0, policy)
-            wait_and_assert_operationid_status(self.nodes[1], opid, 'failed', msg)
+            opid = self.nodes[2].z_sendmany(sproutAddr, recipients, 1, 0, policy)
+            wait_and_assert_operationid_status(self.nodes[2], opid, 'failed', msg)
 
         #
         # Test AllowRevealedAmounts policy
@@ -488,5 +494,24 @@ def run_test(self):
                 {'pools': {'orchard': {'valueZat': 200000000}}, 'minimum_confirmations': 1},
                 self.nodes[0].z_getbalanceforaccount(n0account0))
 
+
+        self.sync_all()
+        self.nodes[1].generate(1)
+        self.sync_all()
+
+        #
+        # Test transparent change
+        #
+
+        recipients = [{"address":n0ua1, "amount": 4}]
+        # Should fail because this generates transparent change, but we don’t have
+        # `AllowRevealedRecipients`
+        opid = self.nodes[2].z_sendmany(mytaddr, recipients, 1, 0, 'AllowRevealedSenders')
+        wait_and_assert_operationid_status(self.nodes[2], opid, 'failed', "This transaction would have transparent change, which is not enabled by default because it will publicly reveal the change address 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.")
+
+        # Should succeed once we include `AllowRevealedRecipients`
+        opid = self.nodes[2].z_sendmany(mytaddr, recipients, 1, 0, 'AllowFullyTransparent')
+        wait_and_assert_operationid_status(self.nodes[2], opid)
+
 if __name__ == '__main__':
     WalletZSendmanyTest().main()
diff --git a/src/wallet/gtest/test_rpc_wallet.cpp b/src/wallet/gtest/test_rpc_wallet.cpp
index 716654c3170..5bcef4ad2d6 100644
--- a/src/wallet/gtest/test_rpc_wallet.cpp
+++ b/src/wallet/gtest/test_rpc_wallet.cpp
@@ -296,7 +296,7 @@ TEST(WalletRPCTests, RPCZsendmanyTaddrToSapling)
             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> operation(new AsyncRPCOperation_sendmany(std::move(builder), selector, recipients, 0, 0, strategy, std::nullopt));
     std::shared_ptr<AsyncRPCOperation_sendmany> ptr = std::dynamic_pointer_cast<AsyncRPCOperation_sendmany> (operation);
 
     // Enable test mode so tx is not sent
diff --git a/src/wallet/test/rpc_wallet_tests.cpp b/src/wallet/test/rpc_wallet_tests.cpp
index 58583c556b0..cd4b4454a76 100644
--- a/src/wallet/test/rpc_wallet_tests.cpp
+++ b/src/wallet/test/rpc_wallet_tests.cpp
@@ -1251,7 +1251,7 @@ BOOST_AUTO_TEST_CASE(rpc_z_sendmany_internals)
         WalletTxBuilder builder(Params(), 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));
+        std::shared_ptr<AsyncRPCOperation> operation(new AsyncRPCOperation_sendmany(std::move(builder), selector, recipients, 1, 1, strategy, std::nullopt));
         operation->main();
         BOOST_CHECK(operation->isFailed());
         std::string msg = operation->getErrorMessage();
@@ -1268,7 +1268,7 @@ BOOST_AUTO_TEST_CASE(rpc_z_sendmany_internals)
         WalletTxBuilder builder(Params(), 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));
+        std::shared_ptr<AsyncRPCOperation> operation(new AsyncRPCOperation_sendmany(std::move(builder), selector, recipients, 1, 1, strategy, std::nullopt));
         operation->main();
         BOOST_CHECK(operation->isFailed());
         std::string msg = operation->getErrorMessage();