Skip to content

Commit

Permalink
Eip1271 tests passing
Browse files Browse the repository at this point in the history
Fixes #29

Adds Support for [EIP-1271](https://eips.ethereum.org/EIPS/eip-1271) for allowing contract accounts to give and receive delegations.

Required one additional bit to the `SignedDelegation` and `SignedInvocation` types: `signerIsContract`. If this bit is `true`, then the first 20 bytes of the `signature` field is treated as the address of a contract to treat as the intended signer, and that contract is sent the remaining `signature: bytes` along with the delegation type hash to determine for itself whether this proof should be treated as valid authorization.

I formerly was somewhat against using EIP-1271 in this way, because I had seen some contract accounts merely allow assigning a single signer as their EIP1271 recovery strategy, which completely undermines the entire point of having a contract account. My position on this has evolved a bit, to  believe that correct usage is possible, and so we shouldn't let the possibility of flawed contract accounts prevent good ones from participating in this.

For example, good usage might look like a multisig might have a custom datastructure for representing which accounts' signatures are included, and including all the signatures combined in the `signature` payload.

Larger controller sets like a token-weighted DAO might involve too many signatures to submit as a delegation/invocation as `calldata`, but I'll leave that research as to do.
  • Loading branch information
danfinlay committed Nov 21, 2022
1 parent 8cb07fa commit 33d05b3
Show file tree
Hide file tree
Showing 6 changed files with 233 additions and 35 deletions.
5 changes: 1 addition & 4 deletions contracts/Delegatable.sol
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
// SPDX-License-Identifier: MIT
pragma solidity 0.8.15;

import "hardhat/console.sol";
// import "hardhat/console.sol";
import {EIP712DOMAIN_TYPEHASH} from "./TypesAndDecoders.sol";
import {Delegation, Invocation, Invocations, SignedInvocation, SignedDelegation} from "./CaveatEnforcer.sol";
import {DelegatableCore} from "./DelegatableCore.sol";
Expand Down Expand Up @@ -119,9 +119,6 @@ abstract contract Delegatable is IDelegatable, DelegatableCore {
if (isContractAccount) {
address intendedSender = address(bytes20(_signature[0:20]));
bytes calldata proof = _signature[20:_signature.length];
console.log("Contract account address?: %s", intendedSender);
console.log("Proof:");
console.logBytes(proof);
_callERC1271isValidSignature(intendedSender, _hash, proof);
return intendedSender;
} else {
Expand Down
48 changes: 38 additions & 10 deletions contracts/DelegatableFacet.sol
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import {EIP712DOMAIN_TYPEHASH} from "./TypesAndDecoders.sol";
import {Delegation, Invocation, Invocations, SignedInvocation, SignedDelegation} from "./CaveatEnforcer.sol";
import {DelegatableCore} from "./DelegatableCore.sol";
import {IDelegatable} from "./interfaces/IDelegatable.sol";
import {IERC1271Wallet} from "./interfaces/IERC1271Wallet.sol";

/* @notice AppStorage is used so ERC2535 Diamond facets do not clobber each others' storage.
* https://eip2535diamonds.substack.com/p/keep-your-data-right-in-eip2535-diamonds?utm_source=substack&utm_campaign=post_embed&utm_medium=web
Expand Down Expand Up @@ -101,30 +102,57 @@ contract DelegatableFacet is IDelegatable, DelegatableCore {
override(IDelegatable, DelegatableCore)
returns (address)
{
Delegation memory delegation = signedDelegation.delegation;
Delegation calldata delegation = signedDelegation.delegation;
bytes32 sigHash = getDelegationTypedDataHash(delegation);
address recoveredSignatureSigner = recover(
address recoveredSignatureSigner = flexibleRecover(
sigHash,
signedDelegation.signature
signedDelegation.signature,
signedDelegation.signerIsContract
);
return recoveredSignatureSigner;
}

function verifyInvocationSignature(SignedInvocation memory signedInvocation)
public
view
returns (address)
{
function verifyInvocationSignature(
SignedInvocation calldata signedInvocation
) public view returns (address) {
bytes32 sigHash = getInvocationsTypedDataHash(
signedInvocation.invocations
);
address recoveredSignatureSigner = recover(
address recoveredSignatureSigner = flexibleRecover(
sigHash,
signedInvocation.signature
signedInvocation.signature,
signedInvocation.signerIsContract
);
return recoveredSignatureSigner;
}

function flexibleRecover(
bytes32 _hash,
bytes calldata _signature,
bool isContractAccount
) internal view returns (address) {
if (isContractAccount) {
address intendedSender = address(bytes20(_signature[0:20]));
bytes calldata proof = _signature[20:_signature.length];
_callERC1271isValidSignature(intendedSender, _hash, proof);
return intendedSender;
} else {
return recover(_hash, _signature);
}
}

function _callERC1271isValidSignature(
address _addr,
bytes32 _hash,
bytes calldata _signature
) internal view {
bytes4 result = IERC1271Wallet(_addr).isValidSignature(
_hash,
_signature
);
require(result == 0x1626ba7e, "INVALID_SIGNATURE");
}

// --------------------------------------
// WRITES
// --------------------------------------
Expand Down
52 changes: 41 additions & 11 deletions contracts/DelegatableRelay.sol
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import {EIP712DOMAIN_TYPEHASH} from "./TypesAndDecoders.sol";
import {Delegation, Invocation, Invocations, SignedInvocation, SignedDelegation} from "./CaveatEnforcer.sol";
import {DelegatableRelayCore} from "./DelegatableRelayCore.sol";
import {IDelegatable} from "./interfaces/IDelegatable.sol";
import {IERC1271Wallet} from "./interfaces/IERC1271Wallet.sol";

contract DelegatableRelay is IDelegatable, DelegatableRelayCore {
/// @notice The hash of the domain separator used in the EIP712 domain hash.
Expand Down Expand Up @@ -75,37 +76,66 @@ contract DelegatableRelay is IDelegatable, DelegatableRelayCore {
return keccak256(encoded);
}

function verifyDelegationSignature(SignedDelegation memory signedDelegation)
function verifyDelegationSignature(
SignedDelegation calldata signedDelegation
)
public
view
virtual
override(IDelegatable, DelegatableRelayCore)
returns (address)
{
Delegation memory delegation = signedDelegation.delegation;
Delegation calldata delegation = signedDelegation.delegation;
bytes32 sigHash = getDelegationTypedDataHash(delegation);
address recoveredSignatureSigner = recover(
address recoveredSignatureSigner = flexibleRecover(
sigHash,
signedDelegation.signature
signedDelegation.signature,
signedDelegation.signerIsContract
);
return recoveredSignatureSigner;
}

function verifyInvocationSignature(SignedInvocation memory signedInvocation)
public
view
returns (address)
{
function verifyInvocationSignature(
SignedInvocation calldata signedInvocation
) public view returns (address) {
bytes32 sigHash = getInvocationsTypedDataHash(
signedInvocation.invocations
);
address recoveredSignatureSigner = recover(
address recoveredSignatureSigner = flexibleRecover(
sigHash,
signedInvocation.signature
signedInvocation.signature,
signedInvocation.signerIsContract
);
return recoveredSignatureSigner;
}

function flexibleRecover(
bytes32 _hash,
bytes calldata _signature,
bool isContractAccount
) internal view returns (address) {
if (isContractAccount) {
address intendedSender = address(bytes20(_signature[0:20]));
bytes calldata proof = _signature[20:_signature.length];
_callERC1271isValidSignature(intendedSender, _hash, proof);
return intendedSender;
} else {
return recover(_hash, _signature);
}
}

function _callERC1271isValidSignature(
address _addr,
bytes32 _hash,
bytes calldata _signature
) internal view {
bytes4 result = IERC1271Wallet(_addr).isValidSignature(
_hash,
_signature
);
require(result == 0x1626ba7e, "INVALID_SIGNATURE");
}

// --------------------------------------
// WRITES
// --------------------------------------
Expand Down
10 changes: 5 additions & 5 deletions contracts/DelegatableRelayCore.sol
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ abstract contract DelegatableRelayCore is EIP712Decoder {
return multiNonce[intendedSender][queue];
}

function verifyDelegationSignature(SignedDelegation memory signedDelegation)
function verifyDelegationSignature(SignedDelegation calldata signedDelegation)
public
view
virtual
Expand Down Expand Up @@ -52,7 +52,7 @@ abstract contract DelegatableRelayCore is EIP712Decoder {
returns (bool success)
{
for (uint256 x = 0; x < batch.length; x++) {
Invocation memory invocation = batch[x];
Invocation calldata invocation = batch[x];
address intendedSender;
address canGrant;

Expand All @@ -65,7 +65,7 @@ abstract contract DelegatableRelayCore is EIP712Decoder {
bytes32 authHash = 0x0;

for (uint256 d = 0; d < invocation.authority.length; d++) {
SignedDelegation memory signedDelegation = invocation.authority[
SignedDelegation calldata signedDelegation = invocation.authority[
d
];
address delegationSigner = verifyDelegationSignature(
Expand All @@ -83,7 +83,7 @@ abstract contract DelegatableRelayCore is EIP712Decoder {
"DelegatableCore:invalid-delegation-signer"
);

Delegation memory delegation = signedDelegation.delegation;
Delegation calldata delegation = signedDelegation.delegation;
require(
delegation.authority == authHash,
"DelegatableCore:invalid-authority-delegation-link"
Expand Down Expand Up @@ -118,7 +118,7 @@ abstract contract DelegatableRelayCore is EIP712Decoder {
}

// Here we perform the requested invocation.
Transaction memory transaction = invocation.transaction;
Transaction calldata transaction = invocation.transaction;

// TODO(@kames): Can we bubble up the error message from the enforcer? Why not? Optimizations?
success = _execute(
Expand Down
5 changes: 0 additions & 5 deletions contracts/test/EIP1271.sol
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
pragma solidity 0.8.15;
import "hardhat/console.sol";
import "../libraries/ECRecovery.sol";

//SPDX-License-Identifier: MIT
Expand All @@ -23,14 +22,10 @@ contract EIP1271 is ECRecovery {
view
returns (bytes4)
{
console.log("Recovered signer to be %s", recover(_hash, _signature));
if (isOwner[recover(_hash, _signature)]) {
console.log("an owner.");
return 0x1626ba7e;
} else {
console.log("Not an owner.");
return 0xffffffff;
}
}

}
148 changes: 148 additions & 0 deletions test/Delegatable1271.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,148 @@
import { ethers } from "hardhat";
import { Contract, ContractFactory, utils, Wallet } from "ethers";
import { SignerWithAddress } from "@nomiclabs/hardhat-ethers/signers";
// @ts-ignore
import { generateUtil } from "eth-delegatable-utils";
import { getPrivateKeys } from "../utils/getPrivateKeys";
import { expect } from "chai";
import { Provider } from "@ethersproject/providers";
import { generateDelegation } from "./utils";
const { getSigners } = ethers;

describe("Delegatable", () => {
const CONTACT_NAME = "Delegatable";
let CONTRACT_INFO: any;
let delegatableUtils: any;
let signer0: SignerWithAddress;
let wallet0: Wallet;
let wallet1: Wallet;
let pk0: string;
let pk1: string;

let AllowedMethodsEnforcer: Contract;
let AllowedMethodsEnforcerFactory: ContractFactory;
let Delegatable: Contract;
let DelegatableFactory: ContractFactory;
let ContractAccount: Contract;
let ContractAccountFactory: ContractFactory;

before(async () => {
[signer0] = await getSigners();
[wallet0, wallet1] = getPrivateKeys(
signer0.provider as unknown as Provider
);
DelegatableFactory = await ethers.getContractFactory("MockDelegatable");
ContractAccountFactory = await ethers.getContractFactory("EIP1271");
AllowedMethodsEnforcerFactory = await ethers.getContractFactory(
"AllowedMethodsEnforcer"
);
pk0 = wallet0._signingKey().privateKey;
pk1 = wallet1._signingKey().privateKey;
});

beforeEach(async () => {
Delegatable = await DelegatableFactory.connect(wallet0).deploy(
CONTACT_NAME
);
AllowedMethodsEnforcer = await AllowedMethodsEnforcerFactory.connect(
wallet0
).deploy();
ContractAccount = await ContractAccountFactory.connect(wallet1).deploy();

CONTRACT_INFO = {
chainId: Delegatable.deployTransaction.chainId,
verifyingContract: Delegatable.address,
name: CONTACT_NAME,
};
delegatableUtils = generateUtil(CONTRACT_INFO);
});

describe("contract accounts", () => {
it("should be able to delegate its own powers", async () => {
await Delegatable.transferOwnership(ContractAccount.address);
expect(await Delegatable.owner()).to.equal(ContractAccount.address);
expect(await Delegatable.owner()).to.equal(ContractAccount.address);

const _delegation: SignedDelegation = generateDelegation(
CONTACT_NAME,
Delegatable,
pk1,
wallet0.address
);

_delegation.signerIsContract = true;
_delegation.signature = `${
ContractAccount.address
}${_delegation.signature.substring(2)}`;

const INVOCATION_MESSAGE = {
replayProtection: {
nonce: "0x01",
queue: "0x00",
},
batch: [
{
authority: [_delegation],
transaction: {
to: Delegatable.address,
gasLimit: "21000000000000",
data: (
await Delegatable.populateTransaction.setPurpose("To delegate!")
).data,
},
},
],
};
const invocation = delegatableUtils.signInvocations(
INVOCATION_MESSAGE,
pk0
);
await Delegatable.invoke([
{
signature: invocation.signature,
invocations: invocation.invocations,
},
]);
expect(await Delegatable.purpose()).to.eq("To delegate!");
});

it("should be able to use powers they have been delegated by an EOA", async () => {
const _delegation: SignedDelegation = generateDelegation(
CONTACT_NAME,
Delegatable,
pk0,
ContractAccount.address
);

const INVOCATION_MESSAGE = {
replayProtection: {
nonce: "0x01",
queue: "0x00",
},
batch: [
{
authority: [_delegation],
transaction: {
to: Delegatable.address,
gasLimit: "21000000000000",
data: (
await Delegatable.populateTransaction.setPurpose("To delegate!")
).data,
},
},
],
};
const invocation = delegatableUtils.signInvocations(
INVOCATION_MESSAGE,
pk1
);

invocation.signerIsContract = true;
invocation.signature = `${
ContractAccount.address
}${invocation.signature.substring(2)}`;
await Delegatable.invoke([invocation]);
expect(await Delegatable.purpose()).to.eq("To delegate!");
});
});
});

0 comments on commit 33d05b3

Please sign in to comment.