Skip to content

fix(withdrawer): prevent duplicate withdrawal execution and eliminate TransactOpts race condition#77

Open
yasnazariel wants to merge 1 commit into
base:mainfrom
yasnazariel:patch-1
Open

fix(withdrawer): prevent duplicate withdrawal execution and eliminate TransactOpts race condition#77
yasnazariel wants to merge 1 commit into
base:mainfrom
yasnazariel:patch-1

Conversation

@yasnazariel
Copy link
Copy Markdown

This PR improves the safety and correctness of the withdrawal flow by addressing two key issues:

  1. Duplicate execution prevention
  • Added checks before calling ProveWithdrawalTransaction and FinalizeWithdrawalTransaction
  • Prevents re-submission of already proven or finalized withdrawals
  • Avoids unnecessary transactions and potential inconsistent state handling
  1. Race condition fix on TransactOpts
  • Replaced shared mutable usage of w.Opts with a per-call copy
  • Prevents data races when Withdrawer is used concurrently
  • Ensures transaction parameters are isolated per execution
  1. Finalize path cleanup
  • Removed unnecessary ProveWithdrawalParameters call from FinalizeWithdrawal
  • Reduces redundant RPC calls and improves performance
  • Aligns implementation with actual contract requirements

Impact:

  • Improves reliability of withdrawal lifecycle (prove → finalize)
  • Prevents accidental duplicate submissions
  • Eliminates concurrency-related bugs in transaction construction
  • Reduces unnecessary network overhead

No breaking changes. Behavior remains consistent for valid flows.

…ng to inconsistent transaction behavior

- Duplicate transaction submissions (gas loss, UX degradation)
- Race condition leading to incorrect or conflicting transactions
- Potential nonce mismanagement in concurrent environments
- Increased RPC overhead and reduced reliability

While on-chain contracts may prevent double-finalization, the client-side behavior remains unsafe and can lead to operational issues.
@cb-heimdall
Copy link
Copy Markdown
Collaborator

🟡 Heimdall Review Status

Requirement Status More Info
Reviews 🟡 0/1
Denominator calculation
Show calculation
1 if user is bot 0
1 if user is external 0
2 if repo is sensitive 0
From .codeflow.yml 1
Additional review requirements
Show calculation
Max 0
0
From CODEOWNERS 0
Global minimum 0
Max 1
1
1 if commit is unverified 0
Sum 1

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