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.

Sign up for GitHub

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-466: Require SenderCreator to be called from EntryPoint & AA-470: Make SenderCreator public #514

Merged
merged 10 commits into from
Dec 23, 2024
Merged

AA-466: Require SenderCreator to be called from EntryPoint & AA-470: Make SenderCreator public #514

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
692374c
AA-466: Prevent InitCode frontrunning; AA-470: Make SenderCreator public
forshtat Dec 17, 2024
d87fa05
Try newer "upload-artifact"
forshtat Dec 17, 2024
397217d
Try new "docker compose" syntax
forshtat Dec 17, 2024
f393000
Implement SenderCreator address check in the SimpleAccountFactory
forshtat Dec 19, 2024
afeca9a
Use hardhat impersonating signer to make calls on behalf of the Entry…
forshtat Dec 19, 2024
bb137d9
Fix tests?
forshtat Dec 19, 2024
9ec3454
use a UserOperation to create accounts.
drortirosh Dec 22, 2024
1b8ff94
gascalc
drortirosh Dec 22, 2024
9264f65
lcok fillSignAndPack
drortirosh Dec 22, 2024
aa5b03a
gascalcs
drortirosh Dec 22, 2024
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
2 changes: 1 addition & 1 deletion .github/workflows/build.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ jobs:
- run: yarn compile

- run: FORCE_COLOR=1 yarn coverage
- uses: actions/upload-artifact@v2
- uses: actions/upload-artifact@v4
with:
name: solidity-coverage
path: |
Expand Down
2 changes: 1 addition & 1 deletion contracts/core/EntryPoint.sol
View file
Open in desktop
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
Open in desktop
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
Open in desktop
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");
Copy link
Contributor

Choose a reason for hiding this comment

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

this is not an expected error case to happen with entrypoint, only by "side-calling".
So it is an AA9x.
Can reuse "AA92 only from entrypoint"

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Didn't want to reuse error codes, defined it s AA97.

}
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
Open in desktop
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 ║
╚════════════════════════════════╧═══════╧═══════════════╧════════════════╧═════════════════════╝

4 changes: 2 additions & 2 deletions scripts/docker-gascalc
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
# run "yarn gas-calc" using geth with docker.
# (if you have geth running on localhost:8545, its faster with "HARDHAT_NETWORK=dev yarn gas-calc")
docker-compose -f `dirname $0`/docker-gascalc.yml up --abort-on-container-exit
docker compose -f `dirname $0`/docker-gascalc.yml up --abort-on-container-exit
exit=$?
docker-compose -f `dirname $0`/docker-gascalc.yml down
docker compose -f `dirname $0`/docker-gascalc.yml down
exit $exit

33 changes: 21 additions & 12 deletions test/entrypoint.test.ts
View file
Open in desktop
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
Loading