core/types: add EffectiveGasPrice in Receipt #26250
Conversation
LaGregance
requested review from
holiman,
karalabe and
rjl493456442
as code owners
| TxHash common.Hash `json:"transactionHash" gencodec:"required"` | ||
| ContractAddress common.Address `json:"contractAddress"` | ||
| GasUsed uint64 `json:"gasUsed" gencodec:"required"` | ||
| EffectiveGasPrice *big.Int `json:"effectiveGasPrice,omitempty"` |
There was a problem hiding this comment.
Where is the value being set?
There was a problem hiding this comment.
Inside UnmarshalJSON in the file gen_receipt_json.go
There was a problem hiding this comment.
He means within execution. At some point, probably during state processing, you need to set the effective gas price. The way it is right now, effective gas is never set, so it's 0 for all receipts
There was a problem hiding this comment.
I think it's set in internal/ethapi/api.go:1641 in the function GetTransactionReceipt.
Personally I checked with my Node (Quicknode) that is running on Geth 1.10.26 and it's perfectly work.
There was a problem hiding this comment.
I see you add this field so you can use it in ethclient.TransactionReceipt()
There was a problem hiding this comment.
I guess we will just have to pass in either the header, or the basefee, as a parameter into DeriveFields.
There was a problem hiding this comment.
Ok, I will try to doing this this or next week.
Do you agree that if we pass the block header as parameter we can remove hash & number params ?
There was a problem hiding this comment.
Yes, they can be removed in that case. Note: I haven't checked myself if the header is easily available at all call sites of DeriveFields. If it is, we're good. Otherwise we'll need to consider it further.
There was a problem hiding this comment.
Not checked too, I keep you updated as soon as I know
There was a problem hiding this comment.
Ok I think everything is done, I have just a little concern about the comment I made to accessors_chain.go that you can see below
fjl
changed the title
| header := ReadHeader(db, hash, number) | ||
| var baseFee *big.Int | ||
| if header == nil { | ||
| baseFee = big.NewInt(0) | ||
| } else { | ||
| baseFee = header.BaseFee | ||
| } |
There was a problem hiding this comment.
Not sure if it's the good way, but accessors_chain_test.go fail at line 646 if we return nil in case the header does not exists.
|
Hi, the pipeline seems to be stuck, someone know how to make it run ? |
|
We had an issue with it today, just re-push and it will start the build again. |
|
Are there any updates on this? Seems like a fairly big and obvious thing to be missing from the receipt |
| // The effective gas price can be calculated based on baseFee for EIP1559 transactions | ||
| if txs[i].Type() == 2 { | ||
| rs[i].EffectiveGasPrice = new(big.Int).Add(baseFee, txs[i].EffectiveGasTipValue(baseFee)) | ||
| } else { | ||
| rs[i].EffectiveGasPrice = new(big.Int).Set(txs[i].GasPrice()) | ||
| } |
There was a problem hiding this comment.
I think this logic will bitrot if we leave it here (as soon as we add a new tx type). I'd rather have a method
func (tx *Transaction) EffectiveGasPrice(baseFee *big.Int) (*big.Int, error)
And that method should use the methods that the transaction types expose. If we do it this way, we can make it so any new type will be 'forced' to handle it
|
Superseded by #26713 |
6 participants
Since the new EIP-1559 transaction format, we need to have a way to get the effective gas price spent by the transaction. This data is already available in the receipt returned by the node so only few change are needed.
This PR will mainly help developers that use go-ethereum as library in their go project to interact with the blockchain.
Basically, I just added the field
EffectiveGasPricetotypes.Receiptand manage it inMarshalJSON&UnmarshalJSONfunctions.