diff --git a/.gas-snapshot b/.gas-snapshot index 3a61111..3139231 100644 --- a/.gas-snapshot +++ b/.gas-snapshot @@ -2,43 +2,52 @@ MultiOwnerPluginIntegration:test_ownerPlugin_successInstallation() (gas: 39135) MultiOwnerPluginIntegration:test_runtimeValidation_alwaysAllow_isValidSignature() (gas: 104093) MultiOwnerPluginIntegration:test_runtimeValidation_ownerOrSelf_standardExecute() (gas: 145057) MultiOwnerPluginIntegration:test_userOpValidation_owner_standardExecute() (gas: 347849) -MultiOwnerPluginTest:testFuzz_isValidSignature_ContractOwner(bytes32) (runs: 256, μ: 110118, ~: 110118) -MultiOwnerPluginTest:testFuzz_isValidSignature_ContractOwnerWithEOAOwner(bytes32) (runs: 256, μ: 120437, ~: 120437) -MultiOwnerPluginTest:testFuzz_isValidSignature_EOAOwner(string,bytes32) (runs: 256, μ: 130754, ~: 130747) -MultiOwnerPluginTest:testFuzz_isValidSignature_PasskeyOwner(bytes32) (runs: 256, μ: 365635, ~: 365718) -MultiOwnerPluginTest:testFuzz_userOpValidationFunction_ContractOwner((address,uint256,bytes,bytes,uint256,uint256,uint256,uint256,uint256,bytes,bytes)) (runs: 256, μ: 130892, ~: 130881) -MultiOwnerPluginTest:testFuzz_userOpValidationFunction_ContractOwnerWithEOAOwner((address,uint256,bytes,bytes,uint256,uint256,uint256,uint256,uint256,bytes,bytes)) (runs: 256, μ: 144491, ~: 144480) -MultiOwnerPluginTest:testFuzz_userOpValidationFunction_EOAOwner(string,(address,uint256,bytes,bytes,uint256,uint256,uint256,uint256,uint256,bytes,bytes)) (runs: 256, μ: 138767, ~: 138770) -MultiOwnerPluginTest:testFuzz_userOpValidationFunction_PasskeyOwner((address,uint256,bytes,bytes,uint256,uint256,uint256,uint256,uint256,bytes,bytes)) (runs: 256, μ: 373522, ~: 373350) -MultiOwnerPluginTest:test_eip712Domain() (gas: 35394) -MultiOwnerPluginTest:test_multiOwnerPlugin_sentinelIsNotOwner() (gas: 19794) -MultiOwnerPluginTest:test_onInstall_failWithInvalidAddress() (gas: 38434) -MultiOwnerPluginTest:test_onInstall_failWithLimitExceeded() (gas: 1714334) -MultiOwnerPluginTest:test_onInstall_success() (gas: 94764) +MultiOwnerPluginTest:testFuzz_isValidSignature_ContractOwner(bytes32) (runs: 256, μ: 110067, ~: 110067) +MultiOwnerPluginTest:testFuzz_isValidSignature_ContractOwnerWithEOAOwner(bytes32) (runs: 256, μ: 120381, ~: 120381) +MultiOwnerPluginTest:testFuzz_isValidSignature_EOAOwner(string,bytes32) (runs: 256, μ: 130741, ~: 130734) +MultiOwnerPluginTest:testFuzz_isValidSignature_PasskeyOwner(bytes32) (runs: 256, μ: 365479, ~: 365612) +MultiOwnerPluginTest:testFuzz_runtimeValidationFunction_BadFunctionId(uint8) (runs: 256, μ: 9747, ~: 9747) +MultiOwnerPluginTest:testFuzz_userOpValidationFunction_BadFunctionId(uint8) (runs: 256, μ: 10744, ~: 10744) +MultiOwnerPluginTest:testFuzz_userOpValidationFunction_ContractOwner((address,uint256,bytes,bytes,uint256,uint256,uint256,uint256,uint256,bytes,bytes)) (runs: 256, μ: 130908, ~: 130897) +MultiOwnerPluginTest:testFuzz_userOpValidationFunction_ContractOwnerWithEOAOwner((address,uint256,bytes,bytes,uint256,uint256,uint256,uint256,uint256,bytes,bytes)) (runs: 256, μ: 144523, ~: 144512) +MultiOwnerPluginTest:testFuzz_userOpValidationFunction_EOAOwner(string,(address,uint256,bytes,bytes,uint256,uint256,uint256,uint256,uint256,bytes,bytes)) (runs: 256, μ: 138777, ~: 138780) +MultiOwnerPluginTest:testFuzz_userOpValidationFunction_PasskeyOwner((address,uint256,bytes,bytes,uint256,uint256,uint256,uint256,uint256,bytes,bytes)) (runs: 256, μ: 373604, ~: 373579) +MultiOwnerPluginTest:test_eip712Domain() (gas: 35438) +MultiOwnerPluginTest:test_isValidSignature_failMalformedAddress() (gas: 15610) +MultiOwnerPluginTest:test_isValidSignature_failWithOutOfBounds() (gas: 12319) +MultiOwnerPluginTest:test_multiOwnerPlugin_sentinelIsNotOwner() (gas: 19805) +MultiOwnerPluginTest:test_onInstall_failWithEmptyOwners() (gas: 36299) +MultiOwnerPluginTest:test_onInstall_failWithInvalidAddress() (gas: 38500) +MultiOwnerPluginTest:test_onInstall_failWithLimitExceeded() (gas: 1714289) +MultiOwnerPluginTest:test_onInstall_success() (gas: 94741) MultiOwnerPluginTest:test_onUninstall_success() (gas: 84935) -MultiOwnerPluginTest:test_pluginInitializeGuards() (gas: 163889) -MultiOwnerPluginTest:test_pluginManifest() (gas: 38747) -MultiOwnerPluginTest:test_runtimeValidationFunction_OwnerOrSelf() (gas: 26747) -MultiOwnerPluginTest:test_updateOwnersPublicKeys_failWithInvalidAddress() (gas: 55749) -MultiOwnerPluginTest:test_updateOwners_failWithDuplicatedAddresses() (gas: 85472) -MultiOwnerPluginTest:test_updateOwners_failWithEmptyOwners() (gas: 70734) +MultiOwnerPluginTest:test_ownerIndexOf_failWithNotExist() (gas: 28310) +MultiOwnerPluginTest:test_pluginInitializeGuards() (gas: 163933) +MultiOwnerPluginTest:test_pluginManifest() (gas: 38695) +MultiOwnerPluginTest:test_pluginMetadata_success() (gas: 16954) +MultiOwnerPluginTest:test_runtimeValidationFunction_OwnerOrSelf() (gas: 26703) +MultiOwnerPluginTest:test_updateOwnersPublicKeys_failWithInvalidAddress() (gas: 55729) +MultiOwnerPluginTest:test_updateOwners_failWithDuplicatedAddresses() (gas: 85516) +MultiOwnerPluginTest:test_updateOwners_failWithEmptyOwners() (gas: 70711) MultiOwnerPluginTest:test_updateOwners_failWithLimitExceeded() (gas: 1924832) -MultiOwnerPluginTest:test_updateOwners_failWithNotExist() (gas: 58568) -MultiOwnerPluginTest:test_updateOwners_failWithZeroAddressOwner() (gas: 56362) +MultiOwnerPluginTest:test_updateOwners_failWithNotExist() (gas: 58523) +MultiOwnerPluginTest:test_updateOwners_failWithZeroAddressOwner() (gas: 56428) MultiOwnerPluginTest:test_updateOwners_success() (gas: 119826) -WebauthnModularAccountFactoryTest:test_2StepOwnershipTransfer() (gas: 87560) -WebauthnModularAccountFactoryTest:test_addStake() (gas: 106151) -WebauthnModularAccountFactoryTest:test_addressMatch() (gas: 801803) +WebauthnModularAccountFactoryTest:test_2StepOwnershipTransfer() (gas: 87593) +WebauthnModularAccountFactoryTest:test_addStake() (gas: 106118) +WebauthnModularAccountFactoryTest:test_addressMatch() (gas: 801814) WebauthnModularAccountFactoryTest:test_badOwnersArray() (gas: 18409) -WebauthnModularAccountFactoryTest:test_deploy() (gas: 810337) -WebauthnModularAccountFactoryTest:test_deployCollision() (gas: 825988) +WebauthnModularAccountFactoryTest:test_deploy() (gas: 810381) +WebauthnModularAccountFactoryTest:test_deployCollision() (gas: 826021) WebauthnModularAccountFactoryTest:test_deployWithDuplicateOwners() (gas: 737357) WebauthnModularAccountFactoryTest:test_deployWithUnsortedOwners() (gas: 737379) -WebauthnModularAccountFactoryTest:test_deploy_PasskeyOwner() (gas: 797170) +WebauthnModularAccountFactoryTest:test_deploy_PasskeyOwner() (gas: 797192) WebauthnModularAccountFactoryTest:test_deployedAccountHasCorrectPlugins() (gas: 801483) -WebauthnModularAccountFactoryTest:test_getAddressWithMaxOwnersAndDeploy() (gas: 2704020) -WebauthnModularAccountFactoryTest:test_getAddressWithTooManyOwners() (gas: 205715) +WebauthnModularAccountFactoryTest:test_getAddressWithMaxOwnersAndDeploy() (gas: 2704053) +WebauthnModularAccountFactoryTest:test_getAddressWithTooManyOwners() (gas: 205692) WebauthnModularAccountFactoryTest:test_getAddressWithUnsortedOwners() (gas: 11320) -WebauthnModularAccountFactoryTest:test_unlockStake() (gas: 147811) +WebauthnModularAccountFactoryTest:test_renounceOwnership() (gas: 10459) +WebauthnModularAccountFactoryTest:test_unlockStake() (gas: 147812) WebauthnModularAccountFactoryTest:test_withdraw() (gas: 128078) -WebauthnModularAccountFactoryTest:test_withdrawStake() (gas: 210368) \ No newline at end of file +WebauthnModularAccountFactoryTest:test_withdrawStake() (gas: 210325) +WebauthnModularAccountFactoryTest:test_withdraw_token() (gas: 995155) \ No newline at end of file diff --git a/src/WebauthnOwnerPlugin.sol b/src/WebauthnOwnerPlugin.sol index d959405..521cf84 100644 --- a/src/WebauthnOwnerPlugin.sol +++ b/src/WebauthnOwnerPlugin.sol @@ -327,7 +327,7 @@ contract WebauthnOwnerPlugin is BasePlugin, IWebauthnOwnerPlugin, IERC1271 { PublicKey memory owner = _owners[account].get(uint8(signature[0])); if (owner.y == 0) { - if (owner.x > type(uint160).max) revert InvalidEthereumAddressOwner(bytes32(owner.x)); + if (owner.x > type(uint160).max) revert InvalidEthereumAddressOwner(bytes32(owner.x)); // should be impossible return address(uint160(owner.x)).isValidSignatureNowCalldata(message, signature[1:]); } diff --git a/test/WebauthnModularAccountFactory.t.sol b/test/WebauthnModularAccountFactory.t.sol index 8240936..c765c29 100644 --- a/test/WebauthnModularAccountFactory.t.sol +++ b/test/WebauthnModularAccountFactory.t.sol @@ -8,11 +8,14 @@ import { EntryPoint } from "account-abstraction/core/EntryPoint.sol"; import { PluginManagerInternals } from "modular-account/src/account/PluginManagerInternals.sol"; import { UpgradeableModularAccount } from "modular-account/src/account/UpgradeableModularAccount.sol"; +import { MockERC20 } from "solady/../test/utils/mocks/MockERC20.sol"; import { ECDSA } from "solady/utils/ECDSA.sol"; import { DeployScript } from "../script/Deploy.s.sol"; import { OwnersLib } from "../src/OwnersLib.sol"; -import { OwnersLimitExceeded, WebauthnModularAccountFactory } from "../src/WebauthnModularAccountFactory.sol"; +import { + InvalidAction, OwnersLimitExceeded, WebauthnModularAccountFactory +} from "../src/WebauthnModularAccountFactory.sol"; import { IMultiOwnerPlugin, MAX_OWNERS, PublicKey, WebauthnOwnerPlugin } from "../src/WebauthnOwnerPlugin.sol"; // solhint-disable func-name-mixedcase @@ -161,6 +164,14 @@ contract WebauthnModularAccountFactoryTest is Test { assertEq(address(factory).balance, 0); } + function test_withdraw_token() external { + MockERC20 token = new MockERC20("A", "A", 18); + token.mint(address(factory), 1); + assertEq(token.balanceOf(address(factory)), 1); + factory.withdraw(payable(address(this)), address(token), 1); + assertEq(token.balanceOf(address(factory)), 0); + } + function test_2StepOwnershipTransfer() external { assertEq(factory.owner(), address(this)); factory.transferOwnership(owner1); @@ -218,6 +229,11 @@ contract WebauthnModularAccountFactoryTest is Test { factory.createAccount(0, tempOwners.toPublicKeys()); } + function test_renounceOwnership() external { + vm.expectRevert(InvalidAction.selector); + factory.renounceOwnership(); + } + // to receive funds from withdraw receive() external payable { } } diff --git a/test/WebauthnOwnerPlugin.t.sol b/test/WebauthnOwnerPlugin.t.sol index 9d423af..4a5698f 100644 --- a/test/WebauthnOwnerPlugin.t.sol +++ b/test/WebauthnOwnerPlugin.t.sol @@ -7,12 +7,12 @@ import { EntryPoint } from "account-abstraction/core/EntryPoint.sol"; import { UpgradeableModularAccount } from "modular-account/src/account/UpgradeableModularAccount.sol"; import { IEntryPoint } from "modular-account/src/interfaces/erc4337/IEntryPoint.sol"; -import { BasePlugin } from "modular-account/src/plugins/BasePlugin.sol"; import { IMultiOwnerPlugin } from "modular-account/src/plugins/owner/IMultiOwnerPlugin.sol"; import { ContractOwner } from "modular-account/test/mocks/ContractOwner.sol"; -import { PluginManifest } from "modular-account-libs/interfaces/IPlugin.sol"; +import { PluginManifest, PluginMetadata } from "modular-account-libs/interfaces/IPlugin.sol"; import { UserOperation } from "modular-account-libs/interfaces/UserOperation.sol"; +import { BasePlugin } from "modular-account-libs/plugins/BasePlugin.sol"; import { ECDSA } from "solady/utils/ECDSA.sol"; @@ -20,7 +20,7 @@ import { Utils, WebAuthnInfo } from "webauthn-sol/../test/Utils.sol"; import { WebAuthn } from "webauthn-sol/WebAuthn.sol"; import { DeployScript } from "../script/Deploy.s.sol"; -import { OwnersLib } from "../src/OwnersLib.sol"; +import { IndexOutOfBounds, OwnersLib } from "../src/OwnersLib.sol"; import { IWebauthnOwnerPlugin, MAX_OWNERS, PublicKey, WebauthnOwnerPlugin } from "../src/WebauthnOwnerPlugin.sol"; import { TestLib } from "./utils/TestLib.sol"; @@ -129,6 +129,13 @@ contract MultiOwnerPluginTest is Test { vm.stopPrank(); } + function test_onInstall_failWithEmptyOwners() external { + vm.expectRevert(IMultiOwnerPlugin.EmptyOwnersNotAllowed.selector); + vm.startPrank(address(contractOwner)); + plugin.onInstall(abi.encode(new PublicKey[](0))); + vm.stopPrank(); + } + function test_onInstall_failWithLimitExceeded() external { address[] memory owners = new address[](MAX_OWNERS + 1); for (uint160 i = 0; i < owners.length; ++i) { @@ -308,6 +315,37 @@ contract MultiOwnerPluginTest is Test { assertEq(_1271_MAGIC_VALUE, plugin.isValidSignature(digest, signature)); } + function test_isValidSignature_failWithOutOfBounds() external { + vm.expectRevert(IndexOutOfBounds.selector); + plugin.isValidSignature("", abi.encodePacked(uint8(4), "")); + } + + function test_isValidSignature_failMalformedAddress() external { + vm.store( + address(plugin), + bytes32(uint256(keccak256(abi.encode(accountA, uint256(0)))) + 1), + bytes32(uint256(type(uint160).max) + 1) + ); + + vm.expectRevert( + abi.encodeWithSelector( + IWebauthnOwnerPlugin.InvalidEthereumAddressOwner.selector, bytes32(uint256(type(uint160).max) + 1) + ) + ); + plugin.isValidSignature("", abi.encodePacked(uint8(0), "")); + } + + function testFuzz_runtimeValidationFunction_BadFunctionId(uint8 functionId) external { + vm.assume(functionId != uint8(IMultiOwnerPlugin.FunctionId.RUNTIME_VALIDATION_OWNER_OR_SELF)); + + vm.expectRevert( + abi.encodeWithSelector( + BasePlugin.NotImplemented.selector, BasePlugin.runtimeValidationFunction.selector, functionId + ) + ); + plugin.runtimeValidationFunction(functionId, address(0), 0, ""); + } + function test_runtimeValidationFunction_OwnerOrSelf() external { // should pass with owner as sender plugin.runtimeValidationFunction( @@ -432,6 +470,23 @@ contract MultiOwnerPluginTest is Test { assertEq(resSuccess, 0); } + function testFuzz_userOpValidationFunction_BadFunctionId(uint8 functionId) external { + vm.assume(functionId != uint8(IMultiOwnerPlugin.FunctionId.USER_OP_VALIDATION_OWNER)); + + UserOperation memory userOp; + vm.expectRevert( + abi.encodeWithSelector( + BasePlugin.NotImplemented.selector, BasePlugin.userOpValidationFunction.selector, functionId + ) + ); + plugin.userOpValidationFunction(functionId, userOp, 0); + } + + function test_ownerIndexOf_failWithNotExist() external { + vm.expectRevert(abi.encodeWithSelector(IMultiOwnerPlugin.OwnerDoesNotExist.selector, address(0xb0b))); + plugin.ownerIndexOf(accountA, address(0xb0b).toPublicKey()); + } + function test_pluginInitializeGuards() external { plugin.onUninstall(bytes("")); @@ -447,4 +502,14 @@ contract MultiOwnerPluginTest is Test { vm.expectRevert(abi.encodeWithSelector(BasePlugin.AlreadyInitialized.selector)); plugin.onInstall(abi.encode(addrArr, new address[](0))); } + + function test_pluginMetadata_success() external view { + PluginMetadata memory metadata = plugin.pluginMetadata(); + + assertEq(metadata.permissionDescriptors.length, 2); + assertEq(metadata.permissionDescriptors[0].permissionDescription, "Modify Ownership"); + assertEq(metadata.permissionDescriptors[1].permissionDescription, "Modify Ownership"); + assertEq(metadata.permissionDescriptors[0].functionSelector, IMultiOwnerPlugin.updateOwners.selector); + assertEq(metadata.permissionDescriptors[1].functionSelector, IWebauthnOwnerPlugin.updateOwnersPublicKeys.selector); + } }