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

Gracefully handle when given an Orchard-only UA #1009

Merged
merged 15 commits into from
Oct 13, 2023
5 changes: 5 additions & 0 deletions zcash_client_backend/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,11 @@ and this library adheres to Rust's notion of

## [Unreleased]

### Added
- `zcash_client_backend::data_api::ShieldedProtocol` has a new variant for `Orchard`, allowing for better reporting to callers trying to perform actions using `Orchard` before it is fully supported.
tw0po1nt marked this conversation as resolved.
Show resolved Hide resolved
- `zcash_client_backend::data_api::error::Error` has new error variant:
- `Error::UnsupportedPoolType(zcash_client_backend::data_api::PoolType)`

### Changed
- `zcash_client_backend::data_api::chain::scan_cached_blocks` now returns
a `ScanSummary` containing metadata about the scanned blocks on success.
Expand Down
15 changes: 13 additions & 2 deletions zcash_client_backend/src/data_api.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

use std::{
collections::{BTreeMap, HashMap},
fmt::Debug,
fmt::{self, Debug},
io,
num::{NonZeroU32, TryFromIntError},
};
Expand Down Expand Up @@ -556,7 +556,8 @@
pub enum ShieldedProtocol {
/// The Sapling protocol
Sapling,
// TODO: Orchard
/// The Orchard protocol
Orchard
daira marked this conversation as resolved.
Show resolved Hide resolved
}

/// A unique identifier for a shielded transaction output
Expand Down Expand Up @@ -603,6 +604,16 @@
Shielded(ShieldedProtocol),
}

impl fmt::Display for PoolType {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
match self {
PoolType::Transparent => f.write_str("Transparent"),
PoolType::Shielded(ShieldedProtocol::Sapling) => f.write_str("Sapling"),
PoolType::Shielded(ShieldedProtocol::Orchard) => f.write_str("Orchard"),

Check warning on line 612 in zcash_client_backend/src/data_api.rs

View check run for this annotation

Codecov / codecov/patch

zcash_client_backend/src/data_api.rs#L609-L612

Added lines #L609 - L612 were not covered by tests
}
}
}

/// A type that represents the recipient of a transaction output; a recipient address (and, for
/// unified addresses, the pool to which the payment is sent) in the case of outgoing output, or an
/// internal account ID and the pool to which funds were sent in the case of a wallet-internal
Expand Down
5 changes: 5 additions & 0 deletions zcash_client_backend/src/data_api/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
zip32::AccountId,
};

use crate::data_api::PoolType;
use crate::data_api::wallet::input_selection::InputSelectorError;

#[cfg(feature = "transparent-inputs")]
Expand Down Expand Up @@ -54,6 +55,9 @@
/// It is forbidden to provide a memo when constructing a transparent output.
MemoForbidden,

/// Attempted to create a spend to an unsupported pool type (currently, Orchard).
UnsupportedPoolType(PoolType),

/// A note being spent does not correspond to either the internal or external
/// full viewing key for an account.
NoteMismatch(NoteRef),
Expand Down Expand Up @@ -111,6 +115,7 @@
Error::ScanRequired => write!(f, "Must scan blocks first"),
Error::Builder(e) => write!(f, "An error occurred building the transaction: {}", e),
Error::MemoForbidden => write!(f, "It is not possible to send a memo to a transparent address."),
Error::UnsupportedPoolType(t) => write!(f, "Attempted to create spend to an unsupported pool type: {}", t),

Check warning on line 118 in zcash_client_backend/src/data_api/error.rs

View check run for this annotation

Codecov / codecov/patch

zcash_client_backend/src/data_api/error.rs#L118

Added line #L118 was not covered by tests
Error::NoteMismatch(n) => write!(f, "A note being spent ({}) does not correspond to either the internal or external full viewing key for the provided spending key.", n),

