Skip to content

fix: add try-finally to executeRecovery to guarantee state cleanup#39

Closed
0x-SquidSol wants to merge 1 commit intonullxnothing:mainfrom
0x-SquidSol:fix/recovery-state-cleanup-on-error
Closed

fix: add try-finally to executeRecovery to guarantee state cleanup#39
0x-SquidSol wants to merge 1 commit intonullxnothing:mainfrom
0x-SquidSol:fix/recovery-state-cleanup-on-error

Conversation

@0x-SquidSol
Copy link
Copy Markdown

Summary

  • If wallet recovery failed mid-execution (network error, invalid key, etc.), cleanup code never ran
  • abortController remained stale, currentStatus stuck in 'executing', and loaded keypairs with private key material stayed in memory
  • Wrapped recovery phases in try-finally to guarantee cleanup: status reset to idle, abortController nulled, and keys securely wiped via clearLoadedWallets()

Test plan

  • Run a recovery that succeeds — verify cleanup and status reset
  • Simulate a recovery failure — verify state resets to idle and keys are cleared
  • Run pnpm run typecheck — passes clean
  • Run pnpm run test — all wallet tests pass (19/19)

If recovery failed mid-execution, abortController remained stale,
currentStatus stayed in 'executing', and loaded keypairs were never
cleared from memory. Wrapped the recovery logic in try-finally to
ensure abortController, status, and key material are always cleaned
up regardless of success or failure.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@nullxnothing
Copy link
Copy Markdown
Owner

Closed as superseded by #75, which reapplies the corrected security hardening on top of current main. This avoids merging stale/conflicted branches and preserves the fixes with passing CI/CodeQL.

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