Merge pull request #1252 from zcash/orchard-test-fixes

Various fixes to Orchard tests
This commit is contained in:
Kris Nuttycombe 2024-03-10 16:15:02 -06:00 committed by GitHub
commit 1003cd66ea
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
5 changed files with 68 additions and 28 deletions

View File

@ -73,6 +73,8 @@ and this library adheres to Rust's notion of
constraint on its `<AccountId>` parameter has been strengthened to `Copy`.
- `zcash_client_backend::fees`:
- Arguments to `ChangeStrategy::compute_balance` have changed.
- `zcash_client_backend::proto`:
- `ProposalDecodingError` has a new variant `TransparentMemo`.
- `zcash_client_backend::zip321::render::amount_str` now takes a
`NonNegativeAmount` rather than a signed `Amount` as its argument.
- `zcash_client_backend::zip321::parse::parse_amount` now parses a

View File

@ -326,6 +326,8 @@ pub enum ProposalDecodingError<DbError> {
ProposalInvalid(ProposalError),
/// An inputs field for the given protocol was present, but contained no input note references.
EmptyShieldedInputs(ShieldedProtocol),
/// A memo field was provided for a transparent output.
TransparentMemo,
/// Change outputs to the specified pool are not supported.
InvalidChangeRecipient(PoolType),
}
@ -378,6 +380,9 @@ impl<E: Display> Display for ProposalDecodingError<E> {
"An inputs field was present for {:?}, but contained no note references.",
protocol
),
ProposalDecodingError::TransparentMemo => {
write!(f, "Transparent outputs cannot have memos.")
}
ProposalDecodingError::InvalidChangeRecipient(pool_type) => write!(
f,
"Change outputs to the {} pool are not supported.",
@ -704,20 +709,26 @@ impl proposal::Proposal {
.proposed_change
.iter()
.map(|cv| -> Result<ChangeValue, ProposalDecodingError<_>> {
let value = NonNegativeAmount::from_u64(cv.value)
.map_err(|_| ProposalDecodingError::BalanceInvalid)?;
let memo = cv
.memo
.as_ref()
.map(|bytes| {
MemoBytes::from_bytes(&bytes.value)
.map_err(ProposalDecodingError::MemoInvalid)
})
.transpose()?;
match cv.pool_type()? {
PoolType::Shielded(ShieldedProtocol::Sapling) => {
Ok(ChangeValue::sapling(
NonNegativeAmount::from_u64(cv.value).map_err(
|_| ProposalDecodingError::BalanceInvalid,
)?,
cv.memo
.as_ref()
.map(|bytes| {
MemoBytes::from_bytes(&bytes.value)
.map_err(ProposalDecodingError::MemoInvalid)
})
.transpose()?,
))
Ok(ChangeValue::sapling(value, memo))
}
#[cfg(feature = "orchard")]
PoolType::Shielded(ShieldedProtocol::Orchard) => {
Ok(ChangeValue::orchard(value, memo))
}
PoolType::Transparent if memo.is_some() => {
Err(ProposalDecodingError::TransparentMemo)
}
t => Err(ProposalDecodingError::InvalidChangeRecipient(t)),
}

View File

@ -806,7 +806,7 @@ impl<Cache> TestState<Cache> {
min_confirmations: u32,
) -> NonNegativeAmount {
self.with_account_balance(account, min_confirmations, |balance| {
balance.sapling_balance().spendable_value()
balance.spendable_value()
})
}
@ -816,8 +816,7 @@ impl<Cache> TestState<Cache> {
min_confirmations: u32,
) -> NonNegativeAmount {
self.with_account_balance(account, min_confirmations, |balance| {
balance.sapling_balance().value_pending_spendability()
+ balance.sapling_balance().change_pending_confirmation()
balance.value_pending_spendability() + balance.change_pending_confirmation()
})
.unwrap()
}
@ -829,7 +828,7 @@ impl<Cache> TestState<Cache> {
min_confirmations: u32,
) -> NonNegativeAmount {
self.with_account_balance(account, min_confirmations, |balance| {
balance.sapling_balance().change_pending_confirmation()
balance.change_pending_confirmation()
})
}

View File

@ -689,7 +689,7 @@ pub(crate) fn spend_fails_on_locked_notes<T: ShieldedPoolTester>() {
// until just before the first transaction expires
for i in 1..42 {
st.generate_next_block(
&T::sk_to_fvk(&T::sk(&[i as u8])),
&T::sk_to_fvk(&T::sk(&[i as u8; 32])),
AddressType::DefaultExternal,
value,
);
@ -717,7 +717,7 @@ pub(crate) fn spend_fails_on_locked_notes<T: ShieldedPoolTester>() {
// Mine block SAPLING_ACTIVATION_HEIGHT + 42 so that the first transaction expires
let (h43, _, _) = st.generate_next_block(
&T::sk_to_fvk(&T::sk(&[42])),
&T::sk_to_fvk(&T::sk(&[42; 32])),
AddressType::DefaultExternal,
value,
);
@ -837,7 +837,7 @@ pub(crate) fn ovk_policy_prevents_recovery_from_chain<T: ShieldedPoolTester>() {
// so that the first transaction expires
for i in 1..=42 {
st.generate_next_block(
&T::sk_to_fvk(&T::sk(&[i as u8])),
&T::sk_to_fvk(&T::sk(&[i as u8; 32])),
AddressType::DefaultExternal,
value,
);

View File

@ -554,6 +554,7 @@ pub(crate) mod tests {
consensus::{BlockHeight, NetworkUpgrade, Parameters},
transaction::components::amount::NonNegativeAmount,
};
use zcash_protocol::ShieldedProtocol;
use crate::{
error::SqliteClientError,
@ -566,7 +567,10 @@ pub(crate) mod tests {
};
#[cfg(feature = "orchard")]
use crate::wallet::orchard::tests::OrchardPoolTester;
use {
crate::wallet::orchard::tests::OrchardPoolTester, orchard::tree::MerkleHashOrchard,
zcash_client_backend::data_api::ORCHARD_SHARD_HEIGHT,
};
#[test]
fn sapling_scan_complete() {
@ -714,22 +718,32 @@ pub(crate) mod tests {
let st = TestBuilder::new()
.with_block_cache()
.with_test_account(|network| {
// We set the Sapling frontier at the birthday height to be 1234 notes
// into the second shard.
// We set the Sapling and Orchard frontiers at the birthday height to be
// 1234 notes into the second shard.
let birthday_height =
network.activation_height(NetworkUpgrade::Nu5).unwrap() + offset;
let sapling_frontier_position = Position::from((0x1 << 16) + 1234);
let frontier_position = Position::from((0x1 << 16) + 1234);
let sapling_frontier = Frontier::from_parts(
sapling_frontier_position,
frontier_position,
Node::empty_leaf(),
vec![Node::empty_leaf(); sapling_frontier_position.past_ommer_count().into()],
vec![Node::empty_leaf(); frontier_position.past_ommer_count().into()],
)
.unwrap();
#[cfg(feature = "orchard")]
let orchard_frontier = Frontier::from_parts(
frontier_position,
MerkleHashOrchard::empty_leaf(),
vec![
MerkleHashOrchard::empty_leaf();
frontier_position.past_ommer_count().into()
],
)
.unwrap();
AccountBirthday::from_parts(
birthday_height,
sapling_frontier,
#[cfg(feature = "orchard")]
Frontier::empty(),
orchard_frontier,
None,
)
})
@ -1138,9 +1152,15 @@ pub(crate) mod tests {
// wallet birthday but before the end of the shard.
let summary = st.get_wallet_summary(1);
assert_eq!(summary.as_ref().map(|s| T::next_subtree_index(s)), Some(0),);
// Progress denominator depends on which pools are enabled (which changes the
// initial tree states in `test_with_nu5_birthday_offset`).
let expected_denom = 1 << SAPLING_SHARD_HEIGHT;
#[cfg(feature = "orchard")]
let expected_denom = expected_denom + (1 << ORCHARD_SHARD_HEIGHT);
assert_eq!(
summary.and_then(|s| s.scan_progress()),
Some(Ratio::new(1, 0x1 << SAPLING_SHARD_HEIGHT))
Some(Ratio::new(1, expected_denom))
);
// Now simulate shutting down, and then restarting 70 blocks later, after a shard
@ -1178,10 +1198,18 @@ pub(crate) mod tests {
// We've crossed a subtree boundary, and so still only have one scanned note but have two
// shards worth of notes to scan.
let expected_denom = expected_denom
+ match T::SHIELDED_PROTOCOL {
ShieldedProtocol::Sapling => 1 << SAPLING_SHARD_HEIGHT,
#[cfg(feature = "orchard")]
ShieldedProtocol::Orchard => 1 << ORCHARD_SHARD_HEIGHT,
#[cfg(not(feature = "orchard"))]
ShieldedProtocol::Orchard => unreachable!(),
};
let summary = st.get_wallet_summary(1);
assert_eq!(
summary.and_then(|s| s.scan_progress()),
Some(Ratio::new(1, 0x1 << (SAPLING_SHARD_HEIGHT + 1)))
Some(Ratio::new(1, expected_denom))
);
}