search

testinprod-io / op-erigon

Optimism implementation on the efficiency frontier
https://op-erigon.testinprod.io
GNU Lesser General Public License v3.0
87 stars 15 forks source link

Fix deposit receipt nonce #149

Closed jyellick closed 8 months ago

jyellick commented 8 months ago

The receipts stored in the DB do not contain the nonce for the deposit transactions. This change triggers the receipt retrieval to go through the more full path which includes the deposit nonce.

Additionally, it fixes the pending RPC block number to correspond to the latest block number as is the behavior in op-geth.

ImTei commented 8 months ago

@jyellick Can you provide me more detailed context about the change, with an example of RPC response if possible?

jyellick commented 8 months ago

For the RPC please see https://github.com/ethereum-optimism/optimism/blob/abfc1e1f37a89405bacd08a3bb6363250d3f68f5/op-e2e/system_test.go#L1518-L1558

For a bit more history, we forked Erigon to use as the sequencer for our network, implementing the Bedrock protocol. As part of this, we worked through getting the op-e2e tests passing. Eventually, we found this fork being maintained as well and we're now syncing some of the changes here to our repo, and trying to push back some the changes from our repo here.

So please expect quite a few more of these in the near future. All of them are backed by the op-e2e tests.

ImTei commented 8 months ago

@jyellick, the change for pending blocks looks to make sense. but I wanna ask you the example of RPC response for deposit nonce issue, because I tested with op-erigon and it shows the correct nonce for deposit txs.

jyellick commented 8 months ago

Sorry for the long delay -- I ended up needing to write a test in the e2e to convince myself that the receipt changes were not needed. Looks like they are indeed superfluous and a holdover from an earlier version that is no longer needed. I've removed them from this PR.

ImTei commented 8 months ago

@jyellick Thank you for confirming!