Skip to content

Commit

Permalink
Merge pull request zcash#1060 from nuttycom/wallet/generalize_proposals
Browse files Browse the repository at this point in the history
Add Orchard support to fees & transaction proposals.
  • Loading branch information
nuttycom authored Jan 9, 2024
2 parents 3bce5af + 24ebe4c commit c3a630b
Show file tree
Hide file tree
Showing 22 changed files with 924 additions and 404 deletions.
1 change: 1 addition & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

55 changes: 29 additions & 26 deletions zcash_client_backend/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -17,17 +17,18 @@ and this library adheres to Rust's notion of
- `ScannedBlockCommitments`
- `Balance::{add_spendable_value, add_pending_change_value, add_pending_spendable_value}`
- `AccountBalance::{
with_sapling_balance_mut,
with_orchard_balance_mut,
with_sapling_balance_mut,
with_orchard_balance_mut,
add_unshielded_value
}`
- `wallet::propose_standard_transfer_to_address`
- `wallet::input_selection::Proposal::from_parts`
- `wallet::input_selection::SaplingInputs`
- `wallet::input_selection::Proposal::{from_parts, shielded_inputs}`
- `wallet::input_selection::ShieldedInputs`
- `wallet::input_selection::ShieldingSelector` has been
factored out from the `InputSelector` trait to separate out transparent
functionality and move it behind the `transparent-inputs` feature flag.
- `zcash_client_backend::fees::{standard, sapling}`
- `zcash_client_backend::fees::{standard, orchard, sapling}`
- `zcash_client_backend::fees::ChangeValue::{new, orchard}`
- `zcash_client_backend::wallet`:
- `Note`
- `ReceivedNote`
Expand Down Expand Up @@ -57,7 +58,7 @@ and this library adheres to Rust's notion of
- `zcash_client_backend::data_api::{NoteId, Recipient}` have
been moved into the `zcash_client_backend::wallet` module.
- `ScannedBlock::{sapling_tree_size, sapling_nullifier_map, sapling_commitments}`
have been moved to `ScannedBlockSapling` and in that context are now
have been moved to `ScannedBlockSapling` and in that context are now
named `{tree_size, nullifier_map, commitments}` respectively.

### Changed
Expand All @@ -68,9 +69,9 @@ and this library adheres to Rust's notion of
- `WalletShieldedOutput` has an additional type parameter which is used for
key scope. `WalletShieldedOutput::from_parts` now takes an additional
argument of this type.
- `WalletTx` has an additional type parameter as a consequence of the
- `WalletTx` has an additional type parameter as a consequence of the
`WalletShieldedOutput` change.
- `ScannedBlock` has an additional type parameter as a consequence of the
- `ScannedBlock` has an additional type parameter as a consequence of the
`WalletTx` change.
- `ScannedBlock::metadata` has been renamed to `to_block_metadata` and now
returns an owned value rather than a reference.
Expand Down Expand Up @@ -124,26 +125,34 @@ and this library adheres to Rust's notion of
argument, instead taking explicit `target_height` and `anchor_height`
arguments. This helps to minimize the set of capabilities that the
`data_api::InputSource` must expose.
- `WalletRead::get_checkpoint_depth` has been removed without replacement. This
is no longer needed given the change to use the stored anchor height for transaction
proposal execution.
- Changes to the `WalletRead` trait:
- Added `get_orchard_nullifiers` (under the `orchard` feature flag.)
- `get_checkpoint_depth` has been removed without replacement. This
is no longer needed given the change to use the stored anchor height for transaction
proposal execution.
- `is_valid_account_extfvk` has been removed; it was unused in
the ECC mobile wallet SDKs and has been superseded by `get_account_for_ufvk`.
- `get_spendable_sapling_notes`, `select_spendable_sapling_notes`, and
`get_unspent_transparent_outputs` have been removed; use
`data_api::InputSource` instead.
- `wallet::{propose_shielding, shield_transparent_funds}` now takes their
`min_confirmations` arguments as `u32` rather than a `NonZeroU32` to permit
implmentations to enable zero-conf shielding.
- `wallet::create_proposed_transaction` now forces implementations to ignore
the database identifiers for its contained notes by universally quantifying
the `NoteRef` type parameter.
- `wallet::input_selection::Proposal::sapling_inputs` now returns type
`Option<&SaplingInputs>`.
- Arguments to `wallet::input_selection::Proposal::from_parts` have changed.
- `wallet::input_selection::Proposal::min_anchor_height` has been removed in
favor of storing this value in `SaplingInputs`.
- `wallet::input_selection::GreedyInputSelector` now has relaxed requirements
for its `InputSource` associated type.

- `zcash_client_backend::fees`:
- `ChangeValue::Sapling` is now a structured variant. In addition to the
existing change value, it now also carries an optional memo to be associated
with the change output.
- `ChangeStrategy::compute_balance` arguments have changed.
- `ChangeValue` is now a struct. In addition to the existing change value, it
now also provides the output pool to which change should be sent and an
optional memo to be associated with the change output.
- `ChangeError` has a new `BundleError` variant.
- `fixed::SingleOutputChangeStrategy::new` and
`zip317::SingleOutputChangeStrategy::new` each now accept an additional
`change_memo` argument.
Expand All @@ -157,7 +166,6 @@ and this library adheres to Rust's notion of
- `error::Error::InsufficientFunds.{available, required}`
- `wallet::input_selection::InputSelectorError::InsufficientFunds.{available, required}`
- `zcash_client_backend::fees`:
- `ChangeValue::Sapling.value`
- `ChangeError::InsufficientFunds.{available, required}`
- `zcash_client_backend::zip321::Payment.amount`
- The following methods now take `NonNegativeAmount` instead of `Amount`:
Expand All @@ -183,7 +191,7 @@ and this library adheres to Rust's notion of
- `zcash_client_backend::address`:
- `RecipientAddress` has been renamed to `Address`
- `Address::Shielded` has been renamed to `Address::Sapling`
- `UnifiedAddress::from_receivers` no longer takes an Orchard receiver
- `UnifiedAddress::from_receivers` no longer takes an Orchard receiver
argument unless the `orchard` feature is enabled.
- `UnifiedAddress::orchard` is now only available when the `orchard` feature
is enabled.
Expand All @@ -199,14 +207,9 @@ and this library adheres to Rust's notion of
### Removed
- `zcash_client_backend::wallet::ReceivedSaplingNote` has been replaced by
`zcash_client_backend::ReceivedNote`.
- `zcash_client_backend::data_api::WalletRead::is_valid_account_extfvk` has been
removed; it was unused in the ECC mobile wallet SDKs and has been superseded by
`get_account_for_ufvk`.
- `zcash_client_backend::data_api::WalletRead::{
get_spendable_sapling_notes
select_spendable_sapling_notes,
get_unspent_transparent_outputs,
}` - use `data_api::InputSource` instead.
- `zcash_client_backend::wallet::input_selection::Proposal::sapling_inputs` has
been replaced by `Proposal::shielded_inputs`
- `zcash_client_backend::data_api`
- `zcash_client_backend::data_api::ScannedBlock::from_parts` has been made crate-private.
- `zcash_client_backend::data_api::ScannedBlock::into_sapling_commitments` has been
replaced by `into_commitments` which returns both Sapling and Orchard note commitments
Expand Down
6 changes: 6 additions & 0 deletions zcash_client_backend/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,12 @@
This library contains Rust structs and traits for creating shielded Zcash light
clients.

## Building

Note that in order to (re)build the GRPC interface, you will need `protoc` on
your `$PATH`. This is not required unless you make changes to any of the files
in `./proto/`.

## License

