Commit d36a4fd4 authored by mergify[bot]'s avatar mergify[bot] Committed by GitHub

Merge pull request #3737 from ethereum-optimism/jg/fix_conf_depth

op-node: Skip conf depth check if L1 head is empty
parents 348505aa 815c7a85
...@@ -6,6 +6,7 @@ import ( ...@@ -6,6 +6,7 @@ import (
"github.com/ethereum-optimism/optimism/op-node/eth" "github.com/ethereum-optimism/optimism/op-node/eth"
"github.com/ethereum-optimism/optimism/op-node/rollup/derive" "github.com/ethereum-optimism/optimism/op-node/rollup/derive"
"github.com/ethereum/go-ethereum" "github.com/ethereum/go-ethereum"
"github.com/ethereum/go-ethereum/common"
) )
// confDepth is an util that wraps the L1 input fetcher used in the pipeline, // confDepth is an util that wraps the L1 input fetcher used in the pipeline,
...@@ -30,7 +31,9 @@ func (c *confDepth) L1BlockRefByNumber(ctx context.Context, num uint64) (eth.L1B ...@@ -30,7 +31,9 @@ func (c *confDepth) L1BlockRefByNumber(ctx context.Context, num uint64) (eth.L1B
// TODO: performance optimization: buffer the l1Unsafe, invalidate any reorged previous buffer content, // TODO: performance optimization: buffer the l1Unsafe, invalidate any reorged previous buffer content,
// and instantly return the origin by number from the buffer if we can. // and instantly return the origin by number from the buffer if we can.
if num == 0 || c.depth == 0 || num+c.depth <= c.l1Head().Number { // Don't apply the conf depth is l1Head is empty (as it is during the startup case before the l1State is initialized).
l1Head := c.l1Head()
if num == 0 || c.depth == 0 || num+c.depth <= l1Head.Number || l1Head.Hash == (common.Hash{}) {
return c.L1Fetcher.L1BlockRefByNumber(ctx, num) return c.L1Fetcher.L1BlockRefByNumber(ctx, num)
} }
return eth.L1BlockRef{}, ethereum.NotFound return eth.L1BlockRef{}, ethereum.NotFound
......
...@@ -9,11 +9,15 @@ import ( ...@@ -9,11 +9,15 @@ import (
"github.com/ethereum-optimism/optimism/op-node/eth" "github.com/ethereum-optimism/optimism/op-node/eth"
"github.com/ethereum-optimism/optimism/op-node/testutils" "github.com/ethereum-optimism/optimism/op-node/testutils"
"github.com/ethereum/go-ethereum" "github.com/ethereum/go-ethereum"
"github.com/ethereum/go-ethereum/common"
) )
var exHash = common.Hash{0xff}
type confTest struct { type confTest struct {
name string name string
head uint64 head uint64
hash common.Hash // hash of head block
req uint64 req uint64
depth uint64 depth uint64
pass bool pass bool
...@@ -21,7 +25,7 @@ type confTest struct { ...@@ -21,7 +25,7 @@ type confTest struct {
func (ct *confTest) Run(t *testing.T) { func (ct *confTest) Run(t *testing.T) {
l1Fetcher := &testutils.MockL1Source{} l1Fetcher := &testutils.MockL1Source{}
l1Head := eth.L1BlockRef{Number: ct.head} l1Head := eth.L1BlockRef{Number: ct.head, Hash: ct.hash}
l1HeadGetter := func() eth.L1BlockRef { return l1Head } l1HeadGetter := func() eth.L1BlockRef { return l1Head }
cd := NewConfDepth(ct.depth, l1HeadGetter, l1Fetcher) cd := NewConfDepth(ct.depth, l1HeadGetter, l1Fetcher)
...@@ -43,18 +47,19 @@ func TestConfDepth(t *testing.T) { ...@@ -43,18 +47,19 @@ func TestConfDepth(t *testing.T) {
// note: we're not testing overflows. // note: we're not testing overflows.
// If a request is large enough to overflow the conf depth check, it's not returning anything anyway. // If a request is large enough to overflow the conf depth check, it's not returning anything anyway.
testCases := []confTest{ testCases := []confTest{
{name: "zero conf future", head: 4, req: 5, depth: 0, pass: true}, {name: "zero conf future", head: 4, hash: exHash, req: 5, depth: 0, pass: true},
{name: "zero conf present", head: 4, req: 4, depth: 0, pass: true}, {name: "zero conf present", head: 4, hash: exHash, req: 4, depth: 0, pass: true},
{name: "zero conf past", head: 4, req: 4, depth: 0, pass: true}, {name: "zero conf past", head: 4, hash: exHash, req: 4, depth: 0, pass: true},
{name: "one conf future", head: 4, req: 5, depth: 1, pass: false}, {name: "one conf future", head: 4, hash: exHash, req: 5, depth: 1, pass: false},
{name: "one conf present", head: 4, req: 4, depth: 1, pass: false}, {name: "one conf present", head: 4, hash: exHash, req: 4, depth: 1, pass: false},
{name: "one conf past", head: 4, req: 3, depth: 1, pass: true}, {name: "one conf past", head: 4, hash: exHash, req: 3, depth: 1, pass: true},
{name: "two conf future", head: 4, req: 5, depth: 2, pass: false}, {name: "two conf future", head: 4, hash: exHash, req: 5, depth: 2, pass: false},
{name: "two conf present", head: 4, req: 4, depth: 2, pass: false}, {name: "two conf present", head: 4, hash: exHash, req: 4, depth: 2, pass: false},
{name: "two conf not like 1", head: 4, req: 3, depth: 2, pass: false}, {name: "two conf not like 1", head: 4, hash: exHash, req: 3, depth: 2, pass: false},
{name: "two conf pass", head: 4, req: 2, depth: 2, pass: true}, {name: "two conf pass", head: 4, hash: exHash, req: 2, depth: 2, pass: true},
{name: "easy pass", head: 100, req: 20, depth: 5, pass: true}, {name: "easy pass", head: 100, hash: exHash, req: 20, depth: 5, pass: true},
{name: "genesis case", head: 0, req: 0, depth: 4, pass: true}, {name: "genesis case", head: 0, hash: exHash, req: 0, depth: 4, pass: true},
{name: "no L1 state", req: 10, depth: 4, pass: true},
} }
for _, tc := range testCases { for _, tc := range testCases {
t.Run(tc.name, tc.Run) t.Run(tc.name, tc.Run)
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment