Skip to content

Consider reverting module installation calls when state is not set (instead of no op) #192

Description

@Amxx

This issue is about

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);
    }

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Fields

    No fields configured for issues without a type.

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions