Skip to content

Implement ERC-7821 calldata compression in ERC7579Utils #5602

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

Conversation

Amxx
Copy link
Collaborator

@Amxx Amxx commented Mar 25, 2025

ERC-7821 implementation uses the ERC7579Utils library. While ERC-7821 looks like a subset of ERC-7579, it includes a specific design choice to help compress calldata that is not strictly part of ERC-7579.

When asked if it would be fine to add this mechanism to ERC-7579, Konrad (ERC-7579 authors) says:

Hadrien Croubois, [21/03/2025 09:56]
[...]. Since we are using the same library for 7821 and 7579 implementations, I was wondering if it would be ok to add the same behavior to 7579.

Konrad | rhinestone, [21/03/2025 22:51]
I dont see a problem with this but one point to note is that this will cause differing behaviour to existing accounts, so you can't really rely on that feature working across accounts

This PR adds the target rewrite mechanism to be used in both 7821 (where it is required) and 7579 (where is is not mandated, but also not a problem).


No need for a changeset entry if this is part of 5.3 audit fixes (unreleased feature).

PR Checklist

  • Tests
  • Changeset entry (run npx changeset add)

Copy link

changeset-bot bot commented Mar 25, 2025

⚠️ No Changeset found

Latest commit: 5558c87

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

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

@Amxx Amxx changed the base branch from master to release-v5.3 March 25, 2025 12:30
@Amxx Amxx force-pushed the account-utils/implement-erc7821-calldata-compression branch from 096c309 to dcd5ade Compare March 25, 2025 12:31
ernestognw
ernestognw previously approved these changes Mar 26, 2025
Copy link
Member

@ernestognw ernestognw left a comment

Choose a reason for hiding this comment

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

I understand the CI is failing because we would need to implement changes from #5600 into the release-v5.3 branch

Can we point to master instead and then cherry pick into release-v5.3? We should do this together during tomorrow's sync

@Amxx Amxx changed the base branch from release-v5.3 to master March 26, 2025 09:19
@Amxx Amxx dismissed ernestognw’s stale review March 26, 2025 09:19

The base branch was changed.

@Amxx Amxx force-pushed the account-utils/implement-erc7821-calldata-compression branch from a504580 to 5558c87 Compare March 26, 2025 09:21
@Amxx
Copy link
Collaborator Author

Amxx commented Mar 26, 2025

updated to target master

@Amxx Amxx merged commit 952775e into OpenZeppelin:master Mar 27, 2025
17 checks passed
@Amxx Amxx deleted the account-utils/implement-erc7821-calldata-compression branch March 27, 2025 08:27
Amxx added a commit to Amxx/openzeppelin-contracts that referenced this pull request Mar 27, 2025
ernestognw pushed a commit that referenced this pull request Mar 28, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants