From 11721906fd30b768b568b138ff06b90a07e72a24 Mon Sep 17 00:00:00 2001 From: Greg Pfeil Date: Mon, 23 Jan 2023 15:57:05 -0700 Subject: [PATCH] =?UTF-8?q?Ensure=20we=20don=E2=80=99t=20make=20Orchard=20?= =?UTF-8?q?change=20pre-NU5?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Kris Nuttycombe --- src/wallet/gtest/test_rpc_wallet.cpp | 2 +- src/wallet/rpcwallet.cpp | 2 +- src/wallet/test/rpc_wallet_tests.cpp | 4 ++-- src/wallet/wallet_tx_builder.cpp | 23 +++++++++++++---------- src/wallet/wallet_tx_builder.h | 8 +++++--- 5 files changed, 22 insertions(+), 17 deletions(-) diff --git a/src/wallet/gtest/test_rpc_wallet.cpp b/src/wallet/gtest/test_rpc_wallet.cpp index 3f258ae08..39739e835 100644 --- a/src/wallet/gtest/test_rpc_wallet.cpp +++ b/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(); diff --git a/src/wallet/rpcwallet.cpp b/src/wallet/rpcwallet.cpp index 7f7bf5f09..25a502dd3 100644 --- a/src/wallet/rpcwallet.cpp +++ b/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 q = getAsyncRPCQueue(); std::shared_ptr operation( diff --git a/src/wallet/test/rpc_wallet_tests.cpp b/src/wallet/test/rpc_wallet_tests.cpp index 40c0b1a55..b5d699f23 100644 --- a/src/wallet/test/rpc_wallet_tests.cpp +++ b/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 recipients = { Payment(zaddr1, 100*COIN, Memo::FromHexOrThrow("DEADBEEF")) }; TransactionStrategy strategy; std::shared_ptr 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 recipients = { Payment(taddr1, 100*COIN, Memo::FromHexOrThrow("DEADBEEF")) }; TransactionStrategy strategy(PrivacyPolicy::AllowRevealedRecipients); std::shared_ptr operation(new AsyncRPCOperation_sendmany(std::move(builder), selector, recipients, 1, 1, strategy)); diff --git a/src/wallet/wallet_tx_builder.cpp b/src/wallet/wallet_tx_builder.cpp index c15ad8201..ffa814815 100644 --- a/src/wallet/wallet_tx_builder.cpp +++ b/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(selected)) { return std::get(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& 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 resolvedPayments; std::optional 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) - && canResolveOrchard + if (canResolveOrchard && ua.GetOrchardReceiver().has_value() && (strategy.AllowRevealedAmounts() || payment.GetAmount() < maxOrchardAvailable) ) { diff --git a/src/wallet/wallet_tx_builder.h b/src/wallet/wallet_tx_builder.h index 901f7ee90..5c42db19b 100644 --- a/src/wallet/wallet_tx_builder.h +++ b/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& 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): - wallet(wallet), minRelayFee(minRelayFee), maxOrchardActions(nOrchardActionLimit) {} + WalletTxBuilder(const CChainParams& params, const CWallet& wallet, CFeeRate minRelayFee): + params(params), wallet(wallet), minRelayFee(minRelayFee), maxOrchardActions(nOrchardActionLimit) {} static bool AllowTransparentCoinbase( const std::vector& payments,