Skip to content

Commit

Permalink
zcash_client_sqlite: Use ZcashAddress for persistence of sent note …
Browse files Browse the repository at this point in the history
…addresses

Prior to this change, the recipient of a sent transaction would always
be shown as the protocol-level address, instead of any unified address
intended as the recipient. Now, instead of reencoding the recipient
address, we use the original `ZcashAddress` value from the payment
request.
  • Loading branch information
nuttycom committed Apr 5, 2024
1 parent 86e1181 commit c610bba
Show file tree
Hide file tree
Showing 14 changed files with 274 additions and 115 deletions.
4 changes: 2 additions & 2 deletions components/zcash_address/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,8 @@ and this library adheres to Rust's notion of
## [Unreleased]

### Added
- `zcash_address::ZcashAddress::{can_receive_memo, can_receive_as}`
- `zcash_address::unified::Address::{can_receive_memo, has_receiver}`
- `zcash_address::ZcashAddress::{can_receive_memo, can_receive_as, matches_receiver}`
- `zcash_address::unified::Address::{can_receive_memo, has_receiver_of_type}`
- Module `zcash_address::testing` under the `test-dependencies` feature.
- Module `zcash_address::unified::address::testing` under the
`test-dependencies` feature.
Expand Down
7 changes: 6 additions & 1 deletion components/zcash_address/src/kind/unified/address.rs
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,7 @@ pub struct Address(pub(crate) Vec<Receiver>);

