Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

SMA-355 [WIP] Gas Optimisations #179

Merged
merged 16 commits into from
Dec 28, 2023
71 changes: 36 additions & 35 deletions .gas-snapshot
Original file line number Diff line number Diff line change
@@ -1,36 +1,37 @@
ERC7484SecurityPolicyPluginTest:testShouldAllowModuleInstallationIfEnoughAttestationsExists() (gas: 784704)
ERC7484SecurityPolicyPluginTest:testShouldNotAllowModuleInstallationIfAttestationsAreRevoked() (gas: 822102)
ERC7484SecurityPolicyPluginTest:testShouldNotAllowModuleInstallationIfInsufficientAttestationsExists() (gas: 622252)
ERC7484SecurityPolicyPluginTest:testShouldNotAllowModuleInstallationIfNotConfigured() (gas: 777891)
ERC7484SecurityPolicyPluginTest:testShouldAllowModuleInstallationIfEnoughAttestationsExists() (gas: 784547)
ERC7484SecurityPolicyPluginTest:testShouldNotAllowModuleInstallationIfAttestationsAreRevoked() (gas: 821945)
ERC7484SecurityPolicyPluginTest:testShouldNotAllowModuleInstallationIfInsufficientAttestationsExists() (gas: 622095)
ERC7484SecurityPolicyPluginTest:testShouldNotAllowModuleInstallationIfNotConfigured() (gas: 777734)
ERC7484SecurityPolicyPluginTest:testShouldSetConfiguration() (gas: 233126)
SABasicsTest:testDeploySAWithDefaultModule() (gas: 306337)
SecurityPolicyManagerPluginModuleInstallationTest:testModuleInstallation() (gas: 277278)
SecurityPolicyManagerPluginModuleInstallationTest:testModuleInstallationWithSetup() (gas: 304839)
SecurityPolicyManagerPluginModuleInstallationTest:testShouldRevertModuleInstallationIfExecTxFromModuleFailsWithSetup() (gas: 117782)
SecurityPolicyManagerPluginModuleInstallationTest:testShouldRevertModuleInstallationIfExecTxFromModuleInstallationFails() (gas: 241349)
SecurityPolicyManagerPluginModuleInstallationTest:testShouldRevertModuleInstallationIfModuleInstallationFails() (gas: 252651)
SecurityPolicyManagerPluginModuleInstallationTest:testShouldRevertModuleInstallationIfModuleInstallationFailsWithSetup() (gas: 129718)
SecurityPolicyManagerPluginModuleInstallationTest:testShouldRevertModuleInstallationIfModuleIsNotAContract() (gas: 116980)
SecurityPolicyManagerPluginModuleInstallationTest:testShouldRevertModuleInstallationIfSecurityPolicyIsNotSatisifedOnInstalledPlugin() (gas: 145722)
SecurityPolicyManagerPluginModuleInstallationTest:testShouldRevertModuleInstallationWithSetupIfModuleIsNotAContract() (gas: 130819)
SecurityPolicyManagerPluginModuleInstallationTest:testShouldRevertModuleInstallationWithSetupIfSecurityPolicyIsNotSatisifedOnInstalledPlugin() (gas: 223574)
SecurityPolicyManagerPluginPluginManagementTest:testAddAndRemoveAllPolicies() (gas: 417180)
SecurityPolicyManagerPluginPluginManagementTest:testDisableSingleSecurityPolicyPlugin() (gas: 280216)
SecurityPolicyManagerPluginPluginManagementTest:testDisableSingleSecurityPolicyPluginsRange() (gas: 293036)
SecurityPolicyManagerPluginPluginManagementTest:testEnableMultipleSecurityPolicyPlugins() (gas: 310496)
SecurityPolicyManagerPluginPluginManagementTest:testEnableSingleSecurityPolicyPlugin() (gas: 165563)
SecurityPolicyManagerPluginPluginManagementTest:testSecurityPoliciesQueryPaginated() (gas: 290097)
SecurityPolicyManagerPluginPluginManagementTest:testShouldNotAllowDisablingAlreadyDisabledPolicySingleDisable() (gas: 285991)
SecurityPolicyManagerPluginPluginManagementTest:testShouldNotAllowDisablingAlreadyEnabledPolicySingleDisable() (gas: 131053)
SecurityPolicyManagerPluginPluginManagementTest:testShouldNotAllowDisablingRangeWithInvalidPointer() (gas: 310156)
SecurityPolicyManagerPluginPluginManagementTest:testShouldNotAllowDisablingRangeWithInvalidRange() (gas: 319688)
SecurityPolicyManagerPluginPluginManagementTest:testShouldNotAllowDisablingSentinelAddressPolicySingleDisable() (gas: 309014)
SecurityPolicyManagerPluginPluginManagementTest:testShouldNotAllowDisablingWithInvalidPointerSingleDisable() (gas: 310291)
SecurityPolicyManagerPluginPluginManagementTest:testShouldNotAllowDisablingZeroAddressPolicySingleDisable() (gas: 309101)
SecurityPolicyManagerPluginPluginManagementTest:testShouldNotAllowEmptyEnableList() (gas: 131359)
SecurityPolicyManagerPluginPluginManagementTest:testShouldNotAllowEnablingAlreadyEnabledPolicySMultiEnable() (gas: 220629)
SecurityPolicyManagerPluginPluginManagementTest:testShouldNotAllowEnablingAlreadyEnabledPolicySingleEnable() (gas: 217968)
SecurityPolicyManagerPluginPluginManagementTest:testShouldNotAllowPolicyAdditionWithSentinelAddressMulti() (gas: 135358)
SecurityPolicyManagerPluginPluginManagementTest:testShouldNotAllowPolicyAdditionWithSentinelAddressSingle() (gas: 131458)
SecurityPolicyManagerPluginPluginManagementTest:testShouldNotAllowPolicyAdditionWithZeroAddressMulti() (gas: 135313)
SecurityPolicyManagerPluginPluginManagementTest:testShouldNotAllowPolicyAdditionWithZeroAddressSingle() (gas: 131545)
SABasicsTest:testDeploySAWithDefaultModule() (gas: 307004)
SABasicsTest:testExecuteBatch() (gas: 291378)
SecurityPolicyManagerPluginModuleInstallationTest:testModuleInstallation() (gas: 277072)
SecurityPolicyManagerPluginModuleInstallationTest:testModuleInstallationWithSetup() (gas: 304682)
SecurityPolicyManagerPluginModuleInstallationTest:testShouldRevertModuleInstallationIfExecTxFromModuleFailsWithSetup() (gas: 117576)
SecurityPolicyManagerPluginModuleInstallationTest:testShouldRevertModuleInstallationIfExecTxFromModuleInstallationFails() (gas: 241143)
SecurityPolicyManagerPluginModuleInstallationTest:testShouldRevertModuleInstallationIfModuleInstallationFails() (gas: 252445)
SecurityPolicyManagerPluginModuleInstallationTest:testShouldRevertModuleInstallationIfModuleInstallationFailsWithSetup() (gas: 129503)
SecurityPolicyManagerPluginModuleInstallationTest:testShouldRevertModuleInstallationIfModuleIsNotAContract() (gas: 116774)
SecurityPolicyManagerPluginModuleInstallationTest:testShouldRevertModuleInstallationIfSecurityPolicyIsNotSatisifedOnInstalledPlugin() (gas: 145516)
SecurityPolicyManagerPluginModuleInstallationTest:testShouldRevertModuleInstallationWithSetupIfModuleIsNotAContract() (gas: 130605)
SecurityPolicyManagerPluginModuleInstallationTest:testShouldRevertModuleInstallationWithSetupIfSecurityPolicyIsNotSatisifedOnInstalledPlugin() (gas: 223417)
SecurityPolicyManagerPluginPluginManagementTest:testAddAndRemoveAllPolicies() (gas: 416523)
SecurityPolicyManagerPluginPluginManagementTest:testDisableSingleSecurityPolicyPlugin() (gas: 279805)
SecurityPolicyManagerPluginPluginManagementTest:testDisableSingleSecurityPolicyPluginsRange() (gas: 292542)
SecurityPolicyManagerPluginPluginManagementTest:testEnableMultipleSecurityPolicyPlugins() (gas: 310086)
SecurityPolicyManagerPluginPluginManagementTest:testEnableSingleSecurityPolicyPlugin() (gas: 165357)
SecurityPolicyManagerPluginPluginManagementTest:testSecurityPoliciesQueryPaginated() (gas: 289892)
SecurityPolicyManagerPluginPluginManagementTest:testShouldNotAllowDisablingAlreadyDisabledPolicySingleDisable() (gas: 285580)
SecurityPolicyManagerPluginPluginManagementTest:testShouldNotAllowDisablingAlreadyEnabledPolicySingleDisable() (gas: 130847)
SecurityPolicyManagerPluginPluginManagementTest:testShouldNotAllowDisablingRangeWithInvalidPointer() (gas: 309745)
SecurityPolicyManagerPluginPluginManagementTest:testShouldNotAllowDisablingRangeWithInvalidRange() (gas: 319277)
SecurityPolicyManagerPluginPluginManagementTest:testShouldNotAllowDisablingSentinelAddressPolicySingleDisable() (gas: 308603)
SecurityPolicyManagerPluginPluginManagementTest:testShouldNotAllowDisablingWithInvalidPointerSingleDisable() (gas: 309880)
SecurityPolicyManagerPluginPluginManagementTest:testShouldNotAllowDisablingZeroAddressPolicySingleDisable() (gas: 308690)
SecurityPolicyManagerPluginPluginManagementTest:testShouldNotAllowEmptyEnableList() (gas: 131153)
SecurityPolicyManagerPluginPluginManagementTest:testShouldNotAllowEnablingAlreadyEnabledPolicySMultiEnable() (gas: 220217)
SecurityPolicyManagerPluginPluginManagementTest:testShouldNotAllowEnablingAlreadyEnabledPolicySingleEnable() (gas: 217556)
SecurityPolicyManagerPluginPluginManagementTest:testShouldNotAllowPolicyAdditionWithSentinelAddressMulti() (gas: 135152)
SecurityPolicyManagerPluginPluginManagementTest:testShouldNotAllowPolicyAdditionWithSentinelAddressSingle() (gas: 131252)
SecurityPolicyManagerPluginPluginManagementTest:testShouldNotAllowPolicyAdditionWithZeroAddressMulti() (gas: 135107)
SecurityPolicyManagerPluginPluginManagementTest:testShouldNotAllowPolicyAdditionWithZeroAddressSingle() (gas: 131339)
23 changes: 16 additions & 7 deletions contracts/smart-account/SmartAccount.sol
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ import {IFallbackManager} from "./interfaces/base/IFallbackManager.sol";
* - It allows to receive and manage assets.
* - It is responsible for managing the modules and fallbacks.
* - The Smart Account can be extended with modules, such as Social Recovery, Session Key and others.
* @author Chirag Titiya - <[email protected]>, Filipp Makarov - <[email protected]>
* @author Chirag Titiya - <[email protected]>, Filipp Makarov - <[email protected]>, Ankur Dubey - <[email protected]>
*/
contract SmartAccount is
BaseSmartAccount,
Expand Down Expand Up @@ -127,10 +127,19 @@ contract SmartAccount is
revert CallerIsNotAnEntryPoint(msg.sender);
}

(, address validationModule) = abi.decode(
userOp.signature,
(bytes, address)
);
address validationModule;
filmakarov marked this conversation as resolved.
Show resolved Hide resolved
{
/*
* userOp.signature = abi.encode(moduleSignature, validationModule)
* Extract validationModule from userOp.signature without copying
* moduleSignature to memory
*/
bytes memory signature = userOp.signature;
assembly ("memory-safe") {
validationModule := mload(add(signature, 0x40))
}
}

if (address(_modules[validationModule]) != address(0)) {
validationData = IAuthorizationModule(validationModule)
.validateUserOp(userOp, userOpHash);
Expand Down Expand Up @@ -289,7 +298,7 @@ contract SmartAccount is
/// @inheritdoc ISignatureValidator
function isValidSignature(
bytes32 dataHash,
bytes memory signature
bytes calldata signature
) public view override returns (bytes4) {
(bytes memory moduleSignature, address validationModule) = abi.decode(
signature,
Expand All @@ -309,7 +318,7 @@ contract SmartAccount is
/// @inheritdoc ISignatureValidator
function isValidSignatureUnsafe(
bytes32 dataHash,
bytes memory signature
bytes calldata signature
) public view returns (bytes4) {
(bytes memory moduleSignature, address validationModule) = abi.decode(
signature,
Expand Down
4 changes: 2 additions & 2 deletions contracts/smart-account/interfaces/ISignatureValidator.sol
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ interface ISignatureValidator {
*/
function isValidSignature(
bytes32 _dataHash,
bytes memory _signature
bytes calldata _signature
) external view returns (bytes4);

/**
Expand All @@ -28,6 +28,6 @@ interface ISignatureValidator {
*/
function isValidSignatureUnsafe(
bytes32 dataHash,
bytes memory moduleSignature
bytes calldata moduleSignature
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

any specific rationale for this?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

calldata can be sliced => cheaper

) external view returns (bytes4);
}
12 changes: 12 additions & 0 deletions test/foundry/base/SATestBase.sol
Original file line number Diff line number Diff line change
Expand Up @@ -173,6 +173,18 @@ abstract contract SATestBase is Test {
return abi.encodeCall(SmartAccount.execute, (_dest, _value, _calldata));
}

function getSmartAccountBatchExecuteCalldata(
address[] memory _dest,
uint256[] memory _values,
bytes[] memory _calldatas
) internal pure returns (bytes memory) {
return
abi.encodeCall(
SmartAccount.executeBatch_y6U,
(_dest, _values, _calldatas)
);
}

function getUserOperationEventData(
Vm.Log[] memory _entries
) internal returns (UserOperationEventData memory data) {
Expand Down
48 changes: 48 additions & 0 deletions test/foundry/smart-account/SA.Basics.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,20 @@ pragma solidity ^0.8.20;
import {SATestBase} from "../base/SATestBase.sol";
import {SmartAccount} from "sa/SmartAccount.sol";
import {EcdsaOwnershipRegistryModule} from "modules/EcdsaOwnershipRegistryModule.sol";
import {UserOperation} from "aa-core/EntryPoint.sol";

contract Test {
event Log(string message);

function emitString(string calldata str) external {
emit Log(str);
}
}

contract SABasicsTest is SATestBase {
event Log(string message);
Test test = new Test();

function setUp() public virtual override {
super.setUp();
}
Expand Down Expand Up @@ -49,4 +61,40 @@ contract SABasicsTest is SATestBase {
"smart account should have 1 token"
);
}

function testExecuteBatch() external {
uint256 smartAccountDeploymentIndex = 0;
bytes memory moduleSetupData = getEcdsaOwnershipRegistryModuleSetupData(
alice.addr
);
SmartAccount sa = getSmartAccountWithModule(
address(ecdsaOwnershipRegistryModule),
moduleSetupData,
smartAccountDeploymentIndex,
"aliceSA"
);

address[] memory dest = new address[](2);
uint256[] memory values = new uint256[](2);
bytes[] memory calldatas = new bytes[](2);

vm.expectEmit(true, true, true, true);
emit Log("hello");
vm.expectEmit(true, true, true, true);
emit Log("world");

dest[0] = address(test);
dest[1] = address(test);
calldatas[0] = abi.encodeCall(test.emitString, ("hello"));
calldatas[1] = abi.encodeCall(test.emitString, ("world"));

UserOperation memory op = makeEcdsaModuleUserOp(
getSmartAccountBatchExecuteCalldata(dest, values, calldatas),
sa,
0,
alice
);
vm.breakpoint("a");
entryPoint.handleOps(arraifyOps(op), owner.addr);
}
}
Loading