Skip to content
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

op-reth: Incorrect and missing fields in eth_getTransactionReceipt #6547

Closed
1 task done
0xZerohero opened this issue Feb 11, 2024 · 7 comments · Fixed by #6607
Closed
1 task done

op-reth: Incorrect and missing fields in eth_getTransactionReceipt #6547

0xZerohero opened this issue Feb 11, 2024 · 7 comments · Fixed by #6607
Assignees
Labels
A-op-reth Related to Optimism and op-reth C-bug An unexpected or incorrect behavior

Comments

@0xZerohero
Copy link
Contributor

Describe the bug

When calling eth_getTransactionReceipt against a non deposit transaction, l1Fee, l1GasUsed fields are missing and l1FeeScalar is always 0

--- /tmp/geth_res.json  2024-02-11 11:19:39.834880064 +0200
+++ /tmp/reth_res.json  2024-02-11 11:18:46.818933728 +0200
@@ -6,10 +6,8 @@
    "effectiveGasPrice" : "0xf438a",
    "from" : "0xcf5254128a5b0565c74903c948b571884a6247a1",
    "gasUsed" : "0x3c0fa",
-   "l1Fee" : "0x4319500e1c27",
-   "l1FeeScalar" : "0.684",
+   "l1FeeScalar" : "0x0",
    "l1GasPrice" : "0x64f4f7c52",
-   "l1GasUsed" : "0xf8c",
    "logs" : [
       {
          "address" : "0x4200000000000000000000000000000000000006",

When calling eth_getTransactionReceipt against a deposit transaction, depositReceiptVersion field is missing

--- /tmp/geth_deposit_res.json  2024-02-11 11:28:55.094405058 +0200
+++ /tmp/reth_deposit_res.json  2024-02-11 11:28:17.254395717 +0200
@@ -4,7 +4,6 @@
    "contractAddress" : null,
    "cumulativeGasUsed" : "0x22dd7",
    "depositNonce" : "0x23e86",
-   "depositReceiptVersion" : "0x1",
    "effectiveGasPrice" : "0x0",
    "from" : "0x977f82a600a1414e583f7f13623f1ac5d58b1c0b",
    "gasUsed" : "0x16892",

Steps to reproduce

  1. cast rpc -r reth_node eth_getTransactionReceipt "0xc76c50bee5cffb7e6a4fea31d0fd360369037d26ff010bec2270916d086aa365" | json_pp > /tmp/reth_res.json
  2. cast rpc -r geth_node eth_getTransactionReceipt "0xc76c50bee5cffb7e6a4fea31d0fd360369037d26ff010bec2270916d086aa365" | json_pp > /tmp/geth_res.json
  3. diff -uN /tmp/geth_res.json /tmp/reth_res.json
  4. cast rpc -r reth_node eth_getTransactionReceipt "0x3321f4dd837da55fad50a828bf093e0a539e3592c3372084e84a713bee51d412" | json_pp > /tmp/reth_deposit_res.json
  5. cast rpc -r geth_node eth_getTransactionReceipt "0x3321f4dd837da55fad50a828bf093e0a539e3592c3372084e84a713bee51d412" | json_pp > /tmp/geth_deposit_res.json
  6. diff -uN /tmp/geth_deposit_res.json /tmp/reth_deposit_res.json

Node logs

No response

Platform(s)

Linux (x86)

What version/commit are you on?

0.1.0-alpha.17

What database version are you on?

Current database version: 1
Local database version: 1

What type of node are you running?

Archive (default)

What prune config do you use, if any?

No response

If you've built Reth from source, provide the full command you used

No response

Code of Conduct

  • I agree to follow the Code of Conduct
@0xZerohero 0xZerohero added C-bug An unexpected or incorrect behavior S-needs-triage This issue needs to be labelled labels Feb 11, 2024
@0xZerohero
Copy link
Contributor Author

Looks like a typo
fcdd31a
https://github.com/0xZerohero/reth/blob/8cfa5efe62e79923b317d9685fc9f8205d5a92b5/crates/rpc/rpc/src/eth/api/transactions.rs#L1118

This should fix the issue with the missing l1Fee and l1GasUsed fields

--- a/crates/rpc/rpc/src/eth/api/transactions.rs
+++ b/crates/rpc/rpc/src/eth/api/transactions.rs
@@ -1115,7 +1115,7 @@ where
         let mut envelope_buf = bytes::BytesMut::new();
         tx.encode_enveloped(&mut envelope_buf);

-        let (l1_fee, l1_data_gas) = if tx.is_deposit() {
+        let (l1_fee, l1_data_gas) = if !tx.is_deposit() {
             let inner_l1_fee = l1_block_info
                 .l1_tx_data_fee(
                     &self.inner.provider.chain_spec(),

@0xZerohero
Copy link
Contributor Author

l1FeeScalar is convertedd to Float in op-geth

feeScalar := big.NewFloat(float64(scalar.Uint64() / 1e6)) 

in op-reth it's U256. That's why 684_000 / 1_000_000 = 0

op_fields.l1_fee_scalar = Some(l1_block_info.l1_base_fee_scalar.div(U256::from(1_000_000)));

@mattsse
Copy link
Collaborator

mattsse commented Feb 11, 2024

nice finds, ty!

do you want to submit the fixes?

@mattsse mattsse added A-op-reth Related to Optimism and op-reth and removed S-needs-triage This issue needs to be labelled labels Feb 11, 2024
@0xZerohero
Copy link
Contributor Author

yes, i'll do it!

@0xZerohero
Copy link
Contributor Author

Hmm to fix the feeScalar issue, the best way IMO involves changing the OptimismTransactionReceiptFields struct, which is in the alloy-rpc-types crate.

- pub l1_fee_scalar: Option<U256>,
+ pub l1_fee_scalar: Option<f64>,

Should I also create a PR there ?
@mattsse

@mattsse
Copy link
Collaborator

mattsse commented Feb 12, 2024

yeah looks like this is a stringified float

we need to change the OptimismTransactionReceiptFields type in https://github.com/alloy-rs/alloy

so please open a PR there

@0xZerohero
Copy link
Contributor Author

I was thinking of fixing the bug about the missing "depositReceiptVersion" field in #6575, but you guys are super fast 🙂

I'll open new PR for it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-op-reth Related to Optimism and op-reth C-bug An unexpected or incorrect behavior
Projects
Archived in project
2 participants