fix(tx-history): token information and query by onur-ozkan · Pull Request #2404 · GLEECBTC/komodo-defi-framework

onur-ozkan · 2025-03-25T12:43:52Z

Previously it was not possible to query history of Tendermint tokens. This PR makes that
possible. It also breaks the tx-history logic due to the new internal ID computation. But still users will not notice this change as it will be handled silently in the background.

Fixes: #2395 and #2315

Signed-off-by: onur-ozkan <work@onurozkan.dev>

onur-ozkan · 2025-03-25T14:52:33Z

mm2src/coins/my_tx_history_v2.rs

+                    // it can be the platform ticker instead of the token ticker for a pre-saved record
+                    if details.coin != request.coin {
+                        details.coin = request.coin.clone();
+                    }


I have no idea how this helps to other protocols but it's buggy (hiding actual token tickers) on Tendermint, so I moved it here.

Apparently this was added for BCH and SLP tx history, SLP tokens are included in a BCH transaction and it might be saved with a BCH ticker. EVMs were not added to v2 tx history so the only tokens protocol that uses this is slp, maybe we should specify this.

awesome let's please add a comment about this then.

Added a TODO note.

I would actually use the BCH/SLP protocol instead of _ wildcard, otherwise this will cause a similar bug that will be hard to detect when we add other protocols with tokens. I am fixing it now in a commit.

Done here 2812ff8 can one of you @mariocynicys @onur-ozkan please check it and resolve this commit if it's fine. I will then merge the PR.

Signed-off-by: onur-ozkan <work@onurozkan.dev>

onur-ozkan · 2025-03-31T08:01:51Z

Would be nice to have an approval from the QA team.

cc @KomodoPlatform/qa

smk762 · 2025-03-31T09:12:35Z

Error in response:


        "sync_status": {
            "state": "Error",
            "additional_info": {
                "message": "StorageError(\"add_transactions_to_history failed: sql_tx_history_storage_v2:474] SqliteFailure(Error { code: ConstraintViolation, extended_code: 2067 }, Some(\\\"UNIQUE constraint failed: IRIS_tx_history.internal_id\\\"))\")"
            }
        },

and console log:

31 09:10:37, coins::tendermint::tendermint_tx_history_v2:1113] INFO Stopping tx history fetching for IRIS. Reason: StorageError("add_transactions_to_history failed: sql_tx_history_storage_v2:474] SqliteFailure(Error { code: ConstraintViolation, extended_code: 2067 }, Some(\"UNIQUE constraint failed: IRIS_tx_history.internal_id\"))")

After moving DB/ folder to backup (i.e. clean slate), response returned expected result.

After restoring backup, on second attempt, response returned expected result. If this is intended (repaired on second restart), I can approve.

onur-ozkan · 2025-03-31T20:48:39Z

Can you re-check? That shouldn't happen anymore @smk762

Signed-off-by: onur-ozkan <work@onurozkan.dev>

… into fix-token-query-and-info

smk762

Confirmed history now returns for IBC tokens. Thanks!

mariocynicys

Thanks!
First review iter, getting to know things.

mm2src/coins/tendermint/tendermint_tx_history_v2.rs

mm2src/coins/lp_coins.rs

mm2src/mm2_test_helpers/dummy_files/iris-ibc-nucleus-history.json

mm2src/coins/tendermint/tendermint_coin.rs

mm2src/coins/tendermint/tendermint_tx_history_v2.rs

mariocynicys

Thanks!
~zero or one last iter for these comments and that's it :)

mariocynicys · 2025-04-03T14:42:08Z

mm2src/coins/tendermint/tendermint_token.rs

