diff --git a/qa/rpc-tests/wallet_z_sendmany.py b/qa/rpc-tests/wallet_z_sendmany.py index 6c710a8a3..07372d80a 100755 --- a/qa/rpc-tests/wallet_z_sendmany.py +++ b/qa/rpc-tests/wallet_z_sendmany.py @@ -372,7 +372,7 @@ class WalletZSendmanyTest(BitcoinTestFramework): 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 insufficent non-Sprout funds, or NU5 has not been activated yet.'), + ('AllowRevealedRecipients', 'Could not send to an Orchard-only receiver, despite a lax privacy policy. Either there are insufficient non-Sprout funds (there is no transaction version that supports both Sprout and Orchard), or NU5 has not been activated yet.'), ]: opid = self.nodes[1].z_sendmany(n1ua0, recipients, 1, 0, policy) wait_and_assert_operationid_status(self.nodes[1], opid, 'failed', msg) @@ -418,7 +418,7 @@ class WalletZSendmanyTest(BitcoinTestFramework): 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 insufficent non-Sprout funds, or NU5 has not been activated yet.'), + ('AllowRevealedRecipients', 'Could not send to an Orchard-only receiver, despite a lax privacy policy. Either there are insufficient non-Sprout funds (there is no transaction version that supports both Sprout and Orchard), or NU5 has not been activated yet.'), ]: opid = self.nodes[1].z_sendmany(n1ua0, recipients, 1, 0, policy) wait_and_assert_operationid_status(self.nodes[1], opid, 'failed', msg) diff --git a/src/wallet/asyncrpcoperation_common.cpp b/src/wallet/asyncrpcoperation_common.cpp index 21bd665b3..f20fb625a 100644 --- a/src/wallet/asyncrpcoperation_common.cpp +++ b/src/wallet/asyncrpcoperation_common.cpp @@ -74,7 +74,8 @@ void ThrowInputSelectionError( throw JSONRPCError( RPC_INVALID_PARAMETER, "Could not send to an Orchard-only receiver, despite a lax privacy policy. " - "Either there are insufficent non-Sprout funds, or NU5 has not been " + "Either there are insufficient non-Sprout funds (there is no transaction " + "version that supports both Sprout and Orchard), or NU5 has not been " "activated yet."); case AddressResolutionError::TransparentReceiverNotAllowed: throw JSONRPCError( diff --git a/src/wallet/wallet_tx_builder.cpp b/src/wallet/wallet_tx_builder.cpp index ffc7c8dac..d9856ba43 100644 --- a/src/wallet/wallet_tx_builder.cpp +++ b/src/wallet/wallet_tx_builder.cpp @@ -72,24 +72,44 @@ PrepareTransactionResult WalletTxBuilder::PrepareTransaction( return result; }; + auto addChangePayment = [&](const std::optional& sendTo) { + assert(sendTo.has_value()); + resolvedPayments.AddPayment( + ResolvedPayment(std::nullopt, sendTo.value(), changeAmount, std::nullopt, true)); + return sendTo.value(); + }; + + auto changeAddressForTransparentSelector = [&](const std::set& receiverTypes) { + return addChangePayment( + pwalletMain->GenerateChangeAddressForAccount( + sendFromAccount, + getAllowedChangePools(receiverTypes))); + }; + + auto changeAddressForSaplingAddress = [&](const libzcash::SaplingPaymentAddress& addr) { + // for Sapling, if using a legacy address, return change to the + // originating address; otherwise return it to the Sapling internal + // address corresponding to the UFVK. + return addChangePayment( + sendFromAccount == ZCASH_LEGACY_ACCOUNT + ? addr + : pwalletMain->GenerateChangeAddressForAccount( + sendFromAccount, + getAllowedChangePools({ReceiverType::Sapling}))); + }; + + auto changeAddressForZUFVK = [&]( + const ZcashdUnifiedFullViewingKey& zufvk, + const std::set& receiverTypes) { + return addChangePayment(zufvk.GetChangeAddress(getAllowedChangePools(receiverTypes))); + }; + changeAddr = std::visit(match { - [&](const CKeyID& keyId) -> ChangeAddress { - auto sendTo = pwalletMain->GenerateChangeAddressForAccount( - sendFromAccount, - getAllowedChangePools({ReceiverType::P2PKH})); - assert(sendTo.has_value()); - resolvedPayments.AddPayment( - ResolvedPayment(std::nullopt, sendTo.value(), changeAmount, std::nullopt, true)); - return sendTo.value(); + [&](const CKeyID&) -> ChangeAddress { + return changeAddressForTransparentSelector({ReceiverType::P2PKH}); }, - [&](const CScriptID& scriptId) -> ChangeAddress { - auto sendTo = pwalletMain->GenerateChangeAddressForAccount( - sendFromAccount, - getAllowedChangePools({ReceiverType::P2SH})); - assert(sendTo.has_value()); - resolvedPayments.AddPayment( - ResolvedPayment(std::nullopt, sendTo.value(), changeAmount, std::nullopt, true)); - return sendTo.value(); + [&](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. @@ -100,60 +120,26 @@ PrepareTransactionResult WalletTxBuilder::PrepareTransaction( return vk.address(); }, [&](const libzcash::SaplingPaymentAddress& addr) -> ChangeAddress { - // 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. - std::optional sendTo = sendFromAccount == ZCASH_LEGACY_ACCOUNT - ? addr - : pwalletMain->GenerateChangeAddressForAccount( - sendFromAccount, - getAllowedChangePools({ReceiverType::Sapling})); - assert(sendTo.has_value()); - resolvedPayments.AddPayment( - ResolvedPayment(std::nullopt, sendTo.value(), changeAmount, std::nullopt, true)); - return sendTo.value(); + return changeAddressForSaplingAddress(addr); }, [&](const libzcash::SaplingExtendedFullViewingKey& fvk) -> ChangeAddress { - // 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. - std::optional sendTo = sendFromAccount == ZCASH_LEGACY_ACCOUNT - ? fvk.DefaultAddress() - : pwalletMain->GenerateChangeAddressForAccount( - sendFromAccount, - getAllowedChangePools({ReceiverType::Sapling})); - assert(sendTo.has_value()); - resolvedPayments.AddPayment( - ResolvedPayment(std::nullopt, sendTo.value(), changeAmount, std::nullopt, true)); - return sendTo.value(); + return changeAddressForSaplingAddress(fvk.DefaultAddress()); }, [&](const libzcash::UnifiedAddress& ua) -> ChangeAddress { auto zufvk = pwalletMain->GetUFVKForAddress(ua); assert(zufvk.has_value()); - auto sendTo = zufvk.value().GetChangeAddress( - getAllowedChangePools(ua.GetKnownReceiverTypes())); - assert(sendTo.has_value()); - resolvedPayments.AddPayment( - ResolvedPayment(std::nullopt, sendTo.value(), changeAmount, std::nullopt, true)); - return sendTo.value(); + return changeAddressForZUFVK(zufvk.value(), ua.GetKnownReceiverTypes()); }, [&](const libzcash::UnifiedFullViewingKey& fvk) -> ChangeAddress { - auto zufvk = ZcashdUnifiedFullViewingKey::FromUnifiedFullViewingKey(params, fvk); - auto sendTo = zufvk.GetChangeAddress( - getAllowedChangePools(fvk.GetKnownReceiverTypes())); - assert(sendTo.has_value()); - resolvedPayments.AddPayment( - ResolvedPayment(std::nullopt, sendTo.value(), changeAmount, std::nullopt, true)); - return sendTo.value(); + return changeAddressForZUFVK( + ZcashdUnifiedFullViewingKey::FromUnifiedFullViewingKey(params, fvk), + fvk.GetKnownReceiverTypes()); }, [&](const AccountZTXOPattern& acct) -> ChangeAddress { - auto sendTo = pwalletMain->GenerateChangeAddressForAccount( - acct.GetAccountId(), - getAllowedChangePools(acct.GetReceiverTypes())); - assert(sendTo.has_value()); - resolvedPayments.AddPayment( - ResolvedPayment(std::nullopt, sendTo.value(), changeAmount, std::nullopt, true)); - return sendTo.value(); + return addChangePayment( + pwalletMain->GenerateChangeAddressForAccount( + acct.GetAccountId(), + getAllowedChangePools(acct.GetReceiverTypes()))); } }, selector.GetPattern()); }