Address WalletTxBuilder review feedback

This commit is contained in:
Greg Pfeil 2023-03-20 14:23:18 -06:00
parent 1f72b42b81
commit af2c7e7e49
No known key found for this signature in database
GPG Key ID: 1193ACD196ED61F2
3 changed files with 50 additions and 63 deletions

View File

@ -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)

View File

@ -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(

View File

@ -72,24 +72,44 @@ PrepareTransactionResult WalletTxBuilder::PrepareTransaction(
return result;
};
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(
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<ReceiverType>& 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<RecipientAddress> 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<RecipientAddress> 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());
}