impl Address {
/// Returns whether this address has the ability to receive transfers of the given pool type.
pub fn has_receiver(&self, pool_type: PoolType) -> bool {
pub fn has_receiver_of_type(&self, pool_type: PoolType) -> bool {
self.0.iter().any(|r| match r {
Receiver::Orchard(_) => pool_type == PoolType::Shielded(ShieldedProtocol::Orchard),
Receiver::Sapling(_) => pool_type == PoolType::Shielded(ShieldedProtocol::Sapling),
Expand All @@ -115,6 +115,11 @@ impl Address {
})
}

/// Returns whether this address contains the given receiver.
pub fn contains_receiver(&self, receiver: &Receiver) -> bool {
self.0.contains(receiver)
}

/// Returns whether this address can receive a memo.
pub fn can_receive_memo(&self) -> bool {
self.0
Expand Down
16 changes: 15 additions & 1 deletion components/zcash_address/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -141,6 +141,7 @@ pub use convert::{
};
pub use encoding::ParseError;
pub use kind::unified;
use kind::unified::Receiver;
pub use zcash_protocol::consensus::NetworkType as Network;
use zcash_protocol::{PoolType, ShieldedProtocol};

Expand Down Expand Up @@ -273,7 +274,7 @@ impl ZcashAddress {
match &self.kind {
AddressKind::Sprout(_) => false,
AddressKind::Sapling(_) => pool_type == PoolType::Shielded(ShieldedProtocol::Sapling),
AddressKind::Unified(addr) => addr.has_receiver(pool_type),
AddressKind::Unified(addr) => addr.has_receiver_of_type(pool_type),
AddressKind::P2pkh(_) => pool_type == PoolType::Transparent,
AddressKind::P2sh(_) => pool_type == PoolType::Transparent,
AddressKind::Tex(_) => pool_type == PoolType::Transparent,
Expand All @@ -291,6 +292,19 @@ impl ZcashAddress {
AddressKind::Tex(_) => false,
}
}

/// Returns whether or not this address contains or corresponds to the given unified address
/// receiver.
pub fn matches_receiver(&self, receiver: &Receiver) -> bool {
match (receiver, &self.kind) {
(r, AddressKind::Unified(ua)) => ua.contains_receiver(r),
(Receiver::Sapling(r), AddressKind::Sapling(d)) => r == d,
(Receiver::P2pkh(r), AddressKind::P2pkh(d)) => r == d,
(Receiver::P2pkh(r), AddressKind::Tex(d)) => r == d,
(Receiver::P2sh(r), AddressKind::P2sh(d)) => r == d,
_ => false,
}
}
}

#[cfg(feature = "test-dependencies")]
Expand Down
2 changes: 2 additions & 0 deletions components/zcash_protocol/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@ and this library adheres to Rust's notion of
[Semantic Versioning](https://semver.org/spec/v2.0.0.html).

## [Unreleased]
### Added
- `zcash_protocol::PoolType::{TRANSPARENT, SAPLING, ORCHARD}`

## [0.1.1] - 2024-03-25
### Added
Expand Down
4 changes: 4 additions & 0 deletions zcash_client_backend/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,10 @@ and this library adheres to Rust's notion of
- `zcash_client_backend::proto::proposal::Proposal::{from_standard_proposal,
try_into_standard_proposal}` each no longer require a `consensus::Parameters`
argument.
- `zcash_client_backend::wallet::Recipient` variants have changed. Instead of
wrapping protocol-address types, the `Recipient` type now wraps a
`zcash_address::ZcashAddress`. This simplifies the process of tracking the
original address to which value was sent.

## [0.12.1] - 2024-03-27

Expand Down
57 changes: 35 additions & 22 deletions zcash_client_backend/src/data_api/wallet.rs
Original file line number Diff line number Diff line change
Expand Up @@ -980,8 +980,8 @@ where
memo.clone(),
)?;
orchard_output_meta.push((
Recipient::Unified(
ua.clone(),
Recipient::External(
payment.recipient_address().clone(),
PoolType::Shielded(ShieldedProtocol::Orchard),
),
payment.amount(),
Expand All @@ -997,8 +997,8 @@ where
memo.clone(),
)?;
sapling_output_meta.push((
Recipient::Unified(
ua.clone(),
Recipient::External(
payment.recipient_address().clone(),
PoolType::Shielded(ShieldedProtocol::Sapling),
),
payment.amount(),
Expand Down Expand Up @@ -1026,15 +1026,23 @@ where
payment.amount(),
memo.clone(),
)?;
sapling_output_meta.push((Recipient::Sapling(addr), payment.amount(), Some(memo)));
sapling_output_meta.push((
Recipient::External(payment.recipient_address().clone(), PoolType::SAPLING),
payment.amount(),
Some(memo),
));
}
Address::Transparent(to) => {
if payment.memo().is_some() {
return Err(Error::MemoForbidden);
} else {
builder.add_transparent_output(&to, payment.amount())?;
}
transparent_output_meta.push((to, payment.amount()));
transparent_output_meta.push((
Recipient::External(payment.recipient_address().clone(), PoolType::TRANSPARENT),
to,
payment.amount(),
));
}
}
}
Expand Down Expand Up @@ -1156,22 +1164,27 @@ where
SentTransactionOutput::from_parts(output_index, recipient, value, memo)
});

let transparent_outputs = transparent_output_meta.into_iter().map(|(addr, value)| {
let script = addr.script();
let output_index = build_result
.transaction()
.transparent_bundle()
.and_then(|b| {
b.vout
.iter()
.enumerate()
.find(|(_, tx_out)| tx_out.script_pubkey == script)
})
.map(|(index, _)| index)
.expect("An output should exist in the transaction for each transparent payment.");

SentTransactionOutput::from_parts(output_index, Recipient::Transparent(addr), value, None)
});
let transparent_outputs =
transparent_output_meta
.into_iter()
.map(|(recipient, addr, value)| {
let script = addr.script();
let output_index = build_result
.transaction()
.transparent_bundle()
.and_then(|b| {
b.vout
.iter()
.enumerate()
.find(|(_, tx_out)| tx_out.script_pubkey == script)
})
.map(|(index, _)| index)
.expect(
"An output should exist in the transaction for each transparent payment.",
);

SentTransactionOutput::from_parts(output_index, recipient, value, None)
});

let mut outputs = vec![];
#[cfg(feature = "orchard")]
Expand Down
18 changes: 6 additions & 12 deletions zcash_client_backend/src/wallet.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
//! light client.
use incrementalmerkletree::Position;
use zcash_keys::address::Address;
use zcash_address::ZcashAddress;
use zcash_note_encryption::EphemeralKeyBytes;
use zcash_primitives::{
consensus::BlockHeight,
Expand All @@ -19,7 +19,7 @@ use zcash_primitives::{
};
use zcash_protocol::value::BalanceError;

use crate::{address::UnifiedAddress, fees::sapling as sapling_fees, PoolType, ShieldedProtocol};
use crate::{fees::sapling as sapling_fees, PoolType, ShieldedProtocol};

#[cfg(feature = "orchard")]
use crate::fees::orchard as orchard_fees;
Expand Down Expand Up @@ -68,22 +68,18 @@ impl NoteId {
/// output.
#[derive(Debug, Clone)]
pub enum Recipient<AccountId, N> {
Transparent(TransparentAddress),
Sapling(sapling::PaymentAddress),
Unified(UnifiedAddress, PoolType),
External(ZcashAddress, PoolType),
InternalAccount {
receiving_account: AccountId,
external_address: Option<Address>,
external_address: Option<ZcashAddress>,
note: N,
},
}

impl<AccountId, N> Recipient<AccountId, N> {
pub fn map_internal_account_note<B, F: FnOnce(N) -> B>(self, f: F) -> Recipient<AccountId, B> {
match self {
Recipient::Transparent(t) => Recipient::Transparent(t),
Recipient::Sapling(s) => Recipient::Sapling(s),
Recipient::Unified(u, p) => Recipient::Unified(u, p),
Recipient::External(addr, pool) => Recipient::External(addr, pool),
Recipient::InternalAccount {
receiving_account,
external_address,
Expand All @@ -100,9 +96,7 @@ impl<AccountId, N> Recipient<AccountId, N> {
impl<AccountId, N> Recipient<AccountId, Option<N>> {
pub fn internal_account_note_transpose_option(self) -> Option<Recipient<AccountId, N>> {
match self {
Recipient::Transparent(t) => Some(Recipient::Transparent(t)),
Recipient::Sapling(s) => Some(Recipient::Sapling(s)),
Recipient::Unified(u, p) => Some(Recipient::Unified(u, p)),
Recipient::External(addr, pool) => Some(Recipient::External(addr, pool)),
Recipient::InternalAccount {
receiving_account,
external_address,
Expand Down
2 changes: 2 additions & 0 deletions zcash_client_sqlite/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,8 @@ This version was yanked, use 0.10.1 instead.
- `zcash_client_sqlite::error::SqliteClientError` has new error variants:
- `SqliteClientError::UnsupportedPoolType`
- `SqliteClientError::BalanceError`
- The `Bech32DecodeError` variant has been replaced with a more general
`DecodingError` type.

## [0.8.1] - 2023-10-18

Expand Down
28 changes: 15 additions & 13 deletions zcash_client_sqlite/src/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,8 @@ use std::error;
use std::fmt;

use shardtree::error::ShardTreeError;
use zcash_client_backend::{
encoding::{Bech32DecodeError, TransparentCodecError},
PoolType,
};
use zcash_address::ParseError;
use zcash_client_backend::PoolType;
use zcash_keys::keys::AddressGenerationError;
use zcash_primitives::zip32;
use zcash_primitives::{consensus::BlockHeight, transaction::components::amount::BalanceError};
Expand All @@ -16,7 +14,10 @@ use crate::wallet::commitment_tree;
use crate::PRUNING_DEPTH;

#[cfg(feature = "transparent-inputs")]
use zcash_primitives::legacy::TransparentAddress;
use {
zcash_client_backend::encoding::TransparentCodecError,
zcash_primitives::legacy::TransparentAddress,
};

/// The primary error type for the SQLite wallet backend.
#[derive(Debug)]
Expand All @@ -33,15 +34,16 @@ pub enum SqliteClientError {
/// Illegal attempt to reinitialize an already-initialized wallet database.
TableNotEmpty,

/// A Bech32-encoded key or address decoding error
Bech32DecodeError(Bech32DecodeError),
/// A Zcash key or address decoding error
DecodingError(ParseError),

/// An error produced in legacy transparent address derivation
#[cfg(feature = "transparent-inputs")]
HdwalletError(hdwallet::error::Error),

/// An error encountered in decoding a transparent address from its
/// serialized form.
#[cfg(feature = "transparent-inputs")]
TransparentAddress(TransparentCodecError),

/// Wrapper for rusqlite errors.
Expand Down Expand Up @@ -116,7 +118,6 @@ impl error::Error for SqliteClientError {
fn source(&self) -> Option<&(dyn error::Error + 'static)> {
match &self {
SqliteClientError::InvalidMemo(e) => Some(e),
SqliteClientError::Bech32DecodeError(Bech32DecodeError::Bech32Error(e)) => Some(e),
SqliteClientError::DbError(e) => Some(e),
SqliteClientError::Io(e) => Some(e),
SqliteClientError::BalanceError(e) => Some(e),
Expand All @@ -136,9 +137,10 @@ impl fmt::Display for SqliteClientError {
SqliteClientError::InvalidNote => write!(f, "Invalid note"),
SqliteClientError::RequestedRewindInvalid(h, r) =>
write!(f, "A rewind must be either of less than {} blocks, or at least back to block {} for your wallet; the requested height was {}.", PRUNING_DEPTH, h, r),
SqliteClientError::Bech32DecodeError(e) => write!(f, "{}", e),
SqliteClientError::DecodingError(e) => write!(f, "{}", e),
#[cfg(feature = "transparent-inputs")]
SqliteClientError::HdwalletError(e) => write!(f, "{:?}", e),
#[cfg(feature = "transparent-inputs")]
SqliteClientError::TransparentAddress(e) => write!(f, "{}", e),
SqliteClientError::TableNotEmpty => write!(f, "Table is not empty"),
SqliteClientError::DbError(e) => write!(f, "{}", e),
Expand Down Expand Up @@ -175,10 +177,9 @@ impl From<std::io::Error> for SqliteClientError {
SqliteClientError::Io(e)
}
}

impl From<Bech32DecodeError> for SqliteClientError {
fn from(e: Bech32DecodeError) -> Self {
SqliteClientError::Bech32DecodeError(e)
impl From<ParseError> for SqliteClientError {
fn from(e: ParseError) -> Self {
SqliteClientError::DecodingError(e)
}
}

Expand All @@ -195,6 +196,7 @@ impl From<hdwallet::error::Error> for SqliteClientError {
}
}

#[cfg(feature = "transparent-inputs")]
impl From<TransparentCodecError> for SqliteClientError {
fn from(e: TransparentCodecError) -> Self {
SqliteClientError::TransparentAddress(e)
Expand Down
Loading

0 comments on commit c610bba

Please sign in to comment.