Update EIP-7069: Rename and update semantics #8287

pdobacz · 2024-03-05T13:38:14Z

The rename as in Rename CREATE3/4, *CALL2 instructions ipsilon/eof#64
The execution semantics to be more aligned with original CALL flow and with evmone implementation and most recent EOF conversations

eth-bot · 2024-03-05T13:38:48Z

✅ All reviewers have approved.

shemnon · 2024-03-05T14:49:32Z

EIPS/eip-7069.md

+7. Calculate the gas available to callee as caller's remaining gas reduced by `max(floor(gas/64), MIN_RETAINED_GAS)`.
+8. Fail with error if the gas available to callee at this point is less than `MIN_CALLEE_GAS`.
+9. If `value` is non-zero, fail lightly if the balance of the current account is less than `value`.
+10. Fail lightly if call stack depth equals `1024`.


What does "fail Lightly" mean? dose "Fail with error" mean revert and take all available gas and "fail lightly" mean fail consuming only already charged gas?

yes (except "Fail with error" is not a revert but an exceptional abort, I presume this was the original intention), I'll specify these.

But you've uncovered an interesting issue - what should be pushed to stack in this case (i.e. light failure)? For a light failure in EOFCREATE/TXCREATE we return 0 (with a different meaning), same for CREATE/CREATE2, but for revamped calls that would mean success.

I also see that evmone would return 0, so making a self note to amend this (I'm working on that now in parallel).

Errata: there's many ways to name the "hard failure", this EIP has "halt with exceptional failure", I think consuming all gas is understood (EOF megaspec and other EIPs don't repeat this), so I'll leave it out.

For light failure I'll be specific.

Proposing a resolution (3 on stack for light failure) 54f6dcd to push the discussion forward. WDYT?

477e269 adheres to what we spoke about on the EOF implementers call - 1 as status code for light failure

EIPS/eip-7069.md

gumb0 · 2024-03-06T11:54:41Z

EIPS/eip-7069.md

+7. Calculate the gas available to callee as caller's remaining gas reduced by `max(floor(gas/64), MIN_RETAINED_GAS)`.
+8. Halt with exceptional failure if the gas available to callee at this point is less than `MIN_CALLEE_GAS`.
+9. If `value` is non-zero, fail lightly if the balance of the current account is less than `value` (push `3` on the stack, only consume gas charged until this point).
+10. Fail lightly if call stack depth equals `1024` (push `3` on the stack, only consume gas charged until this point).


Call depth check is actually superseded by 63/64 rule, see Rationale.

I thought you questioned the correctness of that initial removal of call depth check.

Isn't it a compatibility problem, that call-stack-depth-checking calls can alternate with non-checking ones in the call stack?

Why wasn't the call depth check removed already in legacy calls, if superseded by the 63/64 rule?

for future reference - decided on the EOF Implementers call to keep it

least change to legacy

being cautious about the air-tightness of the (complex) gas-based 63/64 rule, and providing a simpler hard cap

being open to gas limits which are huge for various use cases beyond mainnet

(also I've updated the Rationale section to cover this)

EIPS/eip-7069.md

c.f. ipsilon/eof#64 and ipsilon/eof#68

gumb0 · 2024-03-15T19:59:48Z

EIPS/eip-7069.md

-    11c. `2` if the call has failed.
-11. Gas not used by the callee is returned to the caller.
+7. Calculate the gas available to callee as caller's remaining gas reduced by `max(floor(gas/64), MIN_RETAINED_GAS)`.
+8. Halt with exceptional failure if the gas available to callee at this point is less than `MIN_CALLEE_GAS`.


Why is this exceptional failure? Seems could be light failure

The PR only changes the language on this one. And I see it as a sensible failure mode - consistent with an OOG. But light failure also makes sense on 2nd thought. @chfast @axic @shemnon any more opinions on this, any objections to changing to light failure?

Changed as suggested here: 743a3fb

EIPS/eip-7069.md

gumb0 · 2024-03-21T10:47:09Z

EIPS/eip-7069.md

-    3a. Fail with error if the current frame is in `static-mode`.
-    3b. Fail with error if the balance of the current account is less than `value`.
-    3c. Charge `CALL_VALUE_COST` gas.
+2. Pop required arguments from stack, halt with exceptional failure on stack underflow.


It seems at this point EIP tries to maintain compatibility with both legacy and EOF, so maybe

Suggested change

2. Pop required arguments from stack, halt with exceptional failure on stack underflow.

2. Pop required arguments from stack, halt with exceptional failure on stack underflow.

- *Note: When implemented in EOF, stack underflow check is done during stack validation, and runtime check is omitted.*

The idea is for this EIP to be fully legacy-complete and EOF-independent. EOF (Megaspec) lists the differences required it to be EOF-complete (which is lack of runtime stack underflow check and banning new opcodes in legacy).

Is there any other piece which is EOF-only in this EIP?

(this is just a question to your comment, I have no objections towards adding this note for completeness, and I'll add it)

EIPS/eip-7069.md

Co-authored-by: Andrei Maiboroda <[email protected]>

eth-bot

All Reviewers Have Approved; Performing Automatic Merge...

pdobacz requested a review from eth-bot as a code owner March 5, 2024 13:38

github-actions bot added c-update Modifies an existing proposal s-draft This EIP is a Draft t-core labels Mar 5, 2024

eth-bot added the a-review Waiting on author to review label Mar 5, 2024

shemnon reviewed Mar 5, 2024

View reviewed changes

EIPS/eip-7069.md Outdated Show resolved Hide resolved

gumb0 reviewed Mar 6, 2024

View reviewed changes

pdobacz force-pushed the update-eip-7069 branch from 54f6dcd to 50f8e7a Compare March 7, 2024 07:44

pdobacz requested a review from shemnon March 11, 2024 16:44

axic reviewed Mar 11, 2024

View reviewed changes

EIPS/eip-7069.md Outdated Show resolved Hide resolved

pdobacz added 7 commits March 11, 2024 18:11

Rename *CALL2

520ebb6

c.f. ipsilon/eof#64 and ipsilon/eof#68

Update execution semantics

2cbfc32

Bring back the call stack depth check

5786b32

Clarify failure modes after feedback

260828e

Specify and explain using 1 as status code for light failure

39c6e05

More rationale about the call stack depth check

c3de483

Revised language about the call stack depth check

a26d3a4

pdobacz force-pushed the update-eip-7069 branch from af154c7 to a26d3a4 Compare March 11, 2024 17:11

pdobacz marked this pull request as draft March 13, 2024 16:30

pdobacz requested review from axic and gumb0 March 14, 2024 12:52

pdobacz mentioned this pull request Mar 14, 2024

Mention call depth limit in EOFCREATE spec ipsilon/eof#73

Merged

gumb0 reviewed Mar 15, 2024

View reviewed changes

EIPS/eip-7069.md Outdated Show resolved Hide resolved

gumb0 reviewed Mar 15, 2024

View reviewed changes

EIPS/eip-7069.md Outdated Show resolved Hide resolved

gumb0 reviewed Mar 15, 2024

View reviewed changes

EIPS/eip-7069.md Outdated Show resolved Hide resolved

Improve language after review

c3b528a

chfast approved these changes Mar 18, 2024

View reviewed changes

pdobacz mentioned this pull request Mar 19, 2024

Implement "CALLS 2.0" ipsilon/evmone#630

Merged

Change the MIN_CALLEE_GAS failure to light

743a3fb

gumb0 reviewed Mar 21, 2024

View reviewed changes

EIPS/eip-7069.md Outdated Show resolved Hide resolved

gumb0 reviewed Mar 21, 2024

View reviewed changes

EIPS/eip-7069.md Outdated Show resolved Hide resolved

gumb0 reviewed Mar 21, 2024

View reviewed changes

EIPS/eip-7069.md Outdated Show resolved Hide resolved

Apply suggestions from code review - 7069

de40636

Co-authored-by: Andrei Maiboroda <[email protected]>

pdobacz force-pushed the update-eip-7069 branch from 40ee42a to de40636 Compare March 21, 2024 13:02

pdobacz requested a review from gumb0 March 21, 2024 13:15

gumb0 approved these changes Mar 21, 2024

View reviewed changes

pdobacz marked this pull request as ready for review March 21, 2024 16:31

Merge remote-tracking branch 'origin/master' into update-eip-7069

92a768f

eth-bot enabled auto-merge (squash) March 22, 2024 07:56

eth-bot approved these changes Mar 22, 2024

View reviewed changes

eth-bot merged commit 30a52d0 into ethereum:master Mar 22, 2024

pdobacz deleted the update-eip-7069 branch March 22, 2024 08:22

	2. Pop required arguments from stack, halt with exceptional failure on stack underflow.
	2. Pop required arguments from stack, halt with exceptional failure on stack underflow.
	- Note: When implemented in EOF, stack underflow check is done during stack validation, and runtime check is omitted.

Update EIP-7069: Rename and update semantics #8287

Update EIP-7069: Rename and update semantics #8287

Uh oh!

Conversation

pdobacz commented Mar 5, 2024

Uh oh!

eth-bot commented Mar 5, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

pdobacz Mar 5, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

eth-bot left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

eth-bot commented Mar 5, 2024 •

edited

Loading

pdobacz Mar 5, 2024 •

edited

Loading