From fd4e323199fc82842e527ed36dd24625eabf70b8 Mon Sep 17 00:00:00 2001 From: arya2 Date: Wed, 11 Oct 2023 18:28:55 -0400 Subject: [PATCH 1/8] Uses range_iter in address_transaction_locations --- .../finalized_state/zebra_db/transparent.rs | 42 +++++-------------- 1 file changed, 11 insertions(+), 31 deletions(-) diff --git a/zebra-state/src/service/finalized_state/zebra_db/transparent.rs b/zebra-state/src/service/finalized_state/zebra_db/transparent.rs index 2f80751687f..6d8b616ae6e 100644 --- a/zebra-state/src/service/finalized_state/zebra_db/transparent.rs +++ b/zebra-state/src/service/finalized_state/zebra_db/transparent.rs @@ -235,44 +235,24 @@ 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( + let transaction_location = AddressTransaction::address_iterator_start( address_location, *query_height_range.start(), ); - 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(); - } + let last_transaction_location = AddressTransaction::new( + address_location, + TransactionLocation::from_usize(*query_height_range.end(), usize::MAX), + ); - addr_transactions + self.zs_range_iter( + &tx_loc_by_transparent_addr_loc, + &transaction_location..=&last_transaction_location, + ) + .map(|(tx_loc, ())| tx_loc) + .collect() } // Address index queries From 581829ddbd1ae301029365e2a12972dea751b407 Mon Sep 17 00:00:00 2001 From: arya2 Date: Wed, 11 Oct 2023 18:28:55 -0400 Subject: [PATCH 2/8] Uses range_iter in address_transaction_locations --- .../finalized_state/zebra_db/transparent.rs | 42 +++++-------------- 1 file changed, 11 insertions(+), 31 deletions(-) diff --git a/zebra-state/src/service/finalized_state/zebra_db/transparent.rs b/zebra-state/src/service/finalized_state/zebra_db/transparent.rs index 2f80751687f..6d8b616ae6e 100644 --- a/zebra-state/src/service/finalized_state/zebra_db/transparent.rs +++ b/zebra-state/src/service/finalized_state/zebra_db/transparent.rs @@ -235,44 +235,24 @@ 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( + let transaction_location = AddressTransaction::address_iterator_start( address_location, *query_height_range.start(), ); - 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(); - } + let last_transaction_location = AddressTransaction::new( + address_location, + TransactionLocation::from_usize(*query_height_range.end(), usize::MAX), + ); - addr_transactions + self.zs_range_iter( + &tx_loc_by_transparent_addr_loc, + &transaction_location..=&last_transaction_location, + ) + .map(|(tx_loc, ())| tx_loc) + .collect() } // Address index queries From f442214447f96bc4c883ad492719ca650013c513 Mon Sep 17 00:00:00 2001 From: arya2 Date: Wed, 11 Oct 2023 19:53:01 -0400 Subject: [PATCH 3/8] uses u16::MAX instead of usize::MAX --- .../finalized_state/zebra_db/transparent.rs | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/zebra-state/src/service/finalized_state/zebra_db/transparent.rs b/zebra-state/src/service/finalized_state/zebra_db/transparent.rs index 6d8b616ae6e..5e7b8b131c9 100644 --- a/zebra-state/src/service/finalized_state/zebra_db/transparent.rs +++ b/zebra-state/src/service/finalized_state/zebra_db/transparent.rs @@ -244,15 +244,16 @@ impl ZebraDb { let last_transaction_location = AddressTransaction::new( address_location, - TransactionLocation::from_usize(*query_height_range.end(), usize::MAX), + TransactionLocation::from_usize(*query_height_range.end(), u16::MAX.into()), ); - self.zs_range_iter( - &tx_loc_by_transparent_addr_loc, - &transaction_location..=&last_transaction_location, - ) - .map(|(tx_loc, ())| tx_loc) - .collect() + self.db + .zs_range_iter( + &tx_loc_by_transparent_addr_loc, + &transaction_location..=&last_transaction_location, + ) + .map(|(tx_loc, ())| tx_loc) + .collect() } // Address index queries From 7662b6ba9e27d21bd531565719587938b7bb3337 Mon Sep 17 00:00:00 2001 From: arya2 Date: Thu, 12 Oct 2023 00:16:22 -0400 Subject: [PATCH 4/8] Moves limit code into method --- .../disk_format/transparent.rs | 32 +++++++++---------- .../finalized_state/zebra_db/transparent.rs | 12 ++----- 2 files changed, 18 insertions(+), 26 deletions(-) diff --git a/zebra-state/src/service/finalized_state/disk_format/transparent.rs b/zebra-state/src/service/finalized_state/disk_format/transparent.rs index ace95b04b0f..62c1abdb3f9 100644 --- a/zebra-state/src/service/finalized_state/disk_format/transparent.rs +++ b/zebra-state/src/service/finalized_state/disk_format/transparent.rs @@ -416,36 +416,36 @@ impl AddressTransaction { } } - /// Create an [`AddressTransaction`] which starts iteration for the supplied + /// 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. + /// whichever is greater. Ends at the last transaction index for an [`AddressLocation`]. /// - /// 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. + /// [`DiskDb::zs_range_iter`][1] will fetch all existing + /// (valid) values in the range. /// - /// [1]: super::super::disk_db::ReadDisk::zs_next_key_value_from - pub fn address_iterator_start( + /// [1]: super::super::disk_db::DiskDb + pub fn address_iterator_range( address_location: AddressLocation, query_start: Height, - ) -> AddressTransaction { + ) -> std::ops::RangeInclusive { // 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); + let start_height = max(query_start, address_location.height()); + let first_utxo_idx = address_location.transaction_index().0; - AddressTransaction { - address_location, - transaction_location: max(first_utxo_location, query_start_location), - } + let tx_loc = |tx_idx| TransactionLocation::from_index(start_height, tx_idx); + let addr_tx = |tx_idx| AddressTransaction::new(address_location, tx_loc(tx_idx)); + + addr_tx(first_utxo_idx)..=addr_tx(u16::MAX) } /// Update the transaction location to the next possible transaction for the diff --git a/zebra-state/src/service/finalized_state/zebra_db/transparent.rs b/zebra-state/src/service/finalized_state/zebra_db/transparent.rs index 5e7b8b131c9..38729f4d32f 100644 --- a/zebra-state/src/service/finalized_state/zebra_db/transparent.rs +++ b/zebra-state/src/service/finalized_state/zebra_db/transparent.rs @@ -237,21 +237,13 @@ impl ZebraDb { // A potentially invalid key representing the first UTXO send to the address, // or the query start height. - let transaction_location = AddressTransaction::address_iterator_start( + let transaction_location_range = AddressTransaction::address_iterator_range( address_location, *query_height_range.start(), ); - let last_transaction_location = AddressTransaction::new( - address_location, - TransactionLocation::from_usize(*query_height_range.end(), u16::MAX.into()), - ); - self.db - .zs_range_iter( - &tx_loc_by_transparent_addr_loc, - &transaction_location..=&last_transaction_location, - ) + .zs_range_iter(&tx_loc_by_transparent_addr_loc, transaction_location_range) .map(|(tx_loc, ())| tx_loc) .collect() } From 73290dbeb54ce2eef0de9e1131effd70b658c290 Mon Sep 17 00:00:00 2001 From: arya2 Date: Thu, 12 Oct 2023 00:44:28 -0400 Subject: [PATCH 5/8] adds allow(dead_code) --- .../src/service/finalized_state/disk_format/transparent.rs | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/zebra-state/src/service/finalized_state/disk_format/transparent.rs b/zebra-state/src/service/finalized_state/disk_format/transparent.rs index 62c1abdb3f9..cf81af0e512 100644 --- a/zebra-state/src/service/finalized_state/disk_format/transparent.rs +++ b/zebra-state/src/service/finalized_state/disk_format/transparent.rs @@ -433,13 +433,13 @@ impl AddressTransaction { address_location: AddressLocation, query_start: Height, ) -> std::ops::RangeInclusive { + // Iterating from the start height filters out transactions that aren't needed. + let start_height = max(query_start, address_location.height()); + // 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. - // - // Iterating from the start height filters out transactions that aren't needed. - let start_height = max(query_start, address_location.height()); let first_utxo_idx = address_location.transaction_index().0; let tx_loc = |tx_idx| TransactionLocation::from_index(start_height, tx_idx); @@ -457,6 +457,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. From a4be4e54e56214a596ea12ede1d672dab8bbbe78 Mon Sep 17 00:00:00 2001 From: arya2 Date: Thu, 12 Oct 2023 12:30:13 -0400 Subject: [PATCH 6/8] Simplifies address_iterator_range --- .../service/finalized_state/disk_format/block.rs | 1 - .../finalized_state/disk_format/transparent.rs | 16 ++++++++-------- .../finalized_state/zebra_db/transparent.rs | 6 ++---- 3 files changed, 10 insertions(+), 13 deletions(-) diff --git a/zebra-state/src/service/finalized_state/disk_format/block.rs b/zebra-state/src/service/finalized_state/disk_format/block.rs index 84efd60b016..03a7f648053 100644 --- a/zebra-state/src/service/finalized_state/disk_format/block.rs +++ b/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, diff --git a/zebra-state/src/service/finalized_state/disk_format/transparent.rs b/zebra-state/src/service/finalized_state/disk_format/transparent.rs index cf81af0e512..eac062c2d35 100644 --- a/zebra-state/src/service/finalized_state/disk_format/transparent.rs +++ b/zebra-state/src/service/finalized_state/disk_format/transparent.rs @@ -431,21 +431,21 @@ impl AddressTransaction { /// [1]: super::super::disk_db::DiskDb pub fn address_iterator_range( address_location: AddressLocation, - query_start: Height, + query: std::ops::RangeInclusive, ) -> std::ops::RangeInclusive { - // Iterating from the start height filters out transactions that aren't needed. - let start_height = max(query_start, address_location.height()); - // 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_idx = address_location.transaction_index().0; + 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_index(*query.start(), 0); + let query_end_location = TransactionLocation::from_index(*query.end(), u16::MAX); - let tx_loc = |tx_idx| TransactionLocation::from_index(start_height, tx_idx); - let addr_tx = |tx_idx| AddressTransaction::new(address_location, tx_loc(tx_idx)); + let addr_tx = |tx_loc| AddressTransaction::new(address_location, tx_loc); - addr_tx(first_utxo_idx)..=addr_tx(u16::MAX) + 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 diff --git a/zebra-state/src/service/finalized_state/zebra_db/transparent.rs b/zebra-state/src/service/finalized_state/zebra_db/transparent.rs index 38729f4d32f..2e8d6c3980a 100644 --- a/zebra-state/src/service/finalized_state/zebra_db/transparent.rs +++ b/zebra-state/src/service/finalized_state/zebra_db/transparent.rs @@ -237,10 +237,8 @@ impl ZebraDb { // A potentially invalid key representing the first UTXO send to the address, // or the query start height. - let transaction_location_range = AddressTransaction::address_iterator_range( - address_location, - *query_height_range.start(), - ); + let transaction_location_range = + AddressTransaction::address_iterator_range(address_location, query_height_range); self.db .zs_range_iter(&tx_loc_by_transparent_addr_loc, transaction_location_range) From 0efcbae7f8a8f58bd23e40b039ef891151cf7b38 Mon Sep 17 00:00:00 2001 From: arya2 Date: Thu, 12 Oct 2023 13:20:10 -0400 Subject: [PATCH 7/8] Moves test state init out of loop --- zebra-rpc/src/methods/tests/vectors.rs | 32 +++++++++++++++++++------- 1 file changed, 24 insertions(+), 8 deletions(-) diff --git a/zebra-rpc/src/methods/tests/vectors.rs b/zebra-rpc/src/methods/tests/vectors.rs index 769d561897b..68f08c184b1 100644 --- a/zebra-rpc/src/methods/tests/vectors.rs +++ b/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, - blocks: &[Arc], 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 From cb9a1f66c53758597329b2f36278df3c8126b818 Mon Sep 17 00:00:00 2001 From: arya2 Date: Tue, 17 Oct 2023 17:52:58 -0400 Subject: [PATCH 8/8] Updates docs --- .../finalized_state/disk_format/transparent.rs | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/zebra-state/src/service/finalized_state/disk_format/transparent.rs b/zebra-state/src/service/finalized_state/disk_format/transparent.rs index eac062c2d35..d3fb01c390f 100644 --- a/zebra-state/src/service/finalized_state/disk_format/transparent.rs +++ b/zebra-state/src/service/finalized_state/disk_format/transparent.rs @@ -417,15 +417,16 @@ impl AddressTransaction { } /// 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 last transaction index for an [`AddressLocation`]. + /// 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 transactions with - /// [`DiskDb::zs_range_iter`][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 - /// [`DiskDb::zs_range_iter`][1] will fetch all existing + /// 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. + /// + /// 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 @@ -439,7 +440,7 @@ impl AddressTransaction { // 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. + // 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);