Skip to content

fix(transaction-history): migrate from confirmations to timestamp-based sorting#3338

Merged
ca333 merged 3 commits intodevfrom
patch-fix-tx-history-sync-display
Nov 4, 2025
Merged

fix(transaction-history): migrate from confirmations to timestamp-based sorting#3338
ca333 merged 3 commits intodevfrom
patch-fix-tx-history-sync-display

Conversation

@DeckerSU
Copy link
Copy Markdown
Contributor

@DeckerSU DeckerSU commented Nov 2, 2025

This reverts the changes from 9900372 that attempted to show unconfirmed transactions first. The previous approach caused transactions to appear in a chaotic order during synchronization, creating confusion until all transactions are fully loaded.

This reverts the changes from 9900372 that
attempted to show unconfirmed transactions first. The previous approach
caused transactions to appear in a chaotic order during synchronization,
creating confusion until all transactions are fully loaded.
@DeckerSU DeckerSU requested a review from CharlVS November 2, 2025 20:02
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai bot commented Nov 2, 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 patch-fix-tx-history-sync-display

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.

@github-actions
Copy link
Copy Markdown

github-actions bot commented Nov 2, 2025

Visit the preview URL for this PR (updated for commit fd93e94):

https://walletrc--pull-3338-merge-v9nhvc23.web.app

(expires Tue, 11 Nov 2025 18:12:36 GMT)

🔥 via Firebase Hosting GitHub Action 🌎

Sign: f66a4ff03faa546f12f0ae5a841bd9eff2714dcc

@smk762
Copy link
Copy Markdown
Collaborator

smk762 commented Nov 3, 2025

Possible duplication of #3327
@takenagain pls coordinate with @DeckerSU to resolve these PRs

@DeckerSU
Copy link
Copy Markdown
Contributor Author

DeckerSU commented Nov 3, 2025

Possible duplication of #3327 @takenagain pls coordinate with @DeckerSU to resolve these PRs

Yes, I guess it's a duplication. My suggestion is to let the UI team decide which option fits the codebase better. From my point of view, reverting the unconfirmed transactions logic seems a bit cleaner than adding new conditions for confirmations. But that’s just my humble opinion. The final decision on which one should be closed and which should be merged is up to the UI team.

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.

I am in favour of your solution of reverting to the previous timestamp-based approach, with the small tweaks suggested below. This is also consistent with Onur's comments about the move away from the confirmations field in KDF, and to rather exclusively use the timestamp field in the UI.

Could we copy over the change to update timestamps for existing entries from #3327 to lines 190 and 110. This will update the timestamp for unconfirmed transactions from "Now" to a valid timestamp as events stream in via SSE while on the page.

@CharlVS CharlVS changed the title fix(transaction-history): revert to timestamp-based transaction sorting fix(transaction-history): migrate from confirmations to timestamp-based sorting Nov 3, 2025
CharlVS added a commit that referenced this pull request Nov 3, 2025
- Maintains prioritization of unconfirmed transactions
- Adds multi-level sorting: time, block height, and internal ID
- Uses _sortTime helper for better handling of zero timestamps
- Ensures deterministic ordering for consistent UI display
@ca333 ca333 merged commit 01938ab into dev Nov 4, 2025
6 of 12 checks passed
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.

5 participants