Ensure we don’t make Orchard change pre-NU5

Co-authored-by: Kris Nuttycombe <kris@nutty.land>

src/wallet/gtest/test_rpc_wallet.cpp

@@ -284,7 +284,7 @@ TEST(WalletRPCTests, RPCZsendmanyTaddrToSapling)
     pwalletMain->LoadWalletTx(wtx);
 
     // Context that z_sendmany requires
-    auto builder = WalletTxBuilder(*pwalletMain, minRelayTxFee);
+    auto builder = WalletTxBuilder(Params(), *pwalletMain, minRelayTxFee);
     mtx = CreateNewContextualCMutableTransaction(consensusParams, nextBlockHeight, false);
 
     auto selector = pwalletMain->ZTXOSelectorForAddress(taddr, true, false, TransactionStrategy(PrivacyPolicy::FullPrivacy)).value();

src/wallet/rpcwallet.cpp

@@ -4982,7 +4982,7 @@ UniValue z_sendmany(const UniValue& params, bool fHelp)
 
     // Create operation and add to global queue
     auto nAnchorDepth = std::min((unsigned int) nMinDepth, nAnchorConfirmations);
-    WalletTxBuilder builder(*pwalletMain, minRelayTxFee);
+    WalletTxBuilder builder(Params(), *pwalletMain, minRelayTxFee);
 
     std::shared_ptr<AsyncRPCQueue> q = getAsyncRPCQueue();
     std::shared_ptr<AsyncRPCOperation> operation(

src/wallet/test/rpc_wallet_tests.cpp

@@ -1239,7 +1239,7 @@ BOOST_AUTO_TEST_CASE(rpc_z_sendmany_internals)
     // there are no utxos to spend
     {
         auto selector = pwalletMain->ZTXOSelectorForAddress(taddr1, true, false, FULL_PRIVACY).value();
-        WalletTxBuilder builder(*pwalletMain, minRelayTxFee);
+        WalletTxBuilder builder(Params(), *pwalletMain, minRelayTxFee);
         std::vector<Payment> recipients = { Payment(zaddr1, 100*COIN, Memo::FromHexOrThrow("DEADBEEF")) };
         TransactionStrategy strategy;
         std::shared_ptr<AsyncRPCOperation> operation(new AsyncRPCOperation_sendmany(std::move(builder), selector, recipients, 1, 1, strategy));
@@ -1252,7 +1252,7 @@ BOOST_AUTO_TEST_CASE(rpc_z_sendmany_internals)
     // there are no unspent notes to spend
     {
         auto selector = pwalletMain->ZTXOSelectorForAddress(zaddr1, true, false, FULL_PRIVACY).value();
-        WalletTxBuilder builder(*pwalletMain, minRelayTxFee);
+        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));

src/wallet/wallet_tx_builder.cpp

@@ -6,7 +6,7 @@
 
 using namespace libzcash;
 
-int GetAnchorHeight(const CChain& chain, int anchorConfirmations)
+int GetAnchorHeight(const CChain& chain, uint32_t anchorConfirmations)
 {
     int nextBlockHeight = chain.Height() + 1;
     return nextBlockHeight - anchorConfirmations;
@@ -23,7 +23,8 @@ PrepareTransactionResult WalletTxBuilder::PrepareTransaction(
 {
     assert(fee < MAX_MONEY);
 
-    auto selected = ResolveInputsAndPayments(spendable, payments, chain, strategy, fee, anchorConfirmations);
+    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);
     }
@@ -68,7 +69,8 @@ PrepareTransactionResult WalletTxBuilder::PrepareTransaction(
                         }
                         break;
                     case ReceiverType::Orchard:
-                        if (!spendable.orchardNoteMetadata.empty() || strategy.AllowRevealedAmounts()) {
+                        if (params.GetConsensus().NetworkUpgradeActive(anchorHeight, Consensus::UPGRADE_NU5)
+                            && (!spendable.orchardNoteMetadata.empty() || strategy.AllowRevealedAmounts())) {
                             result.insert(OutputPool::Orchard);
                         }
                         break;
@@ -152,7 +154,7 @@ PrepareTransactionResult WalletTxBuilder::PrepareTransaction(
                 }
             },
             [&](const libzcash::UnifiedFullViewingKey& fvk) {
-                auto zufvk = ZcashdUnifiedFullViewingKey::FromUnifiedFullViewingKey(Params(), fvk);
+                auto zufvk = ZcashdUnifiedFullViewingKey::FromUnifiedFullViewingKey(params, fvk);
                 auto sendTo = zufvk.GetChangeAddress(
                         allowedChangeTypes(fvk.GetKnownReceiverTypes()));
                 if (sendTo.has_value()) {
@@ -189,7 +191,7 @@ PrepareTransactionResult WalletTxBuilder::PrepareTransaction(
             fee,
             ovks.first,
             ovks.second,
-            GetAnchorHeight(chain, anchorConfirmations));
+            anchorHeight);
 }
 
 Payments InputSelection::GetPayments() const {
@@ -232,12 +234,13 @@ bool WalletTxBuilder::AllowTransparentCoinbase(
 }
 
 InputSelectionResult WalletTxBuilder::ResolveInputsAndPayments(
+        const ZTXOSelector& selector,
         SpendableInputs& spendableMut,
         const std::vector<Payment>& payments,
         const CChain& chain,
         TransactionStrategy strategy,
         CAmount fee,
-        uint32_t anchorConfirmations) const
+        int anchorHeight) const
 {
     LOCK2(cs_main, wallet.cs_wallet);
 
@@ -259,10 +262,11 @@ InputSelectionResult WalletTxBuilder::ResolveInputsAndPayments(
 
     // we can only select Orchard addresses if there are sufficient non-Sprout
     // funds to cover the total payments + fee.
-    bool canResolveOrchard = spendableMut.Total() - spendableMut.GetSproutBalance() >= targetAmount;
+    bool canResolveOrchard =
+        params.GetConsensus().NetworkUpgradeActive(anchorHeight, Consensus::UPGRADE_NU5)
+        && spendableMut.Total() - spendableMut.GetSproutBalance() >= targetAmount;
     std::vector<ResolvedPayment> resolvedPayments;
     std::optional<AddressResolutionError> resolutionError;
-    int anchorHeight = GetAnchorHeight(chain, anchorConfirmations);
     for (const auto& payment : payments) {
         std::visit(match {
             [&](const CKeyID& p2pkh) {
@@ -297,8 +301,7 @@ InputSelectionResult WalletTxBuilder::ResolveInputsAndPayments(
             },
             [&](const UnifiedAddress& ua) {
                 bool resolved{false};
-                if (Params().GetConsensus().NetworkUpgradeActive(anchorHeight, Consensus::UPGRADE_NU5)
+                if (canResolveOrchard
-                    && canResolveOrchard
                     && ua.GetOrchardReceiver().has_value()
                     && (strategy.AllowRevealedAmounts() || payment.GetAmount() < maxOrchardAvailable)
                     ) {

src/wallet/wallet_tx_builder.h

@@ -292,6 +292,7 @@ typedef std::variant<
 
 class WalletTxBuilder {
 private:
+    const CChainParams& params;
     const CWallet& wallet;
     CFeeRate minRelayFee;
     uint32_t maxOrchardActions;
@@ -307,12 +308,13 @@ private:
      * and the requested transaction strategy.
      */
     InputSelectionResult ResolveInputsAndPayments(
+            const ZTXOSelector& selector,
             SpendableInputs& spendable,
             const std::vector<Payment>& payments,
             const CChain& chain,
             TransactionStrategy strategy,
             CAmount fee,
-            uint32_t anchorConfirmations) const;
+            int anchorHeight) const;
     /**
      * Compute the internal and external OVKs to use in transaction construction, given
      * the spendable inputs.
@@ -322,8 +324,8 @@ private:
             const SpendableInputs& spendable) const;
 
 public:
-    WalletTxBuilder(const CWallet& wallet, CFeeRate minRelayFee):
+    WalletTxBuilder(const CChainParams& params, const CWallet& wallet, CFeeRate minRelayFee):
-        wallet(wallet), minRelayFee(minRelayFee), maxOrchardActions(nOrchardActionLimit) {}
+        params(params), wallet(wallet), minRelayFee(minRelayFee), maxOrchardActions(nOrchardActionLimit) {}
 
     static bool AllowTransparentCoinbase(
             const std::vector<Payment>& payments,