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

Refactoring to address review comments. 

Signed-off-by: Daira-Emma Hopwood <daira@jacaranda.org>
This commit is contained in:
Daira-Emma Hopwood 2024-07-03 17:05:25 +01:00
parent b63ff5bfcd
commit e97da43409
9 changed files with 140 additions and 79 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

20
zcash_client_backend/CHANGELOG.md
View File

@ -53,14 +53,14 @@ funds to those addresses. See [ZIP 320](https://zips.z.cash/zip-0320) for detail
have changed as a consequence of this extraction; please see the `zip321`
CHANGELOG for details.
- `zcash_client_backend::data_api`:
- `WalletRead` has new `get_known_ephemeral_addresses` and
`get_transparent_address_metadata` methods.
- `WalletWrite` has a new `reserve_next_n_ephemeral_addresses` method.
- `error::Error` has a new `Address` variant.
- `WalletRead` has new `get_known_ephemeral_addresses`,
`find_account_for_ephemeral_address`, and `get_transparent_address_metadata`
methods when the "transparent-inputs" feature is enabled.
- `WalletWrite` has a new `reserve_next_n_ephemeral_addresses` method when
the "transparent-inputs" feature is enabled.
- `error::Error` has new `Address` and (when the "transparent-inputs" feature
is enabled) `PaysEphemeralTransparentAddress` variants.
- `wallet::input_selection::InputSelectorError` has a new `Address` variant.
- `zcash_client_backend::proto::proposal::Proposal::{from_standard_proposal,
try_into_standard_proposal}` each no longer require a `consensus::Parameters`
argument.
- `zcash_client_backend::data_api::fees`
- When the "transparent-inputs" feature is enabled, `ChangeValue` can also
represent an ephemeral transparent output in a proposal. Accordingly, the
@ -78,8 +78,10 @@ funds to those addresses. See [ZIP 320](https://zips.z.cash/zip-0320) for detail
- `zcash_client_backend::proposal::ProposalError` has new variants
`SpendsChange`, `EphemeralOutputLeftUnspent`, and `PaysTexFromShielded`.
(the last two are conditional on the "transparent-inputs" feature).
- `zcash_client_backend::proto::ProposalDecodingError` has a new variant
`InvalidEphemeralRecipient`.
- `zcash_client_backend::proto`:
- `ProposalDecodingError` has a new variant `InvalidEphemeralRecipient`.
- `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 `External` and `InternalAccount` variants
now wrap a `zcash_address::ZcashAddress`. This simplifies the process of

38
zcash_client_backend/src/data_api.rs
View File

@ -998,6 +998,36 @@ pub trait WalletRead {
) -> Result<HashMap<TransparentAddress, TransparentAddressMetadata>, Self::Error> {
Ok(HashMap::new())
}
/// If a given transparent address has been reserved, i.e. would be included in
/// the map returned by `get_known_ephemeral_addresses(account_id, false)` for any
/// of the wallet's accounts, then return `Ok(Some(account_id))`. Otherwise return
/// `Ok(None)`.
///
/// This is equivalent to (but may be implemented more efficiently than):
/// ```compile_fail
/// for account_id in self.get_account_ids()? {
/// if self.get_known_ephemeral_addresses(account_id, false)?.contains_key(address)? {
/// return Ok(Some(account_id));
/// }
/// }
/// Ok(None)
/// ```
#[cfg(feature = "transparent-inputs")]
fn find_account_for_ephemeral_address(
&self,
address: &TransparentAddress,
) -> Result<Option<Self::AccountId>, Self::Error> {
for account_id in self.get_account_ids()? {
if self
.get_known_ephemeral_addresses(account_id, false)?
.contains_key(address)
{
return Ok(Some(account_id));
}
}
Ok(None)
}
}
/// The relevance of a seed to a given wallet.
@ -1998,6 +2028,14 @@ pub mod testing {
) -> Result<HashMap<TransparentAddress, TransparentAddressMetadata>, Self::Error> {
Ok(HashMap::new())
}
#[cfg(feature = "transparent-inputs")]
fn find_account_for_ephemeral_address(
&self,
_address: &TransparentAddress,
) -> Result<Option<Self::AccountId>, Self::Error> {
Ok(None)
}
}
impl WalletWrite for MockWalletDb {

9
zcash_client_backend/src/data_api/error.rs
View File

@ -92,6 +92,11 @@ pub enum Error<DataSourceError, CommitmentTreeError, SelectionError, FeeError> {
/// belonging to the wallet.
#[cfg(feature = "transparent-inputs")]
AddressNotRecognized(TransparentAddress),
/// The wallet tried to pay to an ephemeral transparent address as a normal
/// output.
#[cfg(feature = "transparent-inputs")]
PaysEphemeralTransparentAddress(String),
}
impl<DE, CE, SE, FE> fmt::Display for Error<DE, CE, SE, FE>
@ -161,6 +166,10 @@ where
Error::AddressNotRecognized(_) => {
write!(f, "The specified transparent address was not recognized as belonging to the wallet.")
}
#[cfg(feature = "transparent-inputs")]
Error::PaysEphemeralTransparentAddress(addr) => {
write!(f, "The wallet tried to pay to an ephemeral transparent address as a normal output: {}", addr)
}
}
}
}

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

