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

core: implement auth and authcall #28615

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

lightclient
Copy link
Member

@lightclient lightclient commented Nov 27, 2023

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 and AUTHCALL opcodes!

To use, you'll have to be sure the chain is configured to be post-Prague.

@lightclient
Copy link
Member Author

I temporarily updated dev mode to start in prague, so you can run geth --dev to start a live chain with EIP-3074 active.

core/vm/eips.go Outdated
jt[AUTHCALL] = &operation{
execute: opAuthCall,
constantGas: params.CallGasEIP150,
dynamicGas: gasCallEIP2929,
Copy link
Contributor

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?

Copy link
Contributor

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

Copy link
Contributor

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:

  1. GAS opcode is followed by AUTHCALL. GAS pushes the remaining gas (after GAS op exec)
  2. 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

Copy link
Contributor

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:

https://github.com/ledgerwatch/erigon/pull/10154/files#diff-5fd0ea2f1f3910fe70b219d59a388b920a342bbbf807b89eb6f70543ff0853ddR86

Erigon's draft PR is here, and includes the gas cost calculation differences mentioned in eip-3074.

sudeepdino008 added a commit to ledgerwatch/erigon that referenced this pull request May 10, 2024
- 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.
sudeepdino008 added a commit to ledgerwatch/erigon that referenced this pull request May 10, 2024
- 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.
sudeepdino008

This comment was marked as outdated.

@lightclient lightclient force-pushed the eip-3074 branch 3 times, most recently from 7e66585 to d73fcb8 Compare May 14, 2024 13:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants