Allow key updates in Signers (#72)

Amxx · ernestognw · web-flow · commit 37dfb6e92b31 · 2025-01-29T15:17:59.000+01:00
Co-authored-by: Ernesto García &lt;ernestognw@gmail.com&gt;
diff --git a/contracts/mocks/account/AccountECDSAMock.sol b/contracts/mocks/account/AccountECDSAMock.sol
@@ -8,7 +8,7 @@ import {SignerECDSA} from "../../utils/cryptography/SignerECDSA.sol";
 
 abstract contract AccountECDSAMock is Account, SignerECDSA, ERC7821 {
     constructor(address signerAddr) {
-        _initializeSigner(signerAddr);
+        _setSigner(signerAddr);
     }
 
     /// @inheritdoc ERC7821
diff --git a/contracts/mocks/account/AccountP256Mock.sol b/contracts/mocks/account/AccountP256Mock.sol
@@ -8,7 +8,7 @@ import {SignerP256} from "../../utils/cryptography/SignerP256.sol";
 
 abstract contract AccountP256Mock is Account, SignerP256, ERC7821 {
     constructor(bytes32 qx, bytes32 qy) {
-        _initializeSigner(qx, qy);
+        _setSigner(qx, qy);
     }
 
     /// @inheritdoc ERC7821
diff --git a/contracts/mocks/account/AccountRSAMock.sol b/contracts/mocks/account/AccountRSAMock.sol
@@ -8,7 +8,7 @@ import {SignerRSA} from "../../utils/cryptography/SignerRSA.sol";
 
 abstract contract AccountRSAMock is Account, SignerRSA, ERC7821 {
     constructor(bytes memory e, bytes memory n) {
-        _initializeSigner(e, n);
+        _setSigner(e, n);
     }
 
     /// @inheritdoc ERC7821
diff --git a/contracts/mocks/docs/account/MyAccountECDSA.sol b/contracts/mocks/docs/account/MyAccountECDSA.sol
@@ -3,17 +3,17 @@
 
 pragma solidity ^0.8.20;
 
+import {Initializable} from "@openzeppelin/contracts/proxy/utils/Initializable.sol";
 import {EIP712} from "@openzeppelin/contracts/utils/cryptography/EIP712.sol";
 import {Account} from "../../../account/Account.sol";
 import {ERC7821} from "../../../account/extensions/ERC7821.sol";
 import {SignerECDSA} from "../../../utils/cryptography/SignerECDSA.sol";
 
-contract MyAccountECDSA is Account, SignerECDSA, ERC7821 {
+contract MyAccountECDSA is Initializable, Account, SignerECDSA, ERC7821 {
     constructor() EIP712("MyAccountECDSA", "1") {}
 
-    function initializeSigner(address signerAddr) public virtual {
-        // Will revert if the signer is already initialized
-        _initializeSigner(signerAddr);
+    function initialize(address signerAddr) public initializer {
+        _setSigner(signerAddr);
     }
 
     /// @dev Allows the entry point as an authorized executor.
diff --git a/contracts/mocks/docs/account/MyAccountP256.sol b/contracts/mocks/docs/account/MyAccountP256.sol
@@ -3,17 +3,17 @@
 
 pragma solidity ^0.8.20;
 
+import {Initializable} from "@openzeppelin/contracts/proxy/utils/Initializable.sol";
 import {EIP712} from "@openzeppelin/contracts/utils/cryptography/EIP712.sol";
 import {Account} from "../../../account/Account.sol";
 import {ERC7821} from "../../../account/extensions/ERC7821.sol";
 import {SignerP256} from "../../../utils/cryptography/SignerP256.sol";
 
-contract MyAccountP256 is Account, SignerP256, ERC7821 {
+contract MyAccountP256 is Initializable, Account, SignerP256, ERC7821 {
     constructor() EIP712("MyAccountP256", "1") {}
 
-    function initializeSigner(bytes32 qx, bytes32 qy) public virtual {
-        // Will revert if the signer is already initialized
-        _initializeSigner(qx, qy);
+    function initialize(bytes32 qx, bytes32 qy) public initializer {
+        _setSigner(qx, qy);
     }
 
     /// @dev Allows the entry point as an authorized executor.
diff --git a/contracts/mocks/docs/account/MyAccountRSA.sol b/contracts/mocks/docs/account/MyAccountRSA.sol
@@ -3,17 +3,17 @@
 
 pragma solidity ^0.8.20;
 
+import {Initializable} from "@openzeppelin/contracts/proxy/utils/Initializable.sol";
 import {EIP712} from "@openzeppelin/contracts/utils/cryptography/EIP712.sol";
 import {Account} from "../../../account/Account.sol";
 import {ERC7821} from "../../../account/extensions/ERC7821.sol";
 import {SignerRSA} from "../../../utils/cryptography/SignerRSA.sol";
 
-contract MyAccountRSA is Account, SignerRSA, ERC7821 {
+contract MyAccountRSA is Initializable, Account, SignerRSA, ERC7821 {
     constructor() EIP712("MyAccountRSA", "1") {}
 
-    function initializeSigner(bytes memory e, bytes memory n) public virtual {
-        // Will revert if the signer is already initialized
-        _initializeSigner(e, n);
+    function initialize(bytes memory e, bytes memory n) public initializer {
+        _setSigner(e, n);
     }
 
     /// @dev Allows the entry point as an authorized executor.
diff --git a/contracts/mocks/docs/account/MyFactoryAccount.sol b/contracts/mocks/docs/account/MyFactoryAccount.sol
@@ -32,7 +32,7 @@ contract MyFactoryAccount {
         address predicted = predictAddress(_signerSalt);
         if (predicted.code.length == 0) {
             _impl.cloneDeterministic(_signerSalt);
-            MyAccountECDSA(payable(predicted)).initializeSigner(signer);
+            MyAccountECDSA(payable(predicted)).initialize(signer);
         }
         return predicted;
     }
diff --git a/contracts/mocks/utils/cryptography/ERC7739SignerECDSAMock.sol b/contracts/mocks/utils/cryptography/ERC7739SignerECDSAMock.sol
@@ -9,6 +9,6 @@ import {SignerECDSA} from "../../../utils/cryptography/SignerECDSA.sol";
 
 contract ERC7739ECDSAMock is ERC7739, SignerECDSA {
     constructor(address signerAddr) EIP712("ERC7739ECDSA", "1") {
-        _initializeSigner(signerAddr);
+        _setSigner(signerAddr);
     }
 }
diff --git a/contracts/mocks/utils/cryptography/ERC7739SignerP256Mock.sol b/contracts/mocks/utils/cryptography/ERC7739SignerP256Mock.sol
@@ -8,6 +8,6 @@ import {SignerP256} from "../../../utils/cryptography/SignerP256.sol";
 
 contract ERC7739P256Mock is ERC7739, SignerP256 {
     constructor(bytes32 qx, bytes32 qy) EIP712("ERC7739P256", "1") {
-        _initializeSigner(qx, qy);
+        _setSigner(qx, qy);
     }
 }
diff --git a/contracts/mocks/utils/cryptography/ERC7739SignerRSAMock.sol b/contracts/mocks/utils/cryptography/ERC7739SignerRSAMock.sol
@@ -8,6 +8,6 @@ import {SignerRSA} from "../../../utils/cryptography/SignerRSA.sol";
 
 contract ERC7739RSAMock is ERC7739, SignerRSA {
     constructor(bytes memory e, bytes memory n) EIP712("ERC7739RSA", "1") {
-        _initializeSigner(e, n);
+        _setSigner(e, n);
     }
 }
diff --git a/contracts/utils/cryptography/SignerECDSA.sol b/contracts/utils/cryptography/SignerECDSA.sol
@@ -9,38 +9,32 @@ import {AbstractSigner} from "./AbstractSigner.sol";
  * @dev Implementation of {AbstractSigner} using
  * https://docs.openzeppelin.com/contracts/api/utils#ECDSA[ECDSA] signatures.
  *
- * For {Account} usage, an {_initializeSigner} function is provided to set the {signer} address.
+ * For {Account} usage, an {_setSigner} function is provided to set the {signer} address.
  * Doing so it's easier for a factory, whose likely to use initializable clones of this contract.
  *
  * Example of usage:
  *
  * ```solidity
- * contract MyAccountECDSA is Account, SignerECDSA {
+ * contract MyAccountECDSA is Account, SignerECDSA, Initializable {
  *     constructor() EIP712("MyAccountECDSA", "1") {}
  *
- *     function initializeSigner(address signerAddr) public virtual initializer {
- *       // Will revert if the signer is already initialized
- *       _initializeSigner(signerAddr);
+ *     function initialize(address signerAddr) public initializer {
+ *       _setSigner(signerAddr);
  *     }
  * }
  * ```
  *
- * IMPORTANT: Avoiding to call {_initializeSigner} either during construction (if used standalone)
+ * IMPORTANT: Avoiding to call {_setSigner} either during construction (if used standalone)
  * or during initialization (if used as a clone) may leave the signer either front-runnable or unusable.
  */
 abstract contract SignerECDSA is AbstractSigner {
-    /**
-     * @dev The {signer} is already initialized.
-     */
-    error SignerECDSAUninitializedSigner(address signer);
-
     address private _signer;
 
     /**
-     * @dev Initializes the signer with the address of the native signer. This function can be called only once.
+     * @dev Sets the signer with the address of the native signer. This function should be called during construction
+     * or through an initializater.
      */
-    function _initializeSigner(address signerAddr) internal {
-        if (_signer != address(0)) revert SignerECDSAUninitializedSigner(signerAddr);
+    function _setSigner(address signerAddr) internal {
         _signer = signerAddr;
     }
 
diff --git a/contracts/utils/cryptography/SignerP256.sol b/contracts/utils/cryptography/SignerP256.sol
@@ -9,39 +9,36 @@ import {AbstractSigner} from "./AbstractSigner.sol";
  * @dev Implementation of {AbstractSigner} using
  * https://docs.openzeppelin.com/contracts/api/utils#P256[P256] signatures.
  *
- * For {Account} usage, an {_initializeSigner} function is provided to set the {signer} public key.
+ * For {Account} usage, an {_setSigner} function is provided to set the {signer} public key.
  * Doing so it's easier for a factory, whose likely to use initializable clones of this contract.
  *
  * Example of usage:
  *
  * ```solidity
- * contract MyAccountP256 is Account, SignerP256 {
+ * contract MyAccountP256 is Account, SignerP256, Initializable {
  *     constructor() EIP712("MyAccountP256", "1") {}
  *
- *     function initializeSigner(bytes32 qx, bytes32 qy) public virtual initializer {
- *       // Will revert if the signer is already initialized
- *       _initializeSigner(qx, qy);
+ *     function initializeSigner(bytes32 qx, bytes32 qy) public initializer {
+ *       _setSigner(qx, qy);
  *     }
  * }
  * ```
  *
- * IMPORTANT: Avoiding to call {_initializeSigner} either during construction (if used standalone)
+ * IMPORTANT: Avoiding to call {_setSigner} either during construction (if used standalone)
  * or during initialization (if used as a clone) may leave the signer either front-runnable or unusable.
  */
 abstract contract SignerP256 is AbstractSigner {
-    /**
-     * @dev The {signer} is already initialized.
-     */
-    error SignerP256UninitializedSigner(bytes32 qx, bytes32 qy);
-
     bytes32 private _qx;
     bytes32 private _qy;
 
+    error SignerP256InvalidPublicKey(bytes32 qx, bytes32 qy);
+
     /**
-     * @dev Initializes the signer with the P256 public key. This function can be called only once.
+     * @dev Sets the signer with a P256 public key. This function should be called during construction
+     * or through an initializater.
      */
-    function _initializeSigner(bytes32 qx, bytes32 qy) internal {
-        if (_qx != 0 || _qy != 0) revert SignerP256UninitializedSigner(qx, qy);
+    function _setSigner(bytes32 qx, bytes32 qy) internal {
+        if (!P256.isValidPublicKey(qx, qy)) revert SignerP256InvalidPublicKey(qx, qy);
         _qx = qx;
         _qy = qy;
     }
diff --git a/contracts/utils/cryptography/SignerRSA.sol b/contracts/utils/cryptography/SignerRSA.sol
@@ -9,39 +9,33 @@ import {AbstractSigner} from "./AbstractSigner.sol";
  * @dev Implementation of {AbstractSigner} using
  * https://docs.openzeppelin.com/contracts/api/utils#RSA[RSA] signatures.
  *
- * For {Account} usage, an {_initializeSigner} function is provided to set the {signer} public key.
+ * For {Account} usage, an {_setSigner} function is provided to set the {signer} public key.
  * Doing so it's easier for a factory, whose likely to use initializable clones of this contract.
  *
  * Example of usage:
  *
  * ```solidity
- * contract MyAccountRSA is Account, SignerRSA {
+ * contract MyAccountRSA is Account, SignerRSA, Initializable {
  *     constructor() EIP712("MyAccountRSA", "1") {}
  *
- *     function initializeSigner(bytes memory e, bytes memory n) external {
- *       // Will revert if the signer is already initialized
- *       _initializeSigner(e, n);
+ *     function initializeSigner(bytes memory e, bytes memory n) public initializer {
+ *       _setSigner(e, n);
  *     }
  * }
  * ```
  *
- * IMPORTANT: Avoiding to call {_initializeSigner} either during construction (if used standalone)
+ * IMPORTANT: Avoiding to call {_setSigner} either during construction (if used standalone)
  * or during initialization (if used as a clone) may leave the signer either front-runnable or unusable.
  */
 abstract contract SignerRSA is AbstractSigner {
-    /**
-     * @dev The {signer} is already initialized.
-     */
-    error SignerRSAUninitializedSigner(bytes e, bytes n);
-
     bytes private _e;
     bytes private _n;
 
     /**
-     * @dev Initializes the signer with the RSA public key. This function can be called only once.
+     * @dev Sets the signer with a RSA public key. This function should be called during construction
+     * or through an initializater.
      */
-    function _initializeSigner(bytes memory e, bytes memory n) internal {
-        if (_e.length != 0 || _n.length != 0) revert SignerRSAUninitializedSigner(e, n);
+    function _setSigner(bytes memory e, bytes memory n) internal {
         _e = e;
         _n = n;
     }

Original file line number	Diff line number	Diff line change
`@@ -8,7 +8,7 @@ import {SignerECDSA} from "../../utils/cryptography/SignerECDSA.sol";`
`8`	`8`
`9`	`9`	`abstract contract AccountECDSAMock is Account, SignerECDSA, ERC7821 {`
`10`	`10`	`constructor(address signerAddr) {`
`11`		`- _initializeSigner(signerAddr);`
	`11`	`+ _setSigner(signerAddr);`
`12`	`12`	`}`
`13`	`13`
`14`	`14`	`/// @inheritdoc ERC7821`
Original file line number	Diff line number	Diff line change
`@@ -8,7 +8,7 @@ import {SignerP256} from "../../utils/cryptography/SignerP256.sol";`
`8`	`8`
`9`	`9`	`abstract contract AccountP256Mock is Account, SignerP256, ERC7821 {`
`10`	`10`	`constructor(bytes32 qx, bytes32 qy) {`
`11`		`- _initializeSigner(qx, qy);`
	`11`	`+ _setSigner(qx, qy);`
`12`	`12`	`}`
`13`	`13`
`14`	`14`	`/// @inheritdoc ERC7821`
Original file line number	Diff line number	Diff line change
`@@ -8,7 +8,7 @@ import {SignerRSA} from "../../utils/cryptography/SignerRSA.sol";`
`8`	`8`
`9`	`9`	`abstract contract AccountRSAMock is Account, SignerRSA, ERC7821 {`
`10`	`10`	`constructor(bytes memory e, bytes memory n) {`
`11`		`- _initializeSigner(e, n);`
	`11`	`+ _setSigner(e, n);`
`12`	`12`	`}`
`13`	`13`
`14`	`14`	`/// @inheritdoc ERC7821`
Original file line number	Diff line number	Diff line change
`@@ -32,7 +32,7 @@ contract MyFactoryAccount {`
`32`	`32`	`address predicted = predictAddress(_signerSalt);`
`33`	`33`	`if (predicted.code.length == 0) {`
`34`	`34`	`_impl.cloneDeterministic(_signerSalt);`
`35`		`- MyAccountECDSA(payable(predicted)).initializeSigner(signer);`
	`35`	`+ MyAccountECDSA(payable(predicted)).initialize(signer);`
`36`	`36`	`}`
`37`	`37`	`return predicted;`
`38`	`38`	`}`
Original file line number	Diff line number	Diff line change
`@@ -9,6 +9,6 @@ import {SignerECDSA} from "../../../utils/cryptography/SignerECDSA.sol";`
`9`	`9`
`10`	`10`	`contract ERC7739ECDSAMock is ERC7739, SignerECDSA {`
`11`	`11`	`constructor(address signerAddr) EIP712("ERC7739ECDSA", "1") {`
`12`		`- _initializeSigner(signerAddr);`
	`12`	`+ _setSigner(signerAddr);`
`13`	`13`	`}`
`14`	`14`	`}`
Original file line number	Diff line number	Diff line change
`@@ -8,6 +8,6 @@ import {SignerP256} from "../../../utils/cryptography/SignerP256.sol";`
`8`	`8`
`9`	`9`	`contract ERC7739P256Mock is ERC7739, SignerP256 {`
`10`	`10`	`constructor(bytes32 qx, bytes32 qy) EIP712("ERC7739P256", "1") {`
`11`		`- _initializeSigner(qx, qy);`
	`11`	`+ _setSigner(qx, qy);`
`12`	`12`	`}`
`13`	`13`	`}`
Original file line number	Diff line number	Diff line change
`@@ -8,6 +8,6 @@ import {SignerRSA} from "../../../utils/cryptography/SignerRSA.sol";`
`8`	`8`
`9`	`9`	`contract ERC7739RSAMock is ERC7739, SignerRSA {`
`10`	`10`	`constructor(bytes memory e, bytes memory n) EIP712("ERC7739RSA", "1") {`
`11`		`- _initializeSigner(e, n);`
	`11`	`+ _setSigner(e, n);`
`12`	`12`	`}`
`13`	`13`	`}`