Add token transactions endpoint for the API Server #1991

OBorce · 2025-11-16T16:29:46Z

Add a new endpoint that will return a paginated list of transaction IDs that involve a certain token_id

api-server/web-server/src/api/v2.rs

api-server/scanner-lib/src/blockchain_state/mod.rs

api-server/api-server-common/src/storage/storage_api/mod.rs

ImplOfAnImpl · 2025-11-19T13:37:09Z

api-server/api-server-common/src/storage/impls/in_memory/mod.rs

+        self.token_transactions_table.entry(token_id).or_default().insert(
+            block_height,
+            transaction_ids
+                .into_iter()
+                .enumerate()
+                .map(|(idx, tx_id)| TokenTransaction {
+                    global_tx_index: next_tx_idx + idx as i64,
+                    tx_id,
+                })
+                .collect(),
+        );


Since this whole thing will break if the passed block_height is not the previous block height plus 1, plz assert on this fact.

Not sure what you mean here, it should be okay to have gaps in the block_height

ImplOfAnImpl · 2025-11-19T13:43:58Z

api-server/api-server-common/src/storage/impls/in_memory/mod.rs

+                .last()
+                .expect("not empty")
+                .last()
+                .expect("not empty")


I guess this will break if the previous call to set_token_transactions_at_height had empty transaction_ids.

If so, plz assert that transaction_ids is not empty.

Added a check if the ids are empty just return Ok

api-server/api-server-common/src/storage/storage_api/mod.rs

ImplOfAnImpl · 2025-11-19T14:11:34Z

api-server/api-server-common/src/storage/impls/postgres/queries.rs

+        for transaction_id in transaction_ids {
+            self.tx
+                .execute(
+                    r#"
+                        INSERT INTO ml.token_transactions (token_id, block_height, transaction_id)
+                        VALUES ($1, $2, $3)
+                        ON CONFLICT (token_id, block_height, transaction_id)
+                        DO NOTHING;
+                    "#,
+                    &[&token_id.encode(), &height, &transaction_id.encode()],
+                )
+                .await
+                .map_err(|e| ApiServerStorageError::LowLevelStorageError(e.to_string()))?;
+        }


So, as I can see, in the in-memory implementation, this "global_tx_index" is defined per token.
Also, calling set_token_transactions_at_height twice for the same height will overwrite the previous data.

And here:

global_tx_index is indeed global, but it's unique for each "(token_id, block_height, transaction_id)" combination. I.e. for each particular token there will be gaps in the indices.
On the other hand, it's definitely not the same as the "normal" global tx index (e.g. because here transactions in a block are sorted by their ids, instead of their actual order inside the block; also, the "normal" global index starts from 0 as I can see, and this one starts from 1).

calling set_token_transactions_at_height twice for the same height will keep the previous data.

So,

plz make both implementations consistent; but first plz write some tests that check the produces indices.

regarding what that "global_tx_index" should mean - IMO it should be the same as the "normal" global index, because otherwise the API will be confusing.

I think it should be conistant now between the implementations both being unique globaly not per token_id. And fixed the name to tx_global_index everywhere.

And what about the part:

regarding what that "global_tx_index" should mean - IMO it should be the same as the "normal" global index, because otherwise the API will be confusing.

?

api-server/api-server-common/src/storage/impls/in_memory/mod.rs

ImplOfAnImpl · 2025-11-25T19:55:18Z

api-server/api-server-common/src/storage/storage_api/mod.rs


+    /// Return a page of TX IDs that reference this token_id, with a limit of len and older
+    /// tx_global_index than the specified.
+    /// The tx_global_index is only ordered by block height and are not continuous for a specific
+    /// token_id.
+    async fn get_token_transactions(
+        &self,
+        token_id: TokenId,
+        len: u32,
+        tx_global_index: u64,
+    ) -> Result<Vec<TokenTransaction>, ApiServerStorageError>;


"is only ordered by block height and are not continuous" - inconsistent "is" and "are".

In general, can you plz put more effort into writing documentation comments? "than the specified" - it's not even correct English.

api-server/scanner-lib/src/sync/tests/mod.rs

api-server/api-server-common/src/storage/storage_api/mod.rs

ImplOfAnImpl · 2025-11-25T20:37:39Z

api-server/api-server-common/src/storage/storage_api/mod.rs


