Make init_accounts_table only permit sequential account identifiers.

Also, check to ensure that account creation does not exceed the
maximum account identifier value.

zcash_client_backend/src/data_api.rs

@@ -1,6 +1,5 @@
 //! Interfaces for wallet data persistence & low-level wallet utilities.
 
-use secrecy::SecretVec;
 use std::cmp;
 use std::collections::HashMap;
 use std::fmt::Debug;
@@ -8,6 +7,7 @@ use std::fmt::Debug;
 #[cfg(feature = "transparent-inputs")]
 use std::collections::HashSet;
 
+use secrecy::SecretVec;
 use zcash_primitives::{
     block::BlockHash,
     consensus::BlockHeight,

zcash_client_sqlite/CHANGELOG.md

@@ -18,6 +18,10 @@ and this library adheres to Rust's notion of
     rewinds exceed supported bounds.
   - `SqliteClientError::DiversifierIndexOutOfRange`, to report when the space
     of available diversifier indices has been exhausted. 
+  - `SqliteClientError::AccountIdDiscontinuity`, to report when a user attempts
+    to initialize the accounts table with a noncontiguous set of account identifiers.
+  - `SqliteClientError::AccountIdOutOfRange`, to report when the maximum account
+    identifier has been reached.
 - An `unstable` feature flag; this is added to parts of the API that may change
   in any release. It enables `zcash_client_backend`'s `unstable` feature flag.
 - New summary views that may be directly accessed in the sqlite database.

zcash_client_sqlite/src/error.rs

@@ -73,6 +73,13 @@ pub enum SqliteClientError {
     /// An error occurred deriving a spending key from a seed and an account
     /// identifier.
     KeyDerivationError(AccountId),
+
+    /// A caller attempted to initialize the accounts table with a discontinuous
+    /// set of account identifiers.
+    AccountIdDiscontinuity,
+
+    /// A caller attempted to construct a new account with an invalid account identifier.
+    AccountIdOutOfRange,
 }
 
 impl error::Error for SqliteClientError {
@@ -113,6 +120,8 @@ impl fmt::Display for SqliteClientError {
             SqliteClientError::BackendError(e) => write!(f, "{}", e),
             SqliteClientError::DiversifierIndexOutOfRange => write!(f, "The space of available diversifier indices is exhausted"),
             SqliteClientError::KeyDerivationError(acct_id) => write!(f, "Key derivation failed for account {:?}", acct_id),
+            SqliteClientError::AccountIdDiscontinuity => write!(f, "Wallet account identifiers must be sequential."),
+            SqliteClientError::AccountIdOutOfRange => write!(f, "Wallet account identifiers must be less than 0x7FFFFFFF."),
         }
     }
 }

zcash_client_sqlite/src/lib.rs

@@ -412,6 +412,10 @@ impl<'a, P: consensus::Parameters> WalletWrite for DataConnStmtCache<'a, P> {
                 .map(|a| AccountId::from(u32::from(a) + 1))
                 .unwrap_or_else(|| AccountId::from(0));
 
+            if u32::from(account) >= 0x7FFFFFFF {
+                return Err(SqliteClientError::AccountIdOutOfRange);
+            }
+
             let usk = UnifiedSpendingKey::from_seed(
                 &stmts.wallet_db.params,
                 seed.expose_secret(),

zcash_client_sqlite/src/wallet/init.rs

@@ -586,6 +586,13 @@ pub fn init_accounts_table<P: consensus::Parameters>(
         return Err(SqliteClientError::TableNotEmpty);
     }
 
+    // Ensure that the account identifiers are sequential and begin at zero.
+    if let Some(account_id) = keys.keys().max() {
+        if usize::try_from(u32::from(*account_id)).unwrap() >= keys.len() {
+            return Err(SqliteClientError::AccountIdDiscontinuity);
+        }
+    }
+
     // Insert accounts atomically
     wdb.conn.execute("BEGIN IMMEDIATE", NO_PARAMS)?;
     for (account, key) in keys.iter() {
@@ -677,6 +684,7 @@ mod tests {
     };
 
     use crate::{
+        error::SqliteClientError,
         tests::{self, network},
         wallet::get_address,
         AccountId, WalletDb,
@@ -1370,6 +1378,35 @@ mod tests {
         init_accounts_table(&db_data, &ufvks).unwrap_err();
     }
 
+    #[test]
+    fn init_accounts_table_allows_no_gaps() {
+        let data_file = NamedTempFile::new().unwrap();
+        let mut db_data = WalletDb::for_path(data_file.path(), network()).unwrap();
+        init_wallet_db(&mut db_data, Some(Secret::new(vec![]))).unwrap();
+
+        // allow sequential initialization
+        let seed = [0u8; 32];
+        let ufvks = |ids: &[u32]| {
+            ids.iter()
+                .map(|a| {
+                    let account = AccountId::from(*a);
+                    UnifiedSpendingKey::from_seed(&network(), &seed, account)
+                        .map(|k| (account, k.to_unified_full_viewing_key()))
+                        .unwrap()
+                })
+                .collect::<HashMap<_, _>>()
+        };
+
+        // should fail if we have a gap
+        assert!(matches!(
+            init_accounts_table(&db_data, &ufvks(&[0, 2])),
+            Err(SqliteClientError::AccountIdDiscontinuity)
+        ));
+
+        // should succeed if there are no gaps
+        assert!(init_accounts_table(&db_data, &ufvks(&[0, 1, 2])).is_ok());
+    }
+
     #[test]
     fn init_blocks_table_only_works_once() {
         let data_file = NamedTempFile::new().unwrap();