Commit dc01c039 authored by clabby's avatar clabby Committed by GitHub

Merge pull request #8558 from ethereum-optimism/aj/off-by-one

op-challenger: Fix calculation of bottom game depth
parents 3eada667 e75e86a6
......@@ -86,12 +86,12 @@ func registerOutputAlphabet(
return nil, err
}
prestateProvider := outputs.NewPrestateProvider(ctx, logger, rollupClient, prestateBlock)
splitDepth, err := contract.GetSplitDepth(ctx)
if err != nil {
return nil, err
}
creator := func(ctx context.Context, logger log.Logger, gameDepth uint64, dir string) (faultTypes.TraceAccessor, error) {
splitDepth, err := contract.GetSplitDepth(ctx)
if err != nil {
return nil, err
}
accessor, err := outputs.NewOutputAlphabetTraceAccessor(ctx, logger, m, prestateProvider, rollupClient, gameDepth, splitDepth, prestateBlock, poststateBlock)
accessor, err := outputs.NewOutputAlphabetTraceAccessor(logger, m, prestateProvider, rollupClient, splitDepth, prestateBlock, poststateBlock)
if err != nil {
return nil, err
}
......@@ -129,7 +129,7 @@ func registerOutputCannon(
if err != nil {
return nil, fmt.Errorf("failed to load split depth: %w", err)
}
accessor, err := outputs.NewOutputCannonTraceAccessor(ctx, logger, m, cfg, l2Client, contract, prestateProvider, rollupClient, dir, gameDepth, splitDepth, prestateBlock, poststateBlock)
accessor, err := outputs.NewOutputCannonTraceAccessor(logger, m, cfg, l2Client, contract, prestateProvider, rollupClient, dir, splitDepth, prestateBlock, poststateBlock)
if err != nil {
return nil, err
}
......
......@@ -14,20 +14,17 @@ import (
)
func NewOutputAlphabetTraceAccessor(
ctx context.Context,
logger log.Logger,
m metrics.Metricer,
prestateProvider types.PrestateProvider,
rollupClient OutputRollupClient,
gameDepth uint64,
splitDepth uint64,
prestateBlock uint64,
poststateBlock uint64,
) (*trace.Accessor, error) {
bottomDepth := gameDepth - splitDepth
outputProvider := NewTraceProviderFromInputs(logger, prestateProvider, rollupClient, splitDepth, prestateBlock, poststateBlock)
alphabetCreator := func(ctx context.Context, localContext common.Hash, agreed contracts.Proposal, claimed contracts.Proposal) (types.TraceProvider, error) {
provider := alphabet.NewTraceProvider(localContext.Hex(), bottomDepth)
alphabetCreator := func(ctx context.Context, localContext common.Hash, depth uint64, agreed contracts.Proposal, claimed contracts.Proposal) (types.TraceProvider, error) {
provider := alphabet.NewTraceProvider(localContext.Hex(), depth)
return provider, nil
}
cache := NewProviderCache(m, "output_alphabet_provider", alphabetCreator)
......
......@@ -17,7 +17,6 @@ import (
)
func NewOutputCannonTraceAccessor(
ctx context.Context,
logger log.Logger,
m metrics.Metricer,
cfg *config.Config,
......@@ -26,21 +25,19 @@ func NewOutputCannonTraceAccessor(
prestateProvider types.PrestateProvider,
rollupClient OutputRollupClient,
dir string,
gameDepth uint64,
splitDepth uint64,
prestateBlock uint64,
poststateBlock uint64,
) (*trace.Accessor, error) {
bottomDepth := gameDepth - splitDepth
outputProvider := NewTraceProviderFromInputs(logger, prestateProvider, rollupClient, splitDepth, prestateBlock, poststateBlock)
cannonCreator := func(ctx context.Context, localContext common.Hash, agreed contracts.Proposal, claimed contracts.Proposal) (types.TraceProvider, error) {
cannonCreator := func(ctx context.Context, localContext common.Hash, depth uint64, agreed contracts.Proposal, claimed contracts.Proposal) (types.TraceProvider, error) {
logger := logger.New("pre", agreed.OutputRoot, "post", claimed.OutputRoot, "localContext", localContext)
subdir := filepath.Join(dir, localContext.Hex())
localInputs, err := cannon.FetchLocalInputsFromProposals(ctx, contract, l2Client, agreed, claimed)
if err != nil {
return nil, fmt.Errorf("failed to fetch cannon local inputs: %w", err)
}
provider := cannon.NewTraceProvider(logger, m, cfg, localContext, localInputs, subdir, bottomDepth)
provider := cannon.NewTraceProvider(logger, m, cfg, localContext, localInputs, subdir, depth)
return provider, nil
}
......
......@@ -14,12 +14,12 @@ type ProviderCache struct {
creator ProposalTraceProviderCreator
}
func (c *ProviderCache) GetOrCreate(ctx context.Context, localContext common.Hash, agreed contracts.Proposal, claimed contracts.Proposal) (types.TraceProvider, error) {
func (c *ProviderCache) GetOrCreate(ctx context.Context, localContext common.Hash, depth uint64, agreed contracts.Proposal, claimed contracts.Proposal) (types.TraceProvider, error) {
provider, ok := c.cache.Get(localContext)
if ok {
return provider, nil
}
provider, err := c.creator(ctx, localContext, agreed, claimed)
provider, err := c.creator(ctx, localContext, depth, agreed, claimed)
if err != nil {
return nil, err
}
......
......@@ -23,9 +23,10 @@ func TestProviderCache(t *testing.T) {
L2BlockNumber: big.NewInt(35),
OutputRoot: common.Hash{0xcc},
}
depth := uint64(6)
var createdProvider types.TraceProvider
creator := func(ctx context.Context, localContext common.Hash, agreed contracts.Proposal, claimed contracts.Proposal) (types.TraceProvider, error) {
createdProvider = alphabet.NewTraceProvider("abcdef", 6)
creator := func(ctx context.Context, localContext common.Hash, depth uint64, agreed contracts.Proposal, claimed contracts.Proposal) (types.TraceProvider, error) {
createdProvider = alphabet.NewTraceProvider("abcdef", depth)
return createdProvider, nil
}
localContext1 := common.Hash{0xdd}
......@@ -34,20 +35,20 @@ func TestProviderCache(t *testing.T) {
cache := NewProviderCache(metrics.NoopMetrics, "test", creator)
// Create on first call
provider1, err := cache.GetOrCreate(context.Background(), localContext1, agreed, claimed)
provider1, err := cache.GetOrCreate(context.Background(), localContext1, depth, agreed, claimed)
require.NoError(t, err)
require.Same(t, createdProvider, provider1, "should return created trace provider")
// Return the cached provider on subsequent calls.
createdProvider = nil
cached, err := cache.GetOrCreate(context.Background(), localContext1, agreed, claimed)
cached, err := cache.GetOrCreate(context.Background(), localContext1, depth, agreed, claimed)
require.NoError(t, err)
require.Same(t, provider1, cached, "should return exactly the same instance from cache")
require.Nil(t, createdProvider)
// Create a new provider when the local context is different
createdProvider = nil
otherProvider, err := cache.GetOrCreate(context.Background(), localContext2, agreed, claimed)
otherProvider, err := cache.GetOrCreate(context.Background(), localContext2, depth, agreed, claimed)
require.NoError(t, err)
require.Same(t, otherProvider, createdProvider, "should return newly created trace provider")
require.NotSame(t, otherProvider, provider1, "should not use cached provider for different local context")
......@@ -56,20 +57,20 @@ func TestProviderCache(t *testing.T) {
func TestProviderCache_DoNotCacheErrors(t *testing.T) {
callCount := 0
providerErr := errors.New("boom")
creator := func(ctx context.Context, localContext common.Hash, agreed contracts.Proposal, claimed contracts.Proposal) (types.TraceProvider, error) {
creator := func(ctx context.Context, localContext common.Hash, depth uint64, agreed contracts.Proposal, claimed contracts.Proposal) (types.TraceProvider, error) {
callCount++
return nil, providerErr
}
localContext1 := common.Hash{0xdd}
cache := NewProviderCache(metrics.NoopMetrics, "test", creator)
provider, err := cache.GetOrCreate(context.Background(), localContext1, contracts.Proposal{}, contracts.Proposal{})
provider, err := cache.GetOrCreate(context.Background(), localContext1, 6, contracts.Proposal{}, contracts.Proposal{})
require.Nil(t, provider)
require.ErrorIs(t, err, providerErr)
require.Equal(t, 1, callCount)
// Should call the creator again on the second attempt
provider, err = cache.GetOrCreate(context.Background(), localContext1, contracts.Proposal{}, contracts.Proposal{})
provider, err = cache.GetOrCreate(context.Background(), localContext1, 6, contracts.Proposal{}, contracts.Proposal{})
require.Nil(t, provider)
require.ErrorIs(t, err, providerErr)
require.Equal(t, 2, callCount)
......
......@@ -12,10 +12,10 @@ import (
"github.com/ethereum/go-ethereum/crypto"
)
type ProposalTraceProviderCreator func(ctx context.Context, localContext common.Hash, agreed contracts.Proposal, claimed contracts.Proposal) (types.TraceProvider, error)
type ProposalTraceProviderCreator func(ctx context.Context, localContext common.Hash, depth uint64, agreed contracts.Proposal, claimed contracts.Proposal) (types.TraceProvider, error)
func OutputRootSplitAdapter(topProvider *OutputTraceProvider, creator ProposalTraceProviderCreator) split.ProviderCreator {
return func(ctx context.Context, pre types.Claim, post types.Claim) (types.TraceProvider, error) {
return func(ctx context.Context, depth uint64, pre types.Claim, post types.Claim) (types.TraceProvider, error) {
localContext := createLocalContext(pre, post)
usePrestateBlock := pre == (types.Claim{})
var agreed contracts.Proposal
......@@ -47,7 +47,7 @@ func OutputRootSplitAdapter(topProvider *OutputTraceProvider, creator ProposalTr
OutputRoot: post.Value,
}
return creator(ctx, localContext, agreed, claimed)
return creator(ctx, localContext, depth, agreed, claimed)
}
}
......
......@@ -82,7 +82,7 @@ func TestOutputRootSplitAdapter(t *testing.T) {
OutputRoot: postClaim.Value,
}
_, err := adapter(context.Background(), preClaim, postClaim)
_, err := adapter(context.Background(), 5, preClaim, postClaim)
require.ErrorIs(t, err, creatorError)
require.Equal(t, createLocalContext(preClaim, postClaim), creator.localContext)
require.Equal(t, expectedAgreed, creator.agreed)
......@@ -113,7 +113,7 @@ func TestOutputRootSplitAdapter_FromAbsolutePrestate(t *testing.T) {
OutputRoot: postClaim.Value,
}
_, err := adapter(context.Background(), types.Claim{}, postClaim)
_, err := adapter(context.Background(), 5, types.Claim{}, postClaim)
require.ErrorIs(t, err, creatorError)
require.Equal(t, createLocalContext(types.Claim{}, postClaim), creator.localContext)
require.Equal(t, expectedAgreed, creator.agreed)
......@@ -145,7 +145,7 @@ type capturingCreator struct {
claimed contracts.Proposal
}
func (c *capturingCreator) Create(_ context.Context, localContext common.Hash, agreed contracts.Proposal, claimed contracts.Proposal) (types.TraceProvider, error) {
func (c *capturingCreator) Create(_ context.Context, localContext common.Hash, _ uint64, agreed contracts.Proposal, claimed contracts.Proposal) (types.TraceProvider, error) {
c.localContext = localContext
c.agreed = agreed
c.claimed = claimed
......
......@@ -14,7 +14,7 @@ var (
errRefClaimNotDeepEnough = errors.New("reference claim is not deep enough")
)
type ProviderCreator func(ctx context.Context, pre types.Claim, post types.Claim) (types.TraceProvider, error)
type ProviderCreator func(ctx context.Context, depth uint64, pre types.Claim, post types.Claim) (types.TraceProvider, error)
func NewSplitProviderSelector(topProvider types.TraceProvider, topDepth int, bottomProviderCreator ProviderCreator) trace.ProviderSelector {
return func(ctx context.Context, game types.Game, ref types.Claim, pos types.Position) (types.TraceProvider, error) {
......@@ -56,7 +56,10 @@ func NewSplitProviderSelector(topProvider types.TraceProvider, topDepth int, bot
}
}
}
provider, err := bottomProviderCreator(ctx, pre, post)
// The top game runs from depth 0 to split depth *inclusive*.
// The - 1 here accounts for the fact that the split depth is included in the top game.
bottomDepth := game.MaxDepth() - uint64(topDepth) - 1
provider, err := bottomProviderCreator(ctx, bottomDepth, pre, post)
if err != nil {
return nil, err
}
......
......@@ -14,8 +14,8 @@ import (
)
const (
topDepth = 3
bottomDepth = 4
gameDepth = 7
splitDepth = 3
)
func TestUseTopProvider(t *testing.T) {
......@@ -25,7 +25,7 @@ func TestUseTopProvider(t *testing.T) {
ref := gameBuilder.Game.Claims()[0]
pos := ref.Position
for pos.Depth() <= topDepth {
for pos.Depth() <= splitDepth {
provider, err := selector(ctx, gameBuilder.Game, ref, ref.Position)
require.NoError(t, err)
require.Same(t, topProvider, provider)
......@@ -40,9 +40,9 @@ func TestErrorWhenRefAboveTopGameLeafButPositionInBottom(t *testing.T) {
_, selector, gameBuilder := setupAlphabetSplitSelector(t)
// Generate claims at depths up to but not including the leaf of the top providers
createClaimsToDepth(gameBuilder, topDepth-1)
createClaimsToDepth(gameBuilder, splitDepth-1)
for _, ref := range gameBuilder.Game.Claims() {
pos := types.NewPosition(topDepth+1, big.NewInt(0))
pos := types.NewPosition(splitDepth+1, big.NewInt(0))
provider, err := selector(ctx, gameBuilder.Game, ref, pos)
require.ErrorIsf(t, err, errRefClaimNotDeepEnough, "should not get provider with ref claim at depth: %v", ref.Depth())
require.Nil(t, provider)
......@@ -140,18 +140,18 @@ func TestBottomProviderAttackingTopLeaf(t *testing.T) {
runTest(ref, pos)
runTest(ref, pos.Attack())
runTest(ref, pos.Defend())
if pos.Depth() >= topDepth+bottomDepth {
if pos.Depth() >= gameDepth {
return
}
// If the ref is the leaf of the top claim, ensure we respect whether the test is setup
// to attack or defend the top leaf claim.
if ref.Depth() != topDepth || !pos.RightOf(ref.Position) {
if ref.Depth() != splitDepth || !pos.RightOf(ref.Position) {
gameBuilder.SeqFrom(ref).AttackCorrect()
attackRef := latestClaim(gameBuilder)
testDescendantClaims(attackRef, attackRef.Position)
}
if ref.Depth() != topDepth || pos.RightOf(ref.Position) {
if ref.Depth() != splitDepth || pos.RightOf(ref.Position) {
gameBuilder.SeqFrom(ref).DefendCorrect()
defendRef := latestClaim(gameBuilder)
testDescendantClaims(defendRef, defendRef.Position)
......@@ -284,8 +284,8 @@ func createClaimsToDepth(gameBuilder *test.GameBuilder, depth int) {
func requireBottomProviderForClaims(t *testing.T, actual types.TraceProvider, expectedPre types.Claim, expectedPost types.Claim) {
if expectedPre != (types.Claim{}) {
require.Equal(t,
new(big.Int).Add(expectedPre.TraceIndex(topDepth), big.NewInt(1)),
expectedPost.TraceIndex(topDepth),
new(big.Int).Add(expectedPre.TraceIndex(splitDepth), big.NewInt(1)),
expectedPost.TraceIndex(splitDepth),
"should expect adjacent top level trace indices")
}
......@@ -303,17 +303,17 @@ func asBottomTraceProvider(t *testing.T, actual types.TraceProvider) *bottomTrac
}
func setupAlphabetSplitSelector(t *testing.T) (*alphabet.AlphabetTraceProvider, trace.ProviderSelector, *test.GameBuilder) {
top := alphabet.NewTraceProvider("abcdef", topDepth)
bottomCreator := func(ctx context.Context, pre types.Claim, post types.Claim) (types.TraceProvider, error) {
top := alphabet.NewTraceProvider("abcdef", splitDepth)
bottomCreator := func(ctx context.Context, depth uint64, pre types.Claim, post types.Claim) (types.TraceProvider, error) {
return &bottomTraceProvider{
pre: pre,
post: post,
AlphabetTraceProvider: alphabet.NewTraceProvider(post.Value.Hex(), bottomDepth),
AlphabetTraceProvider: alphabet.NewTraceProvider(post.Value.Hex(), depth),
}, nil
}
selector := NewSplitProviderSelector(top, topDepth, bottomCreator)
selector := NewSplitProviderSelector(top, splitDepth, bottomCreator)
claimBuilder := test.NewAlphabetClaimBuilder(t, topDepth+bottomDepth)
claimBuilder := test.NewAlphabetClaimBuilder(t, gameDepth)
gameBuilder := claimBuilder.GameBuilder(true)
return top, selector, gameBuilder
}
......
package types
import (
"fmt"
"math"
"math/big"
"testing"
......@@ -145,11 +146,106 @@ func TestRelativeToAncestorAtDepth(t *testing.T) {
require.ErrorIs(t, err, ErrPositionDepthTooSmall)
})
t.Run("Success", func(t *testing.T) {
pos := NewPosition(2, big.NewInt(1))
expectedRelativePosition := NewPosition(1, big.NewInt(1))
relativePosition, err := pos.RelativeToAncestorAtDepth(1)
require.NoError(t, err)
require.Equal(t, expectedRelativePosition, relativePosition)
})
tests := []struct {
gindex int64
newRootDepth uint64
expectedGIndex int64
}{
{gindex: 5, newRootDepth: 1, expectedGIndex: 3},
// Depth 0 (should return position unchanged)
{gindex: 1, newRootDepth: 0, expectedGIndex: 1},
{gindex: 2, newRootDepth: 0, expectedGIndex: 2},
// Depth 1
{gindex: 2, newRootDepth: 1, expectedGIndex: 1},
{gindex: 3, newRootDepth: 1, expectedGIndex: 1},
{gindex: 4, newRootDepth: 1, expectedGIndex: 2},
{gindex: 5, newRootDepth: 1, expectedGIndex: 3},
{gindex: 6, newRootDepth: 1, expectedGIndex: 2},
{gindex: 7, newRootDepth: 1, expectedGIndex: 3},
{gindex: 8, newRootDepth: 1, expectedGIndex: 4},
{gindex: 9, newRootDepth: 1, expectedGIndex: 5},
{gindex: 10, newRootDepth: 1, expectedGIndex: 6},
{gindex: 11, newRootDepth: 1, expectedGIndex: 7},
{gindex: 12, newRootDepth: 1, expectedGIndex: 4},
{gindex: 13, newRootDepth: 1, expectedGIndex: 5},
{gindex: 14, newRootDepth: 1, expectedGIndex: 6},
{gindex: 15, newRootDepth: 1, expectedGIndex: 7},
{gindex: 16, newRootDepth: 1, expectedGIndex: 8},
{gindex: 17, newRootDepth: 1, expectedGIndex: 9},
{gindex: 18, newRootDepth: 1, expectedGIndex: 10},
{gindex: 19, newRootDepth: 1, expectedGIndex: 11},
{gindex: 20, newRootDepth: 1, expectedGIndex: 12},
{gindex: 21, newRootDepth: 1, expectedGIndex: 13},
{gindex: 22, newRootDepth: 1, expectedGIndex: 14},
{gindex: 23, newRootDepth: 1, expectedGIndex: 15},
{gindex: 24, newRootDepth: 1, expectedGIndex: 8},
{gindex: 25, newRootDepth: 1, expectedGIndex: 9},
{gindex: 26, newRootDepth: 1, expectedGIndex: 10},
{gindex: 27, newRootDepth: 1, expectedGIndex: 11},
{gindex: 28, newRootDepth: 1, expectedGIndex: 12},
{gindex: 29, newRootDepth: 1, expectedGIndex: 13},
{gindex: 30, newRootDepth: 1, expectedGIndex: 14},
{gindex: 31, newRootDepth: 1, expectedGIndex: 15},
// Depth 2
{gindex: 4, newRootDepth: 2, expectedGIndex: 1},
{gindex: 5, newRootDepth: 2, expectedGIndex: 1},
{gindex: 6, newRootDepth: 2, expectedGIndex: 1},
{gindex: 7, newRootDepth: 2, expectedGIndex: 1},
{gindex: 8, newRootDepth: 2, expectedGIndex: 2},
{gindex: 9, newRootDepth: 2, expectedGIndex: 3},
{gindex: 10, newRootDepth: 2, expectedGIndex: 2},
{gindex: 11, newRootDepth: 2, expectedGIndex: 3},
{gindex: 12, newRootDepth: 2, expectedGIndex: 2},
{gindex: 13, newRootDepth: 2, expectedGIndex: 3},
{gindex: 14, newRootDepth: 2, expectedGIndex: 2},
{gindex: 15, newRootDepth: 2, expectedGIndex: 3},
}
for _, test := range tests {
test := test
t.Run(fmt.Sprintf("From %v SplitAt %v", test.gindex, test.newRootDepth), func(t *testing.T) {
pos := NewPositionFromGIndex(big.NewInt(test.gindex))
expectedRelativePosition := NewPositionFromGIndex(big.NewInt(test.expectedGIndex))
relativePosition, err := pos.RelativeToAncestorAtDepth(test.newRootDepth)
require.NoError(t, err)
require.Equal(t, expectedRelativePosition.ToGIndex(), relativePosition.ToGIndex())
})
}
}
func TestRelativeMoves(t *testing.T) {
tests := []func(pos Position) Position{
func(pos Position) Position {
return pos.Attack()
},
func(pos Position) Position {
return pos.Defend()
},
func(pos Position) Position {
return pos.Attack().Attack()
},
func(pos Position) Position {
return pos.Defend().Defend()
},
func(pos Position) Position {
return pos.Attack().Defend()
},
func(pos Position) Position {
return pos.Defend().Attack()
},
}
for _, test := range tests {
test := test
t.Run("", func(t *testing.T) {
expectedRelativePosition := test(NewPositionFromGIndex(big.NewInt(1)))
relative := NewPositionFromGIndex(big.NewInt(3))
start := test(relative)
relativePosition, err := start.RelativeToAncestorAtDepth(uint64(relative.Depth()))
require.NoError(t, err)
require.Equal(t, expectedRelativePosition.ToGIndex(), relativePosition.ToGIndex())
})
}
}
......@@ -56,12 +56,11 @@ func (g *OutputCannonGameHelper) CreateHonestActor(ctx context.Context, l2Node s
prestateBlock, poststateBlock, err := contract.GetBlockRange(ctx)
g.require.NoError(err, "Failed to load block range")
dir := filepath.Join(cfg.Datadir, "honest")
maxDepth := uint64(g.MaxDepth(ctx))
splitDepth := uint64(g.SplitDepth(ctx))
rollupClient := g.system.RollupClient(l2Node)
prestateProvider := outputs.NewPrestateProvider(ctx, logger, rollupClient, prestateBlock)
accessor, err := outputs.NewOutputCannonTraceAccessor(
ctx, logger, metrics.NoopMetrics, cfg, l2Client, contract, prestateProvider, rollupClient, dir, maxDepth, splitDepth, prestateBlock, poststateBlock)
logger, metrics.NoopMetrics, cfg, l2Client, contract, prestateProvider, rollupClient, dir, splitDepth, prestateBlock, poststateBlock)
g.require.NoError(err, "Failed to create output cannon trace accessor")
return &OutputHonestHelper{
t: g.t,
......
......@@ -72,7 +72,7 @@ func TestOutputCannonDisputeGame(t *testing.T) {
}{
{"StepFirst", 0},
{"StepMiddle", 28},
{"StepInExtension", 2},
{"StepInExtension", 1},
}
for _, test := range tests {
test := test
......
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