+#[derive(Debug, Clone, Copy, PartialEq, Eq)]
+pub struct TokenTransaction {
+    pub tx_global_index: u64,


You've made names consistent, but you haven't made the actual indices consistent. This hasn't made things less confusing.

ImplOfAnImpl · 2025-11-26T09:04:55Z

api-server/api-server-common/src/storage/impls/postgres/queries.rs

+        for transaction_id in transaction_ids {
+            self.tx
+                .execute(
+                    r#"
+                        INSERT INTO ml.token_transactions (token_id, block_height, transaction_id)
+                        VALUES ($1, $2, $3)
+                        ON CONFLICT (token_id, block_height, transaction_id)
+                        DO NOTHING;
+                    "#,
+                    &[&token_id.encode(), &height, &transaction_id.encode()],
+                )
+                .await
+                .map_err(|e| ApiServerStorageError::LowLevelStorageError(e.to_string()))?;
+        }


And what about the part:

regarding what that "global_tx_index" should mean - IMO it should be the same as the "normal" global index, because otherwise the API will be confusing.

?

ImplOfAnImpl · 2025-11-26T09:34:08Z

api-server/api-server-common/src/storage/impls/postgres/queries.rs

+                    SELECT tx_global_index, transaction_id
+                    FROM ml.token_transactions


So, this tx_global_index is different from tx_global_index inside ml.transactions. Such inconsistency, if unavoidable, should be justified - comments must be written about why it has to be so. Also, probably we should invent different names for different indices. Also, we should probably introduce some newtypes to use instead of u64, so that one index can't be accidentally confused with another.

But I don't understand why they should be different:

Can we remove tx_global_index from ml.token_transactions altogether and fetch it from ml.transactions instead?

If the above adds too much overhead, then perhaps set_token_transactions_at_height should accept tx_global_index values together with tx ids, instead of relying on them being automatically generated by the storage. As I can see, in places where set_token_transactions_at_height is called, the tx_global_indexs are already known.

Went with option number 2.

ImplOfAnImpl · 2025-11-26T10:22:00Z

api-server/api-server-common/src/storage/storage_api/mod.rs

        block_height: BlockHeight,
    ) -> Result<(), ApiServerStorageError>;

+    /// Append new token transactions with increasing tx_global_index at this block height


Sorry, but by reading this I still don't understand what this function will do if called 2 times in a row for the same height. "Append" sounds too vague. I'd say that the explanation deserves a separate sentence.

Also, as I can see, this "append" behavior is still inconsistent between the two implementations. E.g. if an existing tx id is passed, the in-memory implementation will ignore it and postgres one will add it with new index.
I'm not actually suggesting making the "append" behavior consistent, because this may be tricky and in reality it's unnecessary. But the interface IMO should define how a function should be used and how it shouldn't. So e.g. if some combination of parameters produces inconsistent results, then it can be said that the result of the function is undefined in such a case, and it's better to have a doc comment that mentions it. Additionally, IMO implementations should better check for undefined cases and panic, at least in debug.

Now both implementations will replace the entry if a duplicate (token_id, tx_id, block_height) is inserted.

ImplOfAnImpl · 2025-12-02T14:52:10Z

api-server/storage-test-suite/src/basic.rs

