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 1247279429..9b2a2a6eb9 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 @@ -2523,26 +2523,24 @@ impl<'a, 'b> QueryFromConnection<'a, 'b> { ) -> Result, ApiServerStorageError> { let len = len as i64; let offset = offset as i64; + // both tables have one row per token state change, hence the DISTINCTs self.tx .query( r#" WITH count_tokens AS ( - SELECT count(token_id) FROM ml.fungible_token + SELECT count(DISTINCT token_id) FROM ml.fungible_token ) - (SELECT token_id + (SELECT DISTINCT token_id FROM ml.fungible_token ORDER BY token_id OFFSET $1 LIMIT $2) UNION ALL - (SELECT nft_id + (SELECT DISTINCT nft_id FROM ml.nft_issuance ORDER BY nft_id OFFSET GREATEST($1 - (SELECT * FROM count_tokens), 0) - LIMIT CASE - WHEN ($1 - (SELECT * FROM count_tokens) >= -$2) - THEN ($2 + $1 - (SELECT * FROM count_tokens)) - ELSE 0 END); + LIMIT GREATEST(LEAST($2, $2 + $1 - (SELECT * FROM count_tokens)), 0)); "#, &[&offset, &len], ) @@ -2572,24 +2570,21 @@ impl<'a, 'b> QueryFromConnection<'a, 'b> { .query( r#" WITH count_tokens AS ( - SELECT count(token_id) FROM ml.fungible_token WHERE ticker ILIKE $3 + SELECT count(DISTINCT token_id) FROM ml.fungible_token WHERE ticker ILIKE $3 ) - (SELECT token_id + (SELECT DISTINCT token_id FROM ml.fungible_token WHERE ticker ILIKE $3 ORDER BY token_id OFFSET $1 LIMIT $2) UNION ALL - (SELECT nft_id + (SELECT DISTINCT nft_id FROM ml.nft_issuance WHERE ticker ILIKE $3 ORDER BY nft_id OFFSET GREATEST($1 - (SELECT * FROM count_tokens), 0) - LIMIT CASE - WHEN ($1 - (SELECT * FROM count_tokens) >= -$2) - THEN ($2 + $1 - (SELECT * FROM count_tokens)) - ELSE 0 END); + LIMIT GREATEST(LEAST($2, $2 + $1 - (SELECT * FROM count_tokens)), 0)); "#, &[&offset, &len, &ticker_patern], ) diff --git a/api-server/stack-test-suite/tests/v2/token_ids.rs b/api-server/stack-test-suite/tests/v2/token_ids.rs index 6a3b6ce947..562203971d 100644 --- a/api-server/stack-test-suite/tests/v2/token_ids.rs +++ b/api-server/stack-test-suite/tests/v2/token_ids.rs @@ -14,7 +14,7 @@ // limitations under the License. use common::chain::{ - make_token_id, + AccountCommand, AccountNonce, UtxoOutPoint, make_token_id, tokens::{IsTokenFreezable, NftIssuance, TokenIssuance, TokenIssuanceV1, TokenTotalSupply}, }; @@ -254,3 +254,199 @@ async fn ok(#[case] seed: Seed) { task.abort(); } + +// Tokens with multiple state changes must appear only once in the response (issue #1982) +#[rstest] +#[trace] +#[case(Seed::from_entropy())] +#[tokio::test] +async fn no_duplicate_ids_for_tokens_with_state_changes(#[case] seed: Seed) { + let listener = tokio::net::TcpListener::bind("127.0.0.1:0").await.unwrap(); + let addr = listener.local_addr().unwrap(); + + let (tx, rx) = tokio::sync::oneshot::channel(); + + let task = tokio::spawn(async move { + let web_server_state = { + let mut rng = make_seedable_rng(seed); + let chain_config = create_unit_test_config(); + + let chainstate_blocks = { + let mut tf = TestFramework::builder(&mut rng) + .with_chain_config(chain_config.clone()) + .build(); + + // AnyoneCanSpend authority so the mint input can go unsigned + let token_issuance = TokenIssuanceV1 { + token_ticker: "XXXX".as_bytes().to_vec(), + number_of_decimals: rng.random_range(1..18), + metadata_uri: "http://uri".as_bytes().to_vec(), + total_supply: TokenTotalSupply::Unlimited, + authority: Destination::AnyoneCanSpend, + is_freezable: IsTokenFreezable::No, + }; + + let genesis_outpoint = UtxoOutPoint::new(tf.best_block_id().into(), 0); + let genesis_coins = chainstate_test_framework::get_output_value( + tf.chainstate.utxo(&genesis_outpoint).unwrap().unwrap().output(), + ) + .unwrap() + .coin_amount() + .unwrap(); + + let issuance_fee = chain_config.fungible_token_issuance_fee(); + let supply_change_fee = chain_config.token_supply_change_fee(BlockHeight::zero()); + + // issue a token + let coins_after_issue = (genesis_coins - issuance_fee).unwrap(); + let issue_tx = TransactionBuilder::new() + .add_input(genesis_outpoint.into(), InputWitness::NoSignature(None)) + .add_output(TxOutput::Transfer( + OutputValue::Coin(coins_after_issue), + Destination::AnyoneCanSpend, + )) + .add_output(TxOutput::IssueFungibleToken(Box::new(TokenIssuance::V1( + token_issuance.clone(), + )))) + .build(); + let minted_token_id = + make_token_id(&chain_config, tf.next_block_height(), issue_tx.inputs()) + .unwrap(); + let issue_tx_id = issue_tx.transaction().get_id(); + tf.make_block_builder() + .add_transaction(issue_tx) + .build_and_process(&mut rng) + .unwrap() + .unwrap(); + + // mint it, giving the token a second state row + let amount_to_mint = Amount::from_atoms(rng.random_range(100..1000)); + let coins_after_mint = (coins_after_issue - supply_change_fee).unwrap(); + let mint_tx = TransactionBuilder::new() + .add_input( + TxInput::from_command( + AccountNonce::new(0), + AccountCommand::MintTokens(minted_token_id, amount_to_mint), + ), + InputWitness::NoSignature(None), + ) + .add_input( + TxInput::from_utxo(issue_tx_id.into(), 0), + InputWitness::NoSignature(None), + ) + .add_output(TxOutput::Transfer( + OutputValue::Coin(coins_after_mint), + Destination::AnyoneCanSpend, + )) + .add_output(TxOutput::Transfer( + OutputValue::TokenV1(minted_token_id, amount_to_mint), + Destination::AnyoneCanSpend, + )) + .build(); + let mint_tx_id = mint_tx.transaction().get_id(); + tf.make_block_builder() + .add_transaction(mint_tx) + .build_and_process(&mut rng) + .unwrap() + .unwrap(); + + // issue a second token with a single state row and the same ticker + let coins_after_second_issue = (coins_after_mint - issuance_fee).unwrap(); + let second_issue_tx = TransactionBuilder::new() + .add_input( + TxInput::from_utxo(mint_tx_id.into(), 0), + InputWitness::NoSignature(None), + ) + .add_output(TxOutput::Transfer( + OutputValue::Coin(coins_after_second_issue), + Destination::AnyoneCanSpend, + )) + .add_output(TxOutput::IssueFungibleToken(Box::new(TokenIssuance::V1( + token_issuance, + )))) + .build(); + let single_row_token_id = make_token_id( + &chain_config, + tf.next_block_height(), + second_issue_tx.inputs(), + ) + .unwrap(); + tf.make_block_builder() + .add_transaction(second_issue_tx) + .build_and_process(&mut rng) + .unwrap() + .unwrap(); + + _ = tx.send((minted_token_id, single_row_token_id)); + + tf.block_indexes + .iter() + .map(|idx| tf.block(tf.to_chain_block_id(idx.block_id().into()))) + .collect::>() + }; + + let storage = { + let mut storage = TransactionalApiServerInMemoryStorage::new(&chain_config); + + let mut db_tx = storage.transaction_rw().await.unwrap(); + db_tx.reinitialize_storage(&chain_config).await.unwrap(); + db_tx.commit().await.unwrap(); + + storage + }; + + let chain_config = Arc::new(chain_config); + + let mut local_node = BlockchainState::new(Arc::clone(&chain_config), storage); + local_node.scan_genesis(chain_config.genesis_block()).await.unwrap(); + local_node.scan_blocks(BlockHeight::new(0), chainstate_blocks).await.unwrap(); + + ApiServerWebServerState { + db: Arc::new(local_node.storage().clone_storage().await), + chain_config: Arc::clone(&chain_config), + rpc: Arc::new(DummyRPC {}), + cached_values: Arc::new(CachedValues { + feerate_points: RwLock::new((get_time(), vec![])), + }), + time_getter: Default::default(), + } + }; + + web_server(listener, web_server_state, false).await + }); + + let chain_config = create_unit_test_config(); + let (minted_token_id, single_row_token_id) = rx.await.unwrap(); + let minted_token_address = Address::new(&chain_config, minted_token_id).unwrap().into_string(); + let single_row_token_address = + Address::new(&chain_config, single_row_token_id).unwrap().into_string(); + + for url in [ + "/api/v2/token?offset=0&items=10".to_string(), + "/api/v2/token/ticker/XXXX?offset=0&items=10".to_string(), + ] { + let response = reqwest::get(format!("http://{}:{}{url}", addr.ip(), addr.port())) + .await + .unwrap(); + + assert_eq!(response.status(), 200, "Failed getting token ids"); + + let body = response.text().await.unwrap(); + let body: serde_json::Value = serde_json::from_str(&body).unwrap(); + let ids = body.as_array().unwrap().iter().map(|v| v.as_str().unwrap()).collect::>(); + + assert_eq!( + ids.iter().filter(|id| **id == minted_token_address).count(), + 1 + ); + assert_eq!( + ids.iter().filter(|id| **id == single_row_token_address).count(), + 1 + ); + + let unique_ids = ids.iter().copied().collect::>(); + assert_eq!(unique_ids.len(), ids.len(), "duplicate token ids: {ids:?}"); + } + + task.abort(); +} diff --git a/api-server/storage-test-suite/src/basic.rs b/api-server/storage-test-suite/src/basic.rs index 664b813a8a..ce6cfd9ebb 100644 --- a/api-server/storage-test-suite/src/basic.rs +++ b/api-server/storage-test-suite/src/basic.rs @@ -2148,6 +2148,222 @@ async fn orders<'a, S: for<'b> Transactional<'b>>(rng: &mut impl CryptoRng, stor ); } +// Tokens with multiple state-change rows must appear exactly once in +// get_token_ids/get_token_ids_by_ticker, on every page (issue #1982) +pub async fn token_ids_dedup_and_pagination( + storage_maker: Arc, + seed_maker: Box Seed + Send>, +) -> Result<(), Failed> +where + S: ApiServerStorage, + Fut: Future + Send + 'static, + F: Fn() -> Fut, +{ + let seed = seed_maker(); + let mut rng = make_seedable_rng(seed); + + 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(); + + let mut db_tx = storage.transaction_rw().await.unwrap(); + + let (_, pk) = PrivateKey::new_from_rng(&mut rng, KeyKind::Secp256k1Schnorr); + let authority = Destination::PublicKeyHash(PublicKeyHash::from(&pk)); + + let matching_ticker = "MATCH"; + let other_ticker = "OTHER"; + // ticker used only by NFTs, so the fungible half of the union matches nothing + let nft_only_ticker = "NFTONLY"; + + let mut fungible_ids = Vec::new(); + let mut matching_fungible_ids = Vec::new(); + for i in 0..rng.random_range(5..10) { + let token_id = TokenId::random_using(&mut rng); + let matches_ticker = i % 2 == 0; + let ticker = if matches_ticker { + matching_ticker + } else { + other_ticker + }; + let token_data = FungibleTokenData { + token_ticker: ticker.as_bytes().to_vec(), + number_of_decimals: rng.random_range(1..18), + metadata_uri: "http://uri".as_bytes().to_vec(), + circulating_supply: Amount::ZERO, + total_supply: TokenTotalSupply::Unlimited, + is_locked: false, + frozen: IsTokenFrozen::No(IsTokenFreezable::Yes), + authority: authority.clone(), + next_nonce: AccountNonce::new(0), + }; + + let mut block_height = BlockHeight::new(rng.random_range(1..100)); + db_tx + .set_fungible_token_issuance(token_id, block_height, token_data.clone()) + .await + .unwrap(); + + let num_state_changes = if i % 2 == 0 { + rng.random_range(1..4) + } else { + 0 + }; + let mut token_data = token_data; + for _ in 0..num_state_changes { + block_height = block_height.next_height(); + let next_nonce = token_data.next_nonce; + token_data = + token_data.mint_tokens(Amount::from_atoms(rng.random_range(1..1000)), next_nonce); + db_tx + .set_fungible_token_data(token_id, block_height, token_data.clone()) + .await + .unwrap(); + } + + fungible_ids.push(token_id); + if matches_ticker { + matching_fungible_ids.push(token_id); + } + } + + let mut nft_ids = Vec::new(); + let mut matching_nft_ids = Vec::new(); + let mut nft_only_ids = Vec::new(); + for i in 0..rng.random_range(5..10) { + let token_id = TokenId::random_using(&mut rng); + let ticker = match i % 3 { + 0 => matching_ticker, + 1 => other_ticker, + _ => nft_only_ticker, + }; + let nft = NftIssuance::V0(NftIssuanceV0 { + metadata: common::chain::tokens::Metadata { + creator: None, + name: "Name".as_bytes().to_vec(), + description: "SomeNFT".as_bytes().to_vec(), + ticker: ticker.as_bytes().to_vec(), + icon_uri: DataOrNoVec::from(None), + additional_metadata_uri: DataOrNoVec::from(None), + media_uri: DataOrNoVec::from(None), + media_hash: "123456".as_bytes().to_vec(), + }, + }); + + let mut block_height = BlockHeight::new(rng.random_range(1..100)); + db_tx + .set_nft_token_issuance(token_id, block_height, nft, &authority) + .await + .unwrap(); + + // an owner change adds a row to the NFT issuance table + let num_owner_changes = if i % 2 == 0 { + rng.random_range(1..4) + } else { + 0 + }; + for _ in 0..num_owner_changes { + block_height = block_height.next_height(); + let (_, new_owner_pk) = PrivateKey::new_from_rng(&mut rng, KeyKind::Secp256k1Schnorr); + let new_owner = Address::new( + &chain_config, + Destination::PublicKeyHash(PublicKeyHash::from(&new_owner_pk)), + ) + .unwrap(); + db_tx + .set_address_balance_at_height( + &new_owner, + Amount::from_atoms(1), + CoinOrTokenId::TokenId(token_id), + block_height, + ) + .await + .unwrap(); + } + + nft_ids.push(token_id); + match i % 3 { + 0 => matching_nft_ids.push(token_id), + 1 => (), + _ => nft_only_ids.push(token_id), + } + } + + // expected order: fungible tokens first, then NFTs, each sorted by id + fungible_ids.sort(); + nft_ids.sort(); + matching_fungible_ids.sort(); + matching_nft_ids.sort(); + nft_only_ids.sort(); + let all_ids = fungible_ids.iter().chain(nft_ids.iter()).copied().collect::>(); + let matching_ids = matching_fungible_ids + .iter() + .chain(matching_nft_ids.iter()) + .copied() + .collect::>(); + + let ids = db_tx.get_token_ids((all_ids.len() * 2) as u32, 0).await.unwrap(); + assert_eq!(ids, all_ids); + + // walk all pages at every page size; the concatenation must equal the full list + for page_size in 1..=all_ids.len() { + let mut collected = Vec::new(); + loop { + let page = db_tx.get_token_ids(page_size as u32, collected.len() as u64).await.unwrap(); + let expected_len = std::cmp::min(page_size, all_ids.len() - collected.len()); + assert_eq!(page.len(), expected_len); + if page.is_empty() { + break; + } + collected.extend(page); + } + assert_eq!(collected, all_ids); + } + + // same for the ticker variant, including a ticker with no fungible matches + for (ticker, expected) in [(matching_ticker, &matching_ids), (nft_only_ticker, &nft_only_ids)] { + let ids = db_tx + .get_token_ids_by_ticker((all_ids.len() * 2) as u32, 0, ticker) + .await + .unwrap(); + assert_eq!(&ids, expected); + + for page_size in 1..=expected.len() { + let mut collected = Vec::new(); + loop { + let page = db_tx + .get_token_ids_by_ticker(page_size as u32, collected.len() as u64, ticker) + .await + .unwrap(); + let expected_len = std::cmp::min(page_size, expected.len() - collected.len()); + assert_eq!(page.len(), expected_len); + if page.is_empty() { + break; + } + collected.extend(page); + } + assert_eq!(&collected, expected); + } + } + + // offsets past the end and zero-size pages return nothing + let past_end = (all_ids.len() * 2) as u64; + assert_eq!(db_tx.get_token_ids(5, past_end).await.unwrap(), Vec::new()); + assert_eq!(db_tx.get_token_ids(0, 0).await.unwrap(), Vec::new()); + assert_eq!( + db_tx.get_token_ids_by_ticker(5, past_end, matching_ticker).await.unwrap(), + Vec::new() + ); + assert_eq!( + db_tx.get_token_ids_by_ticker(0, 0, matching_ticker).await.unwrap(), + Vec::new() + ); + + Ok(()) +} + fn random_order( rng: &mut impl CryptoRng, creation_height: BlockHeight, @@ -2183,7 +2399,8 @@ where { vec![ make_test!(initialization, storage_maker.clone()), - make_test!(set_get, storage_maker), + make_test!(set_get, storage_maker.clone()), + make_test!(token_ids_dedup_and_pagination, storage_maker), ] .into_iter() }