-
Notifications
You must be signed in to change notification settings - Fork 830
feat: implement eip-7918 #4142
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
base: master
Are you sure you want to change the base?
feat: implement eip-7918 #4142
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
Short nit: please label PRs, still important for me |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Few nits to address, looks good otherwise! 👍
// The validation of the fields and 4844 activation is already taken care in BlockHeader constructor | ||
const targetGasConsumed = (this.excessBlobGas ?? BIGINT_0) + (this.blobGasUsed ?? BIGINT_0) | ||
const targetBlobGasPerBlock = childCommon.param('targetBlobGasPerBlock') | ||
const excessBlobGas = this.excessBlobGas ?? 0n |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please do not use the n
notation for BigInts (compatibility reasons) + we have these predefined constants (like BIGINT_1
) for various commonly used BigInt values, which gives a performance benefit and which you can use on various occasions in this PR.
if (!this.cache.hash) { | ||
this.cache.hash = this.keccakFunction(RLP.encode(this.raw())) as Uint8Array | ||
} | ||
this.cache.hash ??= this.keccakFunction(RLP.encode(this.raw())) as Uint8Array |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Did you also had this strange random (?) linter stuff !?
blobGasPriceUpdateFraction: 3338477, // The denominator used in the exponential when calculating a blob gas price | ||
// gasPrices | ||
minBlobGas: 1, // The minimum fee per blob gas | ||
blobBaseCost: 8192, // EIP-7918: Blob base fee bounded by execution cost (2^13) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This can be removed (I guess?).
'should use original EIP-4844 logic when EIP-7918 is not activated', | ||
) | ||
}) | ||
}) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just for informational purposes: is this AI or partly or not?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not matter who to praise for: these are really nicely readable tests! 😄 😂
This PR implements EIP-7918