zip321: Make `Payment` fields private.

This commit is contained in:
Kris Nuttycombe 2024-01-27 21:13:32 -07:00
parent 3ea7d84183
commit 86e1181259
6 changed files with 148 additions and 152 deletions

View File

@ -10,10 +10,14 @@ The entries below are relative to the `zcash_client_backend` crate as of
`zcash_client_backend-0.10.0`.
### Added
- `zip321::Payment::new`
- `impl From<zcash_address:ConversionError<E>> for Zip321Error`
### Changed
- `zip321::Payment::recipient_address` has type `zcash_address::ZcashAddress`
- Fields of `zip321::Payment` are now private. Accessors have been provided for
the fields that are no longer public, and `Payment::new` has been added to
serve the needs of payment construction.
- `zip321::Payment::recipient_address()` returns `zcash_address::ZcashAddress`
- `zip321::Payment::without_memo` now takes a `zcash_address::ZcashAddress` for
its `recipient_address` argument.
- Uses of `zcash_primitives::transaction::components::amount::NonNegartiveAmount`

View File

@ -116,27 +116,41 @@ pub fn memo_from_base64(s: &str) -> Result<MemoBytes, Zip321Error> {
/// A single payment being requested.
#[derive(Debug, Clone, PartialEq, Eq)]
pub struct Payment {
/// The payment address to which the payment should be sent.
pub recipient_address: ZcashAddress,
/// The value of the payment that is being requested, in zatoshis.
pub amount: Zatoshis,
/// A memo that, if included, must be provided with the payment.
/// If a memo is present and [`recipient_address`] is not a shielded
/// address, the wallet should report an error.
///
/// [`recipient_address`]: #structfield.recipient_address
pub memo: Option<MemoBytes>,
/// A human-readable label for this payment within the larger structure
/// of the transaction request.
pub label: Option<String>,
/// A human-readable message to be displayed to the user describing the
/// purpose of this payment.
pub message: Option<String>,
/// A list of other arbitrary key/value pairs associated with this payment.
pub other_params: Vec<(String, String)>,
recipient_address: ZcashAddress,
amount: Zatoshis,
memo: Option<MemoBytes>,
label: Option<String>,
message: Option<String>,
other_params: Vec<(String, String)>,
}
impl Payment {
/// Constructs a new [`Payment`] from its constituent parts.
///
/// Returns `None` if the payment requests that a memo be sent to a recipient that cannot
/// receive a memo.
pub fn new(
recipient_address: ZcashAddress,
amount: Zatoshis,
memo: Option<MemoBytes>,
label: Option<String>,
message: Option<String>,
other_params: Vec<(String, String)>,
) -> Option<Self> {
if memo.is_none() || recipient_address.can_receive_memo() {
Some(Self {
recipient_address,
amount,
memo,
label,
message,
other_params,
})
} else {
None
}
}
/// Constructs a new [`Payment`] paying the given address the specified amount.
pub fn without_memo(recipient_address: ZcashAddress, amount: Zatoshis) -> Self {
Self {
@ -149,6 +163,38 @@ impl Payment {
}
}
/// Returns the payment address to which the payment should be sent.
pub fn recipient_address(&self) -> &ZcashAddress {
&self.recipient_address
}
/// Returns the value of the payment that is being requested, in zatoshis.
pub fn amount(&self) -> Zatoshis {
self.amount
}
/// Returns the memo that, if included, must be provided with the payment.
pub fn memo(&self) -> Option<&MemoBytes> {
self.memo.as_ref()
}
/// A human-readable label for this payment within the larger structure
/// of the transaction request.
pub fn label(&self) -> Option<&String> {
self.label.as_ref()
}
/// A human-readable message to be displayed to the user describing the
/// purpose of this payment.
pub fn message(&self) -> Option<&String> {
self.message.as_ref()
}
/// A list of other arbitrary key/value pairs associated with this payment.
pub fn other_params(&self) -> &[(String, String)] {
self.other_params.as_ref()
}
/// A utility for use in tests to help check round-trip serialization properties.
#[cfg(any(test, feature = "test-dependencies"))]
pub(crate) fn normalize(&mut self) {

View File

@ -497,14 +497,15 @@ where
>,
DbT::NoteRef: Copy + Eq + Ord,
{
let request = zip321::TransactionRequest::new(vec![Payment {
recipient_address: to.to_zcash_address(params),
let request = zip321::TransactionRequest::new(vec![Payment::new(
to.to_zcash_address(params),
amount,
memo,
label: None,
message: None,
other_params: vec![],
}])
None,
None,
vec![],
)
.ok_or(Error::MemoForbidden)?])
.expect(
"It should not be possible for this to violate ZIP 321 request construction invariants.",
);
@ -853,7 +854,7 @@ where
.payments()
.get(&i)
.expect("Payment step references are checked at construction")
.recipient_address
.recipient_address()
.clone()
.convert_if_network(params.network_type())?;
@ -956,16 +957,14 @@ where
(payment, output_pool)
})
{
let recipient_address = payment
.recipient_address
let recipient_address: Address = payment
.recipient_address()
.clone()
.convert_if_network::<Address>(params.network_type())?;
.convert_if_network(params.network_type())?;
match recipient_address {
Address::Unified(ua) => {
let memo = payment
.memo
.as_ref()
.map_or_else(MemoBytes::empty, |m| m.clone());
let memo = payment.memo().map_or_else(MemoBytes::empty, |m| m.clone());
match output_pool {
#[cfg(not(feature = "orchard"))]
@ -977,7 +976,7 @@ where
builder.add_orchard_output(
orchard_external_ovk.clone(),
*ua.orchard().expect("The mapping between payment pool and receiver is checked in step construction"),
payment.amount.into(),
payment.amount().into(),
memo.clone(),
)?;
orchard_output_meta.push((
@ -985,7 +984,7 @@ where
ua.clone(),
PoolType::Shielded(ShieldedProtocol::Orchard),
),
payment.amount,
payment.amount(),
Some(memo),
));
}
@ -994,7 +993,7 @@ where
builder.add_sapling_output(
sapling_external_ovk,
*ua.sapling().expect("The mapping between payment pool and receiver is checked in step construction"),
payment.amount,
payment.amount(),
memo.clone(),
)?;
sapling_output_meta.push((
@ -1002,43 +1001,40 @@ where
ua.clone(),
PoolType::Shielded(ShieldedProtocol::Sapling),
),
payment.amount,
payment.amount(),
Some(memo),
));
}
PoolType::Transparent => {
if payment.memo.is_some() {
if payment.memo().is_some() {
return Err(Error::MemoForbidden);
} else {
builder.add_transparent_output(
ua.transparent().expect("The mapping between payment pool and receiver is checked in step construction."),
payment.amount
payment.amount()
)?;
}
}
}
}
Address::Sapling(addr) => {
let memo = payment
.memo
.as_ref()
.map_or_else(MemoBytes::empty, |m| m.clone());
let memo = payment.memo().map_or_else(MemoBytes::empty, |m| m.clone());
builder.add_sapling_output(
sapling_external_ovk,
addr,
payment.amount,
payment.amount(),
memo.clone(),
)?;
sapling_output_meta.push((Recipient::Sapling(addr), payment.amount, Some(memo)));
sapling_output_meta.push((Recipient::Sapling(addr), payment.amount(), Some(memo)));
}
Address::Transparent(to) => {
if payment.memo.is_some() {
if payment.memo().is_some() {
return Err(Error::MemoForbidden);
} else {
builder.add_transparent_output(&to, payment.amount)?;
builder.add_transparent_output(&to, payment.amount())?;
}
transparent_output_meta.push((to, payment.amount));
transparent_output_meta.push((to, payment.amount()));
}
}
}

View File

@ -360,40 +360,41 @@ where
let mut orchard_outputs = vec![];
let mut payment_pools = BTreeMap::new();
for (idx, payment) in transaction_request.payments() {
let recipient_address = payment
.recipient_address
let recipient_address: Address = payment
.recipient_address()
.clone()
.convert_if_network::<Address>(params.network_type())?;
.convert_if_network(params.network_type())?;
match recipient_address {
Address::Transparent(addr) => {
payment_pools.insert(*idx, PoolType::Transparent);
transparent_outputs.push(TxOut {
value: payment.amount,
value: payment.amount(),
script_pubkey: addr.script(),
});
}
Address::Sapling(_) => {
payment_pools.insert(*idx, PoolType::Shielded(ShieldedProtocol::Sapling));
sapling_outputs.push(SaplingPayment(payment.amount));
sapling_outputs.push(SaplingPayment(payment.amount()));
}
Address::Unified(addr) => {
#[cfg(feature = "orchard")]
if addr.orchard().is_some() {
payment_pools.insert(*idx, PoolType::Shielded(ShieldedProtocol::Orchard));
orchard_outputs.push(OrchardPayment(payment.amount));
orchard_outputs.push(OrchardPayment(payment.amount()));
continue;
}
if addr.sapling().is_some() {
payment_pools.insert(*idx, PoolType::Shielded(ShieldedProtocol::Sapling));
sapling_outputs.push(SaplingPayment(payment.amount));
sapling_outputs.push(SaplingPayment(payment.amount()));
continue;
}
if let Some(addr) = addr.transparent() {
payment_pools.insert(*idx, PoolType::Transparent);
transparent_outputs.push(TxOut {
value: payment.amount,
value: payment.amount(),
script_pubkey: addr.script(),
});
continue;

View File

@ -377,7 +377,7 @@ impl<NoteRef> Step<NoteRef> {
.payments()
.get(idx)
.iter()
.any(|payment| payment.recipient_address.can_receive_as(*pool))
.any(|payment| payment.recipient_address().can_receive_as(*pool))
{
return Err(ProposalError::PaymentPoolsMismatch);
}
@ -404,13 +404,12 @@ impl<NoteRef> Step<NoteRef> {
.get(s_ref.step_index)
.ok_or(ProposalError::ReferenceError(*s_ref))?;
Ok(match s_ref.output_index {
StepOutputIndex::Payment(i) => {
step.transaction_request
.payments()
.get(&i)
.ok_or(ProposalError::ReferenceError(*s_ref))?
.amount
}
StepOutputIndex::Payment(i) => step
.transaction_request
.payments()
.get(&i)
.ok_or(ProposalError::ReferenceError(*s_ref))?
.amount(),
StepOutputIndex::Change(i) => step
.balance
.proposed_change()

View File

@ -168,14 +168,10 @@ pub(crate) fn send_single_step_proposed_transfer<T: ShieldedPoolTester>() {
let to_extsk = T::sk(&[0xf5; 32]);
let to: Address = T::sk_default_address(&to_extsk);
let request = zip321::TransactionRequest::new(vec![Payment {
recipient_address: to.to_zcash_address(&st.network()),
amount: NonNegativeAmount::const_from_u64(10000),
memo: None, // this should result in the creation of an empty memo
label: None,
message: None,
other_params: vec![],
}])
let request = zip321::TransactionRequest::new(vec![Payment::without_memo(
to.to_zcash_address(&st.network()),
NonNegativeAmount::const_from_u64(10000),
)])
.unwrap();
// TODO: This test was originally written to use the pre-zip-313 fee rule
@ -337,14 +333,10 @@ pub(crate) fn send_multi_step_proposed_transfer<T: ShieldedPoolTester>() {
// The first step will deshield to the wallet's default transparent address
let to0 = Address::Transparent(account.usk().default_transparent_address().0);
let request0 = zip321::TransactionRequest::new(vec![Payment {
recipient_address: to0.to_zcash_address(&st.network()),
amount: NonNegativeAmount::const_from_u64(50000),
memo: None,
label: None,
message: None,
other_params: vec![],
}])
let request0 = zip321::TransactionRequest::new(vec![Payment::without_memo(
to0.to_zcash_address(&st.network()),
NonNegativeAmount::const_from_u64(50000),
)])
.unwrap();
let fee_rule = StandardFeeRule::Zip317;
@ -382,14 +374,10 @@ pub(crate) fn send_multi_step_proposed_transfer<T: ShieldedPoolTester>() {
.default_address()
.0,
);
let request1 = zip321::TransactionRequest::new(vec![Payment {
recipient_address: to1.to_zcash_address(&st.network()),
amount: NonNegativeAmount::const_from_u64(40000),
memo: None,
label: None,
message: None,
other_params: vec![],
}])
let request1 = zip321::TransactionRequest::new(vec![Payment::without_memo(
to1.to_zcash_address(&st.network()),
NonNegativeAmount::const_from_u64(40000),
)])
.unwrap();
let step1 = Step::from_parts(
@ -1042,23 +1030,9 @@ pub(crate) fn external_address_change_spends_detected_in_restore_from_seed<
let addr2 = T::fvk_default_address(&dfvk2);
let req = TransactionRequest::new(vec![
// payment to an external recipient
Payment {
recipient_address: addr2.to_zcash_address(&st.network()),
amount: amount_sent,
memo: None,
label: None,
message: None,
other_params: vec![],
},
Payment::without_memo(addr2.to_zcash_address(&st.network()), amount_sent),
// payment back to the originating wallet, simulating legacy change
Payment {
recipient_address: addr.to_zcash_address(&st.network()),
amount: amount_legacy_change,
memo: None,
label: None,
message: None,
other_params: vec![],
},
Payment::without_memo(addr.to_zcash_address(&st.network()), amount_legacy_change),
])
.unwrap();
@ -1151,14 +1125,10 @@ pub(crate) fn zip317_spend<T: ShieldedPoolTester>() {
let input_selector = input_selector(StandardFeeRule::Zip317, None, T::SHIELDED_PROTOCOL);
// This first request will fail due to insufficient non-dust funds
let req = TransactionRequest::new(vec![Payment {
recipient_address: T::fvk_default_address(&dfvk).to_zcash_address(&st.network()),
amount: NonNegativeAmount::const_from_u64(50000),
memo: None,
label: None,
message: None,
other_params: vec![],
}])
let req = TransactionRequest::new(vec![Payment::without_memo(
T::fvk_default_address(&dfvk).to_zcash_address(&st.network()),
NonNegativeAmount::const_from_u64(50000),
)])
.unwrap();
assert_matches!(
@ -1176,14 +1146,10 @@ pub(crate) fn zip317_spend<T: ShieldedPoolTester>() {
// This request will succeed, spending a single dust input to pay the 10000
// ZAT fee in addition to the 41000 ZAT output to the recipient
let req = TransactionRequest::new(vec![Payment {
recipient_address: T::fvk_default_address(&dfvk).to_zcash_address(&st.network()),
amount: NonNegativeAmount::const_from_u64(41000),
memo: None,
label: None,
message: None,
other_params: vec![],
}])
let req = TransactionRequest::new(vec![Payment::without_memo(
T::fvk_default_address(&dfvk).to_zcash_address(&st.network()),
NonNegativeAmount::const_from_u64(41000),
)])
.unwrap();
let txid = st
@ -1479,14 +1445,10 @@ pub(crate) fn pool_crossing_required<P0: ShieldedPoolTester, P1: ShieldedPoolTes
);
let transfer_amount = NonNegativeAmount::const_from_u64(200000);
let p0_to_p1 = zip321::TransactionRequest::new(vec![Payment {
recipient_address: p1_to.to_zcash_address(&st.network()),
amount: transfer_amount,
memo: None,
label: None,
message: None,
other_params: vec![],
}])
let p0_to_p1 = zip321::TransactionRequest::new(vec![Payment::without_memo(
p1_to.to_zcash_address(&st.network()),
transfer_amount,
)])
.unwrap();
let fee_rule = StandardFeeRule::Zip317;
@ -1570,14 +1532,10 @@ pub(crate) fn fully_funded_fully_private<P0: ShieldedPoolTester, P1: ShieldedPoo
);
let transfer_amount = NonNegativeAmount::const_from_u64(200000);
let p0_to_p1 = zip321::TransactionRequest::new(vec![Payment {
recipient_address: p1_to.to_zcash_address(&st.network()),
amount: transfer_amount,
memo: None,
label: None,
message: None,
other_params: vec![],
}])
let p0_to_p1 = zip321::TransactionRequest::new(vec![Payment::without_memo(
p1_to.to_zcash_address(&st.network()),
transfer_amount,
)])
.unwrap();
let fee_rule = StandardFeeRule::Zip317;
@ -1661,14 +1619,10 @@ pub(crate) fn fully_funded_send_to_t<P0: ShieldedPoolTester, P1: ShieldedPoolTes
);
let transfer_amount = NonNegativeAmount::const_from_u64(200000);
let p0_to_p1 = zip321::TransactionRequest::new(vec![Payment {
recipient_address: Address::Transparent(p1_to).to_zcash_address(&st.network()),
amount: transfer_amount,
memo: None,
label: None,
message: None,
other_params: vec![],
}])
let p0_to_p1 = zip321::TransactionRequest::new(vec![Payment::without_memo(
Address::Transparent(p1_to).to_zcash_address(&st.network()),
transfer_amount,
)])
.unwrap();
let fee_rule = StandardFeeRule::Zip317;
@ -2115,14 +2069,10 @@ pub(crate) fn scan_cached_blocks_allows_blocks_out_of_order<T: ShieldedPoolTeste
);
// We can spend the received notes
let req = TransactionRequest::new(vec![Payment {
recipient_address: T::fvk_default_address(&dfvk).to_zcash_address(&st.network()),
amount: NonNegativeAmount::const_from_u64(110_000),
memo: None,
label: None,
message: None,
other_params: vec![],
}])
let req = TransactionRequest::new(vec![Payment::without_memo(
T::fvk_default_address(&dfvk).to_zcash_address(&st.network()),
NonNegativeAmount::const_from_u64(110_000),
)])
.unwrap();
#[allow(deprecated)]