Skip to content

feat(app): live transaction-count hint on transfer/withdraw amounts - #467

Open
Patrick-Ehimen wants to merge 2 commits into
NethermindEth:mainfrom
Patrick-Ehimen:feat/386-live-tx-cost-hint
Open

feat(app): live transaction-count hint on transfer/withdraw amounts#467
Patrick-Ehimen wants to merge 2 commits into
NethermindEth:mainfrom
Patrick-Ehimen:feat/386-live-tx-cost-hint

Conversation

@Patrick-Ehimen

@Patrick-Ehimen Patrick-Ehimen commented Aug 10, 2026

Copy link
Copy Markdown
Contributor

🚀 What’s this PR do?

Shows how many on-chain transactions an amount needs inline, as the user types, in the transfer and withdraw tabs only when that number is greater than one.

#436 already shows this figure in the confirmation dialog, i.e. after the user commits. This adds the part #386 describes as "updated whenever the input is changed by the user", so a split is visible while the amount can still be adjusted.


📎 Related issues

Refs #386 not Closes, since #436 may already satisfy it in your view. Happy to close this if so.


🧠 Context

Reuses the existing PrivatePool.estimate(amount) call, so the inline hint and the dialog can't disagree.

  • Debounced 300ms otherwise one estimate per digit.
  • Only shown when txCount > 1, per the issue.
  • Out-of-order responses discarded by sequence number; a slow estimate must not describe an amount already changed.
  • Recomputed on pool:selected (estimate is pool-specific) and wallet:ready (an amount typed before connecting is skipped for want of a wallet).
  • Cleared on successful submit the field is reset programmatically and fires no input event.
  • Best-effort like txCountRow: a failed estimate clears the hint rather than raising.

Note: txCountRow() guards with if (!txCount), so the dialog still renders "1 transaction" for single-transaction ops, which #386 asks to suppress. Left untouched as out of scope say the word and I'll align it.


✅ Required Checklist

  • I’ve read the contributing guide.
  • I’ve mentioned this PR in the project LinkedIn group (required for review so maintainers can see a human behind the contribution).
  • I’ve tested this locally, run .githooks/pre-push and provided test instructions for maintainers if needed
  • I’ve added relevant docs or comments
  • I’ve updated or created tests if needed

The confirmation dialog already reports how many on-chain transactions an
amount needs, but only after the user commits to the action. Show the same
information inline as the amount is typed, so a split is visible while it can
still be adjusted.

- Debounced (300ms) estimate per keystroke, so a typed-out amount costs one
  call rather than one per digit.
- Shown only when the count exceeds one; a single-transaction amount is
  unremarkable and stays silent.
- Stale responses are discarded by sequence number, since estimates can
  resolve out of order and would otherwise describe a discarded amount.
- Recomputed on pool switch and on wallet connect, both of which change or
  unblock the estimate.
- Best-effort throughout: a failed estimate clears the hint instead of
  raising, leaving the submit path to report anything that actually blocks.

Refs NethermindEth#386

@Fantoni0 Fantoni0 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks for the contribution @Patrick-Ehimen 💯
I like the idea a lot, but I think some race conditions might happen.
My advice would be, rather than showing the number of steps a hint when configuring the transaction, you could show it in the confirm (app/js/confirm) modal. They already have access to the txCountRow() function, and you don't need to worry about calling ensureAppPool and avoid racing issues

Comment thread app/js/ui/transactions.js Outdated
Comment thread app/js/ui/transactions.js Outdated

@Fantoni0 Fantoni0 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks @Patrick-Ehimen
I was checking the deployed version on main, and noticed we already show a transaction-count hint on the confirm modal:

Image

So not sure this PR would be adding anything else on this aspect :(
That being said, I think your changes for hardening the pool session are good and can still be merged. If you could please:

  • Remove the html hints.
  • Any logic related to transaction count/hints that is no longer needed
  • Take a look to navigation.js, I think it could also use the pendingSession guard (on the disconect function we call closeAppPool but a createAppPool might already be running)

I think we can still merge this 🙌

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.

2 participants