zcash_client_sqlite: Update default ephemeral gap limit to 5

zcash_client_backend/src/data_api/testing.rs

@@ -77,6 +77,7 @@ use {
     crate::wallet::TransparentAddressMetadata,
     ::transparent::{address::TransparentAddress, keys::NonHardenedChildIndex},
     std::ops::Range,
+    transparent::GapLimits,
 };
 
 #[cfg(feature = "orchard")]
@@ -1364,7 +1365,11 @@ pub trait DataStoreFactory {
         + WalletCommitmentTrees;
 
     /// Constructs a new data store.
-    fn new_data_store(&self, network: LocalNetwork) -> Result<Self::DataStore, Self::Error>;
+    fn new_data_store(
+        &self,
+        network: LocalNetwork,
+        #[cfg(feature = "transparent-inputs")] gap_limits: GapLimits,
+    ) -> Result<Self::DataStore, Self::Error>;
 }
 
 /// A [`TestState`] builder, that configures the environment for a test.
@@ -1376,6 +1381,8 @@ pub struct TestBuilder<Cache, DataStoreFactory> {
     initial_chain_state: Option<InitialChainState>,
     account_birthday: Option<AccountBirthday>,
     account_index: Option<zip32::AccountId>,
+    #[cfg(feature = "transparent-inputs")]
+    gap_limits: GapLimits,
 }
 
 impl TestBuilder<(), ()> {
@@ -1405,6 +1412,8 @@ impl TestBuilder<(), ()> {
             initial_chain_state: None,
             account_birthday: None,
             account_index: None,
+            #[cfg(feature = "transparent-inputs")]
+            gap_limits: GapLimits::new(10, 5, 5),
         }
     }
 }
@@ -1426,6 +1435,8 @@ impl<A> TestBuilder<(), A> {
             initial_chain_state: self.initial_chain_state,
             account_birthday: self.account_birthday,
             account_index: self.account_index,
+            #[cfg(feature = "transparent-inputs")]
+            gap_limits: self.gap_limits,
         }
     }
 }
@@ -1444,6 +1455,24 @@ impl<A> TestBuilder<A, ()> {
             initial_chain_state: self.initial_chain_state,
             account_birthday: self.account_birthday,
             account_index: self.account_index,
+            #[cfg(feature = "transparent-inputs")]
+            gap_limits: self.gap_limits,
+        }
+    }
+}
+
+impl<A, B> TestBuilder<A, B> {
+    #[cfg(feature = "transparent-inputs")]
+    pub fn with_gap_limits(self, gap_limits: GapLimits) -> TestBuilder<A, B> {
+        TestBuilder {
+            rng: self.rng,
+            network: self.network,
+            cache: self.cache,
+            ds_factory: self.ds_factory,
+            initial_chain_state: self.initial_chain_state,
+            account_birthday: self.account_birthday,
+            account_index: self.account_index,
+            gap_limits,
         }
     }
 }
@@ -1604,7 +1633,10 @@ impl<Cache, DsFactory: DataStoreFactory> TestBuilder<Cache, DsFactory> {
     /// Builds the state for this test.
     pub fn build(self) -> TestState<Cache, DsFactory::DataStore, LocalNetwork> {
         let mut cached_blocks = BTreeMap::new();
-        let mut wallet_data = self.ds_factory.new_data_store(self.network).unwrap();
+        let mut wallet_data = self
+            .ds_factory
+            .new_data_store(self.network, self.gap_limits)
+            .unwrap();
 
         if let Some(initial_state) = &self.initial_chain_state {
             wallet_data

zcash_client_backend/src/data_api/testing/pool.rs

@@ -86,11 +86,6 @@ use zcash_protocol::PoolType;
 #[cfg(feature = "pczt")]
 use pczt::roles::{prover::Prover, signer::Signer};
 
-/// The number of ephemeral addresses that can be safely reserved without observing any
-/// of them to be mined.
-#[cfg(feature = "transparent-inputs")]
-pub const EPHEMERAL_ADDR_GAP_LIMIT: usize = 3;
-
 /// Trait that exposes the pool-specific types and operations necessary to run the
 /// single-shielded-pool tests on a given pool.
 ///
@@ -538,12 +533,14 @@ pub fn send_multi_step_proposed_transfer<T: ShieldedPoolTester, DSF>(
 {
     use ::transparent::builder::TransparentSigningSet;
 
-    use crate::data_api::OutputOfSentTx;
+    use crate::data_api::{testing::transparent::GapLimits, OutputOfSentTx};
 
+    let gap_limits = GapLimits::new(10, 5, 3);
     let mut st = TestBuilder::new()
         .with_data_store_factory(ds_factory)
         .with_block_cache(cache)
         .with_account_from_sapling_activation(BlockHash([0; 32]))
+        .with_gap_limits(gap_limits)
         .build();
 
     let account = st.test_account().cloned().unwrap();
@@ -750,7 +747,10 @@ pub fn send_multi_step_proposed_transfer<T: ShieldedPoolTester, DSF>(
         .wallet()
         .get_known_ephemeral_addresses(account_id, None)
         .unwrap();
-    assert_eq!(known_addrs.len(), EPHEMERAL_ADDR_GAP_LIMIT + 2);
+    assert_eq!(
+        known_addrs.len(),
+        usize::try_from(gap_limits.ephemeral() + 2).unwrap()
+    );
 
     // Check that the addresses are all distinct.
     let known_set: HashSet<_> = known_addrs.iter().map(|(addr, _)| addr).collect();
@@ -775,7 +775,7 @@ pub fn send_multi_step_proposed_transfer<T: ShieldedPoolTester, DSF>(
         },
     );
     let mut transparent_signing_set = TransparentSigningSet::new();
-    let (colliding_addr, _) = &known_addrs[EPHEMERAL_ADDR_GAP_LIMIT - 1];
+    let (colliding_addr, _) = &known_addrs[usize::try_from(gap_limits.ephemeral() - 1).unwrap()];
     let utxo_value = (value - zip317::MINIMUM_FEE).unwrap();
     assert_matches!(
         builder.add_transparent_output(colliding_addr, utxo_value),
@@ -818,7 +818,7 @@ pub fn send_multi_step_proposed_transfer<T: ShieldedPoolTester, DSF>(
     let txid = build_result.transaction().txid();
 
     // Now, store the transaction, pretending it has been mined (we will actually mine the block
-    // next). This will cause the the gap start to move & a new `EPHEMERAL_ADDR_GAP_LIMIT` of
+    // next). This will cause the the gap start to move & a new `gap_limits.ephemeral()` of
     // addresses to be created.
     let target_height = st.latest_cached_block().unwrap().height() + 1;
     st.wallet_mut()
@@ -839,46 +839,52 @@ pub fn send_multi_step_proposed_transfer<T: ShieldedPoolTester, DSF>(
     st.scan_cached_blocks(h, 1);
     assert_eq!(h, target_height);
 
-    // At this point the start of the gap should be at index `EPHEMERAL_ADDR_GAP_LIMIT` and the new
-    // size of the known address set should be `EPHEMERAL_ADDR_GAP_LIMIT * 2`.
+    // At this point the start of the gap should be at index `gap_limits.ephemeral()` and the new
+    // size of the known address set should be `gap_limits.ephemeral() * 2`.
     let new_known_addrs = st
         .wallet()
         .get_known_ephemeral_addresses(account_id, None)
         .unwrap();
-    assert_eq!(new_known_addrs.len(), EPHEMERAL_ADDR_GAP_LIMIT * 2);
+    assert_eq!(
+        new_known_addrs.len(),
+        usize::try_from(gap_limits.ephemeral() * 2).unwrap()
+    );
     assert!(new_known_addrs.starts_with(&known_addrs));
 
-    let reservation_should_succeed = |st: &mut TestState<_, DSF::DataStore, _>, n| {
+    let reservation_should_succeed = |st: &mut TestState<_, DSF::DataStore, _>, n: u32| {
         let reserved = st
             .wallet_mut()
-            .reserve_next_n_ephemeral_addresses(account_id, n)
+            .reserve_next_n_ephemeral_addresses(account_id, n.try_into().unwrap())
             .unwrap();
-        assert_eq!(reserved.len(), n);
+        assert_eq!(reserved.len(), usize::try_from(n).unwrap());
         reserved
     };
     let reservation_should_fail =
-        |st: &mut TestState<_, DSF::DataStore, _>, n, expected_bad_index| {
+        |st: &mut TestState<_, DSF::DataStore, _>, n: u32, expected_bad_index| {
             assert_matches!(st
             .wallet_mut()
-            .reserve_next_n_ephemeral_addresses(account_id, n),
+            .reserve_next_n_ephemeral_addresses(account_id, n.try_into().unwrap()),
             Err(e) if is_reached_gap_limit(&e, account_id, expected_bad_index));
         };
 
     let next_reserved = reservation_should_succeed(&mut st, 1);
-    assert_eq!(next_reserved[0], known_addrs[EPHEMERAL_ADDR_GAP_LIMIT]);
+    assert_eq!(
+        next_reserved[0],
+        known_addrs[usize::try_from(gap_limits.ephemeral()).unwrap()]
+    );
 
     // The range of address indices that are safe to reserve now is
-    // 0..(EPHEMERAL_ADDR_GAP_LIMIT * 2 - 1)`, and we have already reserved or used
-    // `EPHEMERAL_ADDR_GAP_LIMIT + 1`, addresses, so trying to reserve another
-    // `EPHEMERAL_ADDR_GAP_LIMIT` should fail.
+    // 0..(gap_limits.ephemeral() * 2 - 1)`, and we have already reserved or used
+    // `gap_limits.ephemeral() + 1`, addresses, so trying to reserve another
+    // `gap_limits.ephemeral()` should fail.
     reservation_should_fail(
         &mut st,
-        EPHEMERAL_ADDR_GAP_LIMIT,
-        (EPHEMERAL_ADDR_GAP_LIMIT * 2) as u32,
+        gap_limits.ephemeral(),
+        gap_limits.ephemeral() * 2,
     );
-    reservation_should_succeed(&mut st, EPHEMERAL_ADDR_GAP_LIMIT - 1);
+    reservation_should_succeed(&mut st, gap_limits.ephemeral() - 1);
     // Now we've reserved everything we can, we can't reserve one more
-    reservation_should_fail(&mut st, 1, (EPHEMERAL_ADDR_GAP_LIMIT * 2) as u32);
+    reservation_should_fail(&mut st, 1, gap_limits.ephemeral() * 2);
 }
 
 #[cfg(feature = "transparent-inputs")]

zcash_client_backend/src/data_api/testing/transparent.rs

@@ -387,15 +387,15 @@ impl GapLimits {
         }
     }
 
-    pub(crate) fn external(&self) -> u32 {
+    pub fn external(&self) -> u32 {
         self.external
     }
 
-    pub(crate) fn internal(&self) -> u32 {
+    pub fn internal(&self) -> u32 {
         self.internal
     }
 
-    pub(crate) fn ephemeral(&self) -> u32 {
+    pub fn ephemeral(&self) -> u32 {
         self.ephemeral
     }
 }

zcash_client_sqlite/src/lib.rs

@@ -306,7 +306,7 @@ impl GapLimits {
 ///
 /// - external addresses: 10
 /// - transparent internal (change) addresses: 5
-/// - ephemeral addresses: 3
+/// - ephemeral addresses: 5
 ///
 /// These limits are chosen with the following rationale:
 /// - At present, many wallets query light wallet servers with a set of addresses, because querying
@@ -331,12 +331,15 @@ impl Default for GapLimits {
         Self {
             external: 10,
             internal: 5,
-            ephemeral: 3,
+            ephemeral: 5,
         }
     }
 }
 
-#[cfg(all(test, feature = "transparent-inputs"))]
+#[cfg(all(
+    any(test, feature = "test-dependencies"),
+    feature = "transparent-inputs"
+))]
 impl From<GapLimits> for zcash_client_backend::data_api::testing::transparent::GapLimits {
     fn from(value: GapLimits) -> Self {
         zcash_client_backend::data_api::testing::transparent::GapLimits::new(
@@ -347,6 +350,16 @@ impl From<GapLimits> for zcash_client_backend::data_api::testing::transparent::G
     }
 }
 
+#[cfg(all(
+    any(test, feature = "test-dependencies"),
+    feature = "transparent-inputs"
+))]
+impl From<zcash_client_backend::data_api::testing::transparent::GapLimits> for GapLimits {
+    fn from(value: zcash_client_backend::data_api::testing::transparent::GapLimits) -> Self {
+        GapLimits::from_parts(value.external(), value.internal(), value.ephemeral())
+    }
+}
+
 /// A wrapper for the SQLite connection to the wallet database, along with a capability to read the
 /// system from the clock. A `WalletDb` encapsulates the full set of capabilities that are required
 /// in order to implement the [`WalletRead`], [`WalletWrite`] and [`WalletCommitmentTrees`] traits.

zcash_client_sqlite/src/testing/db.rs

@@ -3,6 +3,7 @@ use rusqlite::Connection;
 use std::num::NonZeroU32;
 use std::time::Duration;
 use std::{collections::HashMap, time::SystemTime};
+use testing::transparent::GapLimits;
 use uuid::Uuid;
 
 use tempfile::NamedTempFile;
@@ -173,9 +174,18 @@ impl DataStoreFactory for TestDbFactory {
     type DsError = SqliteClientError;
     type DataStore = TestDb;
 
-    fn new_data_store(&self, network: LocalNetwork) -> Result<Self::DataStore, Self::Error> {
+    fn new_data_store(
+        &self,
+        network: LocalNetwork,
+        #[cfg(feature = "transparent-inputs")] gap_limits: GapLimits,
+    ) -> Result<Self::DataStore, Self::Error> {
         let data_file = NamedTempFile::new().unwrap();
         let mut db_data = WalletDb::for_path(data_file.path(), network, test_clock()).unwrap();
+        #[cfg(feature = "transparent-inputs")]
+        {
+            db_data = db_data.with_gap_limits(gap_limits.into());
+        }
+
         if let Some(migrations) = &self.target_migrations {
             init_wallet_db_internal(&mut db_data, None, migrations, true).unwrap();
         } else {
@@ -190,9 +200,12 @@ impl Reset for TestDb {
 
     fn reset<C>(st: &mut TestState<C, Self, LocalNetwork>) -> NamedTempFile {
         let network = *st.network();
+        let gap_limits = st.wallet().db().gap_limits;
         let old_db = std::mem::replace(
             st.wallet_mut(),
-            TestDbFactory::default().new_data_store(network).unwrap(),
+            TestDbFactory::default()
+                .new_data_store(network, gap_limits.into())
+                .unwrap(),
         );
         old_db.take_data_file()
     }