@@ -513,9 +519,11 @@ impl MmCoin for TendermintToken {
                internal_id,


in withdraw, the internal ID for the token case doesn't seem to consider the token_id https://github.com/KomodoPlatform/komodo-defi-framework/blob/971b40529d076969ad1b4391594876b463de6510/mm2src/coins/tendermint/tendermint_token.rs#L497-L500

...

mariocynicys · 2025-04-03T14:43:52Z

mm2src/coins/my_tx_history_v2.rs

+            TransactionType::TendermintIBCTransfer { token_id }
+            | TransactionType::CustomTendermintMsg { token_id, .. } => {
                if let Some(token_id) = token_id {
                    let mut bytes_for_hash = tx_hash.0.clone();
                    bytes_for_hash.extend_from_slice(&token_id.0);


where as here in tx_hist, we do embed token_id into the internal identifer

That looks weird. 🤔 We shouldn't do that probably, but I don't want remove it and go that far in this PR...

mm2src/coins/tx_history_storage/mod.rs

mariocynicys · 2025-04-03T14:56:02Z

mm2src/coins/tendermint/tendermint_tx_history_v2.rs

+                            for id in [old_internal_id, old_internal_id_for_fees] {
+                                if let Ok(Some(_)) = storage.get_tx_from_history(&wallet_id, &id).await {
+                                    if let Err(e) = storage.remove_tx_from_history(&wallet_id, &id).await {
+                                        log::debug!("Failed to remove old transaction history record. {e:?}");


this shouldn't happen, but if it happened it is worthy to be logged as an error!.

I don't think we should do that.

why don't u think so?

mariocynicys · 2025-04-03T14:56:39Z

mm2src/coins/tendermint/tendermint_tx_history_v2.rs

+                            let old_internal_id_for_fees: BytesJson = H256::from(old_internal_id_hash).to_vec().into();
+
+                            for id in [old_internal_id, old_internal_id_for_fees] {
+                                if let Ok(Some(_)) = storage.get_tx_from_history(&wallet_id, &id).await {


shouldn't we also log errors from here?

No we shouldn't. It's totally fine to skip this silently.

ummm, if it't just not found, that's OK,
but if we encounter a DB error, we probably wanna know why such an error occurs.

mariocynicys · 2025-04-03T15:04:27Z

mm2src/coins/tendermint/tendermint_token.rs

    }

-    fn history_sync_status(&self) -> HistorySyncState { HistorySyncState::NotEnabled }
+    fn history_sync_status(&self) -> HistorySyncState { self.platform_coin.history_sync_status() }


quoting from the PR's post:

Previously it was not possible to query history of Tendermint tokens. This PR makes that
possible.

Q: is this the change that made it possible to query tendermint token history? if not, could you guide me to the change please, as there is no dedicated commit for it.

No, not this. It's the changes made in mm2src/coins/tendermint/tendermint_tx_history_v2.rs.

Signed-off-by: onur-ozkan <work@onurozkan.dev>

shamardy

LGTM! Only nits!

shamardy · 2025-04-10T23:13:13Z

mm2src/coins/my_tx_history_v2.rs

+                    // it can be the platform ticker instead of the token ticker for a pre-saved record
+                    if details.coin != request.coin {
+                        details.coin = request.coin.clone();
+                    }


Apparently this was added for BCH and SLP tx history, SLP tokens are included in a BCH transaction and it might be saved with a BCH ticker. EVMs were not added to v2 tx history so the only tokens protocol that uses this is slp, maybe we should specify this.

mm2src/coins/tendermint/tendermint_tx_history_v2.rs

mariocynicys

My previous comment threads are non-blocking. Will continue the convo in them later.

LGTM

Signed-off-by: onur-ozkan <work@onurozkan.dev>

mariocynicys

Re-lgtm

…OL_TYPE`

* dev: (26 commits) chore(deps): remove base58 and replace it completely with bs58 (GLEECBTC#2427) feat(tron): initial groundwork for full TRON integration (GLEECBTC#2425) fix(UTXO): improve tx fee calculation and min relay fee handling (GLEECBTC#2316) deps(timed-map): bump to 1.3.1 (GLEECBTC#2413) improvement(tendermint): safer IBC channel handler (GLEECBTC#2298) chore(release): complete v2.4.0-beta changelogs (GLEECBTC#2436) fix(event-streaming): initial addresses registration in utxo balance streaming (GLEECBTC#2431) improvement(watchers): re-write use-watchers handling (GLEECBTC#2430) fix(evm): make withdraw_nft work in HD mode (GLEECBTC#2424) feat(taproot): support parsing taproot output address types chore(RPC): use consistent param name for QTUM delegation (GLEECBTC#2419) fix(makerbot): add LiveCoinWatch price provider (GLEECBTC#2416) chore(release): add changelog entries for v2.4.0-beta (GLEECBTC#2415) fix(wallets): prevent path traversal in `wallet_file_path` and update file extension (GLEECBTC#2400) fix(nft): make `update_nft` work with hd wallets using the enabled address (GLEECBTC#2386) fix(wasm): unify error handling for mm2_main (GLEECBTC#2389) fix(tx-history): token information and query (GLEECBTC#2404) test(electrums): fix failing test_one_unavailable_electrum_proto_version (GLEECBTC#2412) improvement(network): remove static IPs from seed lists (GLEECBTC#2407) improvement(best-orders): return an rpc error when we can't find best orders (GLEECBTC#2318) ...

onur-ozkan added 3 commits March 24, 2025 10:28

fill correct token ticker in tx history

21cd458

Signed-off-by: onur-ozkan <work@onurozkan.dev>

fix IBC token query

cee056d

Signed-off-by: onur-ozkan <work@onurozkan.dev>

apply nits

4b47807

Signed-off-by: onur-ozkan <work@onurozkan.dev>

onur-ozkan force-pushed the fix-token-query-and-info branch from 8dde850 to eae2619 Compare March 25, 2025 13:30

update denom_to_ticker

0be59b1

Signed-off-by: onur-ozkan <work@onurozkan.dev>

onur-ozkan force-pushed the fix-token-query-and-info branch 3 times, most recently from ab18e1f to c0af309 Compare March 25, 2025 14:48

onur-ozkan commented Mar 25, 2025

View reviewed changes

onur-ozkan added bug: wallet status: pending review improvement: wallet and removed bug: wallet labels Mar 25, 2025

fix tests

5b8e290

Signed-off-by: onur-ozkan <work@onurozkan.dev>

onur-ozkan force-pushed the fix-token-query-and-info branch from c0af309 to 5b8e290 Compare March 25, 2025 15:18

fix transaction type for tendermint tokens

3fc5e0e

Signed-off-by: onur-ozkan <work@onurozkan.dev>

onur-ozkan force-pushed the fix-token-query-and-info branch from 5e2e150 to 3fc5e0e Compare March 26, 2025 07:01

shamardy requested a review from mariocynicys March 31, 2025 12:55

shamardy added 2.5.0-beta and removed 2.5.0-beta labels Mar 31, 2025

This comment was marked as outdated.

Sign in to view

onur-ozkan force-pushed the fix-token-query-and-info branch 4 times, most recently from 49f3587 to a78a316 Compare April 1, 2025 06:43

update internal-id computation

25bb8ac

Signed-off-by: onur-ozkan <work@onurozkan.dev>

onur-ozkan added 2 commits April 1, 2025 19:13

handle fees

b6eaeca

Signed-off-by: onur-ozkan <work@onurozkan.dev>

Merge branch 'dev' of github.com:KomodoPlatform/komodo-defi-framework…

9c31002

… into fix-token-query-and-info

smk762 self-requested a review April 2, 2025 09:08

smk762 previously approved these changes Apr 2, 2025

View reviewed changes

mariocynicys reviewed Apr 2, 2025

View reviewed changes

onur-ozkan dismissed smk762’s stale review via d1ae3f1 April 2, 2025 12:40

onur-ozkan force-pushed the fix-token-query-and-info branch from d1ae3f1 to 971b405 Compare April 2, 2025 13:14

mariocynicys self-requested a review April 3, 2025 13:45

mariocynicys reviewed Apr 3, 2025

View reviewed changes

apply nit

3aea946

Signed-off-by: onur-ozkan <work@onurozkan.dev>

onur-ozkan force-pushed the fix-token-query-and-info branch from 971b405 to 3aea946 Compare April 3, 2025 17:43

smk762 self-requested a review April 8, 2025 08:38

smk762 previously approved these changes Apr 8, 2025

View reviewed changes

shamardy previously approved these changes Apr 10, 2025

View reviewed changes

mariocynicys previously approved these changes Apr 11, 2025

View reviewed changes

add a TODO note

04392d2

Signed-off-by: onur-ozkan <work@onurozkan.dev>

onur-ozkan dismissed stale reviews from mariocynicys, shamardy, and smk762 via 04392d2 April 11, 2025 07:23

add BCH_PROTOCOL_TYPE and use it instead of wildcard

2812ff8

mariocynicys reviewed Apr 11, 2025

View reviewed changes

mariocynicys previously approved these changes Apr 11, 2025

View reviewed changes

shamardy added 2 commits April 11, 2025 21:59

Merge remote-tracking branch 'origin/dev' into fix-token-query-and-info

528291d

fix test_bch_and_slp_testnet_history by introducing `BCH_TOKEN_PROTOC…

dd550dd

…OL_TYPE`

shamardy dismissed mariocynicys’s stale review via dd550dd April 11, 2025 20:21

shamardy merged commit 468b553 into dev Apr 11, 2025
23 of 24 checks passed

shamardy deleted the fix-token-query-and-info branch April 11, 2025 21:57

shamardy mentioned this pull request Apr 16, 2025

chore(release): v2.4.0-beta #2345

Merged

mariocynicys mentioned this pull request Apr 29, 2025

Tendermint transaction internal ID inconsistency #2429

Closed

		@@ -513,9 +519,11 @@ impl MmCoin for TendermintToken {
		internal_id,

Conversation

onur-ozkan commented Mar 25, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

onur-ozkan Mar 25, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

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!

shamardy Apr 11, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

onur-ozkan commented Mar 31, 2025

Uh oh!

smk762 commented Mar 31, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

This comment was marked as outdated.

onur-ozkan commented Mar 31, 2025

Uh oh!

smk762 left a comment

Choose a reason for hiding this comment

Uh oh!

mariocynicys left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

mariocynicys left a comment

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!

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!

shamardy left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

mariocynicys left a comment

Choose a reason for hiding this comment

Uh oh!

mariocynicys left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

onur-ozkan commented Mar 25, 2025 •

edited

Loading

onur-ozkan Mar 25, 2025 •

edited

Loading

shamardy Apr 11, 2025 •

edited

Loading

smk762 commented Mar 31, 2025 •

edited

Loading