Skip to content

Conversation

MariusVanDerWijden
Copy link
Member

@MariusVanDerWijden MariusVanDerWijden commented Sep 12, 2023

Implements the https://eips.ethereum.org/EIPS/eip-7516 opcode for cancun

core/evm.go Outdated
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are a lot of places that doesn't use this constructor. (Maybe we ought to make them use a constructor, becuause it's very easy to miss updating one of them)
Screenshot from 2023-09-14 09-51-50

46 places where we create a vm.BlockContext struct

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The problem is that most of these cases are in tests where we only set one or two of the fields. We can't really update them to use the constructor since that will need a header to populate the fields. We could theoretically create a constructor that will set every field, but that will result in a lot of NewBlockContext(nil, nil, .-.. nil). Also we pass the struct by value very often so most of the initializatios are actually BlockCOntext{}

Copy link
Member

@lightclient lightclient left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Copy link
Member

@karalabe karalabe left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can remove the ExcessBlobGas from the vm context, its redundant with the fee

if vmContext.ExcessBlobGas != nil {
execRs.CurrentExcessBlobGas = (*math.HexOrDecimal64)(vmContext.ExcessBlobGas)
if vmContext.BlobBaseFee != nil {
// TODO (MariusVanDerWijden): I have no idea why we have this field at all.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the idea here is that if you provide parentExcessBlobGas and parentBlobGasUsed t8n will automatically calculate that block's excessBlobGas. In these instances you would probably want the calculated value output so that you can pipe it in for the next transition.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed, sry

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I still don't think that this is really needed

@holiman holiman added the cancun label Oct 2, 2023
if tx.Type() == types.BlobTxType {
receipt.BlobGasUsed = uint64(len(tx.BlobHashes()) * params.BlobTxBlobGasPerBlob)
receipt.BlobGasPrice = eip4844.CalcBlobFee(*evm.Context.ExcessBlobGas)
receipt.BlobGasPrice = evm.Context.BlobBaseFee
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How come that the "gasprice" is calculated as the basefee? I mean, the base is only one part of the price, no?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is no blob priority fee yet, so it's just base fee!

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What was ExcessBlobGas exactly before?
And the concept of ExcessBlobGas no longer exists?

Copy link
Contributor

@holiman holiman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@holiman holiman added this to the 1.13.3 milestone Oct 2, 2023
@holiman holiman merged commit c39cbc1 into ethereum:master Oct 2, 2023
hyunchel pushed a commit to hyunchel/go-ethereum that referenced this pull request Oct 6, 2023
Implements "EIP-7516: BLOBBASEFEE opcode" for cancun, as per spec: https://eips.ethereum.org/EIPS/eip-7516
ghost pushed a commit to blocknative/go-ethereum that referenced this pull request Oct 12, 2023
Implements "EIP-7516: BLOBBASEFEE opcode" for cancun, as per spec: https://eips.ethereum.org/EIPS/eip-7516
ghost pushed a commit to blocknative/go-ethereum that referenced this pull request Oct 16, 2023
Implements "EIP-7516: BLOBBASEFEE opcode" for cancun, as per spec: https://eips.ethereum.org/EIPS/eip-7516
devopsbo3 pushed a commit to HorizenOfficial/go-ethereum that referenced this pull request Nov 10, 2023
Implements "EIP-7516: BLOBBASEFEE opcode" for cancun, as per spec: https://eips.ethereum.org/EIPS/eip-7516
devopsbo3 added a commit to HorizenOfficial/go-ethereum that referenced this pull request Nov 10, 2023
devopsbo3 added a commit to HorizenOfficial/go-ethereum that referenced this pull request Nov 10, 2023
sduchesneau pushed a commit to streamingfast/go-ethereum that referenced this pull request Feb 26, 2024
implement BLOBBASEFEE opcode (0x4a) (ethereum#28098)

 Conflicts:
  core/evm.go
  core/vm/evm.go
Both nitro and upstream added fields to BlockContextx. Using both.

 Additions:
  accounts/abi/bind/base.go
  eth/tracers/js/goja.go
common.Address no longer has the Hash() function that we sometimes used.
It now, however, has a Big() function.
Previous calls to Address.Hash().Big() were converted to .Big()
gzliudan added a commit to gzliudan/XDPoSChain that referenced this pull request Feb 10, 2025
gzliudan added a commit to XinFinOrg/XDPoSChain that referenced this pull request Mar 26, 2025
gzliudan added a commit to XinFinOrg/XDPoSChain that referenced this pull request Oct 9, 2025
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.

7 participants