-
Notifications
You must be signed in to change notification settings - Fork 36
Open
Description
This issue is about
openzeppelin-community-contracts/contracts/account/modules/ERC7579Signature.sol
Lines 41 to 45 in 0e71ce5
| function onInstall(bytes calldata data) public virtual { | |
| if (signer(msg.sender).length == 0) { | |
| setSigner(data); | |
| } | |
| } |
Basically we have three option if onInstall is called when a signer is already present:
- revert
- override storage
- no op (current behavior)
I'm worried about the choice of the last option, because the installer may expect the signer passed through the argument to be valid. Doing a no op here may cause an issue to be unnoticed.
I would propose something like
function onInstall(bytes calldata data) public virtual {
bytes memory currentSigner = signer(msg.sender);
require(currentSigner.length == 0 || currentSigner.equal(data), Something());
setSigner(msg.sender, data);
}Reactions are currently unavailable
Metadata
Metadata
Assignees
Labels
No labels