Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for transparent-source-only (TEX) addresses #1257

Merged
merged 56 commits into from
Jul 17, 2024
Merged
Changes from 1 commit
Commits
Show all changes
56 commits
Select commit Hold shift + click to select a range
2fae4bb
ZIP 320 implementation.
daira Jun 22, 2024
0f3de63
Apply documentation suggestions from code review.
daira Jun 22, 2024
549fe0b
In `reserve_next_n_ephemeral_addresses`, exclude addresses observed in
daira Jun 22, 2024
eb88461
Address review comment: `EphemeralIvk` should not implement `Incoming…
daira Jun 22, 2024
c6520cf
Change the protobuf schema to explicitly specify whether a `ChangeValue`
daira Jun 22, 2024
2f521d7
If a change memo is supplied, it should not be used in the second step
daira Jun 23, 2024
0f49dae
`mark_ephemeral_address_as_mined` now prefers setting `mined_in_tx` to
daira Jun 23, 2024
637ae92
Add a migration test for the `ephemeral_addresses` migration.
daira Jun 23, 2024
994f6ff
Change type of `n` in `reserve_next_n_ephemeral_addresses`.
daira Jun 24, 2024
25f07da
Add a constraint on the range of `ephemeral_addresses(address_index)`.
daira Jun 24, 2024
e164b59
Move most ephemeral address index handling into helper functions in
daira Jun 24, 2024
914acb5
Move most remaining code for wallet support of ephemeral addresses into
daira Jun 24, 2024
745054b
`find_account_for_transparent_output` now searches unreserved ephemeral
daira Jun 24, 2024
6b465b7
Document the mapping functions on `zcash_client_backend::wallet::Reci…
daira Jun 24, 2024
4f43a01
Refactor transparent address metadata lookups. This is correct as-is but
daira Jun 25, 2024
5a90fff
Factor out the conversion of the `diversifier_index_be` field in the
daira Jun 25, 2024
7fb3557
Implement `WalletRead::get_transparent_address_metadata` for
daira Jun 25, 2024
7d8a96f
Don't cache metadata between steps; it's not an important optimization.
daira Jun 25, 2024
bd6c9f3
Apply documentation suggestions from code review.
daira Jun 26, 2024
0735390
Rename `amount` to `transfer_amount` in `send_multi_step_proposed_tra…
daira Jun 26, 2024
c926e7c
Filter ephemeral transparent `ChangeValue`s by `is_ephemeral()` as we…
daira Jun 26, 2024
ffb2ddf
`zcash_client_backend::fees::ChangeValue` is now an enum, allowing only
daira Jun 26, 2024
b778139
Refactor ephemeral output-related parameters to balance calculation.
daira Jun 26, 2024
7a05b44
Make mutable inputs to closures in `create_proposed_transaction` expl…
daira Jun 26, 2024
6471d4c
Don't assume that prior step outputs are ephemeral iff they are
daira Jun 27, 2024
ec4a6d0
Documentation improvements.
daira Jun 27, 2024
81a2846
Simpler way to calculate `has_shielded_inputs`.
daira Jun 27, 2024
f0e5aab
Improve discrimination of proposal errors.
daira Jun 27, 2024
feabe6d
Remove complicated code to calculate the number of dust spends.
daira Jun 26, 2024
baccb43
Restore the logic to determine whether we are spending inputs that are
daira Jun 28, 2024
9c082dc
`zcash_primitives::transaction::fees::zip317::FeeRule::non_standard` has
daira Jun 28, 2024
3829663
Change note selection query to select notes > 5000 zats, not >= 5000 …
daira Jun 28, 2024
25006ab
Documentation improvement from code review.
daira Jun 28, 2024
3922d71
Change the type of `n` in `reserve_next_n_ephemeral_addresses` back t…
daira Jun 28, 2024
d32b7db
Remove `ChangeValue::new`. Also document `ChangeValue::is_ephemeral` as
daira Jun 28, 2024
286439a
Define a constant `EphemeralParameters::NONE` instead of deriving `De…
daira Jun 29, 2024
bc38f2a
Document the `possible_change` parameter to `check_for_uneconomic_inp…
daira Jun 29, 2024
7838c04
Make `ephemeral_parameters` and `EphemeralParameters::NONE` available
daira Jun 29, 2024
14bdcde
We cannot spend prior outputs at all when "transparent-inputs" is not
daira Jun 29, 2024
8636daa
Tiny simplification.
daira Jun 29, 2024
01ff201
Minor changes responding to review comments.
daira Jul 3, 2024
b63ff5b
Rename `get_reserved_ephemeral_addresses` to `get_known_ephemeral_add…
daira Jul 3, 2024
e97da43
Refactoring to address review comments.
daira Jul 3, 2024
a01588b
Ensure that `mark_ephemeral_address_as_mined` correctly handles indices
daira Jul 4, 2024
86428c4
Refactor `find_account_for_transparent_output` (now called
daira Jul 4, 2024
b48f627
Minor simplification in a test.
daira Jul 4, 2024
6bc22f4
Documentation fixes and improvements.
daira Jul 4, 2024
27ca6e4
Remove unneeded `impl SealedChangeLevelKey for EphemeralIvk`.
daira Jul 4, 2024
bda6451
Change `unwrap`s to `expect`s when constructing `NonHardenedChildIndex`.
daira Jul 4, 2024
22b8cff
The `TxId` argument to `EphemeralAddressReuse` does not need to be op…
daira Jul 4, 2024
9856a70
Simpler handling of a potential overflow.
daira Jul 4, 2024
56aa348
Extend the `send_multi_step_proposed_transfer` test to check the beha…
daira Jul 4, 2024
aa43123
Use `EphemeralBalance` instead of `EphemeralParameters`
nuttycom Jul 16, 2024
dbb5eeb
Fix a potential crash related to varying behavior between change stra…
nuttycom Jul 16, 2024
24b6d50
Apply suggestions from code review
nuttycom Jul 16, 2024
f8bedd8
Make ephemeral_addresses.address unique
nuttycom Jul 17, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
If a change memo is supplied, it should not be used in the second step
of a ZIP 320 proposal.

Signed-off-by: Daira-Emma Hopwood <[email protected]>
  • Loading branch information
daira committed Jun 25, 2024
commit 2f521d78736be6f4b349fd48fbe590745d58f251
9 changes: 5 additions & 4 deletions zcash_client_backend/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -71,11 +71,12 @@ funds to those addresses. See [ZIP 320](https://zips.z.cash/zip-0320) for detail
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
shielded pool.
- `ChangeStrategy::compute_balance`: this trait method has two additional
- `ChangeStrategy::compute_balance`: this trait method has three additional
parameters when the "transparent-inputs" feature is enabled. These are
used to specify amounts of additional transparent P2PKH inputs and outputs,
for which `InputView` or `OutputView` instances may not be available.
Empty slices can be passed to obtain the previous behaviour.
used to specify whether the change memo should be ignored, and the amounts
of additional transparent P2PKH inputs and outputs. Passing `false` for
`ignore_change_memo` and empty slices for the other two arguments will
retain the previous behaviour.
- `zcash_client_backend::input_selection::GreedyInputSelectorError` has a
new variant `UnsupportedTexAddress`.
- `zcash_client_backend::proto::ProposalDecodingError` has a new variant
10 changes: 10 additions & 0 deletions zcash_client_backend/src/data_api/wallet/input_selection.rs
Original file line number Diff line number Diff line change
@@ -475,6 +475,8 @@ where
&orchard_fees::EmptyBundleView,
&self.dust_output_policy,
#[cfg(feature = "transparent-inputs")]
true,
daira marked this conversation as resolved.
Show resolved Hide resolved
#[cfg(feature = "transparent-inputs")]
&[NonNegativeAmount::ZERO],
#[cfg(feature = "transparent-inputs")]
&[],
@@ -494,6 +496,8 @@ where
&orchard_fees::EmptyBundleView,
&self.dust_output_policy,
#[cfg(feature = "transparent-inputs")]
true,
daira marked this conversation as resolved.
Show resolved Hide resolved
#[cfg(feature = "transparent-inputs")]
&[tr1_required_input_value],
#[cfg(feature = "transparent-inputs")]
&[],
@@ -574,6 +578,8 @@ where
),
&self.dust_output_policy,
#[cfg(feature = "transparent-inputs")]
false,
#[cfg(feature = "transparent-inputs")]
&[],
#[cfg(feature = "transparent-inputs")]
&ephemeral_output_amounts,
@@ -764,6 +770,8 @@ where
&orchard_fees::EmptyBundleView,
&self.dust_output_policy,
#[cfg(feature = "transparent-inputs")]
false,
#[cfg(feature = "transparent-inputs")]
&[],
#[cfg(feature = "transparent-inputs")]
&[],
@@ -785,6 +793,8 @@ where
&orchard_fees::EmptyBundleView,
&self.dust_output_policy,
#[cfg(feature = "transparent-inputs")]
nuttycom marked this conversation as resolved.
Show resolved Hide resolved
false,
#[cfg(feature = "transparent-inputs")]
&[],
#[cfg(feature = "transparent-inputs")]
&[],
1 change: 1 addition & 0 deletions zcash_client_backend/src/fees.rs
Original file line number Diff line number Diff line change
@@ -356,6 +356,7 @@ pub trait ChangeStrategy {
sapling: &impl sapling::BundleView<NoteRefT>,
#[cfg(feature = "orchard")] orchard: &impl orchard::BundleView<NoteRefT>,
dust_output_policy: &DustOutputPolicy,
#[cfg(feature = "transparent-inputs")] ignore_change_memo: bool,
#[cfg(feature = "transparent-inputs")] ephemeral_input_amounts: &[NonNegativeAmount],
#[cfg(feature = "transparent-inputs")] ephemeral_output_amounts: &[NonNegativeAmount],
daira marked this conversation as resolved.
Show resolved Hide resolved
) -> Result<TransactionBalance, ChangeError<Self::Error, NoteRefT>>;
14 changes: 13 additions & 1 deletion zcash_client_backend/src/fees/fixed.rs
Original file line number Diff line number Diff line change
@@ -66,9 +66,13 @@ impl ChangeStrategy for SingleOutputChangeStrategy {
sapling: &impl sapling_fees::BundleView<NoteRefT>,
#[cfg(feature = "orchard")] orchard: &impl orchard_fees::BundleView<NoteRefT>,
dust_output_policy: &DustOutputPolicy,
#[cfg(feature = "transparent-inputs")] ignore_change_memo: bool,
#[cfg(feature = "transparent-inputs")] ephemeral_input_amounts: &[NonNegativeAmount],
#[cfg(feature = "transparent-inputs")] ephemeral_output_amounts: &[NonNegativeAmount],
) -> Result<TransactionBalance, ChangeError<Self::Error, NoteRefT>> {
#[cfg(not(feature = "transparent-inputs"))]
let ignore_change_memo = false;

single_change_output_balance(
params,
&self.fee_rule,
@@ -80,7 +84,11 @@ impl ChangeStrategy for SingleOutputChangeStrategy {
orchard,
dust_output_policy,
self.fee_rule().fixed_fee(),
self.change_memo.clone(),
if ignore_change_memo {
None
} else {
self.change_memo.clone()
},
self.fallback_change_pool,
#[cfg(feature = "transparent-inputs")]
ephemeral_input_amounts,
@@ -142,6 +150,8 @@ mod tests {
&orchard_fees::EmptyBundleView,
&DustOutputPolicy::default(),
#[cfg(feature = "transparent-inputs")]
false,
#[cfg(feature = "transparent-inputs")]
&[],
#[cfg(feature = "transparent-inputs")]
&[],
@@ -191,6 +201,8 @@ mod tests {
&orchard_fees::EmptyBundleView,
&DustOutputPolicy::default(),
#[cfg(feature = "transparent-inputs")]
false,
#[cfg(feature = "transparent-inputs")]
&[],
#[cfg(feature = "transparent-inputs")]
&[],
7 changes: 7 additions & 0 deletions zcash_client_backend/src/fees/standard.rs
Original file line number Diff line number Diff line change
@@ -68,6 +68,7 @@ impl ChangeStrategy for SingleOutputChangeStrategy {
sapling: &impl sapling_fees::BundleView<NoteRefT>,
#[cfg(feature = "orchard")] orchard: &impl orchard_fees::BundleView<NoteRefT>,
dust_output_policy: &DustOutputPolicy,
#[cfg(feature = "transparent-inputs")] ignore_change_memo: bool,
#[cfg(feature = "transparent-inputs")] ephemeral_input_amounts: &[NonNegativeAmount],
#[cfg(feature = "transparent-inputs")] ephemeral_output_amounts: &[NonNegativeAmount],
) -> Result<TransactionBalance, ChangeError<Self::Error, NoteRefT>> {
@@ -88,6 +89,8 @@ impl ChangeStrategy for SingleOutputChangeStrategy {
orchard,
dust_output_policy,
#[cfg(feature = "transparent-inputs")]
ignore_change_memo,
#[cfg(feature = "transparent-inputs")]
ephemeral_input_amounts,
#[cfg(feature = "transparent-inputs")]
ephemeral_output_amounts,
@@ -108,6 +111,8 @@ impl ChangeStrategy for SingleOutputChangeStrategy {
orchard,
dust_output_policy,
#[cfg(feature = "transparent-inputs")]
ignore_change_memo,
#[cfg(feature = "transparent-inputs")]
ephemeral_input_amounts,
#[cfg(feature = "transparent-inputs")]
ephemeral_output_amounts,
@@ -128,6 +133,8 @@ impl ChangeStrategy for SingleOutputChangeStrategy {
orchard,
dust_output_policy,
#[cfg(feature = "transparent-inputs")]
ignore_change_memo,
#[cfg(feature = "transparent-inputs")]
ephemeral_input_amounts,
#[cfg(feature = "transparent-inputs")]
ephemeral_output_amounts,
26 changes: 25 additions & 1 deletion zcash_client_backend/src/fees/zip317.rs
Original file line number Diff line number Diff line change
@@ -68,6 +68,7 @@ impl ChangeStrategy for SingleOutputChangeStrategy {
sapling: &impl sapling_fees::BundleView<NoteRefT>,
#[cfg(feature = "orchard")] orchard: &impl orchard_fees::BundleView<NoteRefT>,
dust_output_policy: &DustOutputPolicy,
#[cfg(feature = "transparent-inputs")] ignore_change_memo: bool,
#[cfg(feature = "transparent-inputs")] ephemeral_input_amounts: &[NonNegativeAmount],
#[cfg(feature = "transparent-inputs")] ephemeral_output_amounts: &[NonNegativeAmount],
) -> Result<TransactionBalance, ChangeError<Self::Error, NoteRefT>> {
@@ -203,6 +204,9 @@ impl ChangeStrategy for SingleOutputChangeStrategy {
}
}

#[cfg(not(feature = "transparent-inputs"))]
let ignore_change_memo = false;

single_change_output_balance(
params,
&self.fee_rule,
@@ -214,7 +218,11 @@ impl ChangeStrategy for SingleOutputChangeStrategy {
orchard,
dust_output_policy,
self.fee_rule.marginal_fee(),
self.change_memo.clone(),
if ignore_change_memo {
None
} else {
self.change_memo.clone()
},
self.fallback_change_pool,
#[cfg(feature = "transparent-inputs")]
ephemeral_input_amounts,
@@ -283,6 +291,8 @@ mod tests {
&orchard_fees::EmptyBundleView,
&DustOutputPolicy::default(),
#[cfg(feature = "transparent-inputs")]
false,
#[cfg(feature = "transparent-inputs")]
&[],
#[cfg(feature = "transparent-inputs")]
&[],
@@ -330,6 +340,8 @@ mod tests {
),
&DustOutputPolicy::default(),
#[cfg(feature = "transparent-inputs")]
false,
#[cfg(feature = "transparent-inputs")]
&[],
#[cfg(feature = "transparent-inputs")]
&[],
@@ -386,6 +398,8 @@ mod tests {
&orchard_fees::EmptyBundleView,
dust_output_policy,
#[cfg(feature = "transparent-inputs")]
false,
#[cfg(feature = "transparent-inputs")]
&[],
#[cfg(feature = "transparent-inputs")]
&[],
@@ -433,6 +447,8 @@ mod tests {
&orchard_fees::EmptyBundleView,
&DustOutputPolicy::default(),
#[cfg(feature = "transparent-inputs")]
false,
#[cfg(feature = "transparent-inputs")]
&[],
#[cfg(feature = "transparent-inputs")]
&[],
@@ -480,6 +496,8 @@ mod tests {
&orchard_fees::EmptyBundleView,
&DustOutputPolicy::default(),
#[cfg(feature = "transparent-inputs")]
false,
#[cfg(feature = "transparent-inputs")]
&[],
#[cfg(feature = "transparent-inputs")]
&[],
@@ -533,6 +551,8 @@ mod tests {
Some(NonNegativeAmount::const_from_u64(1000)),
),
#[cfg(feature = "transparent-inputs")]
false,
#[cfg(feature = "transparent-inputs")]
&[],
#[cfg(feature = "transparent-inputs")]
&[],
@@ -597,6 +617,8 @@ mod tests {
&orchard_fees::EmptyBundleView,
dust_output_policy,
#[cfg(feature = "transparent-inputs")]
false,
#[cfg(feature = "transparent-inputs")]
&[],
#[cfg(feature = "transparent-inputs")]
&[],
@@ -653,6 +675,8 @@ mod tests {
&orchard_fees::EmptyBundleView,
&DustOutputPolicy::default(),
#[cfg(feature = "transparent-inputs")]
false,
#[cfg(feature = "transparent-inputs")]
&[],
#[cfg(feature = "transparent-inputs")]
&[],
5 changes: 3 additions & 2 deletions zcash_client_sqlite/src/testing/pool.rs
Original file line number Diff line number Diff line change
@@ -302,6 +302,8 @@ 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 std::str::FromStr;

use zcash_client_backend::fees::ChangeValue;

let mut st = TestBuilder::new()
@@ -346,8 +348,7 @@ pub(crate) fn send_multi_step_proposed_transfer<T: ShieldedPoolTester>() {
TransparentAddress::PublicKeyHash(data) => Address::Tex(data),
_ => unreachable!(),
};
// As of this commit, change memos are not correctly handled in ZIP 320 proposals.
let change_memo = None;
let change_memo = Some(Memo::from_str("change").expect("valid memo").encode());

// We use `st.propose_standard_transfer` here in order to also test round-trip
// serialization of the proposal.