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

AA-525 add Simple7702Account #536

Open
wants to merge 1 commit into
base: AA-521-ep-7702
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
84 changes: 84 additions & 0 deletions contracts/samples/Simple7702Account.sol
Original file line number Diff line number Diff line change
@@ -0,0 +1,84 @@
// SPDX-License-Identifier: MIT
// based on: https://gist.github.com/frangio/e40305b9f99de290b73750dff5ebe50a
pragma solidity ^0.8;

import "../interfaces/PackedUserOperation.sol";
import "../core/Helpers.sol";
import "@openzeppelin/contracts/utils/introspection/IERC165.sol";
import "@openzeppelin/contracts/interfaces/IERC1271.sol";
import "@openzeppelin/contracts/token/ERC1155/utils/ERC1155Holder.sol";
import "@openzeppelin/contracts/token/ERC721/utils/ERC721Holder.sol";
import "@openzeppelin/contracts/utils/cryptography/ECDSA.sol";
import "../core/BaseAccount.sol";

/**
* Simple7702Account.sol
* A minimal account to be used with EIP-7702 (for batching) and ERC-4337 (for gas sponsoring)
*/
contract Simple7702Account is BaseAccount, IERC165, IERC1271, ERC1155Holder, ERC721Holder {

// temporary address of entryPoint v0.8
function entryPoint() public pure override returns (IEntryPoint) {
return IEntryPoint(0x6F4F5099a64044D69EB7419d66760fD4106fcE3C);
}
Comment on lines +21 to +23
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why function and not an an immutable parameter?


/**
* Make this account callable through ERC-4337 EntryPoint.
* The UserOperation should be signed by this account's private key.
*/
function _validateSignature(
PackedUserOperation calldata userOp,
bytes32 userOpHash
) internal virtual override returns (uint256 validationData) {

if (address(this) != ECDSA.recover(userOpHash, userOp.signature)) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

There is a method isValidSignature that does almost the same thing, so the code should kind of be de-duplicated a little bit here.

return SIG_VALIDATION_FAILED;
}
return 0;
}

function _requireFromSelfOrEntryPoint() internal view virtual {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why function and not a modifier?

require(
msg.sender == address(this) ||
msg.sender == address(entryPoint()),
"not from self or EntryPoint"
);
}

struct Call {
address target;
uint256 value;
bytes data;
}

function execute(Call[] calldata calls) external {
_requireFromSelfOrEntryPoint();

for (uint256 i = 0; i < calls.length; i++) {
Call calldata call = calls[i];
(bool ok, bytes memory ret) = call.target.call{value: call.value}(call.data);
if (!ok) {
// solhint-disable-next-line no-inline-assembly
assembly { revert(add(ret, 32), mload(ret)) }
Copy link
Collaborator

Choose a reason for hiding this comment

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

On one hand, it is nice and simple how we are not wrapping the reverted data into our own exception. On the other hand, we are losing some information here - specifically, which Call is the one that reverted. I think this information may be useful enough for the user and developers, so we should reconsider and revert with a custom error here.
What do you think?

}
}
}

function supportsInterface(bytes4 id) public override(ERC1155Holder, IERC165) pure returns (bool) {
return
id == type(IERC165).interfaceId ||
id == type(IAccount).interfaceId ||
id == type(IERC1271).interfaceId ||
id == type(IERC1155Receiver).interfaceId ||
id == type(IERC721Receiver).interfaceId;
}

//deliberately return the same signature as returned by the EOA itself: This way,
// ERC-1271 can be used regardless if the account currently has this code or not.
Comment on lines +76 to +77
Copy link
Collaborator

Choose a reason for hiding this comment

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

This comment is not very clear and not properly formatted.

function isValidSignature(bytes32 hash, bytes memory signature) public view returns (bytes4 magicValue) {
return ECDSA.recover(hash, signature) == address(this) ? this.isValidSignature.selector : bytes4(0);
}

receive() external payable {
Copy link
Collaborator

Choose a reason for hiding this comment

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

If I recall correctly, receive() will revert if there is any msg.data included in an incoming transaction, and you must add fallback() payable as well to catch all incoming value.

}
}
151 changes: 151 additions & 0 deletions test/eip7702-wallet.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,151 @@
import { expect } from 'chai'

import { Simple7702Account, Simple7702Account__factory, EntryPoint, TestPaymasterAcceptAll__factory } from '../typechain'
import { createAccountOwner, createAddress, deployEntryPoint } from './testutils'
import { fillAndSign, packUserOp } from './UserOp'
import { hexConcat, parseEther } from 'ethers/lib/utils'
import { signEip7702Authorization } from './eip7702helpers'
import { GethExecutable } from './GethExecutable'
import { Wallet } from 'ethers'

