Skip to content

Commit

Permalink
AA-466: Prevent InitCode frontrunning; AA-470: Make SenderCreator public
Browse files Browse the repository at this point in the history
  • Loading branch information
@forshtat
forshtat committed Dec 17, 2024
1 parent 6f02f5a commit 692374c
Show file tree
Hide file tree
Showing 5 changed files with 54 additions and 36 deletions.
2 changes: 1 addition & 1 deletion contracts/core/EntryPoint.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ contract EntryPoint is IEntryPoint, StakeManager, NonceManager, ReentrancyGuard,

SenderCreator private immutable _senderCreator = new SenderCreator();

function senderCreator() internal view virtual returns (SenderCreator) {
function senderCreator() public view virtual returns (SenderCreator) {
return _senderCreator;
}

Expand Down
4 changes: 2 additions & 2 deletions contracts/core/EntryPointSimulations.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ contract EntryPointSimulations is EntryPoint, IEntryPointSimulations {
_senderCreator = SenderCreator(createdObj);
}

function senderCreator() internal view virtual override returns (SenderCreator) {
function senderCreator() public view virtual override returns (SenderCreator) {
// return the same senderCreator as real EntryPoint.
// this call is slightly (100) more expensive than EntryPoint's access to immutable member
return _senderCreator;
Expand All @@ -35,7 +35,7 @@ contract EntryPointSimulations is EntryPoint, IEntryPointSimulations {
* it as entrypoint, since the simulation functions don't check the signatures
*/
constructor() {
require(block.number < 100, "should not be deployed");
require(block.number < 1000, "should not be deployed");
}

/// @inheritdoc IEntryPointSimulations
Expand Down
9 changes: 9 additions & 0 deletions contracts/core/SenderCreator.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,12 @@ pragma solidity ^0.8.23;
* which is explicitly not the entryPoint itself.
*/
contract SenderCreator {
address public immutable entryPoint;

constructor(){
entryPoint = msg.sender;
}

/**
* Call the "initCode" factory to create and return the sender account address.
* @param initCode - The initCode value from a UserOp. contains 20 bytes of factory address,
Expand All @@ -15,6 +21,9 @@ contract SenderCreator {
function createSender(
bytes calldata initCode
) external returns (address sender) {
if (msg.sender != entryPoint) {
revert("AAxx should call from EntryPoint");
}
address factory = address(bytes20(initCode[0:20]));
bytes memory initCallData = initCode[20:];
bool success;
Expand Down
42 changes: 21 additions & 21 deletions reports/gas-checker.txt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,52 +4,52 @@
╔══════════════════════════╤════════╗
║ gas estimate "simple" │ 28979 ║
╟──────────────────────────┼────────╢
║ gas estimate "big tx 5k" │ 125224 ║
║ gas estimate "big tx 5k" │ 125212 ║
╚══════════════════════════╧════════╝

╔════════════════════════════════╤═══════╤═══════════════╤════════════════╤═════════════════════╗
║ handleOps description │ count │ total gasUsed │ per UserOp gas │ per UserOp overhead ║
║ │ │ │ (delta for │ (compared to ║
║ │ │ │ one UserOp) │ account.exec()) ║
╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢
║ simple │ 1 │ 79994 │ │ ║
║ simple │ 1 │ 80004 │ │ ║
╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢
║ simple - diff from previous │ 2 │ │ 42192 │ 13213 ║
║ simple - diff from previous │ 2 │ │ 42204 │ 13225 ║
╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢
║ simple │ 10 │ 459921 │ │ ║
║ simple │ 10 │ 459895 │ │ ║
╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢
║ simple - diff from previous │ 11 │ │ 42223 │ 13244 ║
║ simple - diff from previous │ 11 │ │ 42307 │ 13328 ║
╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢
║ simple paymaster │ 1 │ 86113 │ │ ║
║ simple paymaster │ 1 │ 86147 │ │ ║
╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢
║ simple paymaster with diff │ 2 │ │ 41024 │ 12045 ║
║ simple paymaster with diff │ 2 │ │ 41048 │ 12069 ║
╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢
║ simple paymaster │ 10 │ 455444 │ │ ║
║ simple paymaster │ 10 │ 455730 │ │ ║
╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢
║ simple paymaster with diff │ 11 │ │ 41088 │ 12109 ║
║ simple paymaster with diff │ 11 │ │ 41076 │ 12097 ║
╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢
║ big tx 5k │ 1 │ 181026 │ │ ║
║ big tx 5k │ 1 │ 181036 │ │ ║
╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢
║ big tx - diff from previous │ 2 │ │ 142714 │ 17490 ║
║ big tx - diff from previous │ 2 │ │ 142702 │ 17490 ║
╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢
║ big tx 5k │ 10 │ 1465443 │ │ ║
║ big tx 5k │ 10 │ 1465453 │ │ ║
╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢
║ big tx - diff from previous │ 11 │ │ 142686 │ 17462 ║
║ big tx - diff from previous │ 11 │ │ 142710 │ 17498 ║
╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢
║ paymaster+postOp │ 1 │ 87712 │ │ ║
║ paymaster+postOp │ 1 │ 87770 │ │ ║
╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢
║ paymaster+postOp with diff │ 2 │ │ 42671 │ 13692 ║
║ paymaster+postOp with diff │ 2 │ │ 42659 │ 13680 ║
╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢
║ paymaster+postOp │ 10 │ 471754 │ │ ║
║ paymaster+postOp │ 10 │ 471848 │ │ ║
╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢
║ paymaster+postOp with diff │ 11 │ │ 42728 │ 13749 ║
║ paymaster+postOp with diff │ 11 │ │ 42656 │ 13677 ║
╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢
║ token paymaster │ 1 │ 128777 │ │ ║
║ token paymaster │ 1 │ 128821 │ │ ║
╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢
║ token paymaster with diff │ 2 │ │ 66386 │ 37407 ║
║ token paymaster with diff │ 2 │ │ 66408 │ 37429 ║
╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢
║ token paymaster │ 10 │ 726504 │ │ ║
║ token paymaster │ 10 │ 726746 │ │ ║
╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢
║ token paymaster with diff │ 11 │ │ 66394 │ 37415 ║
║ token paymaster with diff │ 11 │ │ 66488 │ 37509 ║
╚════════════════════════════════╧═══════╧═══════════════╧════════════════╧═════════════════════╝

33 changes: 21 additions & 12 deletions test/entrypoint.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,18 @@ import './aa.init'
import { BigNumber, Event, Wallet } from 'ethers'
import { expect } from 'chai'
import {
EntryPoint,
IEntryPoint__factory,
INonceManager__factory,
IStakeManager__factory,
MaliciousAccount__factory,
SenderCreator__factory,
SimpleAccount,
SimpleAccountFactory,
TestAggregatedAccount__factory,
SimpleAccountFactory__factory,
TestAggregatedAccount,
TestAggregatedAccountFactory__factory,
TestAggregatedAccount__factory,
TestCounter,
TestCounter__factory,
TestExpirePaymaster,
Expand All @@ -14,20 +22,13 @@ import {
TestExpiryAccount__factory,
TestPaymasterAcceptAll,
TestPaymasterAcceptAll__factory,
TestPaymasterRevertCustomError__factory,
TestPaymasterWithPostOp,
TestPaymasterWithPostOp__factory,
TestRevertAccount__factory,
TestAggregatedAccount,
TestSignatureAggregator,
TestSignatureAggregator__factory,
MaliciousAccount__factory,
TestWarmColdAccount__factory,
TestPaymasterRevertCustomError__factory,
IEntryPoint__factory,
SimpleAccountFactory__factory,
IStakeManager__factory,
INonceManager__factory,
EntryPoint,
TestPaymasterWithPostOp__factory,
TestPaymasterWithPostOp
TestWarmColdAccount__factory
} from '../typechain'
import {
AddressZero,
Expand Down Expand Up @@ -834,6 +835,14 @@ describe('EntryPoint', function () {
let createOp: PackedUserOperation
const beneficiaryAddress = createAddress() // 1

it('should reject create if SenderCreator not called from EntryPoint', async () => {
const senderCreatorAddress = await entryPoint.senderCreator()
const senderCreator = SenderCreator__factory.connect(senderCreatorAddress, ethersSigner)
await expect(
senderCreator.createSender('0xdeadbeef', { gasLimit: 1000000 })
).to.be.revertedWith('AAxx should call from EntryPoint')
})

it('should reject create if sender address is wrong', async () => {
const op = await fillSignAndPack({
initCode: getAccountInitCode(accountOwner.address, simpleAccountFactory),
Expand Down

0 comments on commit 692374c

Please sign in to comment.