Skip to content

refactor(scan): make remote simulation flow-aware#112

Open
wantedsystem wants to merge 1 commit into
mainfrom
refactor/flow-aware-scan-simulation
Open

refactor(scan): make remote simulation flow-aware#112
wantedsystem wants to merge 1 commit into
mainfrom
refactor/flow-aware-scan-simulation

Conversation

@wantedsystem

Copy link
Copy Markdown
Contributor

Summary

Makes the Blockaid security scan flow-aware so that remote simulation is only requested for the dapp sign-transaction flow. The send and change-trust flows no longer request remote simulation, since they already run local on-chain re-validation that covers submittability. Malicious validation is untouched and still runs for every flow per the useSecurityAlerts preference.

Why

  • send / change-trust already have local re-validation (time bounds, fee, balance, sequence) that re-checks submittability against the latest on-chain state while the confirmation dialog is open. Running remote simulation on top of that is redundant.
  • sign-transaction has no local re-validation, so remote simulation is the only pre-submit signal, we keep it there.
  • On TRON, remote simulation proved unreliable, which is why we don't want it gating the flows that already have a trustworthy local check.

Changes

  • scanRefresher.ts: #getScanOptions now reads the full scan context and only adds Simulation when simulateOnChainActions is enabled and interfaceKey === SignTransaction. Validation is unchanged (all flows, per useSecurityAlerts). Removed the now-unused SecurityScanPreferences alias and updated the 3 call sites.
  • This is the single source of truth for scan options — the initial scan isn't synchronous (scanFetchStatus starts as Fetching), so the refresher performs the only scan call. No controller/handler changes were needed.

Comparison with TRON

This mirrors the spirit of TRON's approach (don't lean on unreliable remote simulation) but applies it per-flow on Stellar: remote simulation is disabled exactly where a local check already exists (send / change-trust), and kept where it's the only option (sign-transaction). SignAuthEntry / SignMessage don't run a transaction scan, so they're unaffected.

Copilot AI 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.

Pull request overview

This PR makes the Blockaid scan “flow-aware” by restricting remote simulation to the dapp sign-transaction confirmation flow, while keeping validation behavior unchanged and still governed by the useSecurityAlerts preference.

Changes:

  • Updated the scan refresher to add TransactionScanOption.Simulation only when simulateOnChainActions is enabled and interfaceKey === SignTransaction.
  • Updated/expanded unit tests to cover simulation inclusion/exclusion across relevant confirmation interface keys.
  • Updated snap.manifest.json source.shasum to reflect the new bundle output.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.

File Description
packages/snap/src/handlers/cronjob/refreshConfirmationContext/scanRefresher.ts Makes scan option selection depend on confirmation flow (simulation only for SignTransaction).
packages/snap/src/handlers/cronjob/refreshConfirmationContext/scanRefresher.test.ts Adds coverage ensuring simulation is skipped for send/change-trust flows and retained for sign-transaction.
packages/snap/snap.manifest.json Updates the snap bundle shasum for the changed build output.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

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