diff --git a/src/wallet/asyncrpcoperation_sendmany.cpp b/src/wallet/asyncrpcoperation_sendmany.cpp index 31dcc114f..83a91838b 100644 --- a/src/wallet/asyncrpcoperation_sendmany.cpp +++ b/src/wallet/asyncrpcoperation_sendmany.cpp @@ -67,12 +67,10 @@ AsyncRPCOperation_sendmany::AsyncRPCOperation_sendmany( for (const SendManyRecipient& recipient : recipients_) { std::visit(match { [&](const CKeyID& addr) { - transparentRecipients_ += 1; txOutputAmounts_.t_outputs_total += recipient.amount; recipientPools_.insert(OutputPool::Transparent); }, [&](const CScriptID& addr) { - transparentRecipients_ += 1; txOutputAmounts_.t_outputs_total += recipient.amount; recipientPools_.insert(OutputPool::Transparent); }, @@ -181,7 +179,7 @@ uint256 AsyncRPCOperation_sendmany::main_impl() { // Allow transparent coinbase inputs if there are no transparent // recipients. - bool allowTransparentCoinbase = transparentRecipients_ == 0; + bool allowTransparentCoinbase = !recipientPools_.count(OutputPool::Transparent); // Set the dust threshold so that we can select enough inputs to avoid // creating dust change amounts. @@ -196,6 +194,8 @@ uint256 AsyncRPCOperation_sendmany::main_impl() { } if (!spendable.LimitToAmount(targetAmount, dustThreshold, recipientPools_)) { CAmount changeAmount{spendable.Total() - targetAmount}; + std::string insufficientFundsMessage = + strprintf("Insufficient funds: have %s", FormatMoney(spendable.Total())); if (changeAmount > 0 && changeAmount < dustThreshold) { // TODO: we should provide the option for the caller to explicitly // forego change (definitionally an amount below the dust amount) @@ -203,34 +203,29 @@ uint256 AsyncRPCOperation_sendmany::main_impl() { // creating dust change, rather than prohibit them from sending // entirely in this circumstance. // (Daira disagrees, as this could leak information to the recipient) - throw JSONRPCError( - RPC_WALLET_INSUFFICIENT_FUNDS, - strprintf( - "Insufficient funds: have %s, need %s more to avoid creating invalid change output %s " - "(dust threshold is %s)", - FormatMoney(spendable.Total()), - FormatMoney(dustThreshold - changeAmount), - FormatMoney(changeAmount), - FormatMoney(dustThreshold))); + insufficientFundsMessage += + strprintf( + ", need %s more to avoid creating invalid change output %s (dust threshold is %s)", + FormatMoney(dustThreshold - changeAmount), + FormatMoney(changeAmount), + FormatMoney(dustThreshold)); } else { - bool isFromUa = std::holds_alternative(ztxoSelector_.GetPattern()); - throw JSONRPCError( - RPC_WALLET_INSUFFICIENT_FUNDS, - strprintf( - "Insufficient funds: have %s, need %s", - FormatMoney(spendable.Total()), FormatMoney(targetAmount)) - + (allowTransparentCoinbase ? "" : - "; note that coinbase outputs will not be selected if you specify " - "ANY_TADDR or if any transparent recipients are included.") - + ((!isFromUa || strategy_.AllowLinkingAccountAddresses()) ? "" : - " (This transaction may require selecting transparent coins that were sent " - "to multiple Unified Addresses, which is not enabled by default because " - "it would create a public link between the transparent receivers of these " - "addresses. THIS MAY AFFECT YOUR PRIVACY. Resubmit with the `privacyPolicy` " - "parameter set to `AllowLinkingAccountAddresses` or weaker if you wish to " - "allow this transaction to proceed anyway.)") - ); + insufficientFundsMessage += strprintf(", need %s", FormatMoney(targetAmount)); } + bool isFromUa = std::holds_alternative(ztxoSelector_.GetPattern()); + throw JSONRPCError( + RPC_WALLET_INSUFFICIENT_FUNDS, + insufficientFundsMessage + + (allowTransparentCoinbase && ztxoSelector_.SelectsTransparentCoinbase() ? "" : + "; note that coinbase outputs will not be selected if you specify " + "ANY_TADDR or if any transparent recipients are included.") + + ((!isFromUa || strategy_.AllowLinkingAccountAddresses()) ? "" : + " (This transaction may require selecting transparent coins that were sent " + "to multiple Unified Addresses, which is not enabled by default because " + "it would create a public link between the transparent receivers of these " + "addresses. THIS MAY AFFECT YOUR PRIVACY. Resubmit with the `privacyPolicy` " + "parameter set to `AllowLinkingAccountAddresses` or weaker if you wish to " + "allow this transaction to proceed anyway.)")); } if (!(spendable.utxos.empty() || strategy_.AllowRevealedSenders())) { diff --git a/src/wallet/asyncrpcoperation_sendmany.h b/src/wallet/asyncrpcoperation_sendmany.h index 0fdcf7c11..5a77dbadb 100644 --- a/src/wallet/asyncrpcoperation_sendmany.h +++ b/src/wallet/asyncrpcoperation_sendmany.h @@ -80,7 +80,6 @@ private: bool isfromsprout_{false}; bool isfromsapling_{false}; TransactionStrategy strategy_; - uint32_t transparentRecipients_{0}; AccountId sendFromAccount_; std::set recipientPools_; TxOutputAmounts txOutputAmounts_; diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index 898cf37dc..da26aa320 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -2214,6 +2214,7 @@ SpendableInputs CWallet::FindSpendableInputs( KeyIO keyIO(Params()); bool selectTransparent{selector.SelectsTransparent()}; + bool selectTransparentCoinbase{selector.SelectsTransparentCoinbase()}; bool selectSprout{selector.SelectsSprout()}; bool selectSapling{selector.SelectsSapling()}; bool selectOrchard{selector.SelectsOrchard()}; @@ -2229,7 +2230,7 @@ SpendableInputs CWallet::FindSpendableInputs( if (selectTransparent && // skip transparent utxo selection if coinbase spend restrictions are not met - (!isCoinbase || (allowTransparentCoinbase && wtx.GetBlocksToMaturity(asOfHeight) <= 0))) { + (!isCoinbase || (selectTransparentCoinbase && allowTransparentCoinbase && wtx.GetBlocksToMaturity(asOfHeight) <= 0))) { for (int i = 0; i < wtx.vout.size(); i++) { const auto& output = wtx.vout[i]; @@ -7822,6 +7823,23 @@ bool ZTXOSelector::SelectsTransparent() const { [](const AccountZTXOPattern& acct) { return acct.IncludesP2PKH() || acct.IncludesP2SH(); } }, this->pattern); } +bool ZTXOSelector::SelectsTransparentCoinbase() const { + return std::visit(match { + [](const CKeyID& keyId) { return true; }, + [](const CScriptID& scriptId) { return true; }, + [](const libzcash::SproutPaymentAddress& addr) { return false; }, + [](const libzcash::SproutViewingKey& vk) { return false; }, + [](const libzcash::SaplingPaymentAddress& addr) { return false; }, + [](const libzcash::SaplingExtendedFullViewingKey& vk) { return false; }, + [](const libzcash::UnifiedAddress& ua) { + return ua.GetP2PKHReceiver().has_value() || ua.GetP2SHReceiver().has_value(); + }, + [](const libzcash::UnifiedFullViewingKey& ufvk) { return ufvk.GetTransparentKey().has_value(); }, + [](const AccountZTXOPattern& acct) { + return (acct.IncludesP2PKH() || acct.IncludesP2SH()) && acct.GetAccountId() != ZCASH_LEGACY_ACCOUNT; + } + }, this->pattern); +} bool ZTXOSelector::SelectsSprout() const { return std::visit(match { [](const libzcash::SproutViewingKey& addr) { return true; }, diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h index 39ecc59c0..dde5fb6b5 100644 --- a/src/wallet/wallet.h +++ b/src/wallet/wallet.h @@ -872,6 +872,7 @@ public: } bool SelectsTransparent() const; + bool SelectsTransparentCoinbase() const; bool SelectsSprout() const; bool SelectsSapling() const; bool SelectsOrchard() const;