ethereum-optimism / optimistic-specs

Optimistic: Bedrock, is a protocol that strives to be an extremely simple optimistic rollup that maintains 1:1 compatibility with Ethereum
MIT License
167 stars 35 forks source link

Fix L2 Output Timestamps #416

Closed trianglesphere closed 2 years ago

trianglesphere commented 2 years ago

Description There was an off by one error in the L2 Output Oracle contract in the timestamp to block number conversion. In addition, the L2 Output Submitter (op proposer) did not recognize that there was a mismatch between the timestamp/block number in the header and the timestamp/block number from the contract.

This bug causes problems when trying to do withdrawals.

Metadata

codecov-commenter commented 2 years ago

Codecov Report

Merging #416 (e56e6ca) into main (55db896) will decrease coverage by 0.86%. The diff coverage is 25.00%.

@@            Coverage Diff             @@
##             main     #416      +/-   ##
==========================================
- Coverage   53.12%   52.26%   -0.87%     
==========================================
  Files          62       63       +1     
  Lines        6500     6703     +203     
==========================================
+ Hits         3453     3503      +50     
- Misses       2608     2764     +156     
+ Partials      439      436       -3     
Impacted Files Coverage Δ
l2os/bindings/l2oo/l2_output_oracle.go 8.60% <ø> (ø)
l2os/drivers/l2output/driver.go 65.19% <14.28%> (-2.05%) :arrow_down:
opnode/rollup/driver/step.go 61.37% <100.00%> (ø)
opnode/cmd/main.go 7.89% <0.00%> (-0.56%) :arrow_down:
opnode/service.go 0.00% <0.00%> (ø)
opnode/cmd/stateviz/main.go 0.00% <0.00%> (ø)
opnode/node/node.go 60.15% <0.00%> (+0.15%) :arrow_up:
opnode/p2p/gossip.go 67.36% <0.00%> (+1.04%) :arrow_up:
l2os/txmgr/txmgr.go 93.78% <0.00%> (+2.48%) :arrow_up:
opnode/rollup/driver/state.go 75.24% <0.00%> (+4.00%) :arrow_up:
... and 2 more

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 55db896...e56e6ca. Read the comment docs.

trianglesphere commented 2 years ago

This is ready for a re-review. The test is failing because the sequencer is reorging on the L1 Info transaction between when it sequences it and when it verifies it