feat(core)!: optionally credit burnt base fee to coinbase address#299
Open
alarso16 wants to merge 6 commits into
Open
feat(core)!: optionally credit burnt base fee to coinbase address#299alarso16 wants to merge 6 commits into
alarso16 wants to merge 6 commits into
Conversation
alarso16
added
the
Status: 🔴 DO NOT MERGE
alarso16
marked this pull request as ready for review
July 21, 2026 22:27
Author
|
DO NOT MERGE until ava-labs/avalanchego#5721 is reviewed |
alarso16
added
Category: Bug 🐛
alarso16
changed the title
alarso16
commented
Jul 22, 2026
ARR4N
reviewed
Jul 23, 2026
| testErr = errors.New("test error") | ||
| baseFee = 3 | ||
| tip = 2 | ||
| feeCap = baseFee + tip + 1 // not binding, so the effective tip is [tip] |
Collaborator
There was a problem hiding this comment.
What do you mean by "not binding"?
Author
There was a problem hiding this comment.
I mean that the fee cap just must be greater than baseFee and tip, but not the whole thing was applied
ARR4N
reviewed
Jul 23, 2026
ARR4N
reviewed
Jul 23, 2026
ARR4N
changed the title
ARR4N
approved these changes
Jul 23, 2026
ARR4N
left a comment
ARR4N
left a comment
Collaborator
There was a problem hiding this comment.
Please can you update the PR description to explain the end goal of the feature, not why the last one didn't work. Otherwise LGTM
rahulmutt-ava
approved these changes
Jul 23, 2026
StephenButtolph
approved these changes
Jul 23, 2026
StephenButtolph
left a comment
StephenButtolph
left a comment
There was a problem hiding this comment.
Will wait to merge until we are closer to merging the stateful RPCs PR.
Don't need to keep merging PRs into libevm if we need to change them immediately after.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why this should be merged
Coreth and Subnet-EVM both credit the entire base fee and tip to the coinbase, which is enforced at verification time to be a specific blackhole address. Prior to #294, this wasn't possible at all through libevm, and was performed after
core.ApplyMessagein SAE.The previous solution with the
AfterExecutingTransactionhook allows modifying the statedb to reflect the blackhole updates, but the tracers did not capture these changes, asCaptureTxEndhad already been called. Additionally, the API surface cannot include arbitrary operations on the StateDB, since the tracers wouldn't know which accounts/storage to look at duringGetResult.How this works
Reverts all prod changes of #294 and provides a different hook which is more targeted to the specific problem SAE faces.
How this was tested
UT including the
prestatetracer to ensure that the common tracer workflows are correctly updated.