zec
/
librustzcash
mirror of https://github.com/zcash/librustzcash.git
1
0
Fork 0
Code Issues Projects Releases Wiki Activity

Change the protobuf schema to explicitly specify whether a `ChangeValue` 

is ephemeral.

This also fixes `try_into_standard_proposal` to allow decoding from the
protobuf representation into a proposal that uses references to prior
ephemeral transparent outputs, provided that the "transparent-inputs"
feature is enabled.

Signed-off-by: Daira-Emma Hopwood <daira@jacaranda.org>
This commit is contained in:
Daira-Emma Hopwood 2024-06-22 19:20:36 +01:00
parent eb8846162c
commit c6520cf6a6
9 changed files with 154 additions and 89 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

7
zcash_client_backend/CHANGELOG.md
View File

@ -29,7 +29,7 @@ funds to those addresses. See [ZIP 320](https://zips.z.cash/zip-0320) for detail
- `chain::BlockCache` trait, behind the `sync` feature flag.
- `WalletRead::get_spendable_transparent_outputs`.
- `zcash_client_backend::fees`:
- `ChangeValue::{transparent, shielded}`
- `ChangeValue::{ephemeral_transparent, shielded}`
- `sapling::EmptyBundleView`
- `orchard::EmptyBundleView`
- `zcash_client_backend::proposal`:
@ -64,6 +64,9 @@ funds to those addresses. See [ZIP 320](https://zips.z.cash/zip-0320) for detail
- The return type of `ChangeValue::output_pool`, and the type of the
`output_pool` argument to `ChangeValue::new`, have changed from
`ShieldedProtocol` to `zcash_protocol::PoolType`.
- `ChangeValue::new` takes an additional `is_ephemeral` parameter indicating
whether the value is ephemeral (i.e. for use in a subsequent proposal step)
or change.
- The return type of `ChangeValue::new` is now optional; it returns `None`
if a memo is given for the transparent pool. Use `ChangeValue::shielded`
to avoid this error case when creating a `ChangeValue` known to be for a
@ -75,6 +78,8 @@ funds to those addresses. See [ZIP 320](https://zips.z.cash/zip-0320) for detail
Empty slices can be passed to obtain the previous behaviour.
- `zcash_client_backend::input_selection::GreedyInputSelectorError` has a
new variant `UnsupportedTexAddress`.
- `zcash_client_backend::proto::ProposalDecodingError` has a new variant
`InvalidEphemeralRecipient`.
- `zcash_client_backend::wallet::Recipient` variants have changed. Instead of
wrapping protocol-address types, the `External` and `InternalAccount` variants
now wrap a `zcash_address::ZcashAddress`. This simplifies the process of

29
zcash_client_backend/proto/proposal.proto
View File

@ -73,14 +73,14 @@ message ReceivedOutput {
uint64 value = 4;
}
// A reference a payment in a prior step of the proposal. This payment must
// A reference to a payment in a prior step of the proposal. This payment must
// belong to the wallet.
message PriorStepOutput {
uint32 stepIndex = 1;
uint32 paymentIndex = 2;
}
// A reference a change output from a prior step of the proposal.
// A reference to a change or ephemeral output from a prior step of the proposal.
message PriorStepChange {
uint32 stepIndex = 1;
uint32 changeIndex = 2;
@ -112,26 +112,29 @@ enum FeeRule {
// The proposed change outputs and fee value.
message TransactionBalance {
// A list of change output values.
//
// Each `ChangeValue` for the transparent value pool must be consumed by
// a subsequent step. These represent ephemeral outputs that will each be
// given a unique t-address.
// A list of change or ephemeral output values.
repeated ChangeValue proposedChange = 1;
// The fee to be paid by the proposed transaction, in zatoshis.
uint64 feeRequired = 2;
}
// A proposed change output. If the transparent value pool is selected,
// the `memo` field must be null.
// A proposed change or ephemeral output. If the transparent value pool is
// selected, the `memo` field must be null.
//
// When the `isEphemeral` field of a `ChangeValue` is set, it represents
// an ephemeral output, which must be spent by a subsequent step. This is
// only supported for transparent outputs. Each ephemeral output will be
// given a unique t-address.
message ChangeValue {
// The value of a change output to be created, in zatoshis.
// The value of a change or ephemeral output to be created, in zatoshis.
uint64 value = 1;
// The value pool in which the change output should be created.
// The value pool in which the change or ephemeral output should be created.
ValuePool valuePool = 2;
// The optional memo that should be associated with the newly created change output.
// Memos must not be present for transparent change outputs.
// The optional memo that should be associated with the newly created output.
// Memos must not be present for transparent outputs.
MemoBytes memo = 3;
// Whether this is to be an ephemeral output.
bool isEphemeral = 4;
}
// An object wrapper for memo bytes, to facilitate representing the

51
zcash_client_backend/src/data_api/wallet.rs
View File

@ -52,7 +52,7 @@ use crate::{
decrypt_transaction,
fees::{self, DustOutputPolicy},
keys::UnifiedSpendingKey,
proposal::{Proposal, ProposalError, Step, StepOutputIndex},
proposal::{Proposal, Step, StepOutputIndex},
wallet::{Note, OvkPolicy, Recipient},
zip321::{self, Payment},
PoolType, ShieldedProtocol,
@ -74,7 +74,11 @@ use zip32::Scope;
#[cfg(feature = "transparent-inputs")]
use {
crate::{fees::ChangeValue, proposal::StepOutput, wallet::TransparentAddressMetadata},
crate::{
fees::ChangeValue,
proposal::{ProposalError, StepOutput},
wallet::TransparentAddressMetadata,
},
input_selection::ShieldingSelector,
std::collections::HashMap,
zcash_keys::encoding::AddressCodec,
@ -624,8 +628,10 @@ where
step_results.push((step, step_result));
}
// Change step outputs represent ephemeral outputs that must be referenced exactly once.
// (We don't support transparent change.)
// Ephemeral outputs must be referenced exactly once. Currently this is all
// transparent outputs using `StepOutputIndex::Change`.
// TODO: if we support transparent change, this will need to be updated to
// not require it to be referenced by a later step.
#[cfg(feature = "transparent-inputs")]
if unused_transparent_outputs
.into_keys()
@ -670,33 +676,40 @@ where
ParamsT: consensus::Parameters + Clone,
FeeRuleT: FeeRule,
{
#[cfg(feature = "transparent-inputs")]
#[allow(unused_variables)]
let step_index = prior_step_results.len();
// Spending shielded outputs of prior multi-step transaction steps (either payments or change)
// is not supported.
// We only support spending transparent payments or ephemeral outputs from a prior step.
//
// TODO: Maybe support this at some point? Doing so would require a higher-level approach in
// the wallet that waits for transactions with shielded outputs to be mined and only then
// attempts to perform the next step.
// TODO: Maybe support spending prior shielded outputs at some point? Doing so would require
// a higher-level approach in the wallet that waits for transactions with shielded outputs to
// be mined and only then attempts to perform the next step.
#[cfg(feature = "transparent-inputs")]
for input_ref in proposal_step.prior_step_inputs() {
let prior_pool = prior_step_results
let supported = prior_step_results
.get(input_ref.step_index())
.and_then(|(prior_step, _)| match input_ref.output_index() {
StepOutputIndex::Payment(i) => prior_step.payment_pools().get(&i).cloned(),
StepOutputIndex::Change(i) => prior_step
.balance()
.proposed_change()
.get(i)
.map(|change| change.output_pool()),
StepOutputIndex::Payment(i) => prior_step
.payment_pools()
.get(&i)
.map(|&pool| pool == PoolType::TRANSPARENT),
StepOutputIndex::Change(i) => {
prior_step.balance().proposed_change().get(i).map(|change| {
change.is_ephemeral() && change.output_pool() == PoolType::TRANSPARENT
})
}
})
.ok_or(Error::Proposal(ProposalError::ReferenceError(*input_ref)))?;
// Return an error on trying to spend a prior shielded output.
if matches!(prior_pool, PoolType::Shielded(_)) {
// Return an error on trying to spend a prior shielded output or non-ephemeral change output.
if !supported {
return Err(Error::ProposalNotSupported);
}
}
#[cfg(not(feature = "transparent-inputs"))]
if !proposal_step.prior_step_inputs().is_empty() {
return Err(Error::ProposalNotSupported);
}
let account_id = wallet_db
.get_account_for_ufvk(&usk.to_unified_full_viewing_key())

2
zcash_client_backend/src/data_api/wallet/input_selection.rs
View File

@ -602,7 +602,7 @@ where
// The ephemeral output should always be at the last change index.
assert_eq!(
*balance.proposed_change().last().expect("nonempty"),
ChangeValue::transparent(ephemeral_output_amounts[0])
ChangeValue::ephemeral_transparent(ephemeral_output_amounts[0])
);
let ephemeral_stepoutput = StepOutput::new(
0,

36
zcash_client_backend/src/fees.rs
View File

@ -27,28 +27,44 @@ pub struct ChangeValue {
output_pool: PoolType,
value: NonNegativeAmount,
memo: Option<MemoBytes>,
is_ephemeral: bool,
}
impl ChangeValue {
/// Constructs a new change value from its constituent parts.
/// Constructs a new change or ephemeral value from its constituent parts.
///
/// Currently, the only supported combinations are change sent to a shielded
/// pool, or (if the "transparent-inputs" feature is enabled) an ephemeral
/// output to the transparent pool.
pub fn new(
output_pool: PoolType,
value: NonNegativeAmount,
memo: Option<MemoBytes>,
is_ephemeral: bool,
) -> Option<Self> {
(matches!(output_pool, PoolType::Shielded(_)) || memo.is_none()).then_some(Self {
match output_pool {
PoolType::Shielded(_) => !is_ephemeral,
#[cfg(feature = "transparent-inputs")]
PoolType::Transparent => is_ephemeral && memo.is_none(),
#[cfg(not(feature = "transparent-inputs"))]
PoolType::Transparent => false,
}
.then_some(Self {
output_pool,
value,
memo,
is_ephemeral,
})
}
/// Constructs a new change value that will be created as a transparent output.
pub fn transparent(value: NonNegativeAmount) -> Self {
/// Constructs a new ephemeral transparent output value.
#[cfg(feature = "transparent-inputs")]
pub fn ephemeral_transparent(value: NonNegativeAmount) -> Self {
Self {
output_pool: PoolType::TRANSPARENT,
value,
memo: None,
is_ephemeral: true,
}
}
@ -62,6 +78,7 @@ impl ChangeValue {
output_pool: PoolType::Shielded(protocol),
value,
memo,
is_ephemeral: false,
}
}
@ -76,20 +93,25 @@ impl ChangeValue {
Self::shielded(ShieldedProtocol::Orchard, value, memo)
}
/// Returns the pool to which the change output should be sent.
/// Returns the pool to which the change or ephemeral output should be sent.
pub fn output_pool(&self) -> PoolType {
self.output_pool
}
/// Returns the value of the change output to be created, in zatoshis.
/// Returns the value of the change or ephemeral output to be created, in zatoshis.
pub fn value(&self) -> NonNegativeAmount {
self.value
}
/// Returns the memo to be associated with the change output.
/// Returns the memo to be associated with the output.
pub fn memo(&self) -> Option<&MemoBytes> {
self.memo.as_ref()
}
/// Whether this is to be an ephemeral output.
pub fn is_ephemeral(&self) -> bool {
self.is_ephemeral
}
}
/// The amount of change and fees required to make a transaction's inputs and

2
zcash_client_backend/src/fees/common.rs
View File

@ -400,7 +400,7 @@ where
change.extend(
ephemeral_output_amounts
.iter()
.map(|&amount| ChangeValue::transparent(amount)),
.map(|&amount| ChangeValue::ephemeral_transparent(amount)),
);
TransactionBalance::new(change, fee).map_err(|_| overflow())

31
zcash_client_backend/src/proto.rs
View File

@ -335,7 +335,7 @@ pub const PROPOSAL_SER_V1: u32 = 1;
/// representation.
#[derive(Debug, Clone)]
pub enum ProposalDecodingError<DbError> {
/// The encoded proposal contained no steps
/// The encoded proposal contained no steps.
NoSteps,
/// The ZIP 321 transaction request URI was invalid.
Zip321(Zip321Error),
@ -366,6 +366,8 @@ pub enum ProposalDecodingError<DbError> {
TransparentMemo,
/// Change outputs to the specified pool are not supported.
InvalidChangeRecipient(PoolType),
/// Ephemeral outputs to the specified pool are not supported.
InvalidEphemeralRecipient(PoolType),
}
impl<E> From<Zip321Error> for ProposalDecodingError<E> {
@ -424,6 +426,11 @@ impl<E: Display> Display for ProposalDecodingError<E> {
"Change outputs to the {} pool are not supported.",
pool_type
),
ProposalDecodingError::InvalidEphemeralRecipient(pool_type) => write!(
f,
"Ephemeral outputs to the {} pool are not supported.",
pool_type
),
}
}
}
@ -572,6 +579,7 @@ impl proposal::Proposal {
memo: change.memo().map(|memo_bytes| proposal::MemoBytes {
value: memo_bytes.as_slice().to_vec(),
}),
is_ephemeral: change.is_ephemeral(),
})
.collect(),
fee_required: step.balance().fee_required().into(),
@ -662,7 +670,7 @@ impl proposal::Proposal {
PoolType::Transparent => {
#[cfg(not(feature = "transparent-inputs"))]
return Err(ProposalDecodingError::ValuePoolNotSupported(
1,
out.value_pool,
));
#[cfg(feature = "transparent-inputs")]
@ -751,18 +759,27 @@ impl proposal::Proposal {
.map_err(ProposalDecodingError::MemoInvalid)
})
.transpose()?;
match cv.pool_type()? {
PoolType::Shielded(ShieldedProtocol::Sapling) => {
match (cv.pool_type()?, cv.is_ephemeral) {
(PoolType::Shielded(ShieldedProtocol::Sapling), false) => {
Ok(ChangeValue::sapling(value, memo))
}
#[cfg(feature = "orchard")]
PoolType::Shielded(ShieldedProtocol::Orchard) => {
(PoolType::Shielded(ShieldedProtocol::Orchard), false) => {
Ok(ChangeValue::orchard(value, memo))
}
PoolType::Transparent if memo.is_some() => {
(PoolType::Transparent, _) if memo.is_some() => {
Err(ProposalDecodingError::TransparentMemo)
}
t => Err(ProposalDecodingError::InvalidChangeRecipient(t)),
#[cfg(feature = "transparent-inputs")]
(PoolType::Transparent, true) => {
Ok(ChangeValue::ephemeral_transparent(value))
}
(pool, false) => {
Err(ProposalDecodingError::InvalidChangeRecipient(pool))
}
(pool, true) => {
Err(ProposalDecodingError::InvalidEphemeralRecipient(pool))
}
}
})
.collect::<Result<Vec<_>, _>>()?,

30
zcash_client_backend/src/proto/proposal.rs generated
View File

@ -72,7 +72,7 @@ pub struct ReceivedOutput {
#[prost(uint64, tag = "4")]
pub value: u64,
}
/// A reference a payment in a prior step of the proposal. This payment must
/// A reference to a payment in a prior step of the proposal. This payment must
/// belong to the wallet.
#[allow(clippy::derive_partial_eq_without_eq)]
#[derive(Clone, PartialEq, ::prost::Message)]
@ -82,7 +82,7 @@ pub struct PriorStepOutput {
#[prost(uint32, tag = "2")]
pub payment_index: u32,
}
/// A reference a change output from a prior step of the proposal.
/// A reference to a change or ephemeral output from a prior step of the proposal.
#[allow(clippy::derive_partial_eq_without_eq)]
#[derive(Clone, PartialEq, ::prost::Message)]
pub struct PriorStepChange {
@ -115,32 +115,36 @@ pub mod proposed_input {
#[allow(clippy::derive_partial_eq_without_eq)]
#[derive(Clone, PartialEq, ::prost::Message)]
pub struct TransactionBalance {
/// A list of change output values.
///
/// Each `ChangeValue` for the transparent value pool must be consumed by
/// a subsequent step. These represent ephemeral outputs that will each be
/// given a unique t-address.
/// A list of change or ephemeral output values.
#[prost(message, repeated, tag = "1")]
pub proposed_change: ::prost::alloc::vec::Vec<ChangeValue>,
/// The fee to be paid by the proposed transaction, in zatoshis.
#[prost(uint64, tag = "2")]
pub fee_required: u64,
}
/// A proposed change output. If the transparent value pool is selected,
/// the `memo` field must be null.
/// A proposed change or ephemeral output. If the transparent value pool is
/// selected, the `memo` field must be null.
///
/// When the `isEphemeral` field of a `ChangeValue` is set, it represents
/// an ephemeral output, which must be spent by a subsequent step. This is
/// only supported for transparent outputs. Each ephemeral output will be
/// given a unique t-address.
#[allow(clippy::derive_partial_eq_without_eq)]
#[derive(Clone, PartialEq, ::prost::Message)]
pub struct ChangeValue {
/// The value of a change output to be created, in zatoshis.
/// The value of a change or ephemeral output to be created, in zatoshis.
#[prost(uint64, tag = "1")]
pub value: u64,
/// The value pool in which the change output should be created.
/// The value pool in which the change or ephemeral output should be created.
#[prost(enumeration = "ValuePool", tag = "2")]
pub value_pool: i32,
/// The optional memo that should be associated with the newly created change output.
/// Memos must not be present for transparent change outputs.
/// The optional memo that should be associated with the newly created output.
/// Memos must not be present for transparent outputs.
#[prost(message, optional, tag = "3")]
pub memo: ::core::option::Option<MemoBytes>,
/// Whether this is to be an ephemeral output.
#[prost(bool, tag = "4")]
pub is_ephemeral: bool,
}
/// An object wrapper for memo bytes, to facilitate representing the
/// `change_memo == None` case.

55
zcash_client_sqlite/src/testing/pool.rs
View File

@ -302,7 +302,6 @@ pub(crate) fn send_single_step_proposed_transfer<T: ShieldedPoolTester>() {
#[cfg(feature = "transparent-inputs")]
pub(crate) fn send_multi_step_proposed_transfer<T: ShieldedPoolTester>() {
use zcash_address::ZcashAddress;
use zcash_client_backend::fees::ChangeValue;
let mut st = TestBuilder::new()
@ -313,12 +312,6 @@ pub(crate) fn send_multi_step_proposed_transfer<T: ShieldedPoolTester>() {
let account = st.test_account().cloned().unwrap();
let dfvk = T::test_account_fvk(&st);
let fee_rule = StandardFeeRule::Zip317;
let input_selector = GreedyInputSelector::new(
standard::SingleOutputChangeStrategy::new(fee_rule, None, T::SHIELDED_PROTOCOL),
DustOutputPolicy::default(),
);
let add_funds = |st: &mut TestState<_>, value| {
let (h, _, _) = st.generate_next_block(&dfvk, AddressType::DefaultExternal, value);
st.scan_cached_blocks(h, 1);
@ -353,34 +346,37 @@ pub(crate) fn send_multi_step_proposed_transfer<T: ShieldedPoolTester>() {
TransparentAddress::PublicKeyHash(data) => Address::Tex(data),
_ => unreachable!(),
};
let request = zip321::TransactionRequest::new(vec![Payment::without_memo(
tex_addr.to_zcash_address(&st.network()),
amount,
)])
.unwrap();
// As of this commit, change memos are not correctly handled in ZIP 320 proposals.
let change_memo = None;
// We use `st.propose_standard_transfer` here in order to also test round-trip
// serialization of the proposal.
let proposal = st
.propose_transfer(
.propose_standard_transfer::<Infallible>(
account.account_id(),
&input_selector,
request,
StandardFeeRule::Zip317,
NonZeroU32::new(1).unwrap(),
&tex_addr,
amount,
None,
change_memo.clone(),
T::SHIELDED_PROTOCOL,
)
.unwrap();
let steps: Vec<_> = proposal.steps().iter().cloned().collect();
assert_eq!(steps.len(), 2);
assert_eq!(steps[0].balance().fee_required(), expected_step0_fee);
assert_eq!(steps[1].balance().fee_required(), expected_step1_fee);
assert_eq!(
steps[0].balance().proposed_change(),
[
ChangeValue::shielded(T::SHIELDED_PROTOCOL, expected_step0_change, None),
ChangeValue::transparent((amount + expected_step1_fee).unwrap()),
ChangeValue::shielded(T::SHIELDED_PROTOCOL, expected_step0_change, change_memo),
ChangeValue::ephemeral_transparent((amount + expected_step1_fee).unwrap()),
]
);
assert_eq!(steps[0].balance().fee_required(), expected_step0_fee);
assert_eq!(steps[1].balance().proposed_change(), []);
assert_eq!(steps[1].balance().fee_required(), expected_step1_fee);
let create_proposed_result = st.create_proposed_transactions::<Infallible, _>(
account.usk(),
@ -451,19 +447,24 @@ pub(crate) fn send_multi_step_proposed_transfer<T: ShieldedPoolTester>() {
assert!(ephemeral0 != ephemeral1);
add_funds(&mut st, value);
let request = zip321::TransactionRequest::new(vec![Payment::without_memo(
ZcashAddress::try_from_encoded(&ephemeral0).expect("valid address"),
amount,
)])
.unwrap();
let ephemeral_taddr = Address::decode(&st.wallet().params, &ephemeral0).expect("valid address");
assert_matches!(
ephemeral_taddr,
Address::Transparent(TransparentAddress::PublicKeyHash(_))
);
// Attempting to use the same address again should cause an error.
let proposal = st
.propose_transfer(
.propose_standard_transfer::<Infallible>(
account.account_id(),
&input_selector,
request,
StandardFeeRule::Zip317,
NonZeroU32::new(1).unwrap(),
&ephemeral_taddr,
amount,
None,
None,
T::SHIELDED_PROTOCOL,
)
.unwrap();