describe('Simple7702Account.sol', function () {
// can't deploy coverage "entrypoint" on geth (contract too large)
if (process.env.COVERAGE != null) {
return
}

let entryPoint: EntryPoint

let eip7702delegate: Simple7702Account
let geth: GethExecutable

before(async function () {
geth = new GethExecutable()
await geth.init()

entryPoint = await deployEntryPoint(geth.provider)

eip7702delegate = await new Simple7702Account__factory(geth.provider.getSigner()).deploy()
expect(await eip7702delegate.entryPoint()).to.equal(entryPoint.address, 'fix entryPoint in Simple7702Account.sol')
console.log('set eip7702delegate=', eip7702delegate.address)
})

after(() => {
geth.done()
})

describe('sanity: normal 7702 batching', () => {
let eoa: Wallet
before(async () => {
eoa = createAccountOwner(geth.provider)

const auth = await signEip7702Authorization(eoa, {
chainId: 0,
nonce: 0,
address: eip7702delegate.address
})
const sendVal = parseEther('10')
const tx = {
to: eoa.address,
value: sendVal.toHexString(),
gas: 1e6,
authorizationList: [auth]
}
await geth.sendTx(tx)
expect(await geth.provider.getBalance(eoa.address)).to.equal(sendVal)
expect(await geth.provider.getCode(eoa.address)).to.equal(hexConcat(['0xef0100', eip7702delegate.address]))
})

it('should fail call from another account', async () => {
const wallet1 = Simple7702Account__factory.connect(eoa.address, geth.provider.getSigner())
await expect(wallet1.execute([])).to.revertedWith('not from self or EntryPoint')
})

it('should succeed sending a batch', async () => {
// submit a batch
const wallet2 = Simple7702Account__factory.connect(eoa.address, eoa)
console.log('eoa balance=', await geth.provider.getBalance(eoa.address))

const addr1 = createAddress()
const addr2 = createAddress()

await wallet2.execute([{
target: addr1, value: 1, data: '0x'
}, {
target: addr2, value: 2, data: '0x'
}]).then(async tx => tx.wait())
expect(await geth.provider.getBalance(addr1)).to.equal(1)
expect(await geth.provider.getBalance(addr2)).to.equal(2)
})
})

it('should be able to use EntryPoint without paymaster', async () => {
const addr1 = createAddress()
const eoa = createAccountOwner(geth.provider)

const callData = eip7702delegate.interface.encodeFunctionData('execute', [[{
target: addr1,
value: 1,
data: '0x'
}]])
const userop = await fillAndSign({
sender: eoa.address,
initCode: '0xef01',
nonce: 0,
callData
}, eoa, entryPoint, { eip7702delegate: eip7702delegate.address })

await geth.sendTx({ to: eoa.address, value: parseEther('1') })
const auth = await signEip7702Authorization(eoa, { chainId: 0, nonce: 0, address: eip7702delegate.address })
const beneficiary = createAddress()
// submit separate tx with tuple: geth's estimateGas doesn't work, and its easier to detect thrown errors..
await geth.sendTx({
to: entryPoint.address,
data: '0x',
gas: 1000000,
authorizationList: [auth]
})
const handleOps = entryPoint.interface.encodeFunctionData('handleOps', [[packUserOp(userop)], beneficiary])
const tx = {
to: entryPoint.address,
data: handleOps
}
await geth.sendTx(tx)
})

it('should use EntryPoint with paymaster', async () => {
const addr1 = createAddress()
const eoa = createAccountOwner(geth.provider)
const paymaster = await new TestPaymasterAcceptAll__factory(geth.provider.getSigner()).deploy(entryPoint.address)
await paymaster.deposit({ value: parseEther('1') })
const callData = eip7702delegate.interface.encodeFunctionData('execute', [[{
target: addr1,
value: 1,
data: '0x'
}]])
const userop = await fillAndSign({
sender: eoa.address,
paymaster: paymaster.address,
initCode: '0xef01',
nonce: 0,
callData
}, eoa, entryPoint, { eip7702delegate: eip7702delegate.address })

const auth = await signEip7702Authorization(eoa, { chainId: 0, nonce: 0, address: eip7702delegate.address })
const beneficiary = createAddress()
console.log('delegate=', eip7702delegate.address)
// submit separate tx with tuple: geth's estimateGas doesn't work, and its easier to detect thrown errors..
await geth.sendTx({
to: entryPoint.address,
data: '0x',
gas: 1000000,
authorizationList: [auth]
})
const handleOps = entryPoint.interface.encodeFunctionData('handleOps', [[packUserOp(userop)], beneficiary])
const tx = {
to: entryPoint.address,
data: handleOps
}
await geth.sendTx(tx)
})
})