fix(account): revert onInstall when module already installed with different config

contracts/account/modules/ERC7579Multisig.sol

@@ -48,6 +48,9 @@ abstract contract ERC7579Multisig is ERC7579Validator {
     /// @dev The `threshold` is unreachable given the number of `signers`.
     error ERC7579MultisigUnreachableThreshold(uint64 signers, uint64 threshold);
 
+    /// @dev Thrown when attempting to install while signers are already configured.
+    error ERC7579MultisigAlreadyInstalled();
+
     mapping(address account => EnumerableSet.BytesSet) private _signersSetByAccount;
     mapping(address account => uint64) private _thresholdByAccount;
 
@@ -62,12 +65,12 @@ abstract contract ERC7579Multisig is ERC7579Validator {
      * If no signers or threshold are provided, the multisignature functionality will be
      * disabled until they are added later.
      *
-     * NOTE: An account can only call onInstall once. If called directly by the account,
-     * the signer will be set to the provided data. Future installations will behave as a no-op.
+     * NOTE: An account can only call onInstall once. Future installations will revert with
+     *       {ERC7579MultisigAlreadyInstalled} if signers are already configured.
      */
     function onInstall(bytes calldata initData) public virtual {
-        if (initData.length > 32 && getSignerCount(msg.sender) == 0) {
-            // More than just delay parameter
+        require(getSignerCount(msg.sender) == 0, ERC7579MultisigAlreadyInstalled());
+        if (initData.length > 32) {
             (bytes[] memory signers_, uint64 threshold_) = abi.decode(initData, (bytes[], uint64));
             _addSigners(msg.sender, signers_);
             _setThreshold(msg.sender, threshold_);

contracts/account/modules/ERC7579MultisigWeighted.sol

@@ -62,8 +62,9 @@ abstract contract ERC7579MultisigWeighted is ERC7579Multisig {
      */
     function onInstall(bytes calldata initData) public virtual override {
         bool installed = getSignerCount(msg.sender) > 0;
+        if (installed) return;
         super.onInstall(initData);
-        if (initData.length > 96 && !installed) {
+        if (initData.length > 96) {
             (bytes[] memory signers, , uint64[] memory weights) = abi.decode(initData, (bytes[], uint64, uint64[]));
             _setSignerWeights(msg.sender, signers, weights);
         }

contracts/account/modules/ERC7579Signature.sol

@@ -27,6 +27,9 @@ contract ERC7579Signature is ERC7579Validator {
     /// @dev Thrown when the signer length is less than 20 bytes.
     error ERC7579SignatureInvalidSignerLength();
 
+    /// @dev Thrown when attempting to install a signer while a different signer is already set
+    error ERC7579SignatureAlreadyInstalled();
+
     /// @dev Return the ERC-7913 signer (i.e. `verifier || key`).
     function signer(address account) public view virtual returns (bytes memory) {
         return _signers[account];
@@ -35,12 +38,18 @@ contract ERC7579Signature is ERC7579Validator {
     /**
      * @dev See {IERC7579Module-onInstall}.
      *
-     * NOTE: An account can only call onInstall once. If called directly by the account,
-     * the signer will be set to the provided data. Future installations will behave as a no-op.
+     * NOTE: On first install, the signer is set to the provided `data`. Subsequent calls are
+     * idempotent only when `data` matches the currently installed signer; otherwise they
+     * revert with {ERC7579SignatureAlreadyInstalled}.
      */
     function onInstall(bytes calldata data) public virtual {
-        if (signer(msg.sender).length == 0) {
-            setSigner(data);
+        bytes memory currentSigner = signer(msg.sender);
+        if (currentSigner.length == 0) {
+            require(data.length >= 20, ERC7579SignatureInvalidSignerLength());
+            _setSigner(msg.sender, data);
+        } else {
+            require(keccak256(currentSigner) == keccak256(data), ERC7579SignatureAlreadyInstalled());
+            // idempotent: no-op for identical data
         }
     }
 

test/account/modules/ERC7579Multisig.test.js

@@ -95,14 +95,12 @@ describe('ERC7579Multisig', function () {
     await expect(this.mock.getSigners(this.mockAccount.address, 0, MAX_UINT64)).to.eventually.deep.equal(this.signers);
     await expect(this.mock.threshold(this.mockAccount.address)).to.eventually.equal(this.threshold);
 
-    // onInstall is allowed again but is a noop
-    await this.mockFromAccount.onInstall(
-      ethers.AbiCoder.defaultAbiCoder().encode(['bytes[]', 'uint256'], [[signerECDSA3.address], 2]),
-    );
-
-    // Should still have the original signers and threshold
-    await expect(this.mock.getSigners(this.mockAccount.address, 0, MAX_UINT64)).to.eventually.deep.equal(this.signers);
-    await expect(this.mock.threshold(this.mockAccount.address)).to.eventually.equal(this.threshold);
+    // onInstall should now revert if already installed
+    await expect(
+      this.mockFromAccount.onInstall(
+        ethers.AbiCoder.defaultAbiCoder().encode(['bytes[]', 'uint256'], [[signerECDSA3.address], 2]),
+      ),
+    ).to.be.revertedWithCustomError(this.mock, 'ERC7579MultisigAlreadyInstalled');
   });
 
   it('cleans up signers and threshold on uninstallation', async function () {

test/account/modules/ERC7579SignatureValidator.test.js

@@ -70,14 +70,32 @@ describe('ERC7579Signature', function () {
     );
   });
 
-  it('behaves as a noop when the validator is already installed for an account', async function () {
-    // First installation should succeed
+  it('installs signer correctly on first call', async function () {
     const signerData = ethers.solidityPacked(['address'], [signerECDSA.address]);
     await expect(this.mockFromAccount.onInstall(signerData)).to.not.be.reverted;
+    await expect(this.mock.signer(this.mockAccount.address)).to.eventually.equal(signerData);
+  });
+
+  it('does not revert when onInstall is called again with the same signer', async function () {
+    const signerData = ethers.solidityPacked(['address'], [signerECDSA.address]);
+    await this.mockFromAccount.onInstall(signerData);
+
+    // Same signer — should NOT revert
+    await expect(this.mockFromAccount.onInstall(signerData)).to.not.be.reverted;
 
-    // Second installation should behave as a no-op
-    await this.mockFromAccount.onInstall(ethers.solidityPacked(['address'], [ethers.Wallet.createRandom().address])); // Not revert
-    await expect(this.mock.signer(this.mockAccount.address)).to.eventually.equal(signerData); // No change in signers
+    // Signer unchanged
+    await expect(this.mock.signer(this.mockAccount.address)).to.eventually.equal(signerData);
+  });
+
+  it('reverts when onInstall is called with a different signer already set', async function () {
+    const signerData = ethers.solidityPacked(['address'], [signerECDSA.address]);
+    await this.mockFromAccount.onInstall(signerData);
+
+    const differentSigner = ethers.solidityPacked(['address'], [ethers.Wallet.createRandom().address]);
+    await expect(this.mockFromAccount.onInstall(differentSigner)).to.be.revertedWithCustomError(
+      this.mock,
+      'ERC7579SignatureAlreadyInstalled',
+    );
   });
 
   it('emits event on ERC7579SignatureSignerSet on both installation and uninstallation', async function () {