diff --git a/api-server/api-server-common/src/storage/impls/in_memory/mod.rs b/api-server/api-server-common/src/storage/impls/in_memory/mod.rs index 7b6a83904..1887ceb40 100644 --- a/api-server/api-server-common/src/storage/impls/in_memory/mod.rs +++ b/api-server/api-server-common/src/storage/impls/in_memory/mod.rs @@ -61,6 +61,13 @@ impl Ord for TokenTransactionOrderedByTxId { } } +/// An offset that doesn't fit a usize is past the end of any in-memory result set. +/// Skipping by usize::MAX yields an empty page instead of wrapping to a small number +/// (which would return the first page) on targets with a narrower pointer width. +fn offset_to_usize(offset: u64) -> usize { + usize::try_from(offset).unwrap_or(usize::MAX) +} + #[derive(Debug, Clone)] /// An in-memory api-server storage, meant for tests. /// @@ -266,7 +273,7 @@ impl ApiServerInMemoryStorage { &self, token_id: TokenId, len: u32, - tx_global_index: u64, + offset: u64, ) -> Result, ApiServerStorageError> { Ok(self .token_transactions_table @@ -280,7 +287,7 @@ impl ApiServerInMemoryStorage { txs.sort_by_key(|tx| std::cmp::Reverse(tx.tx_global_index)); txs }) - .flat_map(|tx| (tx.tx_global_index < tx_global_index).then_some(tx)) + .skip(offset_to_usize(offset)) .cloned() .take(len as usize) .collect() @@ -333,7 +340,7 @@ impl ApiServerInMemoryStorage { }, ) }) - .skip(offset as usize) + .skip(offset_to_usize(offset)) .take(len as usize) .collect()) } @@ -525,7 +532,7 @@ impl ApiServerInMemoryStorage { offset: u64, ) -> Result, ApiServerStorageError> { let len = len as usize; - let offset = offset as usize; + let offset = offset_to_usize(offset); if offset >= self.orders_table.len() { return Ok(vec![]); @@ -558,7 +565,7 @@ impl ApiServerInMemoryStorage { offset: u64, ) -> Result, ApiServerStorageError> { let len = len as usize; - let offset = offset as usize; + let offset = offset_to_usize(offset); let mut order_data: Vec<_> = self .orders_table @@ -687,7 +694,7 @@ impl ApiServerInMemoryStorage { offset: u64, ) -> Result, ApiServerStorageError> { let len = len as usize; - let offset = offset as usize; + let offset = offset_to_usize(offset); let decorated = self.sorted_live_pools()?; if offset >= decorated.len() { return Ok(vec![]); @@ -738,7 +745,7 @@ impl ApiServerInMemoryStorage { offset: u64, ) -> Result, ApiServerStorageError> { let len = len as usize; - let offset = offset as usize; + let offset = offset_to_usize(offset); // deepest pledge first, ties broken by the encoded pool address (byte order), // exactly like the postgres listing; the balance and the sort key are decoded // once per entry, and a pool whose stored balance fails to decode is surfaced @@ -952,7 +959,7 @@ impl ApiServerInMemoryStorage { .fungible_token_data .keys() .chain(self.nft_token_issuances.keys()) - .skip(offset as usize) + .skip(offset_to_usize(offset)) .take(len as usize) .copied() .collect()) @@ -983,7 +990,7 @@ impl ApiServerInMemoryStorage { String::from_utf8_lossy(value_ticker).to_ascii_lowercase(); (lowercased_value_ticker.contains(&lowercased_ticker)).then_some(key) })) - .skip(offset as usize) + .skip(offset_to_usize(offset)) .take(len as usize) .copied() .collect()) diff --git a/api-server/api-server-common/src/storage/impls/in_memory/transactional/read.rs b/api-server/api-server-common/src/storage/impls/in_memory/transactional/read.rs index 865882ed3..4d0336066 100644 --- a/api-server/api-server-common/src/storage/impls/in_memory/transactional/read.rs +++ b/api-server/api-server-common/src/storage/impls/in_memory/transactional/read.rs @@ -81,9 +81,9 @@ impl ApiServerStorageRead for ApiServerInMemoryStorageTransactionalRo<'_> { &self, token_id: TokenId, len: u32, - tx_global_index: u64, + offset: u64, ) -> Result, ApiServerStorageError> { - self.transaction.get_token_transactions(token_id, len, tx_global_index) + self.transaction.get_token_transactions(token_id, len, offset) } async fn get_block( diff --git a/api-server/api-server-common/src/storage/impls/in_memory/transactional/write.rs b/api-server/api-server-common/src/storage/impls/in_memory/transactional/write.rs index d82c1d3d9..282aa8299 100644 --- a/api-server/api-server-common/src/storage/impls/in_memory/transactional/write.rs +++ b/api-server/api-server-common/src/storage/impls/in_memory/transactional/write.rs @@ -366,9 +366,9 @@ impl ApiServerStorageRead for ApiServerInMemoryStorageTransactionalRw<'_> { &self, token_id: TokenId, len: u32, - tx_global_index: u64, + offset: u64, ) -> Result, ApiServerStorageError> { - self.transaction.get_token_transactions(token_id, len, tx_global_index) + self.transaction.get_token_transactions(token_id, len, offset) } async fn get_latest_blocktimestamps( diff --git a/api-server/api-server-common/src/storage/impls/postgres/queries.rs b/api-server/api-server-common/src/storage/impls/postgres/queries.rs index 942d47a40..13010e873 100644 --- a/api-server/api-server-common/src/storage/impls/postgres/queries.rs +++ b/api-server/api-server-common/src/storage/impls/postgres/queries.rs @@ -143,6 +143,13 @@ impl<'a, 'b> QueryFromConnection<'a, 'b> { .map_err(|_| ApiServerStorageError::TxGlobalIndexTooHigh(tx_global_index)) } + /// An offset that doesn't fit a bigint is past the end of any result set. + /// Postgres rejects a negative OFFSET, so callers map overflow to an empty + /// page instead of wrapping. + fn offset_to_bigint(offset: u64) -> Option { + i64::try_from(offset).ok() + } + /// Whether the schema exists and carries a storage version. Note that a `true` /// result does NOT mean the stored version is current: the caller must compare /// [`Self::get_storage_version`] with the current version and reinitialize when @@ -783,9 +790,11 @@ impl<'a, 'b> QueryFromConnection<'a, 'b> { &self, token_id: TokenId, len: u32, - tx_global_index: u64, + offset: u64, ) -> Result, ApiServerStorageError> { - let tx_global_index = Self::tx_global_index_to_postgres_friendly(tx_global_index)?; + let Some(offset) = Self::offset_to_bigint(offset) else { + return Ok(Vec::new()); + }; let len = len as i64; let rows = self .tx @@ -793,11 +802,12 @@ impl<'a, 'b> QueryFromConnection<'a, 'b> { r#" SELECT tx_global_index, transaction_id FROM ml.token_transactions - WHERE token_id = $1 AND tx_global_index < $2 + WHERE token_id = $1 ORDER BY tx_global_index DESC + OFFSET $2 LIMIT $3; "#, - &[&token_id.encode(), &tx_global_index, &len], + &[&token_id.encode(), &offset, &len], ) .await .map_err(|e| ApiServerStorageError::LowLevelStorageError(e.to_string()))?; @@ -2193,7 +2203,9 @@ impl<'a, 'b> QueryFromConnection<'a, 'b> { chain_config: &ChainConfig, ) -> Result, ApiServerStorageError> { let len = len as i64; - let offset = offset as i64; + let Some(offset) = Self::offset_to_bigint(offset) else { + return Ok(Vec::new()); + }; let rows = self .tx .query( @@ -2297,7 +2309,9 @@ impl<'a, 'b> QueryFromConnection<'a, 'b> { chain_config: &ChainConfig, ) -> Result, ApiServerStorageError> { let len = len as i64; - let offset = offset as i64; + let Some(offset) = Self::offset_to_bigint(offset) else { + return Ok(Vec::new()); + }; let rows = self .tx .query( @@ -2481,7 +2495,9 @@ impl<'a, 'b> QueryFromConnection<'a, 'b> { offset: u64, ) -> Result, ApiServerStorageError> { let len = len as i64; - let offset = offset as i64; + let Some(offset) = Self::offset_to_bigint(offset) else { + return Ok(Vec::new()); + }; let rows = self .tx .query( @@ -3088,9 +3104,7 @@ impl<'a, 'b> QueryFromConnection<'a, 'b> { offset: u64, ) -> Result, ApiServerStorageError> { let len = len as i64; - // An offset too big for a bigint is past the end of any result set. Postgres rejects - // a negative OFFSET, so return an empty page rather than let the cast wrap. - let Ok(offset) = i64::try_from(offset) else { + let Some(offset) = Self::offset_to_bigint(offset) else { return Ok(Vec::new()); }; self.tx @@ -3126,9 +3140,7 @@ impl<'a, 'b> QueryFromConnection<'a, 'b> { ticker: &str, ) -> Result, ApiServerStorageError> { let len = len as i64; - // An offset too big for a bigint is past the end of any result set. Postgres rejects - // a negative OFFSET, so return an empty page rather than let the cast wrap. - let Ok(offset) = i64::try_from(offset) else { + let Some(offset) = Self::offset_to_bigint(offset) else { return Ok(Vec::new()); }; let escaped_ticker = escape_for_like(ticker); @@ -3709,7 +3721,9 @@ impl<'a, 'b> QueryFromConnection<'a, 'b> { chain_config: &ChainConfig, ) -> Result, ApiServerStorageError> { let len = len as i64; - let offset = offset as i64; + let Some(offset) = Self::offset_to_bigint(offset) else { + return Ok(Vec::new()); + }; self.tx .query( r#" @@ -3742,7 +3756,9 @@ impl<'a, 'b> QueryFromConnection<'a, 'b> { chain_config: &ChainConfig, ) -> Result, ApiServerStorageError> { let len = len as i64; - let offset = offset as i64; + let Some(offset) = Self::offset_to_bigint(offset) else { + return Ok(Vec::new()); + }; self.tx .query( r#" diff --git a/api-server/api-server-common/src/storage/impls/postgres/transactional/read.rs b/api-server/api-server-common/src/storage/impls/postgres/transactional/read.rs index 7f133b11e..c232b1347 100644 --- a/api-server/api-server-common/src/storage/impls/postgres/transactional/read.rs +++ b/api-server/api-server-common/src/storage/impls/postgres/transactional/read.rs @@ -109,10 +109,10 @@ impl ApiServerStorageRead for ApiServerPostgresTransactionalRo<'_> { &self, token_id: TokenId, len: u32, - tx_global_index: u64, + offset: u64, ) -> Result, ApiServerStorageError> { let conn = QueryFromConnection::new(self.connection.as_ref().expect(CONN_ERR)); - let res = conn.get_token_transactions(token_id, len, tx_global_index).await?; + let res = conn.get_token_transactions(token_id, len, offset).await?; Ok(res) } diff --git a/api-server/api-server-common/src/storage/impls/postgres/transactional/write.rs b/api-server/api-server-common/src/storage/impls/postgres/transactional/write.rs index 63690bf4d..692a3882c 100644 --- a/api-server/api-server-common/src/storage/impls/postgres/transactional/write.rs +++ b/api-server/api-server-common/src/storage/impls/postgres/transactional/write.rs @@ -510,10 +510,10 @@ impl ApiServerStorageRead for ApiServerPostgresTransactionalRw<'_> { &self, token_id: TokenId, len: u32, - tx_global_index: u64, + offset: u64, ) -> Result, ApiServerStorageError> { let conn = QueryFromConnection::new(self.connection.as_ref().expect(CONN_ERR)); - let res = conn.get_token_transactions(token_id, len, tx_global_index).await?; + let res = conn.get_token_transactions(token_id, len, offset).await?; Ok(res) } diff --git a/api-server/api-server-common/src/storage/storage_api/mod.rs b/api-server/api-server-common/src/storage/storage_api/mod.rs index 4bd2aa485..a3f5427e9 100644 --- a/api-server/api-server-common/src/storage/storage_api/mod.rs +++ b/api-server/api-server-common/src/storage/storage_api/mod.rs @@ -652,13 +652,15 @@ pub trait ApiServerStorageRead: Sync { ) -> Result>, ApiServerStorageError>; /// Returns a page of transaction IDs that reference this `token_id`, limited to `len` entries - /// and with a `tx_global_index` older than the specified value. - /// The `tx_global_index` is not continuous for a specific `token_id`. + /// and starting `offset` entries into the full, newest-first list for the token. An offset + /// greater than or equal to the number of transactions for the token yields an empty page. + /// As with the other offset-paginated queries, the cost of a request grows linearly with the + /// offset. async fn get_token_transactions( &self, token_id: TokenId, len: u32, - tx_global_index: u64, + offset: u64, ) -> Result, ApiServerStorageError>; async fn get_best_block(&self) -> Result; diff --git a/api-server/scanner-lib/src/sync/tests/mod.rs b/api-server/scanner-lib/src/sync/tests/mod.rs index 29a343c36..2ffac4011 100644 --- a/api-server/scanner-lib/src/sync/tests/mod.rs +++ b/api-server/scanner-lib/src/sync/tests/mod.rs @@ -1505,7 +1505,7 @@ async fn token_transactions_storage_check(#[case] seed: Seed) { // Check count: Issue(1) + Mint(1) = 2 let db_tx = local_state.storage().transaction_ro().await.unwrap(); let txs = db_tx - .get_token_transactions(token_id, 100, u64::MAX) + .get_token_transactions(token_id, 100, 0) .await .unwrap() .into_iter() @@ -1597,7 +1597,7 @@ async fn token_transactions_storage_check(#[case] seed: Seed) { // Verify Storage: 2 previous + 4 new = 6 transactions let db_tx = local_state.storage().transaction_ro().await.unwrap(); let txs = db_tx - .get_token_transactions(token_id, 100, u64::MAX) + .get_token_transactions(token_id, 100, 0) .await .unwrap() .into_iter() @@ -1638,7 +1638,7 @@ async fn token_transactions_storage_check(#[case] seed: Seed) { token_txs.insert(tx_spend_id); let db_tx = local_state.storage().transaction_ro().await.unwrap(); let txs = db_tx - .get_token_transactions(token_id, 100, u64::MAX) + .get_token_transactions(token_id, 100, 0) .await .unwrap() .into_iter() @@ -1693,7 +1693,7 @@ async fn token_transactions_storage_check(#[case] seed: Seed) { // Verify Storage: Order creation involves the token (in 'Give'), so it should be indexed. let db_tx = local_state.storage().transaction_ro().await.unwrap(); let txs = db_tx - .get_token_transactions(token_id, 100, u64::MAX) + .get_token_transactions(token_id, 100, 0) .await .unwrap() .into_iter() @@ -1743,7 +1743,7 @@ async fn token_transactions_storage_check(#[case] seed: Seed) { // Verify Storage: Fill Order should be indexed for the token let db_tx = local_state.storage().transaction_ro().await.unwrap(); let txs = db_tx - .get_token_transactions(token_id, 100, u64::MAX) + .get_token_transactions(token_id, 100, 0) .await .unwrap() .into_iter() @@ -1785,7 +1785,7 @@ async fn token_transactions_storage_check(#[case] seed: Seed) { // Verify Storage: Conclude Order should be indexed for the token let db_tx = local_state.storage().transaction_ro().await.unwrap(); let txs = db_tx - .get_token_transactions(token_id, 100, u64::MAX) + .get_token_transactions(token_id, 100, 0) .await .unwrap() .into_iter() @@ -1827,7 +1827,7 @@ async fn token_transactions_storage_check(#[case] seed: Seed) { // Verify Storage: Conclude Order should be indexed for the token let db_tx = local_state.storage().transaction_ro().await.unwrap(); let txs = db_tx - .get_token_transactions(token_id, 100, u64::MAX) + .get_token_transactions(token_id, 100, 0) .await .unwrap() .into_iter() diff --git a/api-server/stack-test-suite/tests/v2/token_transactions.rs b/api-server/stack-test-suite/tests/v2/token_transactions.rs index 9ccc0b3ff..146f0f939 100644 --- a/api-server/stack-test-suite/tests/v2/token_transactions.rs +++ b/api-server/stack-test-suite/tests/v2/token_transactions.rs @@ -268,7 +268,7 @@ async fn ok(#[case] seed: Seed) { let (token_id, expected_transactions) = rx.await.unwrap(); let num_tx = expected_transactions.len(); - let url = format!("/api/v2/token/{token_id}/transactions?offset=999&items={num_tx}"); + let url = format!("/api/v2/token/{token_id}/transactions?offset=0&items={num_tx}"); let response = reqwest::get(format!("http://{}:{}{url}", addr.ip(), addr.port())) .await @@ -296,10 +296,7 @@ async fn ok(#[case] seed: Seed) { let offset = rng.random_range(1..num_tx); let items = num_tx - offset; - let tx_global_index = &arr_body[offset - 1].get("tx_global_index").unwrap(); - eprintln!("tx_global_index: '{tx_global_index}'"); - let url = - format!("/api/v2/token/{token_id}/transactions?offset={tx_global_index}&items={items}"); + let url = format!("/api/v2/token/{token_id}/transactions?offset={offset}&items={items}"); let response = reqwest::get(format!("http://{}:{}{url}", addr.ip(), addr.port())) .await diff --git a/api-server/storage-test-suite/src/basic.rs b/api-server/storage-test-suite/src/basic.rs index 902c8d8a0..09e015abb 100644 --- a/api-server/storage-test-suite/src/basic.rs +++ b/api-server/storage-test-suite/src/basic.rs @@ -981,28 +981,26 @@ where } let len = 5; - let global_idx = 10; - let token_txs = - db_tx.get_token_transactions(random_token_id, len, global_idx).await.unwrap(); + let offset = 0; + let token_txs = db_tx.get_token_transactions(random_token_id, len, offset).await.unwrap(); let expected_txs: Vec<_> = token_transactions .iter() .rev() - .filter(|tx| tx.tx_global_index < global_idx) + .skip(offset as usize) .take(len as usize) .cloned() .collect(); assert_eq!(token_txs, expected_txs); let len = 5; - let global_idx = 5; - let token_txs = - db_tx.get_token_transactions(random_token_id, len, global_idx).await.unwrap(); + let offset = 5; + let token_txs = db_tx.get_token_transactions(random_token_id, len, offset).await.unwrap(); let expected_txs: Vec<_> = token_transactions .iter() .rev() - .filter(|tx| tx.tx_global_index < global_idx) + .skip(offset as usize) .take(len as usize) .cloned() .collect(); @@ -1033,14 +1031,13 @@ where } let len = 5; - let global_idx = 200; - let token_txs = - db_tx.get_token_transactions(random_token_id, len, global_idx).await.unwrap(); + let offset = 0; + let token_txs = db_tx.get_token_transactions(random_token_id, len, offset).await.unwrap(); let expected_txs: Vec<_> = updated_token_transactions .iter() .rev() - .filter(|tx| tx.tx_global_index < global_idx) + .skip(offset as usize) .take(len as usize) .cloned() .collect(); @@ -1048,14 +1045,13 @@ where assert_eq!(token_txs, expected_txs); let len = 5; - let global_idx = 105; - let token_txs = - db_tx.get_token_transactions(random_token_id, len, global_idx).await.unwrap(); + let offset = 5; + let token_txs = db_tx.get_token_transactions(random_token_id, len, offset).await.unwrap(); let expected_txs: Vec<_> = updated_token_transactions .iter() .rev() - .filter(|tx| tx.tx_global_index < global_idx) + .skip(offset as usize) .take(len as usize) .cloned() .collect(); @@ -1082,20 +1078,20 @@ where } let len = rng.random_range(0..5); - let global_idx = rng.random_range(5..=10); - let token_txs = - db_tx.get_token_transactions(random_token_id, len, global_idx).await.unwrap(); - eprintln!("getting len: {len} < idx {global_idx}"); + let offset = rng.random_range(0..=5); + let token_txs = db_tx.get_token_transactions(random_token_id, len, offset).await.unwrap(); + eprintln!("getting len: {len}, offset {offset}"); let expected_token_txs: Vec<_> = token_transactions .iter() .rev() - .filter_map(|(idx, tx_id, _)| { + .map(|(idx, tx_id, _)| { let idx = *idx; - ((idx) < global_idx).then_some(TokenTransaction { + TokenTransaction { tx_global_index: idx, tx_id: *tx_id, - }) + } }) + .skip(offset as usize) .take(len as usize) .collect(); @@ -2698,6 +2694,85 @@ fn random_order( (order_id, order) } +/// The offset passed to `get_token_transactions` skips over the newest entries of the token's +/// transaction list, matching the skip-N semantics of the offset parameter the HTTP API passes: +/// offset 0 starts at the newest transaction, and an offset at or past the end of the list +/// yields an empty page. +pub async fn token_transactions_offset_paging( + storage_maker: Arc, + seed_maker: Box Seed + Send>, +) -> Result<(), Failed> +where + S: ApiServerStorage, + Fut: Future + Send + 'static, + F: Fn() -> Fut, +{ + let mut rng = make_seedable_rng(seed_maker()); + let token_id = TokenId::random_using(&mut rng); + let mut storage = storage_maker().await; + { + let mut tx = storage.transaction_rw().await.unwrap(); + let chain_config = create_unit_test_config(); + tx.reinitialize_storage(&chain_config).await.unwrap(); + tx.commit().await.unwrap(); + } + + // 10 transactions for the token, with continuous tx_global_index values 0..10. + let token_transactions: Vec<_> = (0..10) + .map(|idx| { + let random_tx_id = Id::::random_using(&mut rng); + TokenTransaction { + tx_global_index: idx, + tx_id: random_tx_id, + } + }) + .collect(); + + let mut tx = storage.transaction_rw().await.unwrap(); + for txn in &token_transactions { + tx.set_token_transaction_at_height( + token_id, + txn.tx_id, + BlockHeight::new(txn.tx_global_index), + txn.tx_global_index, + ) + .await + .unwrap(); + } + + let newest_first: Vec<_> = token_transactions.iter().rev().cloned().collect(); + + // Default paging (offset 0) returns the newest transactions first. + assert_eq!( + tx.get_token_transactions(token_id, 10, 0).await.unwrap(), + newest_first + ); + + // The page length is respected. + assert_eq!( + tx.get_token_transactions(token_id, 2, 0).await.unwrap(), + newest_first[..2] + ); + + // The offset skips over the newest entries. + assert_eq!( + tx.get_token_transactions(token_id, 4, 3).await.unwrap(), + &newest_first[3..7] + ); + + // An offset at or past the end of the list yields an empty page, however large. + assert!(tx.get_token_transactions(token_id, 10, 10).await.unwrap().is_empty()); + assert!(tx.get_token_transactions(token_id, 10, 12345).await.unwrap().is_empty()); + assert!(tx.get_token_transactions(token_id, 10, u64::MAX).await.unwrap().is_empty()); + // An offset that fits a 64-bit usize but not a 32-bit one must not wrap to a small value + // (which would return the first page) on narrow-pointer targets. + assert!(tx.get_token_transactions(token_id, 10, 1 << 35).await.unwrap().is_empty()); + + tx.commit().await.unwrap(); + + Ok(()) +} + pub fn build_tests Fut + Send + Sync + 'static>( storage_maker: Arc, ) -> impl Iterator @@ -2708,6 +2783,7 @@ where vec![ make_test!(initialization, storage_maker.clone()), make_test!(set_get, storage_maker.clone()), + make_test!(token_transactions_offset_paging, storage_maker.clone()), make_test!(pool_data_keyset, storage_maker.clone()), make_test!(top_address_amounts, storage_maker.clone()), make_test!(order_book_entries, storage_maker.clone()),