API server: fix duplicate token ids in token listing queries#2088
Open
mannutech wants to merge 1 commit into
Open
API server: fix duplicate token ids in token listing queries#2088mannutech wants to merge 1 commit into
mannutech wants to merge 1 commit into
Conversation
03a8fc9 to
bb6b240
Compare
The Postgres fungible_token and nft_issuance tables keep one row per token state change, but get_token_ids/get_token_ids_by_ticker selected ids without collapsing history, so every token appeared once per state change. Deduplicate both halves of the union, count distinct tokens (not rows) when computing the NFT half's offset, and clamp its limit to the requested page size, which used to be exceeded when the offset landed past the fungible tokens. Add a storage-test-suite trial covering dedup and full page walks on both backends, and a stack-test-suite HTTP test for the /token and /token/ticker/:ticker endpoints. Fixes mintlayer#1982
bb6b240 to
cb2c43e
Compare
Author
|
@ImplOfAnImplmeasured the cost of the added At current production size (~400 tokens) the new query runs in 0.25 ms. At ~1000x that (100k tokens / 1.05M fungible rows, 100k NFTs / 350k rows),
Almost all of the difference is |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #1982
What's wrong
GET /api/v2/tokenreturns the same token id multiple times. This is still reproducible right now on the public testnet server —https://api-server-lovelace.mintlayer.org/api/v2/token?offset=0&items=100returns the id from the original report (tmltk1rz9hmgm...sugcfq6) 4 times, and other ids up to 8 times.Why it happens
ml.fungible_tokenandml.nft_issuancestore one row per token state change — their primary key is(id, block_height). Point queries already handle this correctly (get_fungible_token_issuancepicks the latest row), but the two listing queries select ids straight off the tables. So a token appears once per issuance/mint/lock/authority change — which is exactly why the issue shows repeat counts of 2x, 3x, 4x rather than uniform duplication: the count is the number of state changes that token has had.The in-memory backend keys its maps by token id and was never affected. The two backends had quietly diverged on these two methods, and no shared test covered them.
Digging into the query exposed two more pagination bugs in the same place:
count_tokens, which decides where the NFT half of theUNION ALLstarts, counted rows instead of distinct tokens — so the fungible→NFT handoff point drifts as tokens accumulate history, even with the ids deduplicated.LIMIT($2 + $1 - count) exceeds the requested page size once the offset points past the last fungible token. With 3 fungible tokens,offset=5&items=3returns 5 NFTs instead of 3.The fix
In
get_token_idsandget_token_ids_by_ticker:SELECT DISTINCTin both halves of the union (NFTs accumulate rows too, from owner changes)count(DISTINCT token_id)incount_tokensLIMITclamped to the page size:GREATEST(LEAST($2, $2 + $1 - count), 0)A token's ticker never changes, so plain
DISTINCTis enough for the ticker variant — no latest-row handling needed there.How it's tested
token_ids_dedup_and_pagination, running against both backends: seeds tokens and NFTs with multi-row histories, checks the exact listings, walks every page size end to end, and covers a ticker that only NFTs use (fungible count = 0), offsets past the end, and zero-size pages. Without the fix it fails on Postgres with the same repeated-id pattern as the issue report, and passes on in-memory — so it's the regression test and the missing backend-parity test in one.no_duplicate_ids_for_tokens_with_state_changes: over HTTP,/tokenand/token/ticker/:tickerreturn a minted (multi-row) token exactly once.