Extend the `send_multi_step_proposed_transfer` test to check the behaviour

when another wallet creates a transaction with an output to one of our
ephemeral addresses, and repair the implementation to pass this test.

Signed-off-by: Daira-Emma Hopwood <daira@jacaranda.org>

zcash_client_sqlite/src/lib.rs

@@ -1099,6 +1099,8 @@ impl<P: consensus::Parameters> WalletWrite for WalletDb<rusqlite::Connection, P>
         self.transactionally(|wdb| {
             let tx_ref = wallet::put_tx_data(wdb.conn.0, d_tx.tx(), None, None)?;
             let funding_accounts = wallet::get_funding_accounts(wdb.conn.0, d_tx.tx())?;
+
+            // TODO(#1305): Correctly track accounts that fund each transaction output.
             let funding_account = funding_accounts.iter().next().copied();
             if funding_accounts.len() > 1 {
                 warn!(
@@ -1287,38 +1289,27 @@ impl<P: consensus::Parameters> WalletWrite for WalletDb<rusqlite::Connection, P>
                 wallet::transparent::mark_transparent_utxo_spent(wdb.conn.0, tx_ref, &txin.prevout)?;
             }
 
-            // If we have some transparent outputs:
+            // This `if` is just an optimization for cases where we would do nothing in the loop.
-            if d_tx
+            if funding_account.is_some() || cfg!(feature = "transparent-inputs") {
-                .tx()
+                for (output_index, txout) in d_tx
-                .transparent_bundle()
+                    .tx()
-                .iter()
+                    .transparent_bundle()
-                .any(|b| !b.vout.is_empty())
+                    .iter()
-            {
+                    .flat_map(|b| b.vout.iter())
-                // If the transaction contains spends from our wallet, we will store z->t
+                    .enumerate()
-                // transactions we observe in the same way they would be stored by
+                {
-                // create_spend_to_address.
+                    if let Some(address) = txout.recipient_address() {
-                let funding_accounts = wallet::get_funding_accounts(wdb.conn.0, d_tx.tx())?;
+                        // The transaction is not necessarily mined yet, but we want to record
-                let funding_account = funding_accounts.iter().next().copied();
+                        // that an output to the address was seen in this tx anyway. This will
-                if let Some(account_id) = funding_account {
+                        // advance the gap regardless of whether it is mined, but an output in
-                    if funding_accounts.len() > 1 {
+                        // an unmined transaction won't advance the range of safe indices.
-                        warn!(
+                        #[cfg(feature = "transparent-inputs")]
-                            "More than one wallet account detected as funding transaction {:?}, selecting {:?}",
+                        wallet::transparent::ephemeral::mark_ephemeral_address_as_seen(wdb, &address, tx_ref)?;
-                            d_tx.tx().txid(),
-                            account_id
-                        )
-                    }
-
-                    for (output_index, txout) in d_tx
-                        .tx()
-                        .transparent_bundle()
-                        .iter()
-                        .flat_map(|b| b.vout.iter())
-                        .enumerate()
-                    {
-                        if let Some(address) = txout.recipient_address() {
-                            #[cfg(feature = "transparent-inputs")]
-                            wallet::transparent::ephemeral::mark_ephemeral_address_as_mined(wdb, &address, tx_ref)?;
 
+                        // If a transaction we observe contains spends from our wallet, we will
+                        // store its transparent outputs in the same way they would be stored by
+                        // create_spend_to_address.
+                        if let Some(account_id) = funding_account {
                             let receiver = Receiver::Transparent(address);
 
                             #[cfg(feature = "transparent-inputs")]

zcash_client_sqlite/src/testing/pool.rs

@@ -302,9 +302,19 @@ pub(crate) fn send_single_step_proposed_transfer<T: ShieldedPoolTester>() {
 
 #[cfg(feature = "transparent-inputs")]
 pub(crate) fn send_multi_step_proposed_transfer<T: ShieldedPoolTester>() {
-    use std::str::FromStr;
+    use std::{collections::HashSet, str::FromStr};
 
-    use zcash_client_backend::fees::ChangeValue;
+    use rand_core::OsRng;
+    use zcash_client_backend::{
+        fees::ChangeValue,
+        wallet::{TransparentAddressMetadata, WalletTx},
+    };
+    use zcash_primitives::{
+        legacy::keys::{NonHardenedChildIndex, TransparentKeyScope},
+        transaction::builder::{BuildConfig, Builder},
+    };
+
+    use crate::wallet::{sapling::tests::test_prover, GAP_LIMIT};
 
     let mut st = TestBuilder::new()
         .with_block_cache()
@@ -312,6 +322,8 @@ pub(crate) fn send_multi_step_proposed_transfer<T: ShieldedPoolTester>() {
         .build();
 
     let account = st.test_account().cloned().unwrap();
+    let account_id = account.account_id();
+    let (default_addr, default_index) = account.usk().default_transparent_address();
     let dfvk = T::test_account_fvk(&st);
 
     let add_funds = |st: &mut TestState<_>, value| {
@@ -325,7 +337,8 @@ pub(crate) fn send_multi_step_proposed_transfer<T: ShieldedPoolTester>() {
                 .block_height(),
             h
         );
-        assert_eq!(st.get_spendable_balance(account.account_id(), 1), value);
+        assert_eq!(st.get_spendable_balance(account_id, 1), value);
+        h
     };
 
     let value = NonNegativeAmount::const_from_u64(100000);
@@ -344,7 +357,7 @@ pub(crate) fn send_multi_step_proposed_transfer<T: ShieldedPoolTester>() {
 
         // Generate a ZIP 320 proposal, sending to the wallet's default transparent address
         // expressed as a TEX address.
-        let tex_addr = match account.usk().default_transparent_address().0 {
+        let tex_addr = match default_addr {
             TransparentAddress::PublicKeyHash(data) => Address::Tex(data),
             _ => unreachable!(),
         };
@@ -354,7 +367,7 @@ pub(crate) fn send_multi_step_proposed_transfer<T: ShieldedPoolTester>() {
         // serialization of the proposal.
         let proposal = st
             .propose_standard_transfer::<Infallible>(
-                account.account_id(),
+                account_id,
                 StandardFeeRule::Zip317,
                 NonZeroU32::new(1).unwrap(),
                 &tex_addr,
@@ -439,15 +452,15 @@ pub(crate) fn send_multi_step_proposed_transfer<T: ShieldedPoolTester>() {
             (sent_v, sent_to_addr, None, None)
             if sent_v == u64::try_from(transfer_amount).unwrap() && sent_to_addr == Some(tex_addr.encode(&st.wallet().params)));
 
-        (ephemeral_addr.unwrap(), txids.head)
+        (ephemeral_addr.unwrap(), txids)
     };
 
     // Each transfer should use a different ephemeral address.
-    let (ephemeral0, _) = run_test(&mut st, 0);
+    let (ephemeral0, txids0) = run_test(&mut st, 0);
-    let (ephemeral1, _) = run_test(&mut st, 1);
+    let (ephemeral1, txids1) = run_test(&mut st, 1);
     assert_ne!(ephemeral0, ephemeral1);
 
-    add_funds(&mut st, value);
+    let height = add_funds(&mut st, value);
 
     let ephemeral_taddr = Address::decode(&st.wallet().params, &ephemeral0).expect("valid address");
     assert_matches!(
@@ -458,7 +471,7 @@ pub(crate) fn send_multi_step_proposed_transfer<T: ShieldedPoolTester>() {
     // Attempting to pay to an ephemeral address should cause an error.
     let proposal = st
         .propose_standard_transfer::<Infallible>(
-            account.account_id(),
+            account_id,
             StandardFeeRule::Zip317,
             NonZeroU32::new(1).unwrap(),
             &ephemeral_taddr,
@@ -477,6 +490,184 @@ pub(crate) fn send_multi_step_proposed_transfer<T: ShieldedPoolTester>() {
     assert_matches!(
         &create_proposed_result,
         Err(Error::PaysEphemeralTransparentAddress(address_str)) if address_str == &ephemeral0);
+
+    // Simulate another wallet sending to an ephemeral address with an index
+    // within the current gap limit. The `PaysEphemeralTransparentAddress` error
+    // prevents us from doing so straightforwardly, so we'll do it by building
+    // a transaction and calling `store_decrypted_tx` with it.
+    let known_addrs = st
+        .wallet()
+        .get_known_ephemeral_addresses(account_id, None)
+        .unwrap();
+    assert_eq!(known_addrs.len(), (GAP_LIMIT as usize) + 2);
+
+    // Check that the addresses are all distinct.
+    let known_set: HashSet<_> = known_addrs.iter().map(|(addr, _)| addr).collect();
+    assert_eq!(known_set.len(), known_addrs.len());
+    // Check that the metadata is as expected.
+    for (i, (_, meta)) in known_addrs.iter().enumerate() {
+        assert_eq!(
+            meta,
+            &TransparentAddressMetadata::new(
+                TransparentKeyScope::EPHEMERAL,
+                NonHardenedChildIndex::from_index(i.try_into().unwrap()).unwrap()
+            )
+        );
+    }
+
+    let mut builder = Builder::new(
+        st.wallet().params,
+        height + 1,
+        BuildConfig::Standard {
+            sapling_anchor: None,
+            orchard_anchor: None,
+        },
+    );
+    let (colliding_addr, _) = &known_addrs[10];
+    assert_matches!(
+        builder.add_transparent_output(colliding_addr, (value - zip317::MINIMUM_FEE).unwrap()),
+        Ok(_)
+    );
+    let sk = account
+        .usk()
+        .transparent()
+        .derive_secret_key(Scope::External.into(), default_index)
+        .unwrap();
+    let outpoint = OutPoint::fake();
+    let txout = TxOut {
+        script_pubkey: default_addr.script(),
+        value,
+    };
+    assert_matches!(builder.add_transparent_input(sk, outpoint, txout), Ok(_));
+    let test_prover = test_prover();
+    let build_result = builder
+        .build(
+            OsRng,
+            &test_prover,
+            &test_prover,
+            &zip317::FeeRule::standard(),
+        )
+        .unwrap();
+    let decrypted_tx = DecryptedTransaction::<AccountId>::new(
+        build_result.transaction(),
+        vec![],
+        #[cfg(feature = "orchard")]
+        vec![],
+    );
+    st.wallet_mut().store_decrypted_tx(decrypted_tx).unwrap();
+
+    // That should have advanced the start of the gap to index 11.
+    let new_known_addrs = st
+        .wallet()
+        .get_known_ephemeral_addresses(account_id, None)
+        .unwrap();
+    assert_eq!(new_known_addrs.len(), (GAP_LIMIT as usize) + 11);
+    assert!(new_known_addrs.starts_with(&known_addrs));
+
+    let reservation_should_succeed = |st: &mut TestState<_>, n| {
+        let reserved = st
+            .wallet_mut()
+            .reserve_next_n_ephemeral_addresses(account_id, n)
+            .unwrap();
+        assert_eq!(reserved.len(), n);
+        reserved
+    };
+    let reservation_should_fail = |st: &mut TestState<_>, n, expected_bad_index| {
+        assert_matches!(st
+            .wallet_mut()
+            .reserve_next_n_ephemeral_addresses(account_id, n),
+            Err(SqliteClientError::ReachedGapLimit(acct, bad_index))
+            if acct == account_id && bad_index == expected_bad_index);
+    };
+
+    let next_reserved = reservation_should_succeed(&mut st, 1);
+    assert_eq!(next_reserved[0], known_addrs[11]);
+
+    // Calling `reserve_next_n_ephemeral_addresses(account_id, 1)` will have advanced
+    // the start of the gap to index 12. This also tests the `index_range` parameter.
+    let newer_known_addrs = st
+        .wallet()
+        .get_known_ephemeral_addresses(account_id, Some(5..100))
+        .unwrap();
+    assert_eq!(newer_known_addrs.len(), (GAP_LIMIT as usize) + 12 - 5);
+    assert!(newer_known_addrs.starts_with(&new_known_addrs[5..]));
+
+    // None of the five transactions created above (two from each proposal and the
+    // one built manually) have been mined yet. So, the range of address indices
+    // that are safe to reserve is still 0..20, and we have already reserved 12
+    // addresses, so trying to reserve another 9 should fail.
+    reservation_should_fail(&mut st, 9, 20);
+    reservation_should_succeed(&mut st, 8);
+    reservation_should_fail(&mut st, 1, 20);
+
+    // Now mine the transaction with the ephemeral output at index 1.
+    // We already reserved 20 addresses, so this should allow 2 more (..22).
+    // It does not matter that the transaction with ephemeral output at index 0
+    // remains unmined.
+    let (h, _) = st.generate_next_block_including(txids1.head);
+    st.scan_cached_blocks(h, 1);
+    reservation_should_succeed(&mut st, 2);
+    reservation_should_fail(&mut st, 1, 22);
+
+    // Mining the transaction with the ephemeral output at index 0 at this point
+    // should make no difference.
+    let (h, _) = st.generate_next_block_including(txids0.head);
+    st.scan_cached_blocks(h, 1);
+    reservation_should_fail(&mut st, 1, 22);
+
+    // Now mine the transaction with the ephemeral output at index 10.
+    let tx = build_result.transaction();
+    let tx_index = 1;
+    let (h, _) = st.generate_next_block_from_tx(tx_index, tx);
+    st.scan_cached_blocks(h, 1);
+
+    // The rest of this test would currently fail without the explicit call to
+    // `put_tx_meta` below. Ideally the above `scan_cached_blocks` would be
+    // sufficient, but it does not detect the transaction as interesting to the
+    // wallet. If a transaction is in the database with a null `mined_height`,
+    // as in this case, its `mined_height` will remain null unless `put_tx_meta`
+    // is called on it. Normally `put_tx_meta` would be called via `put_blocks`
+    // as a result of scanning, but that won't happen for any fully transparent
+    // transaction, and currently it also will not happen for a partially shielded
+    // transaction unless it is interesting to the wallet for another reason.
+    // Therefore we will not currently detect either collisions with uses of
+    // ephemeral outputs by other wallets, or refunds of funds sent to TEX
+    // addresses. (#1354, #1379)
+
+    // Check that what we say in the above paragraph remains true, so that we
+    // don't accidentally fix it without updating this test.
+    reservation_should_fail(&mut st, 1, 22);
+
+    // For now, we demonstrate that this problem is the only obstacle to the rest
+    // of the ZIP 320 code doing the right thing, by manually calling `put_tx_meta`:
+    crate::wallet::put_tx_meta(
+        &st.wallet_mut().conn,
+        &WalletTx::new(
+            tx.txid(),
+            tx_index,
+            vec![],
+            vec![],
+            #[cfg(feature = "orchard")]
+            vec![],
+            #[cfg(feature = "orchard")]
+            vec![],
+        ),
+        h,
+    )
+    .unwrap();
+
+    // We already reserved 22 addresses, so mining the transaction with the
+    // ephemeral output at index 10 should allow 9 more (..31).
+    reservation_should_succeed(&mut st, 9);
+    reservation_should_fail(&mut st, 1, 31);
+
+    let newest_known_addrs = st
+        .wallet()
+        .get_known_ephemeral_addresses(account_id, None)
+        .unwrap();
+    assert_eq!(newest_known_addrs.len(), (GAP_LIMIT as usize) + 31);
+    assert!(newest_known_addrs.starts_with(&known_addrs));
+    assert!(newest_known_addrs[5..].starts_with(&newer_known_addrs));
 }
 
 #[cfg(feature = "transparent-inputs")]
@@ -490,6 +681,7 @@ pub(crate) fn proposal_fails_if_not_all_ephemeral_outputs_consumed<T: ShieldedPo
         .build();
 
     let account = st.test_account().cloned().unwrap();
+    let account_id = account.account_id();
     let dfvk = T::test_account_fvk(&st);
 
     let add_funds = |st: &mut TestState<_>, value| {
@@ -503,7 +695,7 @@ pub(crate) fn proposal_fails_if_not_all_ephemeral_outputs_consumed<T: ShieldedPo
                 .block_height(),
             h
         );
-        assert_eq!(st.get_spendable_balance(account.account_id(), 1), value);
+        assert_eq!(st.get_spendable_balance(account_id, 1), value);
     };
 
     let value = NonNegativeAmount::const_from_u64(100000);
@@ -521,7 +713,7 @@ pub(crate) fn proposal_fails_if_not_all_ephemeral_outputs_consumed<T: ShieldedPo
 
     let proposal = st
         .propose_standard_transfer::<Infallible>(
-            account.account_id(),
+            account_id,
             StandardFeeRule::Zip317,
             NonZeroU32::new(1).unwrap(),
             &tex_addr,

zcash_client_sqlite/src/wallet/db.rs

@@ -94,38 +94,41 @@ CREATE INDEX "addresses_accounts" ON "addresses" (
 /// - `address` contains the string (Base58Check) encoding of a transparent P2PKH address.
 /// - `used_in_tx` indicates that the address has been used by this wallet in a transaction (which
 ///   has not necessarily been mined yet). This should only be set once, when the txid is known.
-/// - `mined_in_tx` is non-null iff the address has been observed in a mined transaction (which may
+/// - `seen_in_tx` is non-null iff an output to the address has been seed in a transaction observed
-///   have been sent by this wallet or another one using the same seed, or by a TEX address recipient
+///   on the network and passed to `store_decrypted_tx`. The transaction may have been sent by this
-///   sending back the funds). This is used to advance the "gap limit", as well as to heuristically
+//    wallet or another one using the same seed, or by a TEX address recipient sending back the
-///   reduce the chance of address reuse collisions with another wallet using the same seed.
+///   funds. This is used to advance the "gap", as well as to heuristically reduce the chance of
-///
+///   address reuse collisions with another wallet using the same seed.
-/// Note that the fact that `used_in_tx` and `mined_in_tx` reference specific transactions is primarily
-/// a debugging aid (although the latter allows us to account for whether the referenced transaction
-/// is unmined). We only really care which addresses have been used, and whether we can allocate a
-/// new address within the gap limit.
 ///
 /// It is an external invariant that within each account:
 /// - the address indices are contiguous and start from 0;
-/// - the last `GAP_LIMIT` addresses have `used_in_tx` and `mined_in_tx` both NULL.
+/// - the last `GAP_LIMIT` addresses have `used_in_tx` and `seen_in_tx` both NULL.
 ///
 /// All but the last `GAP_LIMIT` addresses are defined to be "reserved" addresses. Since the next
 /// index to reserve is determined by dead reckoning from the last stored address, we use dummy
 /// entries after the maximum valid index in order to allow the last `GAP_LIMIT` addresses at the
 /// end of the index range to be used.
+///
+/// Note that the fact that `used_in_tx` references a specific transaction is just a debugging aid.
+/// The same is mostly true of `seen_in_tx`, but we also take into account whether the referenced
+/// transaction is unmined in order to determine the last index that is safe to reserve.
 pub(super) const TABLE_EPHEMERAL_ADDRESSES: &str = r#"
 CREATE TABLE ephemeral_addresses (
     account_id INTEGER NOT NULL,
     address_index INTEGER NOT NULL,
     address TEXT,
     used_in_tx INTEGER,
-    mined_in_tx INTEGER,
+    seen_in_tx INTEGER,
     FOREIGN KEY (account_id) REFERENCES accounts(id),
     FOREIGN KEY (used_in_tx) REFERENCES transactions(id_tx),
-    FOREIGN KEY (mined_in_tx) REFERENCES transactions(id_tx),
+    FOREIGN KEY (seen_in_tx) REFERENCES transactions(id_tx),
     PRIMARY KEY (account_id, address_index),
+    CONSTRAINT used_implies_seen CHECK (
+        used_in_tx IS NULL OR seen_in_tx IS NOT NULL
+    ),
     CONSTRAINT index_range_and_address_nullity CHECK (
         (address_index BETWEEN 0 AND 0x7FFFFFFF AND address IS NOT NULL) OR
-        (address_index BETWEEN 0x80000000 AND 0x7FFFFFFF + 20 AND address IS NULL AND used_in_tx IS NULL AND mined_in_tx IS NULL)
+        (address_index BETWEEN 0x80000000 AND 0x7FFFFFFF + 20 AND address IS NULL AND used_in_tx IS NULL AND seen_in_tx IS NULL)
     )
 ) WITHOUT ROWID"#;
 // Hexadecimal integer literals were added in SQLite version 3.8.6 (2014-08-15).

zcash_client_sqlite/src/wallet/init/migrations/ephemeral_addresses.rs

@@ -47,14 +47,17 @@ impl<P: consensus::Parameters> RusqliteMigration for Migration<P> {
                 address_index INTEGER NOT NULL,
                 address TEXT,
                 used_in_tx INTEGER,
-                mined_in_tx INTEGER,
+                seen_in_tx INTEGER,
                 FOREIGN KEY (account_id) REFERENCES accounts(id),
                 FOREIGN KEY (used_in_tx) REFERENCES transactions(id_tx),
-                FOREIGN KEY (mined_in_tx) REFERENCES transactions(id_tx),
+                FOREIGN KEY (seen_in_tx) REFERENCES transactions(id_tx),
                 PRIMARY KEY (account_id, address_index),
+                CONSTRAINT used_implies_seen CHECK (
+                    used_in_tx IS NULL OR seen_in_tx IS NOT NULL
+                ),
                 CONSTRAINT index_range_and_address_nullity CHECK (
                     (address_index BETWEEN 0 AND 0x7FFFFFFF AND address IS NOT NULL) OR
-                    (address_index BETWEEN 0x80000000 AND 0x7FFFFFFF + 20 AND address IS NULL AND used_in_tx IS NULL AND mined_in_tx IS NULL)
+                    (address_index BETWEEN 0x80000000 AND 0x7FFFFFFF + 20 AND address IS NULL AND used_in_tx IS NULL AND seen_in_tx IS NULL)
                 )
             ) WITHOUT ROWID;
             CREATE INDEX ephemeral_addresses_address ON ephemeral_addresses (

zcash_client_sqlite/src/wallet/transparent/ephemeral.rs

@@ -70,7 +70,7 @@ pub(crate) fn first_unsafe_index(
     account_id: AccountId,
 ) -> Result<u32, SqliteClientError> {
     // The inner join with `transactions` excludes addresses for which
-    // `mined_in_tx` is NULL. The query also excludes addresses observed
+    // `seen_in_tx` is NULL. The query also excludes addresses observed
     // to have been mined in a transaction that we currently see as unmined.
     // This is conservative in terms of avoiding violation of the gap
     // invariant: it can only cause us to get to the end of the gap sooner.
@@ -80,7 +80,7 @@ pub(crate) fn first_unsafe_index(
     let first_unmined_index: u32 = match conn
         .query_row(
             "SELECT address_index FROM ephemeral_addresses
-             JOIN transactions t ON t.id_tx = mined_in_tx
+             JOIN transactions t ON t.id_tx = seen_in_tx
              WHERE account_id = :account_id AND t.mined_height IS NOT NULL
              ORDER BY address_index DESC
              LIMIT 1",
@@ -164,12 +164,11 @@ pub(crate) fn get_known_ephemeral_addresses<P: consensus::Parameters>(
     Ok(result)
 }
 
-/// If this is an ephemeral address in any account, return its account id.
+/// If this is a known ephemeral address in any account, return its account id.
 pub(crate) fn find_account_for_ephemeral_address_str(
     conn: &rusqlite::Connection,
     address_str: &str,
 ) -> Result<Option<AccountId>, SqliteClientError> {
-    // Search ephemeral addresses that have already been reserved.
     Ok(conn
         .query_row(
             "SELECT account_id FROM ephemeral_addresses WHERE address = :address",
@@ -179,7 +178,7 @@ pub(crate) fn find_account_for_ephemeral_address_str(
         .optional()?)
 }
 
-#[cfg(feature = "transparent-inputs")]
+/// If this is a known ephemeral address in the given account, return its index.
 pub(crate) fn find_index_for_ephemeral_address_str(
     conn: &rusqlite::Connection,
     account_id: AccountId,
@@ -259,7 +258,7 @@ pub(crate) fn init_account<P: consensus::Parameters>(
 ///
 /// # Panics
 ///
-/// Panics if `next_to_reserve > (1 << 31)`.
+/// Panics if the precondition `next_to_reserve <= (1 << 31)` does not hold.
 fn reserve_until<P: consensus::Parameters>(
     conn: &rusqlite::Transaction,
     params: &P,
@@ -276,7 +275,7 @@ fn reserve_until<P: consensus::Parameters>(
 
     let ephemeral_ivk = get_ephemeral_ivk(conn, params, account_id)?;
 
-    // used_in_tx and mined_in_tx are initially NULL
+    // used_in_tx and seen_in_tx are initially NULL
     let mut stmt_insert_ephemeral_address = conn.prepare_cached(
         "INSERT INTO ephemeral_addresses (account_id, address_index, address)
          VALUES (:account_id, :address_index, :address)",
@@ -314,18 +313,18 @@ fn ephemeral_address_reuse_check<P: consensus::Parameters>(
     // using a given seed, because such a wallet will not reuse an address that
     // it ever reserved.
     //
-    // `COALESCE(used_in_tx, mined_in_tx)` can only differ from `used_in_tx`
+    // `COALESCE(used_in_tx, seen_in_tx)` can only differ from `used_in_tx`
     // if the address was reserved, an error occurred in transaction creation
-    // before calling `mark_ephemeral_address_as_used`, and then we observed
+    // before calling `mark_ephemeral_address_as_used`, and then we saw the
-    // the address to have been used in a mined transaction (presumably by
+    // address in another transaction (presumably created by another wallet
-    // another wallet instance, or due to a bug) anyway.
+    // instance, or as a result of a bug) anyway.
     let res = wdb
         .conn
         .0
         .query_row(
             "SELECT t.txid FROM ephemeral_addresses
              LEFT OUTER JOIN transactions t
-             ON t.id_tx = COALESCE(used_in_tx, mined_in_tx)
+             ON t.id_tx = COALESCE(used_in_tx, seen_in_tx)
              WHERE address = :address",
             named_params![":address": address_str],
             |row| row.get::<_, Option<Vec<u8>>>(0),
@@ -362,10 +361,28 @@ pub(crate) fn mark_ephemeral_address_as_used<P: consensus::Parameters>(
     let address_str = ephemeral_address.encode(&wdb.params);
     ephemeral_address_reuse_check(wdb, &address_str)?;
 
-    wdb.conn.0.execute(
+    // We update both `used_in_tx` and `seen_in_tx` here, because a used address has
-        "UPDATE ephemeral_addresses SET used_in_tx = :used_in_tx WHERE address = :address",
+    // necessarily been seen in a transaction. We will not treat this as extending the
-        named_params![":used_in_tx": &tx_ref, ":address": address_str],
+    // range of addresses that are safe to reserve unless and until the transaction is
-    )?;
+    // observed as mined.
+    let update_result = wdb
+        .conn
+        .0
+        .query_row(
+            "UPDATE ephemeral_addresses
+             SET used_in_tx = :tx_ref, seen_in_tx = :tx_ref
+             WHERE address = :address
+             RETURNING account_id, address_index",
+            named_params![":tx_ref": &tx_ref, ":address": address_str],
+            |row| Ok((AccountId(row.get::<_, u32>(0)?), row.get::<_, u32>(1)?)),
+        )
+        .optional()?;
+
+    // Maintain the invariant that the last `GAP_LIMIT` addresses are unused and unseen.
+    if let Some((account_id, address_index)) = update_result {
+        let next_to_reserve = address_index.checked_add(1).expect("ensured by constraint");
+        reserve_until(wdb.conn.0, &wdb.params, account_id, next_to_reserve)?;
+    }
     Ok(())
 }
 
@@ -374,7 +391,7 @@ pub(crate) fn mark_ephemeral_address_as_used<P: consensus::Parameters>(
 ///
 /// `tx_ref` must be a valid transaction reference. This call has no effect if
 /// `address` is not one of our ephemeral addresses.
-pub(crate) fn mark_ephemeral_address_as_mined<P: consensus::Parameters>(
+pub(crate) fn mark_ephemeral_address_as_seen<P: consensus::Parameters>(
     wdb: &mut WalletDb<SqlTransaction<'_>, P>,
     address: &TransparentAddress,
     tx_ref: i64,
@@ -382,7 +399,7 @@ pub(crate) fn mark_ephemeral_address_as_mined<P: consensus::Parameters>(
     let address_str = address.encode(&wdb.params);
 
     // Figure out which transaction was mined earlier: `tx_ref`, or any existing
-    // tx referenced by `mined_in_tx` for the given address. Prefer the existing
+    // tx referenced by `seen_in_tx` for the given address. Prefer the existing
     // reference in case of a tie or if both transactions are unmined.
     // This slightly reduces the chance of unnecessarily reaching the gap limit
     // too early in some corner cases (because the earlier transaction is less
@@ -392,34 +409,32 @@ pub(crate) fn mark_ephemeral_address_as_mined<P: consensus::Parameters>(
     let earlier_ref = wdb.conn.0.query_row(
         "SELECT id_tx FROM transactions
          LEFT OUTER JOIN ephemeral_addresses e
-         ON id_tx = e.mined_in_tx
+         ON id_tx = e.seen_in_tx
          WHERE id_tx = :tx_ref OR e.address = :address
          ORDER BY mined_height ASC NULLS LAST,
                   tx_index ASC NULLS LAST,
-                  e.mined_in_tx ASC NULLS LAST
+                  e.seen_in_tx ASC NULLS LAST
          LIMIT 1",
         named_params![":tx_ref": &tx_ref, ":address": address_str],
         |row| row.get::<_, i64>(0),
     )?;
 
-    let mined_ephemeral = wdb
+    let update_result = wdb
         .conn
         .0
         .query_row(
             "UPDATE ephemeral_addresses
-            SET mined_in_tx = :mined_in_tx
+             SET seen_in_tx = :seen_in_tx
-            WHERE address = :address
+             WHERE address = :address
-            RETURNING (account_id, address_index)",
+             RETURNING account_id, address_index",
-            named_params![":mined_in_tx": &earlier_ref, ":address": address_str],
+            named_params![":seen_in_tx": &earlier_ref, ":address": address_str],
             |row| Ok((AccountId(row.get::<_, u32>(0)?), row.get::<_, u32>(1)?)),
         )
         .optional()?;
 
-    // If this is a known ephemeral address for an account in this wallet, we might need
+    // Maintain the invariant that the last `GAP_LIMIT` addresses are unused and unseen.
-    // to extend the indices stored for that account to maintain the invariant that the
+    if let Some((account_id, address_index)) = update_result {
-    // last `GAP_LIMIT` addresses are unused and unmined.
+        let next_to_reserve = address_index.checked_add(1).expect("ensured by constraint");
-    if let Some((account_id, address_index)) = mined_ephemeral {
-        let next_to_reserve = min(1 << 31, address_index.saturating_add(1));
         reserve_until(wdb.conn.0, &wdb.params, account_id, next_to_reserve)?;
     }
     Ok(())