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

Witness charging order with limits #502

Open
wants to merge 8 commits into
base: jsign-witness-fix
Choose a base branch
from

Conversation

gballet
Copy link
Owner

@gballet gballet commented Sep 24, 2024

This is another take on #495, which doesn't pass the gas usage function as a parameter, but the total amount of gas available. This approach has two advantages:

  1. it doesn't require removing the verkle gas functions, which should simplify mainlining the PR
  2. it returns the gas to be consumed at the call site, which helps with readability (incidentally, there is a change to the UseGas function signature in master for the live tracer, which will make it trickier to handle

The difficulty was distinguishing the gas that is to be charged (consumed), from the gas that is requested (wanted). If wanted > charged then it is known that the contract ran out of gas.

In order to do so

  • The touch* functions are changed to return the consumed and wanted gas. Locations will only be added to the witness if consumed == wanted.
  • The Touch functions change to only return the wanted gas, so that the gas* functions can return it (in order to let the interpreter loop know that the dynamic gas is too much, while only adding locations that were fully paid for to the witness.
  • Calls and contract creation use the consumed < wanted comparison to figure out if the subcall ran out of gas.

@gballet gballet force-pushed the witness-charging-order-with-limits branch 2 times, most recently from caa00f3 to f7db43f Compare September 24, 2024 17:10
@gballet gballet changed the base branch from kaustinen-with-shapella to jsign-witness-fix September 27, 2024 13:20
@gballet gballet force-pushed the witness-charging-order-with-limits branch from f96a41c to e220c74 Compare October 11, 2024 13:11
@gballet gballet force-pushed the witness-charging-order-with-limits branch from e220c74 to f2b8d24 Compare October 11, 2024 13:48
jsign and others added 2 commits October 11, 2024 15:58
* ci: new workflows for testing

Signed-off-by: Ignacio Hagopian <[email protected]>

* nit

Signed-off-by: Ignacio Hagopian <[email protected]>

* ci: fix stable consumption

Signed-off-by: Ignacio Hagopian <[email protected]>

* fix

Signed-off-by: Ignacio Hagopian <[email protected]>

* more fixes

Signed-off-by: Ignacio Hagopian <[email protected]>

* more fixes

Signed-off-by: Ignacio Hagopian <[email protected]>

* update tag

Signed-off-by: Ignacio Hagopian <[email protected]>

---------

Signed-off-by: Ignacio Hagopian <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants