Skip to content

Commit

Permalink
op-proposer: (DFG) skip new game creation if outputRoot is unchanged (#…
Browse files Browse the repository at this point in the history 
…13288)

* op-proposer: do not propose if outputRoot is the same as last proposed game

* fixed porposer tests

* PR comments

* Fix log message.

---------

Co-authored-by: Adrian Sutton <[email protected]>
Co-authored-by: Adrian Sutton <[email protected]>
  • Loading branch information
@PierreOssun @ajsutton
3 people authored Dec 23, 2024
1 parent 90c18bc commit d356d92
Show file tree
Hide file tree
Showing 4 changed files with 34 additions and 19 deletions.
19 changes: 11 additions & 8 deletions op-proposer/contracts/disputegamefactory.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ type gameMetadata struct {
Timestamp time.Time
Address common.Address
Proposer common.Address
Claim common.Hash
}

type DisputeGameFactory struct {
Expand Down Expand Up @@ -62,30 +63,30 @@ func (f *DisputeGameFactory) Version(ctx context.Context) (string, error) {
// HasProposedSince attempts to find a game with the specified game type created by the specified proposer after the
// given cut off time. If one is found, returns true and the time the game was created at.
// If no matching proposal is found, returns false, time.Time{}, nil
func (f *DisputeGameFactory) HasProposedSince(ctx context.Context, proposer common.Address, cutoff time.Time, gameType uint32) (bool, time.Time, error) {
func (f *DisputeGameFactory) HasProposedSince(ctx context.Context, proposer common.Address, cutoff time.Time, gameType uint32) (bool, time.Time, common.Hash, error) {
gameCount, err := f.gameCount(ctx)
if err != nil {
return false, time.Time{}, fmt.Errorf("failed to get dispute game count: %w", err)
return false, time.Time{}, common.Hash{}, fmt.Errorf("failed to get dispute game count: %w", err)
}
if gameCount == 0 {
return false, time.Time{}, nil
return false, time.Time{}, common.Hash{}, nil
}
for idx := gameCount - 1; ; idx-- {
game, err := f.gameAtIndex(ctx, idx)
if err != nil {
return false, time.Time{}, fmt.Errorf("failed to get dispute game %d: %w", idx, err)
return false, time.Time{}, common.Hash{}, fmt.Errorf("failed to get dispute game %d: %w", idx, err)
}
if game.Timestamp.Before(cutoff) {
// Reached a game that is before the expected cutoff, so we haven't found a suitable proposal
return false, time.Time{}, nil
return false, time.Time{}, common.Hash{}, nil
}
if game.GameType == gameType && game.Proposer == proposer {
// Found a matching proposal
return true, game.Timestamp, nil
return true, game.Timestamp, game.Claim, nil
}
if idx == 0 { // Need to check here rather than in the for condition to avoid underflow
// Checked every game and didn't find a match
return false, time.Time{}, nil
return false, time.Time{}, common.Hash{}, nil
}
}
}
Expand Down Expand Up @@ -135,13 +136,15 @@ func (f *DisputeGameFactory) gameAtIndex(ctx context.Context, idx uint64) (gameM
if err != nil {
return gameMetadata{}, fmt.Errorf("failed to load root claim of game %v: %w", idx, err)
}
// We don't need most of the claim data, only the claimant which is the game proposer
// We don't need most of the claim data, only the claim and the claimant which is the game proposer
claimant := result.GetAddress(2)
claim := result.GetHash(4)

return gameMetadata{
GameType: gameType,
Timestamp: time.Unix(int64(timestamp), 0),
Address: address,
Proposer: claimant,
Claim: claim,
}, nil
}
18 changes: 12 additions & 6 deletions op-proposer/contracts/disputegamefactory_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,10 +25,11 @@ func TestHasProposedSince(t *testing.T) {
stubRpc, factory := setupDisputeGameFactoryTest(t)
withClaims(stubRpc)

proposed, proposalTime, err := factory.HasProposedSince(context.Background(), proposerAddr, cutOffTime, 0)
proposed, proposalTime, claim, err := factory.HasProposedSince(context.Background(), proposerAddr, cutOffTime, 0)
require.NoError(t, err)
require.False(t, proposed)
require.Equal(t, time.Time{}, proposalTime)
require.Equal(t, common.Hash{}, claim)
})

t.Run("NoMatchingProposal", func(t *testing.T) {
Expand All @@ -49,10 +50,11 @@ func TestHasProposedSince(t *testing.T) {
},
)

proposed, proposalTime, err := factory.HasProposedSince(context.Background(), proposerAddr, cutOffTime, 0)
proposed, proposalTime, claim, err := factory.HasProposedSince(context.Background(), proposerAddr, cutOffTime, 0)
require.NoError(t, err)
require.False(t, proposed)
require.Equal(t, time.Time{}, proposalTime)
require.Equal(t, common.Hash{}, claim)
})

t.Run("MatchingProposalBeforeCutOff", func(t *testing.T) {
Expand All @@ -79,10 +81,11 @@ func TestHasProposedSince(t *testing.T) {
},
)

proposed, proposalTime, err := factory.HasProposedSince(context.Background(), proposerAddr, cutOffTime, 0)
proposed, proposalTime, claim, err := factory.HasProposedSince(context.Background(), proposerAddr, cutOffTime, 0)
require.NoError(t, err)
require.False(t, proposed)
require.Equal(t, time.Time{}, proposalTime)
require.Equal(t, common.Hash{}, claim)
})

