change(state): Set upper bound when reading from deleting column family tx_loc_by_transparent_addr_loc (#7732)

* Uses range_iter in address_transaction_locations

* Uses range_iter in address_transaction_locations

* uses u16::MAX instead of usize::MAX

* Moves limit code into method

* adds allow(dead_code)

* Simplifies address_iterator_range

* Moves test state init out of loop

* Updates docs

zebra-rpc/src/methods/tests/vectors.rs

@@ -16,6 +16,7 @@ use zebra_chain::{
 };
 use zebra_node_services::BoxError;
 
+use zebra_state::{LatestChainTip, ReadStateService};
 use zebra_test::mock_service::MockService;
 
 use super::super::*;
@@ -674,19 +675,36 @@ async fn rpc_getaddresstxids_response() {
             .address(network)
             .unwrap();
 
+        // Create a populated state service
+        let (_state, read_state, latest_chain_tip, _chain_tip_change) =
+            zebra_state::populated_state(blocks.to_owned(), network).await;
+
         if network == Mainnet {
             // Exhaustively test possible block ranges for mainnet.
             //
             // TODO: if it takes too long on slower machines, turn this into a proptest with 10-20 cases
             for start in 1..=10 {
                 for end in start..=10 {
-                    rpc_getaddresstxids_response_with(network, start..=end, &blocks, &address)
-                        .await;
+                    rpc_getaddresstxids_response_with(
+                        network,
+                        start..=end,
+                        &address,
+                        &read_state,
+                        &latest_chain_tip,
+                    )
+                    .await;
                 }
             }
         } else {
             // Just test the full range for testnet.
-            rpc_getaddresstxids_response_with(network, 1..=10, &blocks, &address).await;
+            rpc_getaddresstxids_response_with(
+                network,
+                1..=10,
+                &address,
+                &read_state,
+                &latest_chain_tip,
+            )
+            .await;
         }
     }
 }
