Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## develop #10143 +/- ##
============================================
- Coverage 42.33% 29.22% -13.11%
============================================
Files 73 31 -42
Lines 4845 2898 -1947
Branches 766 614 -152
============================================
- Hits 2051 847 -1204
+ Misses 2684 1976 -708
+ Partials 110 75 -35
Flags with carried forward coverage won't be shown. Click here to find out more. |
|
Looks really great on my first pass. Next week I'd like to re-review the spec and give a more security-focused review of the code and tests in this PR |
|
@mds1 Needs versions such that they are |
5c6a6c1 to
095375f
Compare
|
implemented comments here: #10219 |
015aa3e to
7ec7956
Compare
|
Semgrep found 6
Named return arguments to functions must be appended with an underscore ( |
|
@mds1 This branch has been rebased on top of your codesize checks and also includes all of the recent specs changes ( |
02085fe to
50b6f58
Compare
Custom gas token semantics are strengthened and cleaned up to ensure invariants in spec are held true.
c9c91eb to
e532b85
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
Out of diff range and nitpick comments (1)
packages/contracts-bedrock/test/L1/OptimismPortal.t.sol (1)
Line range hint
697-697: Avoid usingblockhash(block.number)as it always returns 0.- blockhash(block.number) + blockhash(block.number - 1) // Use the hash of the previous block
There was a problem hiding this comment.
Actionable comments posted: 0
Out of diff range and nitpick comments (1)
packages/contracts-bedrock/test/L1/OptimismPortal.t.sol (1)
Line range hint
697-697: Avoid usingblockhash(block.number)as it always returns 0.- optimismPortal.l2Oracle().proposeL2Output(proposal.outputRoot, optimismPortal.l2Oracle().nextBlockNumber(), blockhash(block.number), block.number); + optimismPortal.l2Oracle().proposeL2Output(proposal.outputRoot, optimismPortal.l2Oracle().nextBlockNumber(), blockhash(block.number - 1), block.number);Using
blockhash(block.number)results in a value of 0, which is not useful. Useblockhash(block.number - 1)instead to get the hash of the most recent block.
Description
Implements the custom gas token spec found in ethereum-optimism/specs#111
Includes the full feature set and extensive test coverage.