t.Run("MatchingProposalAtCutOff", func(t *testing.T) {
Expand All @@ -109,10 +112,11 @@ func TestHasProposedSince(t *testing.T) {
},
)

proposed, proposalTime, err := factory.HasProposedSince(context.Background(), proposerAddr, cutOffTime, 0)
proposed, proposalTime, claim, err := factory.HasProposedSince(context.Background(), proposerAddr, cutOffTime, 0)
require.NoError(t, err)
require.True(t, proposed)
require.Equal(t, cutOffTime, proposalTime)
require.Equal(t, common.Hash{0xdd}, claim)
})

t.Run("MatchingProposalAfterCutOff", func(t *testing.T) {
Expand Down Expand Up @@ -140,10 +144,11 @@ func TestHasProposedSince(t *testing.T) {
},
)

proposed, proposalTime, err := factory.HasProposedSince(context.Background(), proposerAddr, cutOffTime, 0)
proposed, proposalTime, claim, err := factory.HasProposedSince(context.Background(), proposerAddr, cutOffTime, 0)
require.NoError(t, err)
require.True(t, proposed)
require.Equal(t, expectedProposalTime, proposalTime)
require.Equal(t, common.Hash{0xdd}, claim)
})

t.Run("MultipleMatchingProposalAfterCutOff", func(t *testing.T) {
Expand Down Expand Up @@ -171,11 +176,12 @@ func TestHasProposedSince(t *testing.T) {
},
)

proposed, proposalTime, err := factory.HasProposedSince(context.Background(), proposerAddr, cutOffTime, 0)
proposed, proposalTime, claim, err := factory.HasProposedSince(context.Background(), proposerAddr, cutOffTime, 0)
require.NoError(t, err)
require.True(t, proposed)
// Should find the most recent proposal
require.Equal(t, expectedProposalTime, proposalTime)
require.Equal(t, common.Hash{0xdd}, claim)
})
}

Expand Down
12 changes: 9 additions & 3 deletions op-proposer/proposer/driver.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ type L2OOContract interface {

type DGFContract interface {
Version(ctx context.Context) (string, error)
HasProposedSince(ctx context.Context, proposer common.Address, cutoff time.Time, gameType uint32) (bool, time.Time, error)
HasProposedSince(ctx context.Context, proposer common.Address, cutoff time.Time, gameType uint32) (bool, time.Time, common.Hash, error)
ProposalTx(ctx context.Context, gameType uint32, outputRoot common.Hash, l2BlockNum uint64) (txmgr.TxCandidate, error)
}

Expand Down Expand Up @@ -269,7 +269,7 @@ func (l *L2OutputSubmitter) FetchL2OOOutput(ctx context.Context) (*eth.OutputRes
// context will be derived from it.
func (l *L2OutputSubmitter) FetchDGFOutput(ctx context.Context) (*eth.OutputResponse, bool, error) {
cutoff := time.Now().Add(-l.Cfg.ProposalInterval)
proposedRecently, proposalTime, err := l.dgfContract.HasProposedSince(ctx, l.Txmgr.From(), cutoff, l.Cfg.DisputeGameType)
proposedRecently, proposalTime, claim, err := l.dgfContract.HasProposedSince(ctx, l.Txmgr.From(), cutoff, l.Cfg.DisputeGameType)
if err != nil {
return nil, false, fmt.Errorf("could not check for recent proposal: %w", err)
}
Expand All @@ -278,7 +278,6 @@ func (l *L2OutputSubmitter) FetchDGFOutput(ctx context.Context) (*eth.OutputResp
l.Log.Debug("Duration since last game not past proposal interval", "duration", time.Since(proposalTime))
return nil, false, nil
}
l.Log.Info("No proposals found for at least proposal interval, submitting proposal now", "proposalInterval", l.Cfg.ProposalInterval)

// Fetch the current L2 heads
currentBlockNumber, err := l.FetchCurrentBlockNumber(ctx)
Expand All @@ -296,6 +295,13 @@ func (l *L2OutputSubmitter) FetchDGFOutput(ctx context.Context) (*eth.OutputResp
return nil, false, fmt.Errorf("could not fetch output at current block number %d: %w", currentBlockNumber, err)
}

if claim == common.Hash(output.OutputRoot) {
l.Log.Debug("Skipping proposal: output root unchanged since last proposed game", "last_proposed_root", claim, "output_root", output.OutputRoot)
return nil, false, nil
}

l.Log.Info("No proposals found for at least proposal interval, submitting proposal now", "proposalInterval", l.Cfg.ProposalInterval)

return output, true, nil
}

Expand Down
4 changes: 2 additions & 2 deletions op-proposer/proposer/driver_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -42,9 +42,9 @@ type StubDGFContract struct {
hasProposedCount int
}

func (m *StubDGFContract) HasProposedSince(_ context.Context, _ common.Address, _ time.Time, _ uint32) (bool, time.Time, error) {
func (m *StubDGFContract) HasProposedSince(_ context.Context, _ common.Address, _ time.Time, _ uint32) (bool, time.Time, common.Hash, error) {
m.hasProposedCount++
return false, time.Unix(1000, 0), nil
return false, time.Unix(1000, 0), common.Hash{0xdd}, nil
}

func (m *StubDGFContract) ProposalTx(_ context.Context, _ uint32, _ common.Hash, _ uint64) (txmgr.TxCandidate, error) {
Expand Down

0 comments on commit d356d92

Please sign in to comment.