@ -77,6 +77,7 @@ use {
core::convert::Infallible,
input_selection::ShieldingSelector,
std::collections::HashMap,
zcash_keys::encoding::AddressCodec,
zcash_primitives::transaction::components::TxOut,
};
@ -1017,11 +1018,19 @@ where
transparent_output_meta: &mut Vec<_>,
to: TransparentAddress|
-> Result<(), ErrorT<DbT, InputsErrT, FeeRuleT>> {
// Always reject sending to one of our known ephemeral addresses.
#[cfg(feature = "transparent-inputs")]
if wallet_db
.find_account_for_ephemeral_address(&to)
.map_err(Error::DataSource)?
.is_some()
{
return Err(Error::PaysEphemeralTransparentAddress(to.encode(params)));
}
if payment.memo().is_some() {
return Err(Error::MemoForbidden);
} else {
builder.add_transparent_output(&to, payment.amount())?;
}
builder.add_transparent_output(&to, payment.amount())?;
transparent_output_meta.push((
Recipient::External(recipient_address.clone(), PoolType::TRANSPARENT),
to,

20
zcash_client_sqlite/src/lib.rs
View File

@ -88,6 +88,7 @@ use {
#[cfg(feature = "transparent-inputs")]
use {
zcash_client_backend::wallet::TransparentAddressMetadata,
zcash_keys::encoding::AddressCodec,
zcash_primitives::{
legacy::TransparentAddress,
transaction::components::{OutPoint, TxOut},
@ -561,6 +562,17 @@ impl<C: Borrow<rusqlite::Connection>, P: consensus::Parameters> WalletRead for W
for_detection,
)
}
#[cfg(feature = "transparent-inputs")]
fn find_account_for_ephemeral_address(
&self,
address: &TransparentAddress,
) -> Result<Option<Self::AccountId>, Self::Error> {
wallet::transparent::ephemeral::find_account_for_ephemeral_address_str(
self.conn.borrow(),
&address.encode(&self.params),
)
}
}
impl<P: consensus::Parameters> WalletWrite for WalletDb<rusqlite::Connection, P> {
@ -1470,14 +1482,6 @@ impl<P: consensus::Parameters> WalletWrite for WalletDb<rusqlite::Connection, P>
tx_ref,
)?;
}
#[cfg(feature = "transparent-inputs")]
Recipient::External(zcash_address, PoolType::Transparent) => {
// Always reject sending to one of our ephemeral addresses.
wallet::transparent::ephemeral::check_address_is_not_ephemeral(
wdb,
&zcash_address.encode(),
)?;
}
_ => {}
}
}

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

@ -443,7 +443,7 @@ pub(crate) fn send_multi_step_proposed_transfer<T: ShieldedPoolTester>() {
};
// Each transfer should use a different ephemeral address.
let (ephemeral0, txid0) = run_test(&mut st, 0);
let (ephemeral0, _) = run_test(&mut st, 0);
let (ephemeral1, _) = run_test(&mut st, 1);
assert!(ephemeral0 != ephemeral1);
@ -455,7 +455,7 @@ pub(crate) fn send_multi_step_proposed_transfer<T: ShieldedPoolTester>() {
Address::Transparent(TransparentAddress::PublicKeyHash(_))
);
// Attempting to use the same address again should cause an error.
// Attempting to pay to an ephemeral address should cause an error.
let proposal = st
.propose_standard_transfer::<Infallible>(
account.account_id(),
@ -476,7 +476,7 @@ pub(crate) fn send_multi_step_proposed_transfer<T: ShieldedPoolTester>() {
);
assert_matches!(
&create_proposed_result,
Err(Error::DataSource(SqliteClientError::EphemeralAddressReuse(addr, Some(txid)))) if addr == &ephemeral0 && txid == &txid0);
Err(Error::PaysEphemeralTransparentAddress(address_str)) if address_str == &ephemeral0);
}
#[cfg(feature = "transparent-inputs")]