@@ -694,13 +712,11 @@ async fn rpc_getaddresstxids_response() {
 async fn rpc_getaddresstxids_response_with(
     network: Network,
     range: RangeInclusive<u32>,
-    blocks: &[Arc<Block>],
     address: &transparent::Address,
+    read_state: &ReadStateService,
+    latest_chain_tip: &LatestChainTip,
 ) {
     let mut mempool: MockService<_, _, _, BoxError> = MockService::build().for_unit_tests();
-    // Create a populated state service
-    let (_state, read_state, latest_chain_tip, _chain_tip_change) =
-        zebra_state::populated_state(blocks.to_owned(), network).await;
 
     let (rpc, rpc_tx_queue_task_handle) = RpcImpl::new(
         "RPC test",
@@ -710,7 +726,7 @@ async fn rpc_getaddresstxids_response_with(
         true,
         Buffer::new(mempool.clone(), 1),
         Buffer::new(read_state.clone(), 1),
-        latest_chain_tip,
+        latest_chain_tip.clone(),
     );
 
     // call the method with valid arguments

zebra-state/src/service/finalized_state/disk_format/block.rs

@@ -132,7 +132,6 @@ pub struct TransactionLocation {
 
 impl TransactionLocation {
     /// Creates a transaction location from a block height and transaction index.
-    #[allow(dead_code)]
     pub fn from_index(height: Height, transaction_index: u16) -> TransactionLocation {
         TransactionLocation {
             height,

zebra-state/src/service/finalized_state/disk_format/transparent.rs

@@ -416,36 +416,37 @@ impl AddressTransaction {
         }
     }
 
-    /// Create an [`AddressTransaction`] which starts iteration for the supplied
-    /// address. Starts at the first UTXO, or at the `query_start` height,
-    /// whichever is greater.
+    /// Create a range of [`AddressTransaction`]s which starts iteration for the supplied
+    /// address. Starts at the first UTXO, or at the `query` start height, whichever is greater.
+    /// Ends at the maximum possible transaction index for the end height.
     ///
-    /// Used to look up the first transaction with
-    /// [`ReadDisk::zs_next_key_value_from`][1].
+    /// Used to look up transactions with [`DiskDb::zs_range_iter`][1].
     ///
-    /// The transaction location might be invalid, if it is based on the
-    /// `query_start` height. But this is not an issue, since
-    /// [`ReadDisk::zs_next_key_value_from`][1] will fetch the next existing
-    /// (valid) value.
+    /// The transaction locations in the:
+    /// - start bound might be invalid, if it is based on the `query` start height.
+    /// - end bound will always be invalid.
     ///
-    /// [1]: super::super::disk_db::ReadDisk::zs_next_key_value_from
-    pub fn address_iterator_start(
+    /// But this is not an issue, since [`DiskDb::zs_range_iter`][1] will fetch all existing
+    /// (valid) values in the range.
+    ///
+    /// [1]: super::super::disk_db::DiskDb
+    pub fn address_iterator_range(
         address_location: AddressLocation,
-        query_start: Height,
-    ) -> AddressTransaction {
+        query: std::ops::RangeInclusive<Height>,
+    ) -> std::ops::RangeInclusive<AddressTransaction> {
         // Iterating from the lowest possible transaction location gets us the first transaction.
         //
         // The address location is the output location of the first UTXO sent to the address,
         // and addresses can not spend funds until they receive their first UTXO.
         let first_utxo_location = address_location.transaction_location();
 
-        // Iterating from the start height filters out transactions that aren't needed.
-        let query_start_location = TransactionLocation::from_usize(query_start, 0);
+        // Iterating from the start height to the end height filters out transactions that aren't needed.
+        let query_start_location = TransactionLocation::from_index(*query.start(), 0);
+        let query_end_location = TransactionLocation::from_index(*query.end(), u16::MAX);
 
-        AddressTransaction {
-            address_location,
-            transaction_location: max(first_utxo_location, query_start_location),
-        }
+        let addr_tx = |tx_loc| AddressTransaction::new(address_location, tx_loc);
+
+        addr_tx(max(first_utxo_location, query_start_location))..=addr_tx(query_end_location)
     }
 
     /// Update the transaction location to the next possible transaction for the
@@ -457,6 +458,7 @@ impl AddressTransaction {
     /// existing (valid) value.
     ///
     /// [1]: super::super::disk_db::ReadDisk::zs_next_key_value_from
+    #[allow(dead_code)]
     pub fn address_iterator_next(&mut self) {
         // Iterating from the next possible output location gets us the next output,
         // even if it is in a later block or transaction.

zebra-state/src/service/finalized_state/zebra_db/transparent.rs

@@ -235,44 +235,15 @@ impl ZebraDb {
         let tx_loc_by_transparent_addr_loc =
             self.db.cf_handle("tx_loc_by_transparent_addr_loc").unwrap();
 
-        // Manually fetch the entire addresses' transaction locations
-        let mut addr_transactions = BTreeSet::new();
-
         // A potentially invalid key representing the first UTXO send to the address,
         // or the query start height.
-        let mut transaction_location = AddressTransaction::address_iterator_start(
-            address_location,
-            *query_height_range.start(),
-        );
+        let transaction_location_range =
+            AddressTransaction::address_iterator_range(address_location, query_height_range);
 
-        loop {
-            // Seek to a valid entry for this address, or the first entry for the next address
-            transaction_location = match self
-                .db
-                .zs_next_key_value_from(&tx_loc_by_transparent_addr_loc, &transaction_location)
-            {
-                Some((transaction_location, ())) => transaction_location,
-                // We're finished with the final address in the column family
-                None => break,
-            };
-
-            // We found the next address, so we're finished with this address
-            if transaction_location.address_location() != address_location {
-                break;
-            }
-
-            // We're past the end height, so we're finished with this query
-            if transaction_location.transaction_location().height > *query_height_range.end() {
-                break;
-            }
-
-            addr_transactions.insert(transaction_location);
-
-            // A potentially invalid key representing the next possible output
-            transaction_location.address_iterator_next();
-        }
-
-        addr_transactions
+        self.db
+            .zs_range_iter(&tx_loc_by_transparent_addr_loc, transaction_location_range)
+            .map(|(tx_loc, ())| tx_loc)
+            .collect()
     }
 
     // Address index queries