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

Check and correct tracing after fee currency addition #69

Closed
Tracked by #185
palango opened this issue Nov 7, 2023 · 2 comments · Fixed by #211
Closed
Tracked by #185

Check and correct tracing after fee currency addition #69

palango opened this issue Nov 7, 2023 · 2 comments · Fixed by #211
Assignees
Labels
type:bug Something isn't working type:enhancement New feature or request

Comments

@palango
Copy link

palango commented Nov 7, 2023

Part of celo-org/optimism#6

@carterqw2 carterqw2 transferred this issue from celo-org/optimism Apr 2, 2024
@carterqw2 carterqw2 mentioned this issue Apr 2, 2024
@carterqw2 carterqw2 added this to the 3 - Celo MVP Testnet milestone Apr 12, 2024
@karlb karlb self-assigned this May 15, 2024
@karlb
Copy link

karlb commented Jul 17, 2024

Without further modifications, we currently see the following issues when tracing fee currency txs:

  • Call tracer: the tracer only supports a single top level call. Calling creditGasFees after the main tx overwrites the trace.
  • Prestate tracer: the tracing is initialized after the debit and the tracer tries to undo this by adding the debit amount back to the tx sender's account. When using fee currencies, this adds the amount to the native balance, although it has been debited from the fee currency

The problems with the prestate tracer can only be fixed by initializing the tracing earlier, since there is not safe generic way to undo the debit. The major tracing changes in geth 1.14 will help with this but are not yet included in op-geth (update PR). Since waiting for these changes should make a proper solution for the prestate tracer possible and will reduce rebasing work, I suggest to delay implementing this.

Some commits I collected while experimenting can be found at https://github.com/celo-org/op-geth/compare/celo7...celo-org:op-geth:karlb/tracing-stash?expand=1.

@karlb
Copy link

karlb commented Jul 17, 2024

But we can already collect some thoughts on how these tracers should work.

Call tracer

Usually, debiting and crediting does not show up in the call trace, since no calls are used in the process. With fee currencies, two calls are used in addition to the main tx call. I assume that most users will expect the same call trace result no matter how the fees are paid. Therefore, tracing should be disabled for debiting and crediting by default.

We could provide a special fee currency call tracer that includes debit and credit and returns three top level calls. It is unclear if there is any demand for this and whether people will find out how to enable non-standard tracers. Thoughts?

Prestate tracer

The prestate tracer's result must be sufficient to execute the traced tx. This only works if

  • we include the debit (and probably credit) in the trace. Otherwise, we would miss the fee currency contract code and state.
  • we include the required exchange rate as a special field in the pre state. The exchange rates won't show up in the trace because they are read once per block outside of the tx execution.
  • we capture the pre state balance correctly before the debit is made

karlb added a commit that referenced this issue Jul 17, 2024
This test exercises both the call tracer and the transfer precompile. It
is the first test that checks for the gas cost and return value of the
precompile.

See #69
karlb added a commit that referenced this issue Jul 17, 2024
The precompile must show up as a CALL in call tracing.

This test exercises both the call tracer and the transfer precompile. It
is the first test that checks for the gas cost and return value of the
precompile.

See #69
karlb added a commit that referenced this issue Jul 17, 2024
The precompile must show up as a CALL in call tracing.

This test exercises both the call tracer and the transfer precompile. It
is the first test that checks for the gas cost and return value of the
precompile.

See #69
@lvpeschke lvpeschke added the type:bug Something isn't working label Jul 23, 2024
karlb added a commit that referenced this issue Aug 20, 2024
The precompile must show up as a CALL in call tracing.

This test exercises both the call tracer and the transfer precompile. It
is the first test that checks for the gas cost and return value of the
precompile.

See #69
karlb added a commit that referenced this issue Aug 26, 2024
The precompile must show up as a CALL in call tracing.

This test exercises both the call tracer and the transfer precompile. It
is the first test that checks for the gas cost and return value of the
precompile.

See #69
karlb added a commit that referenced this issue Aug 30, 2024
The precompile must show up as a CALL in call tracing.

This test exercises both the call tracer and the transfer precompile. It
is the first test that checks for the gas cost and return value of the
precompile.

See #69
@karlb karlb closed this as completed in #211 Sep 2, 2024
karlb added a commit that referenced this issue Oct 11, 2024
The precompile must show up as a CALL in call tracing.

This test exercises both the call tracer and the transfer precompile. It
is the first test that checks for the gas cost and return value of the
precompile.

See #69
piersy pushed a commit that referenced this issue Oct 15, 2024
The precompile must show up as a CALL in call tracing.

This test exercises both the call tracer and the transfer precompile. It
is the first test that checks for the gas cost and return value of the
precompile.

See #69
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type:bug Something isn't working type:enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants