Skip to content

use withdraw preview signed hex for broadcast#3309

Merged
ca333 merged 3 commits intodevfrom
use-withdraw-preview-for-broadcast
Oct 31, 2025
Merged

use withdraw preview signed hex for broadcast#3309
ca333 merged 3 commits intodevfrom
use-withdraw-preview-for-broadcast

Conversation

@smk762
Copy link
Copy Markdown
Collaborator

@smk762 smk762 commented Oct 30, 2025

Closes #3307 & #3196

To test:

  • Make a withdrawal with a trezor. You should only need to sign the hex once.
  • Make a withdrawal with of ARRR. You should only need to generate the hex once.

Summary

Avoids calling the withdraw RPC twice by reusing the transaction preview (tx_hex) for broadcast on confirmation. This eliminates a second device-sign prompt for hardware wallets (Trezor) and removes unnecessary duplicate transaction generation on slow chains (e.g., ZHTLC).

Linked Issues

  • Trezor withdraws should not require two signatures (#3196)
  • ZHTLC transactions should not need to be generated twice (#3307)

Problem

Current flow generates the transaction twice:

  1. First withdraw call for preview (user reviews fees/details)
  2. Second withdraw call on confirm (broadcast)

For hardware wallets, this results in two device confirmations for the same transaction. For chains with slower generation (ZHTLC), the duplicate work is visible and wasteful.

Approach

Perform a single withdraw (preview) to generate the transaction and cache it in local state. On confirm, broadcast the cached tx_hex using send_raw_transaction instead of calling withdraw again.

Implementation Details

  • Keep the first WithdrawalPreview in WithdrawFormState.preview when the confirmation screen is shown.
  • On Confirm:
    • Call send_raw_transaction with preview.txHex and preview.coin.
    • Build WithdrawalResult using the preview data and the broadcast txHash.
  • Clear cached preview on Cancel/Back (via reset) or successful broadcast.
  • Do not re-trigger Trezor signing on confirm; the only user interaction with the device happens during preview/initial signing.

Files Touched (high level)

  • lib/bloc/withdraw_form/withdraw_form_bloc.dart
    • Inject Mm2Api into the bloc.
    • Replace second withdraw call with send_raw_transaction using cached preview.txHex.
    • Clear preview on success to avoid stale state.
  • lib/views/wallet/coin_details/withdraw_form/withdraw_form.dart
    • Provide Mm2Api to WithdrawFormBloc constructor.

User Impact

  • Trezor: single device-sign prompt, then immediate broadcast on Confirm (no second prompt).
  • ZHTLC and other slow chains: no second generation delay; Confirm broadcasts immediately.

Testing Steps

  1. Login with Trezor.
  2. Create a withdraw and press Preview.
    • Expect device to prompt once for signing (if required by the chain).
    • Preview screen shows amounts and network fee.
  3. Press Confirm.
    • Expect immediate broadcast using the cached tx_hex.
    • No second Trezor prompt.
  4. Cancel/back from confirm screen.
    • Preview cache is cleared (no leak into next attempts).
  5. Repeat on a ZHTLC asset.
    • Confirm should not trigger a second generation; broadcast is immediate.

Edge Cases

  • If preview is unexpectedly null on Confirm, an error is surfaced and the transaction fails gracefully.
  • If send_raw_transaction returns an error, the failure is shown on the failed screen with details.

Risks and Mitigations

  • Risk: Preview cache not cleared on all exits.
    • Mitigation: Cache cleared on success and via existing reset on Cancel/Back.
  • Risk: Divergence from legacy paths.
    • Mitigation: The approach mirrors existing KMD rewards claim flow that uses send_raw_transaction after constructing tx_hex.

Checklist

  • Reuse preview tx_hex for broadcast on Confirm
  • Remove second device-sign prompt for Trezor
  • Clear cached preview on cancel/back and on success
  • Minimal surface-area changes (bloc + view wiring only)

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai bot commented Oct 30, 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 use-withdraw-preview-for-broadcast

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 Oct 30, 2025

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

https://walletrc--pull-3309-merge-ftd7uwcn.web.app

(expires Thu, 06 Nov 2025 12:38:24 GMT)

🔥 via Firebase Hosting GitHub Action 🌎

Sign: f66a4ff03faa546f12f0ae5a841bd9eff2714dcc

@ca333 ca333 merged commit 1e3daa0 into dev Oct 31, 2025
2 of 11 checks passed

WithdrawFormBloc({
required Asset asset,
required KomodoDefiSdk sdk,
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

@smk762 Thank you for this PR. Btw, for the future, the SDK has 1:1 type-safe mapping for most RPCs, which is accessible through the SDK, so passing _mm2Api isn't necessary here. e.g. _sdk.rpc.withdraw.withdraw or _sdk.rpc.withdraw.sendRawTransaction.

But in general, we want to avoid having to use RPCs instead of the abstracted/managed SDK features so that devs don't need to learn and understand KDF's API. With the SDK's withdrawal feature now unused, it suggests it doesn't meet our needs, and therefore won't meet the needs of other developers.

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.

ZHTLC transactions should not need to be generated twice Trezor withdraws should not require two signatures

3 participants