Skip to content

Commit

Permalink
chore: install new view func for convenience (#22)
Browse files Browse the repository at this point in the history
## Summary
The tradeoff is installation gas cost vs convenient access to states
through MSCA itself. For more details, please refer to
[How to write an ERC-6900
Plugin](https://dev.collab.land/blog/how-to-write-an-erc-6900-plugin/#3-map-plugin-logic-to-msca-functions).

## Detail
### Changeset
* install `getReplaySafeMessageHash` for both `SingleOwnerPlugin` and
`WeightedWebauthnMultisigPlugin` so they could be both called through
MSCA itself

### Checklist
- [x] Did you add new tests and confirm all tests pass? (`yarn test`)
- [ ] Did you update relevant docs? (docs are found in the `docs`
folder)
- [x] Do your commits follow the [Conventional
Commits](https://www.conventionalcommits.org/en/v1.0.0/) standard?
- [x] Does your PR title also follow the [Conventional
Commits](https://www.conventionalcommits.org/en/v1.0.0/) standard?
- [ ] If you have a breaking change, is it [correctly reflected in your
commit
message](https://www.conventionalcommits.org/en/v1.0.0/#examples)? (e.g.
`feat!: breaking change`)
- [x] Did you run lint (`yarn lint`) and fix any issues?
- [x] Did you run formatter (`yarn format:check`) and fix any issues
(`yarn format:write`)?

## Testing
* Updated the tests to reflect the installation 

## Documentation
n/a
  • Loading branch information
huaweigu authored Oct 15, 2024
1 parent e1e2eee commit 2a17ad5
Show file tree
Hide file tree
Showing 5 changed files with 19 additions and 52 deletions.
7 changes: 5 additions & 2 deletions src/msca/6900/v0.7/plugins/v1_0_0/acl/SingleOwnerPlugin.sol
Original file line number Diff line number Diff line change
Expand Up @@ -147,11 +147,12 @@ contract SingleOwnerPlugin is BasePlugin, ISingleOwnerPlugin, IERC1271, BaseERC7
/// @inheritdoc BasePlugin
function pluginManifest() external pure override returns (PluginManifest memory) {
PluginManifest memory manifest;
manifest.executionFunctions = new bytes4[](4);
manifest.executionFunctions = new bytes4[](5);
manifest.executionFunctions[0] = this.transferOwnership.selector;
manifest.executionFunctions[1] = this.getOwner.selector;
manifest.executionFunctions[2] = this.getOwnerOf.selector;
manifest.executionFunctions[3] = this.isValidSignature.selector;
manifest.executionFunctions[4] = this.getReplaySafeMessageHash.selector;

ManifestFunction memory userOpValidationAssociatedFunction =
ManifestFunction(ManifestAssociatedFunctionType.SELF, uint8(FunctionId.USER_OP_VALIDATION_OWNER), 0);
Expand All @@ -177,7 +178,7 @@ contract SingleOwnerPlugin is BasePlugin, ISingleOwnerPlugin, IERC1271, BaseERC7
ManifestFunction memory runtimeAlwaysAllowAssociatedFunction =
ManifestFunction(ManifestAssociatedFunctionType.RUNTIME_VALIDATION_ALWAYS_ALLOW, 0, 0);
// the following direct function calls (from EOA/SC) should be gated by the runtimeValidationAssociatedFunction
manifest.runtimeValidationFunctions = new ManifestAssociatedFunction[](9);
manifest.runtimeValidationFunctions = new ManifestAssociatedFunction[](10);
// plugin functions
manifest.runtimeValidationFunctions[0] =
ManifestAssociatedFunction(this.transferOwnership.selector, runtimeValidationAssociatedFunction);
Expand All @@ -199,6 +200,8 @@ contract SingleOwnerPlugin is BasePlugin, ISingleOwnerPlugin, IERC1271, BaseERC7
ManifestAssociatedFunction(this.getOwnerOf.selector, runtimeAlwaysAllowAssociatedFunction);
manifest.runtimeValidationFunctions[8] =
ManifestAssociatedFunction(this.isValidSignature.selector, runtimeAlwaysAllowAssociatedFunction);
manifest.runtimeValidationFunctions[9] =
ManifestAssociatedFunction(this.getReplaySafeMessageHash.selector, runtimeAlwaysAllowAssociatedFunction);
manifest.interfaceIds = new bytes4[](2);
manifest.interfaceIds[0] = type(IERC1271).interfaceId;
manifest.interfaceIds[1] = type(ISingleOwnerPlugin).interfaceId;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -158,11 +158,12 @@ contract WeightedWebauthnMultisigPlugin is BaseWeightedMultisigPlugin, BaseERC71
function pluginManifest() external pure virtual override returns (PluginManifest memory) {
PluginManifest memory manifest;

manifest.executionFunctions = new bytes4[](4);
manifest.executionFunctions = new bytes4[](5);
manifest.executionFunctions[0] = this.addOwners.selector;
manifest.executionFunctions[1] = this.removeOwners.selector;
manifest.executionFunctions[2] = this.updateMultisigWeights.selector;
manifest.executionFunctions[3] = this.isValidSignature.selector;
manifest.executionFunctions[4] = this.getReplaySafeMessageHash.selector;

ManifestFunction memory ownerUserOpValidationFunction = ManifestFunction({
functionType: ManifestAssociatedFunctionType.SELF,
Expand All @@ -171,7 +172,7 @@ contract WeightedWebauthnMultisigPlugin is BaseWeightedMultisigPlugin, BaseERC71
});

// Update native functions to use userOpValidationFunction provided by this plugin
// The view functions `isValidSignature` and `eip712Domain` are excluded from being assigned a user
// The view functions `isValidSignature` and `getReplaySafeMessageHash` are excluded from being assigned a user
// operation validation function since they should only be called via the runtime path.
manifest.userOpValidationFunctions = new ManifestAssociatedFunction[](8);
manifest.userOpValidationFunctions[0] = ManifestAssociatedFunction({
Expand Down
47 changes: 2 additions & 45 deletions src/msca/6900/v0.8/modules/multisig/BaseWeightedMultisigModule.sol
Original file line number Diff line number Diff line change
Expand Up @@ -259,51 +259,8 @@ abstract contract BaseWeightedMultisigModule is IWeightedMultisigModule, BaseMul
OwnerData[] memory newOwnersData,
uint256 newThresholdWeight
) internal {
uint256 ownersToUpdateLen = ownersToUpdate.length;
uint256 weightsToUpdateLen = newOwnersData.length;

// If ownersToUpdate and newWeights are differing lengths, revert.
// (If both are 0, just update threshold)
if (ownersToUpdateLen != weightsToUpdateLen) {
revert OwnersWeightsMismatch();
}

uint256 totalWeightAdditions = 0;
uint256 totalWeightReductions = 0;

if (ownersToUpdateLen > 0) {
for (uint256 i = 0; i < ownersToUpdateLen; ++i) {
// confirm the owner has a nonzero weight, else revert
uint256 _ownerCurrentWeight = ownerDataPerAccount[ownersToUpdate[i]][msg.sender].weight;
uint256 _newWeight = newOwnersData[i].weight;
if (_ownerCurrentWeight == 0) {
// setting owner weights for uninitialized owners via setMultisigWeights() is not allowed. Owners
// must be added via addOwners().
revert InvalidOwner(ownersToUpdate[i]);
}
if (_ownerCurrentWeight < _newWeight) {
_setOwnerWeight(ownersToUpdate[i], newOwnersData[i].weight);
totalWeightAdditions += (_newWeight - _ownerCurrentWeight);
} else if (_ownerCurrentWeight > _newWeight) {
_setOwnerWeight(ownersToUpdate[i], newOwnersData[i].weight);
totalWeightReductions += (_ownerCurrentWeight - _newWeight);
}
}
}

// 2. Update metadata
OwnershipMetadata storage metadata = _ownerMetadata[msg.sender];
uint256 _initialTotalWeight = metadata.totalWeight;
uint256 _newTotalWeight = _initialTotalWeight + totalWeightAdditions - totalWeightReductions;
metadata.totalWeight = _newTotalWeight;

_validateAndOptionallySetThresholdWeight(newThresholdWeight, _newTotalWeight, metadata);

// 3. Emit events
// (updateMultisigWeights permits updating threshold without updating individual owner weights.)
if (ownersToUpdateLen > 0) {
emit OwnersUpdated(msg.sender, ownersToUpdate, newOwnersData);
}
(ownersToUpdate, newOwnersData, newThresholdWeight);
revert NotImplemented(msg.sig, 0);
}

/// @notice Adds owners, or reverts if any owner cannot be added.
Expand Down
5 changes: 5 additions & 0 deletions test/msca/6900/v0.7/SingleOwnerPlugin.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -312,6 +312,11 @@ contract SingleOwnerPluginTest is TestUtils {

// verify supportedInterfaces
assertEq(bytes32(msca1.getSupportedInterface(type(IERC1271).interfaceId)), bytes32(uint256(1)));

executionDetail = msca1.getExecutionDetail(singleOwnerPlugin.getReplaySafeMessageHash.selector);
assertEq(executionDetail.plugin, singleOwnerPluginAddr);
assertEq(executionDetail.userOpValidationFunction.pack(), EMPTY_FUNCTION_REFERENCE);
assertEq(executionDetail.runtimeValidationFunction.pack(), RUNTIME_VALIDATION_ALWAYS_ALLOW_FUNCTION_REFERENCE);
}

function testTransferOwnership() public {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -274,13 +274,14 @@ contract WeightedWebauthnMultisigPluginTest is TestUtils {

function test_pluginManifest() public {
PluginManifest memory manifest = plugin.pluginManifest();
// 4 execution functions (addOwners, removeOwners, updateMultisigWeights, isValidSignature)
assertEq(4, manifest.executionFunctions.length);
// 4 execution functions (addOwners, removeOwners, updateMultisigWeights, isValidSignature,
// getReplaySafeMessageHash)
assertEq(5, manifest.executionFunctions.length);

// 7 native + 1 plugin exec func
assertEq(8, manifest.userOpValidationFunctions.length);

// 10 runtime validations (isValidSignature, eip712Domain, 8 disabled functions)
// 10 runtime validations (isValidSignature, getReplaySafeMessageHash, 8 disabled functions)
assertEq(10, manifest.runtimeValidationFunctions.length);
}

Expand Down

0 comments on commit 2a17ad5

Please sign in to comment.