Skip to content

Commit

Permalink
update ecdsa validation and add more test
Browse files Browse the repository at this point in the history
  • Loading branch information
fangting-alchemy committed Jul 12, 2024
1 parent 058629c commit e0b605f
Show file tree
Hide file tree
Showing 5 changed files with 65 additions and 40 deletions.
13 changes: 10 additions & 3 deletions src/plugins/validation/EcdsaValidation.sol
Original file line number Diff line number Diff line change
Expand Up @@ -10,11 +10,18 @@ import {IValidation} from "../../interfaces/IValidation.sol";
import {BasePlugin} from "../BasePlugin.sol";
import {IEcdsaValidation} from "./IEcdsaValidation.sol";

/// @title ECSDA Validation
/// @author ERC-6900 Authors
/// @notice This validation enables any ECDSA (secp256k1 curve) signature validation. It handles installation by
/// each entity (validationId). Uninstallation will NOT disable all installed validation entities. None of the
/// functions are installed on the account. Account states are to be retrieved from this global singleton directly.
/// Note: This validation also support composition that other validation can relay on entities in this validation
/// to validate partially or fully.
contract EcdsaValidation is IEcdsaValidation, BasePlugin {
using ECDSA for bytes32;
using MessageHashUtils for bytes32;

string public constant NAME = "Ecdsa Validation Plugin";
string public constant NAME = "Ecdsa Validation";
string public constant VERSION = "1.0.0";
string public constant AUTHOR = "ERC-6900 Authors";

Expand Down Expand Up @@ -74,7 +81,7 @@ contract EcdsaValidation is IEcdsaValidation, BasePlugin {
{
// Validate the user op signature against the owner.
(address sigSigner,,) = (userOpHash.toEthSignedMessageHash()).tryRecover(userOp.signature);
if (sigSigner == address(0) || sigSigner != signer[validationId][msg.sender]) {
if (sigSigner == address(0) || sigSigner != signer[validationId][userOp.sender]) {
return _SIG_VALIDATION_FAILED;
}
return _SIG_VALIDATION_PASSED;
Expand All @@ -87,7 +94,7 @@ contract EcdsaValidation is IEcdsaValidation, BasePlugin {
override
{
// Validate that the sender is the owner of the account or self.
if (sender != signer[validationId][msg.sender] && sender != msg.sender) {
if (sender != signer[validationId][msg.sender]) {
revert NotAuthorized();
}
return;
Expand Down
3 changes: 1 addition & 2 deletions test/mocks/EcdsaFactoryFixture.sol
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ import {IEntryPoint} from "@eth-infinitism/account-abstraction/interfaces/IEntry

import {ValidationConfigLib} from "../../src/helpers/ValidationConfigLib.sol";
import {UpgradeableModularAccount} from "../../src/account/UpgradeableModularAccount.sol";
import {PackedPluginEntityLib} from "../../src/helpers/PackedPluginEntityLib.sol";
import {EcdsaValidation} from "../../src/plugins/validation/EcdsaValidation.sol";

import {OptimizedTest} from "../utils/OptimizedTest.sol";
Expand Down Expand Up @@ -52,7 +51,7 @@ contract EcdsaFactoryFixture is OptimizedTest {

// point proxy to actual implementation and init plugins
UpgradeableModularAccount(payable(addr)).initializeWithValidation(
ValidationConfigLib.pack(address(ecdsaValidation), TEST_DEFAULT_VALIDATION_ID, true, true),
ValidationConfigLib.pack(address(ecdsaValidation), TEST_DEFAULT_VALIDATION_ID, true, false),
new bytes4[](0),
pluginInstallData,
"",
Expand Down
7 changes: 7 additions & 0 deletions test/utils/AccountTestBase.sol
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import {EntryPoint} from "@eth-infinitism/account-abstraction/core/EntryPoint.so
import {PackedUserOperation} from "@eth-infinitism/account-abstraction/interfaces/PackedUserOperation.sol";
import {MessageHashUtils} from "@openzeppelin/contracts/utils/cryptography/MessageHashUtils.sol";

import {EcdsaValidation} from "../../src/plugins/validation/EcdsaValidation.sol";
import {PluginEntity, PluginEntityLib} from "../../src/helpers/PluginEntityLib.sol";
import {IStandardExecutor, Call} from "../../src/interfaces/IStandardExecutor.sol";
import {UpgradeableModularAccount} from "../../src/account/UpgradeableModularAccount.sol";
Expand All @@ -13,6 +14,7 @@ import {SingleOwnerPlugin} from "../../src/plugins/owner/SingleOwnerPlugin.sol";
import {OptimizedTest} from "./OptimizedTest.sol";
import {TEST_DEFAULT_OWNER_FUNCTION_ID as EXT_CONST_TEST_DEFAULT_OWNER_FUNCTION_ID} from "./TestConstants.sol";

import {EcdsaFactoryFixture} from "../mocks/EcdsaFactoryFixture.sol";
import {MSCAFactoryFixture} from "../mocks/MSCAFactoryFixture.sol";

/// @dev This contract handles common boilerplate setup for tests using UpgradeableModularAccount with
Expand All @@ -26,6 +28,9 @@ abstract contract AccountTestBase is OptimizedTest {
SingleOwnerPlugin public singleOwnerPlugin;
MSCAFactoryFixture public factory;

EcdsaValidation public ecdsaValidation;
EcdsaFactoryFixture public ecdsaFactory;

address public owner1;
uint256 public owner1Key;
UpgradeableModularAccount public account1;
Expand Down Expand Up @@ -53,6 +58,8 @@ abstract contract AccountTestBase is OptimizedTest {

singleOwnerPlugin = _deploySingleOwnerPlugin();
factory = new MSCAFactoryFixture(entryPoint, singleOwnerPlugin);
ecdsaValidation = _deployEcdsaValidation();
ecdsaFactory = new EcdsaFactoryFixture(entryPoint, ecdsaValidation);

account1 = factory.createAccount(owner1, 0);
vm.deal(address(account1), 100 ether);
Expand Down
2 changes: 1 addition & 1 deletion test/utils/TestConstants.sol
Original file line number Diff line number Diff line change
Expand Up @@ -2,4 +2,4 @@
pragma solidity ^0.8.25;

uint32 constant TEST_DEFAULT_OWNER_FUNCTION_ID = 0;
uint32 constant TEST_DEFAULT_VALIDATION_ID = 0;
uint32 constant TEST_DEFAULT_VALIDATION_ID = 1;
80 changes: 46 additions & 34 deletions test/validation/EcdsaValidation.t.sol
Original file line number Diff line number Diff line change
@@ -1,53 +1,34 @@
// SPDX-License-Identifier: UNLICENSED
pragma solidity ^0.8.19;

import {console} from "forge-std/Test.sol";
import {EntryPoint} from "@eth-infinitism/account-abstraction/core/EntryPoint.sol";
import {PackedUserOperation} from "@eth-infinitism/account-abstraction/interfaces/PackedUserOperation.sol";
import {MessageHashUtils} from "@openzeppelin/contracts/utils/cryptography/MessageHashUtils.sol";

import {UpgradeableModularAccount} from "../../src/account/UpgradeableModularAccount.sol";
import {EcdsaValidation} from "../../src/plugins/validation/EcdsaValidation.sol";
import {PluginEntityLib} from "../../src/helpers/PluginEntityLib.sol";
import {ValidationConfigLib} from "../../src/helpers/ValidationConfigLib.sol";

import {OptimizedTest} from "../utils/OptimizedTest.sol";
import {EcdsaFactoryFixture} from "../mocks/EcdsaFactoryFixture.sol";
import {AccountTestBase} from "../utils/AccountTestBase.sol";
import {TEST_DEFAULT_VALIDATION_ID} from "../utils/TestConstants.sol";

contract EcdsaValidationTest is OptimizedTest {
contract EcdsaValidationTest is AccountTestBase {
using MessageHashUtils for bytes32;

EntryPoint public entryPoint;
EcdsaValidation public ecdsaValidation;
address payable public beneficiary;
address public ethRecipient;

address public owner1;
uint256 public owner1Key;
EcdsaFactoryFixture public factory;
UpgradeableModularAccount public account1;
bytes32 public validationId1;

uint256 public constant CALL_GAS_LIMIT = 50000;
uint256 public constant VERIFICATION_GAS_LIMIT = 1200000;
uint8 public constant SELECTOR_ASSOCIATED_VALIDATION = 0;
uint8 public constant DEFAULT_VALIDATION = 1;
address public owner2;
uint256 public owner2Key;
UpgradeableModularAccount public account;

function setUp() public {
entryPoint = new EntryPoint();
(owner1, owner1Key) = makeAddrAndKey("owner1");
beneficiary = payable(makeAddr("beneficiary"));
ethRecipient = makeAddr("ethRecipient");

ecdsaValidation = _deployEcdsaValidation();
factory = new EcdsaFactoryFixture(entryPoint, ecdsaValidation);

account1 = factory.createAccount(owner1, 0);
vm.deal(address(account1), 100 ether);
(owner2, owner2Key) = makeAddrAndKey("owner2");
account = ecdsaFactory.createAccount(owner1, 0);
vm.deal(address(account), 100 ether);
}

function test_userOpValidation() public {
PackedUserOperation memory userOp = PackedUserOperation({
sender: address(account1),
sender: address(account),
nonce: 0,
initCode: "",
callData: abi.encodeCall(UpgradeableModularAccount.execute, (ethRecipient, 1 wei, "")),
Expand All @@ -61,7 +42,11 @@ contract EcdsaValidationTest is OptimizedTest {
// Generate signature
bytes32 userOpHash = entryPoint.getUserOpHash(userOp);
(uint8 v, bytes32 r, bytes32 s) = vm.sign(owner1Key, userOpHash.toEthSignedMessageHash());
userOp.signature = abi.encodePacked(TEST_DEFAULT_VALIDATION_ID, DEFAULT_VALIDATION, r, s, v);
userOp.signature = _encodeSignature(
PluginEntityLib.pack(address(ecdsaValidation), TEST_DEFAULT_VALIDATION_ID),
GLOBAL_VALIDATION,
abi.encodePacked(r, s, v)
);

PackedUserOperation[] memory userOps = new PackedUserOperation[](1);
userOps[0] = userOp;
Expand All @@ -71,8 +56,35 @@ contract EcdsaValidationTest is OptimizedTest {
assertEq(ethRecipient.balance, 1 wei);
}

// helper function to compress 2 gas values into a single bytes32
function _encodeGas(uint256 g1, uint256 g2) internal pure returns (bytes32) {
return bytes32(uint256((g1 << 128) + uint128(g2)));
function test_runtime() public {
vm.prank(owner1);
account.executeWithAuthorization(
abi.encodeCall(UpgradeableModularAccount.execute, (ethRecipient, 1 wei, "")),
_encodeSignature(
PluginEntityLib.pack(address(ecdsaValidation), TEST_DEFAULT_VALIDATION_ID), GLOBAL_VALIDATION, ""
)
);
assertEq(ethRecipient.balance, 1 wei);
}

function test_runtime_with2ndValidation() public {
uint32 newValidationId = TEST_DEFAULT_OWNER_FUNCTION_ID + 1;
vm.prank(address(entryPoint));
account.installValidation(
ValidationConfigLib.pack(address(ecdsaValidation), newValidationId, true, false),
new bytes4[](0),
abi.encode(newValidationId, owner2),
"",
""
);

vm.prank(owner2);
account.executeWithAuthorization(
abi.encodeCall(UpgradeableModularAccount.execute, (ethRecipient, 1 wei, "")),
_encodeSignature(
PluginEntityLib.pack(address(ecdsaValidation), newValidationId), GLOBAL_VALIDATION, ""
)
);
assertEq(ethRecipient.balance, 1 wei);
}
}

0 comments on commit e0b605f

Please sign in to comment.