Skip to content

Conversation

@Amxx
Copy link
Collaborator

@Amxx Amxx commented Jun 20, 2025

When the signer were part of the community repository, we did not have an upgradeable version. It made sens to not have a constructor, because we wanted to be able to use them in an "initializable" manner, without a key set at construction. This was still not upgrade-friendly because of the lack of namespaced storage.

This PR proposes to align the signers with the normal practice in the library:

  • have a constructor that calls _setSigner(). This will force that the signer is set at construction, no longer possible to "forget calling it". This is fine for "one time deployment" (non upgradeable, non proxy) accounts.
  • rely on the upgradeable version (produced by the transpiler) to have a version that use initializers and 7702 storage.

checklist

  • Tests
  • Documentation
  • Changeset entry (run npx changeset add)

@Amxx Amxx added this to the 5.4 milestone Jun 20, 2025
@Amxx Amxx requested a review from a team as a code owner June 20, 2025 08:26
@changeset-bot
Copy link

changeset-bot bot commented Jun 20, 2025

🦋 Changeset detected

Latest commit: c9b8b7a

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

This PR includes changesets to release 1 package
Name Type
openzeppelin-solidity 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

@Amxx Amxx changed the title Add signer constructors + fix vulnerability in MultiSignerERC7913.sol Add signer constructors Jun 20, 2025
@Amxx
Copy link
Collaborator Author

Amxx commented Jun 20, 2025

Note: the changeset is just for RC-1. Will not be part of the final release.

@Amxx Amxx merged commit 6079eb3 into OpenZeppelin:master Jun 20, 2025
27 of 28 checks passed
@Amxx Amxx deleted the refactor/signer-constructors branch June 20, 2025 11:09
Amxx added a commit that referenced this pull request Jun 20, 2025
Co-authored-by: ernestognw <[email protected]>
Signed-off-by: Hadrien Croubois <[email protected]>
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