Skip to content

core/types: Populate Jovian receipt fields #710

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.

Sign up for GitHub

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

Open
wants to merge 1 commit into
base: optimism
Choose a base branch
from
+585 −619
Open

core/types: Populate Jovian receipt fields #710

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 8 additions & 6 deletions core/types/gen_receipt_json.go
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

42 changes: 13 additions & 29 deletions core/types/receipt.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -86,15 +86,16 @@ type Receipt struct {
TransactionIndex uint `json:"transactionIndex"`

// Optimism: extend receipts with L1 and operator fee info
L1GasPrice *big.Int `json:"l1GasPrice,omitempty"` // Present from pre-bedrock. L1 Basefee after Bedrock
L1BlobBaseFee *big.Int `json:"l1BlobBaseFee,omitempty"` // Always nil prior to the Ecotone hardfork
L1GasUsed *big.Int `json:"l1GasUsed,omitempty"` // Present from pre-bedrock, deprecated as of Fjord
L1Fee *big.Int `json:"l1Fee,omitempty"` // Present from pre-bedrock
FeeScalar *big.Float `json:"l1FeeScalar,omitempty"` // Present from pre-bedrock to Ecotone. Nil after Ecotone
L1BaseFeeScalar *uint64 `json:"l1BaseFeeScalar,omitempty"` // Always nil prior to the Ecotone hardfork
L1BlobBaseFeeScalar *uint64 `json:"l1BlobBaseFeeScalar,omitempty"` // Always nil prior to the Ecotone hardfork
OperatorFeeScalar *uint64 `json:"operatorFeeScalar,omitempty"` // Always nil prior to the Isthmus hardfork
OperatorFeeConstant *uint64 `json:"operatorFeeConstant,omitempty"` // Always nil prior to the Isthmus hardfork
L1GasPrice *big.Int `json:"l1GasPrice,omitempty"` // Present from pre-bedrock. L1 Basefee after Bedrock
L1BlobBaseFee *big.Int `json:"l1BlobBaseFee,omitempty"` // Always nil prior to the Ecotone hardfork
L1GasUsed *big.Int `json:"l1GasUsed,omitempty"` // Present from pre-bedrock, deprecated as of Fjord
L1Fee *big.Int `json:"l1Fee,omitempty"` // Present from pre-bedrock
FeeScalar *big.Float `json:"l1FeeScalar,omitempty"` // Present from pre-bedrock to Ecotone. Nil after Ecotone
L1BaseFeeScalar *uint64 `json:"l1BaseFeeScalar,omitempty"` // Always nil prior to the Ecotone hardfork
L1BlobBaseFeeScalar *uint64 `json:"l1BlobBaseFeeScalar,omitempty"` // Always nil prior to the Ecotone hardfork
OperatorFeeScalar *uint64 `json:"operatorFeeScalar,omitempty"` // Always nil prior to the Isthmus hardfork
OperatorFeeConstant *uint64 `json:"operatorFeeConstant,omitempty"` // Always nil prior to the Isthmus hardfork
DAFootprintGasScalar *uint64 `json:"daFootprintGasScalar,omitempty"` // Always nil prior to the Jovian hardfork
}

type receiptMarshaling struct {
Expand All @@ -121,6 +122,7 @@ type receiptMarshaling struct {
DepositReceiptVersion *hexutil.Uint64
OperatorFeeScalar *hexutil.Uint64
OperatorFeeConstant *hexutil.Uint64
DAFootprintGasScalar *hexutil.Uint64
}

// receiptRLP is the consensus encoding of a receipt.
Expand Down Expand Up @@ -612,26 +614,8 @@ func (rs Receipts) DeriveFields(config *params.ChainConfig, blockHash common.Has
logIndex += uint(len(rs[i].Logs))
}

if config.Optimism != nil && len(txs) >= 2 && config.IsBedrock(new(big.Int).SetUint64(blockNumber)) {
gasParams, err := extractL1GasParams(config, blockTime, txs[0].Data())
if err != nil {
return err
}
for i := 0; i < len(rs); i++ {
if txs[i].IsDepositTx() {
continue
}
rs[i].L1GasPrice = gasParams.l1BaseFee
rs[i].L1BlobBaseFee = gasParams.l1BlobBaseFee
rs[i].L1Fee, rs[i].L1GasUsed = gasParams.costFunc(txs[i].RollupCostData())
rs[i].FeeScalar = gasParams.feeScalar
rs[i].L1BaseFeeScalar = u32ptrTou64ptr(gasParams.l1BaseFeeScalar)
rs[i].L1BlobBaseFeeScalar = u32ptrTou64ptr(gasParams.l1BlobBaseFeeScalar)
if gasParams.operatorFeeScalar != nil && gasParams.operatorFeeConstant != nil && (*gasParams.operatorFeeScalar != 0 || *gasParams.operatorFeeConstant != 0) {
rs[i].OperatorFeeScalar = u32ptrTou64ptr(gasParams.operatorFeeScalar)
rs[i].OperatorFeeConstant = gasParams.operatorFeeConstant
}
}
if config.IsOptimismBedrock(new(big.Int).SetUint64(blockNumber)) && len(txs) >= 2 {
Copy link
Contributor

Choose a reason for hiding this comment

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

Condition changed: we still need the config.Optimism != nil, correct? Suggesting we leave the condition unchanged since it doesn't seem to have an impact on this PR.

Suggested change
if config.IsOptimismBedrock(new(big.Int).SetUint64(blockNumber)) && len(txs) >= 2 {
if config.Optimism != nil && len(txs) >= 2 && config.IsOptimismBedrock(new(big.Int).SetUint64(blockNumber)) {

Copy link
Contributor

Choose a reason for hiding this comment

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

I think the condition is equivalent because of this:

op-geth/params/config.go

Lines 907 to 910 in 96738d2

// IsOptimismBedrock returns true iff this is an optimism node & bedrock is active
func (c *ChainConfig) IsOptimismBedrock(num *big.Int) bool {
return c.IsOptimism() && c.IsBedrock(num)
}

return rs.deriveOPStackFields(config, blockTime, txs)
}
return nil
}
Expand Down
48 changes: 48 additions & 0 deletions core/types/receipt_opstack.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
package types

import (
"fmt"

"github.com/ethereum/go-ethereum/params"
)

// deriveOPStackFields derives the OP Stack specific fields for each receipt.
// It must only be called for blocks with at least two transactions.
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: it will still work even if there is only the system deposit transaction right?

Suggested change
// It must only be called for blocks with at least two transactions.
// It must only be called for blocks with at least one transaction (the system deposit).

Copy link
Contributor

@geoknee geoknee Oct 27, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

I think the system deposit (l1 attribs) tx would be skipped below because of the isDepositTx check.

I'm generally unsure about this requirement, it does match the condition in the calling function DeriveFields(), but I don't understand it. We already skip deposit transactions in the loop below, so I don't know why we can't just call this unconditionally (perhaps it is a performance optimisation?) And what if we had multiple deposits in a block, we would still call this function and potentially skip all the transactions in the loop.

Copy link
Contributor

@joshklop joshklop Oct 27, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

so I don't know why we can't just call this unconditionally (perhaps it is a performance optimisation?)

It would panic since we read txs[0].Data(). Ofc we can guard against this by checking the length of the txs slice, but I don't want to bikeshed this comment too much: as long as it accurately documents the behavior I'm happy.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah completely empty tx slice would be problematic, but shouldn't happen on an optimism chain, so the tx length check in the caller seems redundant to me still. Doesn't need to block this PR since I think it preserves original semantics, but I would like to either understand this better or circle back and change it.

Copy link
Contributor

Choose a reason for hiding this comment

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

The genesis block might be an edge case.

func (rs Receipts) deriveOPStackFields(config *params.ChainConfig, blockTime uint64, txs []*Transaction) error {
l1AttributesData := txs[0].Data()
gasParams, err := extractL1GasParams(config, blockTime, l1AttributesData)
if err != nil {
return fmt.Errorf("failed to extract L1 gas params: %w", err)
}

var daFootprintGasScalar uint64
if config.IsJovian(blockTime) {
scalar, err := ExtractDAFootprintGasScalar(l1AttributesData)
if err != nil {
return fmt.Errorf("failed to extract DA footprint gas scalar: %w", err)
}
daFootprintGasScalar = uint64(scalar)
}

for i := range rs {
if txs[i].IsDepositTx() {
continue
}
rs[i].L1GasPrice = gasParams.l1BaseFee
rs[i].L1BlobBaseFee = gasParams.l1BlobBaseFee
rcd := txs[i].RollupCostData()
rs[i].L1Fee, rs[i].L1GasUsed = gasParams.costFunc(rcd)
rs[i].FeeScalar = gasParams.feeScalar
rs[i].L1BaseFeeScalar = u32ptrTou64ptr(gasParams.l1BaseFeeScalar)
rs[i].L1BlobBaseFeeScalar = u32ptrTou64ptr(gasParams.l1BlobBaseFeeScalar)
if gasParams.operatorFeeScalar != nil && gasParams.operatorFeeConstant != nil && (*gasParams.operatorFeeScalar != 0 || *gasParams.operatorFeeConstant != 0) {
rs[i].OperatorFeeScalar = u32ptrTou64ptr(gasParams.operatorFeeScalar)
rs[i].OperatorFeeConstant = gasParams.operatorFeeConstant
}
if config.IsJovian(blockTime) {
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: we can have isJovian := config.IsJovian(blockTime) at the top of the function to avoid checking on every receipt.

rs[i].DAFootprintGasScalar = &daFootprintGasScalar
Copy link
Contributor

Choose a reason for hiding this comment

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

It feels a tad wasteful to include the scalar (a block-level attribute) in every receipt when it can be statically inferred from the BlobGasUsed and the transaction.

I see we include other block-level attributes like the operatorFeeScalar in receipts, although I'm not sure if they can be inferred like the daFootprintGasScalar.

Any thoughts?

Copy link
Contributor

Choose a reason for hiding this comment

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

Just noting also that we should specify whatever behaviour we want to enshrine in the specs, as was done here https://specs.optimism.io/protocol/isthmus/exec-engine.html#receipts.

rs[i].BlobGasUsed = daFootprintGasScalar * rcd.EstimatedDASize().Uint64()
}
}
return nil
}
Loading