Skip to content

fix single tx history display for SIA coins#3405

Closed
smk762 wants to merge 1 commit intotest-siafrom
fix/sia-tx-history
Closed

fix single tx history display for SIA coins#3405
smk762 wants to merge 1 commit intotest-siafrom
fix/sia-tx-history

Conversation

@smk762
Copy link
Copy Markdown
Collaborator

@smk762 smk762 commented Nov 19, 2025

Fixes bug where only a single tx entry would appear in SIA coin page tx history.

  • Added a _txKey helper in transaction_history_bloc.dart that falls back to txHash/id when internalId is empty, ensuring Sia transactions receive unique keys even though MM2 returns "" for internal_id.
  • Updated all in-memory maps (byId, _firstSeenAtById, _processedTxIds) to use _txKey(...) so batches no longer overwrite each other when internal_id is blank, letting every SC transaction display.

To test:

  • login to a wallet which has more than one SC transaction in its history
  • activate SC
  • go to SC page
  • confirm tx history is complete, and not redacted after a single entry

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai bot commented Nov 19, 2025

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

✨ Finishing touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch fix/sia-tx-history

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

https://github.com/KomodoPlatform/komodo-wallet/blob/9dff1144db3e215d834c6fc8f7b833c6d6cec031/lib/bloc/transaction_history/transaction_history_bloc.dart#L111-L114
P1 Badge Update existing transaction using wrong key

After switching the in‑memory map to use _txKey so Sia transactions can be keyed by txHash or id instead of the empty internalId, the merge logic still writes back with byId[sanitized.internalId] = .... When internalId is blank (the case this commit targets), existing is read from byId[txKey] but the update is stored under the empty string, leaving the original entry untouched and adding a duplicate value. Subsequent batches that refresh confirmations will keep showing the stale copy keyed by txHash, while the latest data sits under '', so users may see duplicated transactions or confirmations that never advance. The update should use the same txKey that was used to look up the existing entry.

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

@mariocynicys
Copy link
Copy Markdown

is this deployed somewhere?

mariocynicys added a commit to GLEECBTC/komodo-defi-framework that referenced this pull request Nov 20, 2025
set the internal id correctly in transaction details for sia history fetching loop

this should fix what GLEECBTC/gleec-wallet#3405 is trying to fix but from KDF side.
Copy link
Copy Markdown
Contributor

@takenagain takenagain left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was resolved in KDF #2691, and SC/SCZEN transactions appear as expected for me in #3398 (once confirmed). I believe this PR can be closed, leaving internal_id as a unique identifier for transactions in the wallet and the SDK (luckily only one reference in each, so changing this behaviour is doable).

@takenagain
Copy link
Copy Markdown
Contributor

is this deployed somewhere?

Deployments only run for PRs into specified branch prefixes, like dev in the base PR #3398, so there is unfortunately no preview link available for this PR.

@smk762
Copy link
Copy Markdown
Collaborator Author

smk762 commented Nov 23, 2025

This was resolved in KDF #2691, and SC/SCZEN transactions appear as expected for me in #3398 (once confirmed). I believe this PR can be closed, leaving internal_id as a unique identifier for transactions in the wallet and the SDK (luckily only one reference in each, so changing this behaviour is doable).

confirmed, closing

@smk762 smk762 closed this Nov 23, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants