-
Notifications
You must be signed in to change notification settings - Fork 19.5k
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
core: implement auth and authcall #28615
base: master
Are you sure you want to change the base?
Conversation
55189f1
to
bae5387
Compare
I temporarily updated dev mode to start in prague, so you can run |
67662bc
to
5f5dd54
Compare
69f4d74
to
b93f1b6
Compare
core/vm/eips.go
Outdated
jt[AUTHCALL] = &operation{ | ||
execute: opAuthCall, | ||
constantGas: params.CallGasEIP150, | ||
dynamicGas: gasCallEIP2929, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
gasCallEIP2929 relies on warm storage cost to be already included. The constantGas should then have additional params.WarmStorageReadCostEIP2929?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also, gasCallEIP2929 itself uses logic which doesn't satisfy all steps needed for AUTHCALL dynamic gas calculation. For example, when "If the gas operand is equal to 0, the instruction will send all available gas as per EIP-150."
gasCallEIP2929
doesn't obey this
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
in fact the testcase should ideally fail. Because:
- GAS opcode is followed by AUTHCALL. GAS pushes the remaining gas (after GAS op exec)
- for AUTHCALL execution, dynamic gas is calculated and then out of the remaining gas,
all_but_one_64th
is calculated. This would be always less than gas on stack (pushed by GAS above).
The EIP mentions this logic:
if gas == 0:
subcall_gas = all_but_one_64th
elif all_but_one_64th < gas:
raise # Execution is invalid.
else:
subcall_gas = gas
gas is not 0, and the second case is always satisfied, leading to invalid execution
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I edit the test, replacing GAS opcode with PUSH0:
Erigon's draft PR is here, and includes the gas cost calculation differences mentioned in eip-3074.
- unit test imported from ethereum/go-ethereum#28615 - the invoker contract needed to be modified a little (use PUSH0 instead of GAS), so as to allow all gas to be available for the invoked contract. - fix for AUTHCALL constant gas to include warm storage access cost.
- unit test imported from ethereum/go-ethereum#28615 - the invoker contract needed to be modified a little (use PUSH0 instead of GAS), so as to allow all gas to be available for the invoked contract. - fix for AUTHCALL constant gas to include warm storage access cost.
7e66585
to
d73fcb8
Compare
This is an implementation of EIP-3074: AUTH and AUTHCALL. Still a WIP, I think there may be some bugs in the gas calculations and some edge cases which are not correctly dealt with. But it should be good enough for people wanting to experiment with the
AUTH
andAUTHCALL
opcodes!To use, you'll have to be sure the chain is configured to be post-Prague.