feat: eth_getTransactionReceipt and eth_getBlockReceipts RPCs#150
feat: eth_getTransactionReceipt and eth_getBlockReceipts RPCs#150JonathanOppenheimer wants to merge 32 commits intoarr4n/recoveryfrom
eth_getTransactionReceipt and eth_getBlockReceipts RPCs#150Conversation
eth_getTransactionReceipt and eth_getBlockReceipts RPCs
saexec/execution.go
Outdated
| // The [types.Header] that we pass to [core.ApplyTransaction] is adjusted to | ||
| // reduce worst‑case gas, which changes the block hash. DeriveFields recomputes | ||
| // receipt/log metadata (e.g., block hash, effective gas price, etc.) against the final | ||
| // block header so cached receipts match the DB path. | ||
| var blobGasPrice *big.Int | ||
| if header.ExcessBlobGas != nil { | ||
| blobGasPrice = eip4844.CalcBlobFee(*header.ExcessBlobGas) | ||
| } | ||
| if err := receipts.DeriveFields(e.chainConfig, b.Hash(), b.NumberU64(), header.Time, header.BaseFee, blobGasPrice, b.Transactions()); err != nil { | ||
| return err | ||
| } | ||
|
|
There was a problem hiding this comment.
This was surfaced after writing tests that compare the receipts of cached blocks vs settled blocks, and we found that the effective gas price was missing from the receipts of blocks in the cache, but not from those of the database. There was an inconsistency here with how receipts are provided from the database and manually resetting the block hash pointed to that.
In geth, receipts are only retrieved from settled blocks, which have all their fields properly derived via DeriveFields(). We match that behavior here.
Thanks to @alarso16 for helping me figure this out.
|
|
||
| func TestReceiptAPIs(t *testing.T) { | ||
| opt, vmTime := withVMTime(t, time.Unix(saeparams.TauSeconds, 0)) | ||
| ctx, sut := newSUT(t, 5, opt) |
There was a problem hiding this comment.
Creating the sut here is acceptable from my understanding. We create and use 5 accounts to avoid logging logging nonce stuff.
b28849b to
13c89e4
Compare
|
The |
Yup all good, I'm going to leave this as a draft until your base PR is merged |
Signed-off-by: Jonathan Oppenheimer <147infiniti@gmail.com>
26be9e3 to
c82e54e
Compare
c82e54e to
ac595dc
Compare
c6c280b to
9cf6641
Compare
Signed-off-by: Jonathan Oppenheimer <147infiniti@gmail.com>
9cf6641 to
6a6a6a8
Compare
Signed-off-by: Jonathan Oppenheimer <147infiniti@gmail.com>
sae/rpc_test.go
Outdated
| blockMultiTxs := sut.createAndAcceptBlock(t, txMulti1, txMulti2, txMulti3) | ||
| require.NoErrorf(t, blockMultiTxs.WaitUntilExecuted(ctx), "%T.WaitUntilExecuted()", blockMultiTxs) | ||
|
|
||
| // Block 4: Triggers settlement of block 2 |
There was a problem hiding this comment.
I think block 3 settled block 4
There was a problem hiding this comment.
We don't call WaitUntilSettled until here.
sae/rpc_test.go
Outdated
|
|
||
| var receipt *types.Receipt | ||
| err := sut.CallContext(ctx, &receipt, "eth_getTransactionReceipt", txSettled.Hash()) | ||
| require.ErrorContains(t, err, "reading execution-result base fee") |
There was a problem hiding this comment.
Why not use require.ErrorIs?
There was a problem hiding this comment.
We need Tsvetan's PR to clean this up.
ca81fd1 to
433de16
Compare
433de16 to
36b0e29
Compare
9129673 to
bc709d1
Compare
Signed-off-by: Jonathan Oppenheimer <147infiniti@gmail.com>
512ee0f to
6774ac3
Compare
| // modified to reduce gas price from the worst-case value agreed by | ||
| // consensus. This changes the hash, which is what is copied to receipts | ||
| // and logs. | ||
| receipt.BlockHash = b.Hash() |
There was a problem hiding this comment.
The below block does this -- confirmed with Cey that we can remove this.
36b0e29 to
c9a3080
Compare
Signed-off-by: Jonathan Oppenheimer <147infiniti@gmail.com>
36787bf to
9e046d4
Compare
Co-authored-by: Austin Larson <78000745+alarso16@users.noreply.github.com> Signed-off-by: Jonathan Oppenheimer <147infiniti@gmail.com>
eef7035 to
1baee20
Compare
1baee20 to
333e8ae
Compare
| @@ -316,6 +328,23 @@ func readByHash[T any](b *ethAPIBackend, hash common.Hash, read canonicalReader[ | |||
|
|
|||
| var errFutureBlockNotResolved = errors.New("not accepted yet") | |||
There was a problem hiding this comment.
Why is this variable declared here. I'd move it but it would create an irrelevant diff. Seems very strange to declare an error after it is used.
Closes #91
This PR adds receipt retrieval for
eth_getTransactionReceiptandeth_getBlockReceipts. To do so, it adds the following backend methods:GetReceipts(ctx, hash)- Cache-first receipt lookup (checksvm.blocks, falls back to DB)BlockByNumberOrHash(ctx, blockNrOrHash)- Unified block lookupHeaderByNumberOrHash(ctx, blockNrOrHash)- Unified header lookupreadByNumberOrHash[T]()- Generic helper for number-or-hash lookupsnecessary to support these RPCs. I also added a (fairly comprehensive test),
TestReceiptAPIswhich covers:cmp.Diffwithcmputils.Receipts()TransactionIndexordering)eth_getBlockReceiptsby hash and by number produce equivalent resultsWhile working on this PR, I surfaced some inconsistencies about the fields of receipts, which I fixed in
execution.go. See comments below.