Licensed under either of
Expand Down
54 changes: 29 additions & 25 deletions zcash_client_backend/proto/proposal.proto
Original file line number Diff line number Diff line change
Expand Up @@ -11,11 +11,13 @@ message Proposal {
uint32 protoVersion = 1;
// ZIP 321 serialized transaction request
string transactionRequest = 2;
// The transparent UTXOs to use as inputs to the transaction.
repeated ProposedInput transparentInputs = 3;
// The Sapling input notes and anchor height to be used in creating the transaction.
SaplingInputs saplingInputs = 4;
// The total value, fee amount, and change outputs of the proposed
// The anchor height to be used in creating the transaction, if any.
// Setting the anchor height to zero will disallow the use of any shielded
// inputs.
uint32 anchorHeight = 3;
// The inputs to be used in creating the transaction.
repeated ProposedInput inputs = 4;
// The total value, fee value, and change outputs of the proposed
// transaction
TransactionBalance balance = 5;
// The fee rule used in constructing this proposal
Expand All @@ -30,18 +32,26 @@ message Proposal {
bool isShielding = 8;
}

message SaplingInputs {
// The Sapling anchor height to be used in creating the transaction
uint32 anchorHeight = 1;
// The unique identifier and amount for each proposed Sapling input
repeated ProposedInput inputs = 2;
enum ValuePool {
// Protobuf requires that enums have a zero discriminant as the default
// value. However, we need to require that a known value pool is selected,
// and we do not want to fall back to any default, so sending the
// PoolNotSpecified value will be treated as an error.
PoolNotSpecified = 0;
// The transparent value pool (P2SH is not distinguished from P2PKH)
Transparent = 1;
// The Sapling value pool
Sapling = 2;
// The Orchard value pool
Orchard = 3;
}

// The unique identifier and amount for each proposed input.
// The unique identifier and value for each proposed input.
message ProposedInput {
bytes txid = 1;
uint32 index = 2;
uint64 value = 3;
ValuePool valuePool = 2;
uint32 index = 3;
uint64 value = 4;
}

// The fee rule used in constructing a Proposal
Expand All @@ -59,28 +69,22 @@ enum FeeRule {
Zip317 = 3;
}

// The proposed change outputs and fee amount.
// The proposed change outputs and fee value.
message TransactionBalance {
repeated ChangeValue proposedChange = 1;
uint64 feeRequired = 2;
}

// An enumeration of change value types.
// A proposed change output. If the transparent value pool is selected,
// the `memo` field must be null.
message ChangeValue {
oneof value {
SaplingChange saplingValue = 1;
}
uint64 value = 1;
ValuePool valuePool = 2;
MemoBytes memo = 3;
}

// An object wrapper for memo bytes, to facilitate representing the
// `change_memo == None` case.
message MemoBytes {
bytes value = 1;
}

// The amount and memo for a proposed Sapling change output.
message SaplingChange {
uint64 amount = 1;
MemoBytes memo = 2;
}

35 changes: 26 additions & 9 deletions zcash_client_backend/src/data_api.rs
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ use crate::{
decrypt::DecryptedOutput,
keys::{UnifiedFullViewingKey, UnifiedSpendingKey},
proto::service::TreeState,
wallet::{NoteId, ReceivedNote, Recipient, WalletTransparentOutput, WalletTx},
wallet::{Note, NoteId, ReceivedNote, Recipient, WalletTransparentOutput, WalletTx},
ShieldedProtocol,
};

Expand Down Expand Up @@ -360,7 +360,7 @@ pub trait InputSource {
txid: &TxId,
protocol: ShieldedProtocol,
index: u32,
) -> Result<Option<ReceivedNote<Self::NoteRef>>, Self::Error>;
) -> Result<Option<ReceivedNote<Self::NoteRef, Note>>, Self::Error>;

/// Returns a list of spendable notes sufficient to cover the specified target value, if
/// possible. Only spendable notes corresponding to the specified shielded protocol will
Expand All @@ -372,7 +372,7 @@ pub trait InputSource {
sources: &[ShieldedProtocol],
anchor_height: BlockHeight,
exclude: &[Self::NoteRef],
) -> Result<Vec<ReceivedNote<Self::NoteRef>>, Self::Error>;
) -> Result<Vec<ReceivedNote<Self::NoteRef, Note>>, Self::Error>;