3
zcash_client_sqlite/src/wallet.rs
View File

@ -68,7 +68,6 @@ use incrementalmerkletree::{Marking, Retention};
use rusqlite::{self, named_params, OptionalExtension};
use secrecy::{ExposeSecret, SecretVec};
use shardtree::{error::ShardTreeError, store::ShardStore, ShardTree};
use zcash_keys::encoding::encode_transparent_address_p;
use zip32::fingerprint::SeedFingerprint;
use std::collections::{HashMap, HashSet};
@ -2148,7 +2147,7 @@ fn recipient_params<P: consensus::Parameters>(
ephemeral_address,
..
} => (
Some(encode_transparent_address_p(params, ephemeral_address)),
Some(ephemeral_address.encode(params)),
Some(*receiving_account),
PoolType::TRANSPARENT,
),

21
zcash_client_sqlite/src/wallet/transparent.rs
View File

@ -3,7 +3,6 @@ use std::collections::{HashMap, HashSet};
use rusqlite::OptionalExtension;
use rusqlite::{named_params, Connection, Row};
use zcash_keys::encoding::encode_transparent_address_p;
use zip32::{DiversifierIndex, Scope};
use zcash_address::unified::{Encoding, Ivk, Uivk};
@ -468,7 +467,7 @@ pub(crate) fn get_transparent_address_metadata<P: consensus::Parameters>(
account_id: AccountId,
address: &TransparentAddress,
) -> Result<Option<TransparentAddressMetadata>, SqliteClientError> {
let address_str = encode_transparent_address_p(params, address);
let address_str = address.encode(params);
if let Some(di_vec) = conn
.query_row(
@ -494,14 +493,8 @@ pub(crate) fn get_transparent_address_metadata<P: consensus::Parameters>(
}
// Search ephemeral addresses that have already been reserved.
if let Some(raw_index) = conn
.query_row(
"SELECT address_index FROM ephemeral_addresses
WHERE account_id = :account_id AND address = :address",
named_params![":account_id": account_id.0, ":address": &address_str],
|row| row.get::<_, u32>(0),
)
.optional()?
if let Some(raw_index) =
ephemeral::find_index_for_ephemeral_address_str(conn, account_id, &address_str)?
{
let address_index = NonHardenedChildIndex::from_index(raw_index).unwrap();
return Ok(Some(ephemeral::metadata(address_index)));
@ -542,13 +535,7 @@ pub(crate) fn find_account_for_transparent_output<P: consensus::Parameters>(
}
// Search ephemeral addresses that have already been reserved.
if let Some(account_id) = conn
.query_row(
"SELECT account_id FROM ephemeral_addresses WHERE address = :address",
named_params![":address": &address_str],
|row| Ok(AccountId(row.get(0)?)),
)
.optional()?
if let Some(account_id) = ephemeral::find_account_for_ephemeral_address_str(conn, &address_str)?
{
return Ok(Some(account_id));
}

89
zcash_client_sqlite/src/wallet/transparent/ephemeral.rs
View File

@ -6,10 +6,7 @@ use std::ops::RangeInclusive;
use rusqlite::{named_params, OptionalExtension};
use zcash_client_backend::{data_api::Account, wallet::TransparentAddressMetadata};
use zcash_keys::{
encoding::{encode_transparent_address_p, AddressCodec},
keys::AddressGenerationError,
};
use zcash_keys::{encoding::AddressCodec, keys::AddressGenerationError};
use zcash_primitives::{
legacy::{
keys::{EphemeralIvk, NonHardenedChildIndex, TransparentKeyScope},
@ -158,6 +155,37 @@ pub(crate) fn get_known_ephemeral_addresses<P: consensus::Parameters>(
Ok(result)
}
/// If this is an ephemeral address in any account, return its account id.
pub(crate) fn find_account_for_ephemeral_address_str(
conn: &rusqlite::Connection,
address_str: &str,
) -> Result<Option<AccountId>, SqliteClientError> {
// Search ephemeral addresses that have already been reserved.
Ok(conn
.query_row(
"SELECT account_id FROM ephemeral_addresses WHERE address = :address",
named_params![":address": &address_str],
|row| Ok(AccountId(row.get(0)?)),
)
.optional()?)
}
#[cfg(feature = "transparent-inputs")]
pub(crate) fn find_index_for_ephemeral_address_str(
conn: &rusqlite::Connection,
account_id: AccountId,
address_str: &str,
) -> Result<Option<u32>, SqliteClientError> {
Ok(conn
.query_row(
"SELECT address_index FROM ephemeral_addresses
WHERE account_id = :account_id AND address = :address",
named_params![":account_id": account_id.0, ":address": &address_str],
|row| row.get::<_, u32>(0),
)
.optional()?)
}
/// Returns a vector with the next `n` previously unreserved ephemeral addresses for
/// the given account.
///
@ -215,29 +243,18 @@ pub(crate) fn reserve_next_n_ephemeral_addresses<P: consensus::Parameters>(
stmt_insert_ephemeral_address.execute(named_params![
":account_id": account_id.0,
":address_index": raw_index,
":address": encode_transparent_address_p(&wdb.params, &address)
":address": address.encode(&wdb.params),
])?;
Ok((address, metadata(address_index)))
})
.collect()
}
/// Returns a `SqliteClientError::EphemeralAddressReuse` error if `address` is
/// an ephemeral transparent address.
pub(crate) fn check_address_is_not_ephemeral<P: consensus::Parameters>(
wdb: &mut WalletDb<SqlTransaction<'_>, P>,
address_str: &str,
) -> Result<(), SqliteClientError> {
ephemeral_address_check_internal(wdb, address_str, true)
}
/// Returns a `SqliteClientError::EphemeralAddressReuse` error if the address was
/// already used. If `reject_all_ephemeral` is set, return an error if the address
/// is ephemeral at all, regardless of reuse.
fn ephemeral_address_check_internal<P: consensus::Parameters>(
/// already used.
fn ephemeral_address_reuse_check<P: consensus::Parameters>(
wdb: &mut WalletDb<SqlTransaction<'_>, P>,
address_str: &str,
reject_all_ephemeral: bool,
) -> Result<(), SqliteClientError> {
// It is intentional that we don't require `t.mined_height` to be non-null.
// That is, we conservatively treat an ephemeral address as potentially
@ -263,25 +280,21 @@ fn ephemeral_address_check_internal<P: consensus::Parameters>(
named_params![":address": address_str],
|row| row.get::<_, Option<Vec<u8>>>(0),
)
.optional()?;
.optional()?
.flatten();
match res {
Some(Some(txid_bytes)) => {
let txid = TxId::from_bytes(
txid_bytes
.try_into()
.map_err(|_| SqliteClientError::CorruptedData("invalid txid".to_owned()))?,
);
Err(SqliteClientError::EphemeralAddressReuse(
address_str.to_owned(),
Some(txid),
))
}
Some(None) if reject_all_ephemeral => Err(SqliteClientError::EphemeralAddressReuse(
if let Some(txid_bytes) = res {
let txid = TxId::from_bytes(
txid_bytes
.try_into()
.map_err(|_| SqliteClientError::CorruptedData("invalid txid".to_owned()))?,
);
Err(SqliteClientError::EphemeralAddressReuse(
address_str.to_owned(),
None,
)),
_ => Ok(()),
Some(txid),
))
} else {
Ok(())
}
}
@ -296,8 +309,8 @@ pub(crate) fn mark_ephemeral_address_as_used<P: consensus::Parameters>(
ephemeral_address: &TransparentAddress,
tx_ref: i64,
) -> Result<(), SqliteClientError> {
let address_str = encode_transparent_address_p(&wdb.params, ephemeral_address);
ephemeral_address_check_internal(wdb, &address_str, false)?;
let address_str = ephemeral_address.encode(&wdb.params);
ephemeral_address_reuse_check(wdb, &address_str)?;
wdb.conn.0.execute(
"UPDATE ephemeral_addresses SET used_in_tx = :used_in_tx WHERE address = :address",
@ -316,7 +329,7 @@ pub(crate) fn mark_ephemeral_address_as_mined<P: consensus::Parameters>(
address: &TransparentAddress,
tx_ref: i64,
) -> Result<(), SqliteClientError> {
let address_str = encode_transparent_address_p(&wdb.params, address);
let address_str = address.encode(&wdb.params);
// Figure out which transaction was mined earlier: `tx_ref`, or any existing
// tx referenced by `mined_in_tx` for the given address. Prefer the existing