+        let random_token_id = TokenId::new(H256::random_using(&mut rng));
+        let token_transactions: Vec<_> = (0..10)
+            .map(|idx| {
+                let random_tx_id = Id::<Transaction>::new(H256::random_using(&mut rng));


FYI: Id also has the random_using method, i.e. this line could be a bit shorter.

ImplOfAnImpl · 2025-12-02T15:01:14Z

api-server/stack-test-suite/tests/v2/token_transactions.rs

+                // Unmint tokens
+                let coins_after_unmint = (coins_after_mint - token_supply_change_fee).unwrap();
+                let tokens_to_unmint = Amount::from_atoms(1);
+                let tokens_leff_after_unmint = (amount_to_mint - tokens_to_unmint).unwrap();


typo - leff

ImplOfAnImpl · 2025-12-02T15:03:14Z

api-server/stack-test-suite/tests/v2/token_transactions.rs

+                let token_issuance_fee =
+                    tf.chainstate.get_chain_config().fungible_token_issuance_fee();
+
+                let issuance = test_utils::token_utils::random_token_issuance_v1(


Any particular reason not to use issue_and_mint_tokens_from_genesis from helpers?
(if it's not generic enough, then perhaps it can be generalized more)

ImplOfAnImpl · 2025-12-02T15:11:38Z

api-server/scanner-lib/src/sync/tests/mod.rs

+    // 2. Token Authority Management Commands
+    // ------------------------------------------------------------------------
+
+    // Helper to create simple command txs


Outdated comment?
(I don't see any helper here)

ImplOfAnImpl · 2025-12-02T15:23:00Z

api-server/scanner-lib/src/sync/tests/mod.rs

+    assert_eq!(txs.len(), 6);
+    let ids: Vec<_> = txs.iter().map(|t| t.tx_id).collect();
+    assert!(ids.contains(&tx_freeze_id));
+    assert!(ids.contains(&tx_unfreeze_id));
+    assert!(ids.contains(&tx_metadata_id));
+    assert!(ids.contains(&tx_authority_id));


IMO it'd be a bit more robust if you collected ids to a set and then compared it as a whole to the expected set of ids (thus you'd check that the previously checked ids are still there). Same in other places.
(If you don't want to repeat expected tx ids every time, you could have the expected set as a mutable var and extend it with new ids every time.)

ImplOfAnImpl · 2025-12-02T16:16:35Z

api-server/api-server-common/src/storage/storage_api/mod.rs

    AddressableError,
    #[error("Block timestamp too high: {0}")]
    TimestampTooHigh(BlockTimestamp),
+    #[error("Tx global index too hight: {0}")]


hight -> high

ImplOfAnImpl · 2025-12-02T16:20:35Z

api-server/api-server-common/src/storage/storage_api/mod.rs


+    /// 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` and is not continuous for a specific `token_id`.


The tx_global_index and is not continuous

It's ironic that this change was made as a response to the review comment "can you plz put more effort into writing documentation comments".

ImplOfAnImpl · 2025-12-02T16:34:58Z

api-server/api-server-common/src/storage/impls/in_memory/mod.rs

+    TokenTransaction, TransactionInfo, TransactionWithBlockInfo, Utxo, UtxoLock, UtxoWithExtraInfo,
 };

+use itertools::Itertools as _;


Put it after the std imports?

ImplOfAnImpl · 2025-12-02T17:00:09Z

api-server/api-server-common/src/storage/impls/postgres/queries.rs

+                    ON CONFLICT (token_id, transaction_id, block_height)
+                    DO NOTHING;


This contradicts the documentation - "If the pair already exists at that block_height, the tx_global_index is updated."

P.S. It'd be nice to have a test for this in the storage test suite.

ImplOfAnImpl · 2025-12-02T17:21:08Z

api-server/api-server-common/src/storage/impls/in_memory/mod.rs

+        Ok(self
+            .token_transactions_table
+            .get(&token_id)
+            .map_or_else(Vec::new, |transactions| {
+                transactions
+                    .iter()
+                    .rev()
+                    .flat_map(|(_, txs)| txs.iter().map(|tx| &tx.0))
+                    .flat_map(|tx| (tx.tx_global_index < tx_global_index).then_some(tx))
+                    .cloned()
+                    .take(len as usize)
+                    .collect()
+            }))


This doesn't look correct:

The transactions in the inner set are not ordered by the global index. So, if there are more than len transactions at a particular height, the function will return arbitrary len transactions, not the latest len ones.

Even if the inner set was ordered by the global index first, the initial .iter().rev().flat_map(...) would still mess it up, because it produces an iterator that goes backwards over the heights, but forward over transactions. So, the transactions wouldn't be ordered by the global index anyway and taking the first len of them is wrong.

P.S. It'd be nice to have a test for this in the storage test suite.

OBorce requested review from ImplOfAnImpl and erubboli as code owners November 16, 2025 16:29

OBorce force-pushed the feature/api-server-token-transactions branch 2 times, most recently from c55d771 to d804d77 Compare November 18, 2025 02:21

ImplOfAnImpl requested changes Nov 19, 2025

View reviewed changes

Add token transactions endpoint for the API Server

50b1af5

OBorce force-pushed the feature/api-server-token-transactions branch from d804d77 to 3eccbe6 Compare November 24, 2025 05:53

OBorce changed the base branch from master to add_changelogs November 24, 2025 05:53

OBorce force-pushed the feature/api-server-token-transactions branch 3 times, most recently from 0f040e3 to a856b93 Compare November 24, 2025 09:04

Base automatically changed from add_changelogs to master November 24, 2025 10:31

OBorce force-pushed the feature/api-server-token-transactions branch from a856b93 to 1d47121 Compare November 24, 2025 15:12

fix comments

0f1d451

OBorce force-pushed the feature/api-server-token-transactions branch from 1d47121 to 0f1d451 Compare November 24, 2025 15:41

ImplOfAnImpl reviewed Nov 26, 2025

View reviewed changes

OBorce added 2 commits November 26, 2025 18:27

Add HTLC's refund key to UTXOs addresses

11039a0

fix tx_global_index consistency with transactions table

ab7232c

ImplOfAnImpl approved these changes Dec 2, 2025

View reviewed changes

		SELECT tx_global_index, transaction_id
		FROM ml.token_transactions

		ON CONFLICT (token_id, transaction_id, block_height)
		DO NOTHING;

Add token transactions endpoint for the API Server #1991

Are you sure you want to change the base?

Add token transactions endpoint for the API Server #1991

Conversation

OBorce commented Nov 16, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

OBorce commented Nov 16, 2025 •

edited

Loading