#[cfg(feature = "transparent-inputs")]
Expand Down
29 changes: 23 additions & 6 deletions zcash_client_backend/src/data_api/wallet.rs
Original file line number Diff line number Diff line change
Expand Up @@ -425,6 +425,9 @@
///
/// Returns the database identifier for the newly constructed transaction, or an error if
/// an error occurs in transaction construction, proving, or signing.
///
/// Note: If the payment includes a recipient with an Orchard-only UA, this will attempt to fallback
/// to the transparent receiver until full Orchard support is implemented
tw0po1nt marked this conversation as resolved.
Show resolved Hide resolved
#[allow(clippy::too_many_arguments)]
#[allow(clippy::type_complexity)]
pub fn create_proposed_transaction<DbT, ParamsT, InputsErrT, FeeRuleT>(
Expand Down Expand Up @@ -546,12 +549,26 @@
.memo
.as_ref()
.map_or_else(MemoBytes::empty, |m| m.clone());
builder.add_sapling_output(
external_ovk,
*ua.sapling().expect("TODO: Add Orchard support to builder"),
payment.amount,
memo.clone(),
)?;

if let Some(sapling_receiver) = ua.sapling() {
builder.add_sapling_output(
external_ovk,
*sapling_receiver,
payment.amount,
memo.clone(),

Check warning on line 558 in zcash_client_backend/src/data_api/wallet.rs

View check run for this annotation

Codecov / codecov/patch

zcash_client_backend/src/data_api/wallet.rs#L553-L558

Added lines #L553 - L558 were not covered by tests
)?;
} else if let Some(taddr) = ua.transparent() {
if payment.memo.is_some() {
tw0po1nt marked this conversation as resolved.
Show resolved Hide resolved
return Err(Error::MemoForbidden);

Check warning on line 562 in zcash_client_backend/src/data_api/wallet.rs

View check run for this annotation

Codecov / codecov/patch

zcash_client_backend/src/data_api/wallet.rs#L560-L562

Added lines #L560 - L562 were not covered by tests
} else {
builder.add_transparent_output(
taddr,
payment.amount

Check warning on line 566 in zcash_client_backend/src/data_api/wallet.rs

View check run for this annotation

Codecov / codecov/patch

zcash_client_backend/src/data_api/wallet.rs#L564-L566

Added lines #L564 - L566 were not covered by tests
)?;
}
} else {
return Err(Error::UnsupportedPoolType(PoolType::Shielded(ShieldedProtocol::Orchard)));

Check warning on line 570 in zcash_client_backend/src/data_api/wallet.rs

View check run for this annotation

Codecov / codecov/patch

zcash_client_backend/src/data_api/wallet.rs#L570

Added line #L570 was not covered by tests
}
sapling_output_meta.push((
Recipient::Unified(ua.clone(), PoolType::Shielded(ShieldedProtocol::Sapling)),
tw0po1nt marked this conversation as resolved.
Show resolved Hide resolved
payment.amount,
Expand Down
4 changes: 4 additions & 0 deletions zcash_client_sqlite/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,10 @@ and this library adheres to Rust's notion of

## [Unreleased]

### Added
- `zcash_client_sqlite::error::SqliteClientError` has new error variant:
- `SqliteClientError::UnsupportedPoolType(zcash_client_backend::data_api::PoolType)`

## [0.8.0] - 2023-09-25

### Notable Changes
Expand Down
7 changes: 6 additions & 1 deletion zcash_client_sqlite/src/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
use std::fmt;

use shardtree::error::ShardTreeError;
use zcash_client_backend::data_api::PoolType;
use zcash_client_backend::encoding::{Bech32DecodeError, TransparentCodecError};
use zcash_primitives::{consensus::BlockHeight, zip32::AccountId};

Expand Down Expand Up @@ -98,6 +99,9 @@
/// [`WalletWrite::update_chain_tip`]:
/// zcash_client_backend::data_api::WalletWrite::update_chain_tip
ChainHeightUnknown,

/// Unsupported pool type
UnsupportedPoolType(PoolType)
}

impl error::Error for SqliteClientError {
Expand Down Expand Up @@ -144,7 +148,8 @@
SqliteClientError::AddressNotRecognized(_) => write!(f, "The address associated with a received txo is not identifiable as belonging to the wallet."),
SqliteClientError::CommitmentTree(err) => write!(f, "An error occurred accessing or updating note commitment tree data: {}.", err),
SqliteClientError::CacheMiss(height) => write!(f, "Requested height {} does not exist in the block cache.", height),
SqliteClientError::ChainHeightUnknown => write!(f, "Chain height unknown; please call `update_chain_tip`")
SqliteClientError::ChainHeightUnknown => write!(f, "Chain height unknown; please call `update_chain_tip`"),
SqliteClientError::UnsupportedPoolType(t) => write!(f, "Pool type is not currently supported: {}", t)

Check warning on line 152 in zcash_client_sqlite/src/error.rs

View check run for this annotation

Codecov / codecov/patch

zcash_client_sqlite/src/error.rs#L151-L152

Added lines #L151 - L152 were not covered by tests
}
}
}
Expand Down
9 changes: 8 additions & 1 deletion zcash_client_sqlite/src/wallet.rs
Original file line number Diff line number Diff line change
Expand Up @@ -136,6 +136,7 @@
match pool_type {
PoolType::Transparent => 0i64,
PoolType::Shielded(ShieldedProtocol::Sapling) => 2i64,
PoolType::Shielded(ShieldedProtocol::Orchard) => 3i64

Check warning on line 139 in zcash_client_sqlite/src/wallet.rs

View check run for this annotation

Codecov / codecov/patch

zcash_client_sqlite/src/wallet.rs#L139

Added line #L139 was not covered by tests
}
}

Expand Down Expand Up @@ -758,7 +759,12 @@
conn: &rusqlite::Connection,
note_id: NoteId,
) -> Result<Option<Memo>, SqliteClientError> {
let memo_bytes: Option<Vec<_>> = match note_id.protocol() {
let protocol = note_id.protocol();
if let ShieldedProtocol::Orchard = protocol {
return Err(SqliteClientError::UnsupportedPoolType(PoolType::Shielded(protocol)));

Check warning on line 764 in zcash_client_sqlite/src/wallet.rs

View check run for this annotation

Codecov / codecov/patch

zcash_client_sqlite/src/wallet.rs#L764

Added line #L764 was not covered by tests
}

tw0po1nt marked this conversation as resolved.
Show resolved Hide resolved
let memo_bytes: Option<Vec<_>> = match protocol {
ShieldedProtocol::Sapling => conn
.query_row(
"SELECT memo FROM sapling_received_notes
Expand All @@ -773,6 +779,7 @@
)
.optional()?
.flatten(),
ShieldedProtocol::Orchard => None

Check warning on line 782 in zcash_client_sqlite/src/wallet.rs

View check run for this annotation

Codecov / codecov/patch

zcash_client_sqlite/src/wallet.rs#L782

Added line #L782 was not covered by tests
tw0po1nt marked this conversation as resolved.
Show resolved Hide resolved
};

memo_bytes
Expand Down
Loading