diff --git a/zcash_client_backend/CHANGELOG.md b/zcash_client_backend/CHANGELOG.md index 0f47b0ad4..0b4cb668d 100644 --- a/zcash_client_backend/CHANGELOG.md +++ b/zcash_client_backend/CHANGELOG.md @@ -73,6 +73,8 @@ and this library adheres to Rust's notion of constraint on its `` 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 diff --git a/zcash_client_backend/src/proto.rs b/zcash_client_backend/src/proto.rs index 50cf4b09f..f020e731d 100644 --- a/zcash_client_backend/src/proto.rs +++ b/zcash_client_backend/src/proto.rs @@ -326,6 +326,8 @@ pub enum ProposalDecodingError { 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 Display for ProposalDecodingError { "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> { + 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)), } diff --git a/zcash_client_sqlite/src/testing.rs b/zcash_client_sqlite/src/testing.rs index 7d63401a8..1fd22e1c5 100644 --- a/zcash_client_sqlite/src/testing.rs +++ b/zcash_client_sqlite/src/testing.rs @@ -806,7 +806,7 @@ impl TestState { 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 TestState { 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 TestState { min_confirmations: u32, ) -> NonNegativeAmount { self.with_account_balance(account, min_confirmations, |balance| { - balance.sapling_balance().change_pending_confirmation() + balance.change_pending_confirmation() }) } diff --git a/zcash_client_sqlite/src/testing/pool.rs b/zcash_client_sqlite/src/testing/pool.rs index 25da006ef..16150165f 100644 --- a/zcash_client_sqlite/src/testing/pool.rs +++ b/zcash_client_sqlite/src/testing/pool.rs @@ -689,7 +689,7 @@ pub(crate) fn spend_fails_on_locked_notes() { // 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() { // 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() { // 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, ); diff --git a/zcash_client_sqlite/src/wallet/scanning.rs b/zcash_client_sqlite/src/wallet/scanning.rs index 83bff8698..e23dfc46b 100644 --- a/zcash_client_sqlite/src/wallet/scanning.rs +++ b/zcash_client_sqlite/src/wallet/scanning.rs @@ -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)) ); }