Skip to content

finalizeWithdrawal: specify the proofSubmitter#3204

Merged
jxom merged 7 commits intowevm:mainfrom
hamdiallam:finalize.proofsubmittor
Jan 8, 2025
Merged

finalizeWithdrawal: specify the proofSubmitter#3204
jxom merged 7 commits intowevm:mainfrom
hamdiallam:finalize.proofsubmittor

Conversation

@hamdiallam
Copy link
Contributor

@hamdiallam hamdiallam commented Jan 7, 2025

OptimismPortal2 introduces finalizeWithdrawalTransactionExternalProof such that the calling sender does not have to match the original proof submittor.

With the current structure of the viem hook, the caller must be the proof submitter in order for the finalizing tx to succesfully execute

Introduce an optional proofSubmittor argument to the simulating and executing hook for finalizeWithdrawalTransaction. If set, use the the altered function. This introduces this functionality in a backwards compatible way.

  • Although from a DevX perspective, it would make more sense for viem to auto-select a proof submitting address that is passing such that the user doesn't need to explicitly know. There's no security implications as long as a valid proof submitter is used. The optionality is to prevent grieving of valid proof submissions by frontrunning with invalid games.
  • Note: Inclusion of a valid proof submitter in the withdrawal status (it already queries this internally) would be a breaking change with the return return type. Let me know thoughts here, happy to refactor this in agreement

@changeset-bot
Copy link

changeset-bot bot commented Jan 7, 2025

🦋 Changeset detected

Latest commit: 13f1bee

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
viem Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@vercel
Copy link

vercel bot commented Jan 7, 2025

@hamdiallam is attempting to deploy a commit to the Wevm Team on Vercel.

A member of the Team first needs to authorize it.

@hamdiallam hamdiallam changed the title finalizeWithdrawal: specifying the proofSubmittor finalizeWithdrawal: proofSubmittor Jan 7, 2025
Copy link
Member

@jxom jxom left a comment

Choose a reason for hiding this comment

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

  • Please rename from "submittor" to "submitter".
  • Needs a changeset (pnpm changeset)
  • Need to add proofSubmitter to docs.

@hamdiallam hamdiallam changed the title finalizeWithdrawal: proofSubmittor finalizeWithdrawal: proofSubmitter Jan 8, 2025
@hamdiallam hamdiallam changed the title finalizeWithdrawal: proofSubmitter finalizeWithdrawal: specify the proofSubmitter Jan 8, 2025
@jxom jxom merged commit be5516f into wevm:main Jan 8, 2025
2 checks passed
@github-actions github-actions bot mentioned this pull request Jan 8, 2025
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