Merge pull request #1491 from daira/consistent-orchard-enabling

Ensure that `zcash_client_sqlite` only enables "orchard" for `zcash_keys` when its own "orchard" feature is enabled
This commit is contained in:
Kris Nuttycombe 2024-08-11 19:14:56 -06:00 committed by GitHub
commit c6ff58c042
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
9 changed files with 48 additions and 38 deletions

View File

@ -441,13 +441,13 @@ where
}
Address::Unified(addr) => {
#[cfg(feature = "orchard")]
if addr.orchard().is_some() {
if addr.has_orchard() {
payment_pools.insert(*idx, PoolType::ORCHARD);
orchard_outputs.push(OrchardPayment(payment.amount()));
continue;
}
if addr.sapling().is_some() {
if addr.has_sapling() {
payment_pools.insert(*idx, PoolType::SAPLING);
sapling_outputs.push(SaplingPayment(payment.amount()));
continue;

View File

@ -29,7 +29,7 @@ rustdoc-args = ["--cfg", "docsrs"]
zcash_address.workspace = true
zcash_client_backend = { workspace = true, features = ["unstable-serialization", "unstable-spanning-tree"] }
zcash_encoding.workspace = true
zcash_keys = { workspace = true, features = ["orchard", "sapling"] }
zcash_keys = { workspace = true, features = ["sapling"] }
zcash_primitives.workspace = true
zcash_protocol.workspace = true
zip32.workspace = true

View File

@ -2159,8 +2159,6 @@ mod tests {
ufvk.sapling().cloned(),
#[cfg(feature = "orchard")]
ufvk.orchard().cloned(),
#[cfg(not(feature = "orchard"))]
None, // see zcash/librustzcash#1488
)
.unwrap();
assert_matches!(

View File

@ -373,12 +373,17 @@ pub(crate) fn add_account<P: consensus::Parameters>(
AccountSource::Imported { purpose } => (None, None, purpose == AccountPurpose::Spending),
};
#[cfg(feature = "orchard")]
let orchard_item = viewing_key
.ufvk()
.and_then(|ufvk| ufvk.orchard().map(|k| k.to_bytes()));
#[cfg(not(feature = "orchard"))]
let orchard_item: Option<Vec<u8>> = None;
let sapling_item = viewing_key
.ufvk()
.and_then(|ufvk| ufvk.sapling().map(|k| k.to_bytes()));
#[cfg(feature = "transparent-inputs")]
let transparent_item = viewing_key
.ufvk()
@ -685,6 +690,13 @@ pub(crate) fn get_account_for_ufvk<P: consensus::Parameters>(
params: &P,
ufvk: &UnifiedFullViewingKey,
) -> Result<Option<Account>, SqliteClientError> {
#[cfg(feature = "orchard")]
let orchard_item = ufvk.orchard().map(|k| k.to_bytes());
#[cfg(not(feature = "orchard"))]
let orchard_item: Option<Vec<u8>> = None;
let sapling_item = ufvk.sapling().map(|k| k.to_bytes());
#[cfg(feature = "transparent-inputs")]
let transparent_item = ufvk.transparent().map(|k| k.serialize());
#[cfg(not(feature = "transparent-inputs"))]
@ -701,8 +713,8 @@ pub(crate) fn get_account_for_ufvk<P: consensus::Parameters>(
let accounts = stmt
.query_and_then::<_, SqliteClientError, _, _>(
named_params![
":orchard_fvk_item_cache": ufvk.orchard().map(|k| k.to_bytes()),
":sapling_fvk_item_cache": ufvk.sapling().map(|k| k.to_bytes()),
":orchard_fvk_item_cache": orchard_item,
":sapling_fvk_item_cache": sapling_item,
":p2pkh_fvk_item_cache": transparent_item,
],
|row| {

View File

@ -166,9 +166,9 @@ mod tests {
Ok(Address::decode(&db_data.params, &row.get::<_, String>(0)?).unwrap())
}) {
Ok(Address::Unified(ua)) => {
assert!(ua.orchard().is_none());
assert!(ua.sapling().is_some());
assert_eq!(ua.transparent().is_some(), UA_TRANSPARENT);
assert!(!ua.has_orchard());
assert!(ua.has_sapling());
assert_eq!(ua.has_transparent(), UA_TRANSPARENT);
}
other => panic!("Unexpected result from address decoding: {:?}", other),
}
@ -184,9 +184,9 @@ mod tests {
Ok(Address::decode(&db_data.params, &row.get::<_, String>(0)?).unwrap())
}) {
Ok(Address::Unified(ua)) => {
assert_eq!(ua.orchard().is_some(), UA_ORCHARD);
assert!(ua.sapling().is_some());
assert_eq!(ua.transparent().is_some(), UA_TRANSPARENT);
assert_eq!(ua.has_orchard(), UA_ORCHARD);
assert!(ua.has_sapling());
assert_eq!(ua.has_transparent(), UA_TRANSPARENT);
}
other => panic!("Unexpected result from address decoding: {:?}", other),
}

View File

@ -133,8 +133,13 @@ mod tests {
.map_err(|_| SqliteClientError::KeyDerivationError(account_index))?;
let ufvk = usk.to_unified_full_viewing_key();
#[cfg(feature = "orchard")]
let orchard_item = ufvk.orchard().map(|k| k.to_bytes());
#[cfg(not(feature = "orchard"))]
let orchard_item: Option<Vec<u8>> = None;
let sapling_item = ufvk.sapling().map(|k| k.to_bytes());
#[cfg(feature = "transparent-inputs")]
let transparent_item = ufvk.transparent().map(|k| k.serialize());
#[cfg(not(feature = "transparent-inputs"))]

View File

@ -172,6 +172,13 @@ impl<P: consensus::Parameters> RusqliteMigration for Migration<P> {
.to_unified_incoming_viewing_key()
.encode(&self.params);
#[cfg(feature = "orchard")]
let orchard_item = ufvk_parsed.orchard().map(|k| k.to_bytes());
#[cfg(not(feature = "orchard"))]
let orchard_item: Option<Vec<u8>> = None;
let sapling_item = ufvk_parsed.sapling().map(|k| k.to_bytes());
#[cfg(feature = "transparent-inputs")]
let transparent_item = ufvk_parsed.transparent().map(|k| k.serialize());
#[cfg(not(feature = "transparent-inputs"))]
@ -220,8 +227,8 @@ impl<P: consensus::Parameters> RusqliteMigration for Migration<P> {
":account_index": account_index,
":ufvk": ufvk,
":uivk": uivk,
":orchard_fvk_item_cache": ufvk_parsed.orchard().map(|k| k.to_bytes()),
":sapling_fvk_item_cache": ufvk_parsed.sapling().map(|k| k.to_bytes()),
":orchard_fvk_item_cache": orchard_item,
":sapling_fvk_item_cache": sapling_item,
":p2pkh_fvk_item_cache": transparent_item,
":birthday_height": birthday_height,
":birthday_sapling_tree_size": birthday_sapling_tree_size,

View File

@ -415,21 +415,9 @@ impl Address {
matches!(pool_type, PoolType::Transparent)
}
Address::Unified(ua) => match pool_type {
PoolType::Transparent => ua.transparent().is_some(),
PoolType::Shielded(ShieldedProtocol::Sapling) => {
#[cfg(feature = "sapling")]
return ua.sapling().is_some();
#[cfg(not(feature = "sapling"))]
return false;
}
PoolType::Shielded(ShieldedProtocol::Orchard) => {
#[cfg(feature = "orchard")]
return ua.orchard().is_some();
#[cfg(not(feature = "orchard"))]
return false;
}
PoolType::Transparent => ua.has_transparent(),
PoolType::Shielded(ShieldedProtocol::Sapling) => ua.has_sapling(),
PoolType::Shielded(ShieldedProtocol::Orchard) => ua.has_orchard(),
},
}
}
@ -540,15 +528,15 @@ mod tests {
fn ua_parsing() {
for tv in test_vectors::UNIFIED {
match Address::decode(&MAIN_NETWORK, tv.unified_addr) {
Some(Address::Unified(_ua)) => {
Some(Address::Unified(ua)) => {
assert_eq!(
_ua.transparent().is_some(),
ua.has_transparent(),
tv.p2pkh_bytes.is_some() || tv.p2sh_bytes.is_some()
);
#[cfg(feature = "sapling")]
assert_eq!(_ua.sapling().is_some(), tv.sapling_raw_addr.is_some());
assert_eq!(ua.has_sapling(), tv.sapling_raw_addr.is_some());
#[cfg(feature = "orchard")]
assert_eq!(_ua.orchard().is_some(), tv.orchard_raw_addr.is_some());
assert_eq!(ua.has_orchard(), tv.orchard_raw_addr.is_some());
}
Some(_) => {
panic!(

View File

@ -1477,11 +1477,11 @@ mod tests {
Some(Address::Unified(tvua)) => {
// We always derive transparent and Sapling receivers, but not
// every value in the test vectors has these present.
if tvua.transparent().is_some() {
if tvua.has_transparent() {
assert_eq!(tvua.transparent(), ua.transparent());
}
#[cfg(feature = "sapling")]
if tvua.sapling().is_some() {
if tvua.has_sapling() {
assert_eq!(tvua.sapling(), ua.sapling());
}
}
@ -1658,11 +1658,11 @@ mod tests {
Some(Address::Unified(tvua)) => {
// We always derive transparent and Sapling receivers, but not
// every value in the test vectors has these present.
if tvua.transparent().is_some() {
if tvua.has_transparent() {
assert_eq!(tvua.transparent(), ua.transparent());
}
#[cfg(feature = "sapling")]
if tvua.sapling().is_some() {
if tvua.has_sapling() {
assert_eq!(tvua.sapling(), ua.sapling());
}
}