Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

finalizeWithdrawal: specify the proofSubmitter #3204

Merged
merged 7 commits into from
Jan 8, 2025

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

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

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 of 3 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