/// Fetches a spendable transparent output.
///
Expand Down Expand Up @@ -526,14 +526,23 @@ pub trait WalletRead {
/// Returns a transaction.
fn get_transaction(&self, txid: TxId) -> Result<Transaction, Self::Error>;

/// Returns the nullifiers for notes that the wallet is tracking, along with their associated
/// account IDs, that are either unspent or have not yet been confirmed as spent (in that a
/// spending transaction known to the wallet has not yet been included in a block).
/// Returns the nullifiers for Sapling notes that the wallet is tracking, along with their
/// associated account IDs, that are either unspent or have not yet been confirmed as spent (in
/// that a spending transaction known to the wallet has not yet been included in a block).
fn get_sapling_nullifiers(
&self,
query: NullifierQuery,
) -> Result<Vec<(AccountId, sapling::Nullifier)>, Self::Error>;

/// Returns the nullifiers for Orchard notes that the wallet is tracking, along with their
/// associated account IDs, that are either unspent or have not yet been confirmed as spent (in
/// that a spending transaction known to the wallet has not yet been included in a block).
#[cfg(feature = "orchard")]
fn get_orchard_nullifiers(
&self,
query: NullifierQuery,
) -> Result<Vec<(AccountId, orchard::note::Nullifier)>, Self::Error>;

/// Returns the set of all transparent receivers associated with the given account.
///
/// The set contains all transparent receivers that are known to have been derived
Expand Down Expand Up @@ -1096,7 +1105,7 @@ pub mod testing {
use crate::{
address::{AddressMetadata, UnifiedAddress},
keys::{UnifiedFullViewingKey, UnifiedSpendingKey},
wallet::{NoteId, ReceivedNote, WalletTransparentOutput},
wallet::{Note, NoteId, ReceivedNote, WalletTransparentOutput},
ShieldedProtocol,
};

Expand Down Expand Up @@ -1133,7 +1142,7 @@ pub mod testing {
_txid: &TxId,
_protocol: ShieldedProtocol,
_index: u32,
) -> Result<Option<ReceivedNote<Self::NoteRef>>, Self::Error> {
) -> Result<Option<ReceivedNote<Self::NoteRef, Note>>, Self::Error> {
Ok(None)
}

Expand All @@ -1144,7 +1153,7 @@ pub mod testing {
_sources: &[ShieldedProtocol],
_anchor_height: BlockHeight,
_exclude: &[Self::NoteRef],
) -> Result<Vec<ReceivedNote<Self::NoteRef>>, Self::Error> {
) -> Result<Vec<ReceivedNote<Self::NoteRef, Note>>, Self::Error> {
Ok(Vec::new())
}
}
Expand Down Expand Up @@ -1251,6 +1260,14 @@ pub mod testing {
Ok(Vec::new())
}

#[cfg(feature = "orchard")]
fn get_orchard_nullifiers(
&self,
_query: NullifierQuery,
) -> Result<Vec<(AccountId, orchard::note::Nullifier)>, Self::Error> {
Ok(Vec::new())
}

fn get_transparent_receivers(
&self,
_account: AccountId,
Expand Down
25 changes: 18 additions & 7 deletions zcash_client_backend/src/data_api/wallet.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ use crate::{
SentTransactionOutput, WalletCommitmentTrees, WalletRead, WalletWrite,
},
decrypt_transaction,
fees::{self, ChangeValue, DustOutputPolicy},
fees::{self, DustOutputPolicy},
keys::UnifiedSpendingKey,
wallet::{Note, OvkPolicy, Recipient},
zip321::{self, Payment},
Expand Down Expand Up @@ -573,7 +573,7 @@ where
Some(dfvk.to_ovk(Scope::Internal))
};

let (sapling_anchor, sapling_inputs) = proposal.sapling_inputs().map_or_else(
let (sapling_anchor, sapling_inputs) = proposal.shielded_inputs().map_or_else(
|| Ok((sapling::Anchor::empty_tree(), vec![])),
|inputs| {
wallet_db.with_sapling_tree_mut::<_, _, Error<_, _, _, _>>(|sapling_tree| {
Expand Down Expand Up @@ -718,24 +718,35 @@ where
}

for change_value in proposal.balance().proposed_change() {
match change_value {
ChangeValue::Sapling { value, memo } => {
let memo = memo.as_ref().map_or_else(MemoBytes::empty, |m| m.clone());
let memo = change_value
.memo()
.map_or_else(MemoBytes::empty, |m| m.clone());
match change_value.output_pool() {
ShieldedProtocol::Sapling => {
builder.add_sapling_output(
internal_ovk(),
dfvk.change_address().1,
*value,
change_value.value(),
memo.clone(),
)?;
sapling_output_meta.push((
Recipient::InternalAccount(
account,
PoolType::Shielded(ShieldedProtocol::Sapling),
),
*value,
change_value.value(),
Some(memo),
))
}
ShieldedProtocol::Orchard => {
#[cfg(not(feature = "orchard"))]
return Err(Error::UnsupportedPoolType(PoolType::Shielded(
ShieldedProtocol::Orchard,
)));

#[cfg(feature = "orchard")]
unimplemented!("FIXME: implement Orchard change output creation.")
}
}
}

Expand Down
Loading

0 comments on commit c3a630b

Please sign in to comment.