Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

op-dispute-mon: Use game data from previous update cycle if update fails #12481

Merged
merged 1 commit into from
Oct 23, 2024
Merged

op-dispute-mon: Use game data from previous update cycle if update fails #12481

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 14 additions & 1 deletion op-dispute-mon/metrics/metrics.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -183,6 +183,8 @@ type Metricer interface {

RecordL2Challenges(agreement bool, count int)

RecordOldestGameUpdateTime(t time.Time)

caching.Metrics
contractMetrics.ContractMetricer
}
Expand Down Expand Up @@ -215,7 +217,8 @@ type Metrics struct {
credits prometheus.GaugeVec
honestWithdrawableAmounts prometheus.GaugeVec

lastOutputFetch prometheus.Gauge
lastOutputFetch prometheus.Gauge
oldestGameUpdateTime prometheus.Gauge

gamesAgreement prometheus.GaugeVec
latestValidProposalL2Block prometheus.Gauge
Expand Down Expand Up @@ -269,6 +272,12 @@ func NewMetrics() *Metrics {
Name: "last_output_fetch",
Help: "Timestamp of the last output fetch",
}),
oldestGameUpdateTime: factory.NewGauge(prometheus.GaugeOpts{
Namespace: Namespace,
Name: "oldest_game_update_time",
Help: "Timestamp the least recently updated game " +
"or the time of the last update cycle if there were no games in the monitoring window",
}),
honestActorClaims: *factory.NewGaugeVec(prometheus.GaugeOpts{
Namespace: Namespace,
Name: "honest_actor_claims",
Expand Down Expand Up @@ -499,6 +508,10 @@ func (m *Metrics) RecordOutputFetchTime(timestamp float64) {
m.lastOutputFetch.Set(timestamp)
}

func (m *Metrics) RecordOldestGameUpdateTime(t time.Time) {
m.oldestGameUpdateTime.Set(float64(t.Unix()))
}

func (m *Metrics) RecordGameAgreement(status GameAgreementStatus, count int) {
m.gamesAgreement.WithLabelValues(labelValuesFor(status)...).Set(float64(count))
}
Expand Down
2 changes: 2 additions & 0 deletions op-dispute-mon/metrics/noop.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,8 @@ func (*NoopMetricsImpl) RecordWithdrawalRequests(_ common.Address, _ bool, _ int

func (*NoopMetricsImpl) RecordOutputFetchTime(_ float64) {}

func (*NoopMetricsImpl) RecordOldestGameUpdateTime(_ time.Time) {}

func (*NoopMetricsImpl) RecordGameAgreement(_ GameAgreementStatus, _ int) {}

func (*NoopMetricsImpl) RecordLatestValidProposalL2Block(_ uint64) {}
Expand Down
22 changes: 17 additions & 5 deletions op-dispute-mon/mon/extract/extractor.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,11 @@ import (

gameTypes "github.com/ethereum-optimism/optimism/op-challenger/game/types"
monTypes "github.com/ethereum-optimism/optimism/op-dispute-mon/mon/types"
"github.com/ethereum-optimism/optimism/op-service/clock"
"github.com/ethereum-optimism/optimism/op-service/sources/batching/rpcblock"
"github.com/ethereum/go-ethereum/common"
"github.com/ethereum/go-ethereum/log"
"golang.org/x/exp/maps"
)

var (
Expand All @@ -29,20 +31,23 @@ type Enricher interface {

type Extractor struct {
logger log.Logger
clock clock.Clock
createContract CreateGameCaller
fetchGames FactoryGameFetcher
maxConcurrency int
enrichers []Enricher
ignoredGames map[common.Address]bool
latestGameData map[common.Address]*monTypes.EnrichedGameData
}

func NewExtractor(logger log.Logger, creator CreateGameCaller, fetchGames FactoryGameFetcher, ignoredGames []common.Address, maxConcurrency uint, enrichers ...Enricher) *Extractor {
func NewExtractor(logger log.Logger, cl clock.Clock, creator CreateGameCaller, fetchGames FactoryGameFetcher, ignoredGames []common.Address, maxConcurrency uint, enrichers ...Enricher) *Extractor {
ignored := make(map[common.Address]bool)
for _, game := range ignoredGames {
ignored[game] = true
}
return &Extractor{
logger: logger,
clock: cl,
createContract: creator,
fetchGames: fetchGames,
maxConcurrency: int(maxConcurrency),
Expand All @@ -61,7 +66,6 @@ func (e *Extractor) Extract(ctx context.Context, blockHash common.Hash, minTimes
}

func (e *Extractor) enrichGames(ctx context.Context, blockHash common.Hash, games []gameTypes.GameMetadata) ([]*monTypes.EnrichedGameData, int, int) {
var enrichedGames []*monTypes.EnrichedGameData
var ignored atomic.Int32
var failed atomic.Int32

Expand Down Expand Up @@ -101,8 +105,14 @@ func (e *Extractor) enrichGames(ctx context.Context, blockHash common.Hash, game
}()
}

// Push each game into the channel
// Create a new store for game data. This ensures any games no longer in the monitoring set are dropped.
updatedGameData := make(map[common.Address]*monTypes.EnrichedGameData)
// Push each game into the channel and store the latest cached game data as a default if fetching fails
for _, game := range games {
previousData := e.latestGameData[game.Proxy]
if previousData != nil {
updatedGameData[game.Proxy] = previousData
}
gameCh <- game
}
close(gameCh)
Expand All @@ -112,9 +122,10 @@ func (e *Extractor) enrichGames(ctx context.Context, blockHash common.Hash, game

// Read the results
for enrichedGame := range enrichedCh {
enrichedGames = append(enrichedGames, enrichedGame)
updatedGameData[enrichedGame.Proxy] = enrichedGame
}
return enrichedGames, int(ignored.Load()), int(failed.Load())
e.latestGameData = updatedGameData
return maps.Values(updatedGameData), int(ignored.Load()), int(failed.Load())
}

func (e *Extractor) enrichGame(ctx context.Context, blockHash common.Hash, game gameTypes.GameMetadata) (*monTypes.EnrichedGameData, error) {
Expand All @@ -138,6 +149,7 @@ func (e *Extractor) enrichGame(ctx context.Context, blockHash common.Hash, game
enrichedClaims[i] = monTypes.EnrichedClaim{Claim: claim}
}
enrichedGame := &monTypes.EnrichedGameData{
LastUpdateTime: e.clock.Now(),
GameMetadata: game,
L1Head: meta.L1Head,
L2BlockNumber: meta.L2BlockNum,
Expand Down
78 changes: 63 additions & 15 deletions op-dispute-mon/mon/extract/extractor_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import (

"github.com/ethereum-optimism/optimism/op-challenger/game/fault/contracts"
monTypes "github.com/ethereum-optimism/optimism/op-dispute-mon/mon/types"
"github.com/ethereum-optimism/optimism/op-service/clock"
"github.com/ethereum-optimism/optimism/op-service/sources/batching/rpcblock"
"github.com/stretchr/testify/require"

Expand All @@ -26,15 +27,15 @@ var (

func TestExtractor_Extract(t *testing.T) {
t.Run("FetchGamesError", func(t *testing.T) {
extractor, _, games, _ := setupExtractorTest(t)
extractor, _, games, _, _ := setupExtractorTest(t)
games.err = errors.New("boom")
_, _, _, err := extractor.Extract(context.Background(), common.Hash{}, 0)
require.ErrorIs(t, err, games.err)
require.Equal(t, 1, games.calls)
})

t.Run("CreateGameErrorLog", func(t *testing.T) {
extractor, creator, games, logs := setupExtractorTest(t)
extractor, creator, games, logs, _ := setupExtractorTest(t)
games.games = []gameTypes.GameMetadata{{}}
creator.err = errors.New("boom")
enriched, ignored, failed, err := extractor.Extract(context.Background(), common.Hash{}, 0)
Expand All @@ -50,7 +51,7 @@ func TestExtractor_Extract(t *testing.T) {
})

t.Run("MetadataFetchErrorLog", func(t *testing.T) {
extractor, creator, games, logs := setupExtractorTest(t)
extractor, creator, games, logs, _ := setupExtractorTest(t)
games.games = []gameTypes.GameMetadata{{}}
creator.caller.metadataErr = errors.New("boom")
enriched, ignored, failed, err := extractor.Extract(context.Background(), common.Hash{}, 0)
Expand All @@ -66,7 +67,7 @@ func TestExtractor_Extract(t *testing.T) {
})

t.Run("ClaimsFetchErrorLog", func(t *testing.T) {
extractor, creator, games, logs := setupExtractorTest(t)
extractor, creator, games, logs, _ := setupExtractorTest(t)
games.games = []gameTypes.GameMetadata{{}}
creator.caller.claimsErr = errors.New("boom")
enriched, ignored, failed, err := extractor.Extract(context.Background(), common.Hash{}, 0)
Expand All @@ -82,7 +83,7 @@ func TestExtractor_Extract(t *testing.T) {
})

t.Run("Success", func(t *testing.T) {
extractor, creator, games, _ := setupExtractorTest(t)
extractor, creator, games, _, _ := setupExtractorTest(t)
games.games = []gameTypes.GameMetadata{{}}
enriched, ignored, failed, err := extractor.Extract(context.Background(), common.Hash{}, 0)
require.NoError(t, err)
Expand All @@ -97,7 +98,7 @@ func TestExtractor_Extract(t *testing.T) {

t.Run("EnricherFails", func(t *testing.T) {
enricher := &mockEnricher{err: errors.New("whoops")}
extractor, _, games, logs := setupExtractorTest(t, enricher)
extractor, _, games, logs, _ := setupExtractorTest(t, enricher)
games.games = []gameTypes.GameMetadata{{}}
enriched, ignored, failed, err := extractor.Extract(context.Background(), common.Hash{}, 0)
require.NoError(t, err)
Expand All @@ -110,7 +111,7 @@ func TestExtractor_Extract(t *testing.T) {

t.Run("EnricherSuccess", func(t *testing.T) {
enricher := &mockEnricher{}
extractor, _, games, _ := setupExtractorTest(t, enricher)
extractor, _, games, _, _ := setupExtractorTest(t, enricher)
games.games = []gameTypes.GameMetadata{{}}
enriched, ignored, failed, err := extractor.Extract(context.Background(), common.Hash{}, 0)
require.NoError(t, err)
Expand All @@ -123,8 +124,8 @@ func TestExtractor_Extract(t *testing.T) {
t.Run("MultipleEnrichersMultipleGames", func(t *testing.T) {
enricher1 := &mockEnricher{}
enricher2 := &mockEnricher{}
extractor, _, games, _ := setupExtractorTest(t, enricher1, enricher2)
games.games = []gameTypes.GameMetadata{{}, {}}
extractor, _, games, _, _ := setupExtractorTest(t, enricher1, enricher2)
games.games = []gameTypes.GameMetadata{{Proxy: common.Address{0xaa}}, {Proxy: common.Address{0xbb}}}
enriched, ignored, failed, err := extractor.Extract(context.Background(), common.Hash{}, 0)
require.NoError(t, err)
require.Zero(t, ignored)
Expand All @@ -136,7 +137,7 @@ func TestExtractor_Extract(t *testing.T) {

t.Run("IgnoreGames", func(t *testing.T) {
enricher1 := &mockEnricher{}
extractor, _, games, logs := setupExtractorTest(t, enricher1)
extractor, _, games, logs, _ := setupExtractorTest(t, enricher1)
// Two games, one of which is ignored
games.games = []gameTypes.GameMetadata{{Proxy: ignoredGames[0]}, {Proxy: common.Address{0xaa}}}
enriched, ignored, failed, err := extractor.Extract(context.Background(), common.Hash{}, 0)
Expand All @@ -152,6 +153,47 @@ func TestExtractor_Extract(t *testing.T) {
testlog.NewMessageFilter("Ignoring game"),
testlog.NewAttributesFilter("game", ignoredGames[0].Hex())))
})

t.Run("UseCachedValueOnFailure", func(t *testing.T) {
enricher := &mockEnricher{}
extractor, _, games, _, cl := setupExtractorTest(t, enricher)
gameA := common.Address{0xaa}
gameB := common.Address{0xbb}
games.games = []gameTypes.GameMetadata{{Proxy: gameA}, {Proxy: gameB}}

// First fetch succeeds and the results should be cached
enriched, ignored, failed, err := extractor.Extract(context.Background(), common.Hash{}, 0)
require.NoError(t, err)
require.Zero(t, ignored)
require.Zero(t, failed)
require.Len(t, enriched, 2)
require.Equal(t, 2, enricher.calls)
firstUpdateTime := cl.Now()
require.Equal(t, firstUpdateTime, enriched[0].LastUpdateTime)
require.Equal(t, firstUpdateTime, enriched[1].LastUpdateTime)

cl.AdvanceTime(2 * time.Minute)
secondUpdateTime := cl.Now()
enricher.action = func(game *monTypes.EnrichedGameData) error {
if game.Proxy == gameA {
return errors.New("boom")
}
// Updated games will have a different status
game.Status = gameTypes.GameStatusChallengerWon
return nil
}
// Second fetch fails for one of the two games, it's cached value should be used.
enriched, ignored, failed, err = extractor.Extract(context.Background(), common.Hash{}, 0)
require.NoError(t, err)
require.Zero(t, ignored)
require.Equal(t, 1, failed)
require.Len(t, enriched, 2)
require.Equal(t, 4, enricher.calls)
require.Equal(t, enriched[0].Status, gameTypes.GameStatusInProgress) // Uses cached value from game A
require.Equal(t, enriched[1].Status, gameTypes.GameStatusChallengerWon) // Updates game B
require.Equal(t, firstUpdateTime, enriched[0].LastUpdateTime)
require.Equal(t, secondUpdateTime, enriched[1].LastUpdateTime)
})
}

func verifyLogs(t *testing.T, logs *testlog.CapturingHandler, createErr, metadataErr, claimsErr, durationErr int) {
Expand All @@ -170,20 +212,22 @@ func verifyLogs(t *testing.T, logs *testlog.CapturingHandler, createErr, metadat
require.Len(t, l, durationErr)
}

func setupExtractorTest(t *testing.T, enrichers ...Enricher) (*Extractor, *mockGameCallerCreator, *mockGameFetcher, *testlog.CapturingHandler) {
func setupExtractorTest(t *testing.T, enrichers ...Enricher) (*Extractor, *mockGameCallerCreator, *mockGameFetcher, *testlog.CapturingHandler, *clock.DeterministicClock) {
logger, capturedLogs := testlog.CaptureLogger(t, log.LvlDebug)
games := &mockGameFetcher{}
caller := &mockGameCaller{rootClaim: mockRootClaim}
creator := &mockGameCallerCreator{caller: caller}
cl := clock.NewDeterministicClock(time.Unix(48294294, 58))
extractor := NewExtractor(
logger,
cl,
creator.CreateGameCaller,
games.FetchGames,
ignoredGames,
5,
enrichers...,
)
return extractor, creator, games, capturedLogs
return extractor, creator, games, capturedLogs, cl
}

type mockGameFetcher struct {
Expand Down Expand Up @@ -311,11 +355,15 @@ func (m *mockGameCaller) IsResolved(_ context.Context, _ rpcblock.Block, claims
}

type mockEnricher struct {
err error
calls int
err error
calls int
action func(game *monTypes.EnrichedGameData) error
}

func (m *mockEnricher) Enrich(_ context.Context, _ rpcblock.Block, _ GameCaller, _ *monTypes.EnrichedGameData) error {
func (m *mockEnricher) Enrich(_ context.Context, _ rpcblock.Block, _ GameCaller, game *monTypes.EnrichedGameData) error {
m.calls++
if m.action != nil {
return m.action(game)
}
return m.err
}
35 changes: 9 additions & 26 deletions op-dispute-mon/mon/monitor.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,6 @@ import (
)

type ForecastResolution func(games []*types.EnrichedGameData, ignoredCount, failedCount int)
type Bonds func(games []*types.EnrichedGameData)
type Resolutions func(games []*types.EnrichedGameData)
type Monitor func(games []*types.EnrichedGameData)
type BlockHashFetcher func(ctx context.Context, number *big.Int) (common.Hash, error)
type BlockNumberFetcher func(ctx context.Context) (uint64, error)
Expand All @@ -38,11 +36,7 @@ type gameMonitor struct {
monitorInterval time.Duration

forecast ForecastResolution
bonds Bonds
resolutions Resolutions
claims Monitor
withdrawals Monitor
l2Challenges Monitor
monitors []Monitor
extract Extract
fetchBlockHash BlockHashFetcher
fetchBlockNumber BlockNumberFetcher
Expand All @@ -55,16 +49,11 @@ func newGameMonitor(
metrics MonitorMetrics,
monitorInterval time.Duration,
gameWindow time.Duration,
forecast ForecastResolution,
bonds Bonds,
resolutions Resolutions,
claims Monitor,
withdrawals Monitor,
l2Challenges Monitor,
extract Extract,
fetchBlockNumber BlockNumberFetcher,
fetchBlockHash BlockHashFetcher,
) *gameMonitor {
fetchBlockNumber BlockNumberFetcher,
extract Extract,
forecast ForecastResolution,
monitors ...Monitor) *gameMonitor {
return &gameMonitor{
logger: logger,
clock: cl,
Expand All @@ -74,11 +63,7 @@ func newGameMonitor(
monitorInterval: monitorInterval,
gameWindow: gameWindow,
forecast: forecast,
bonds: bonds,
resolutions: resolutions,
claims: claims,
withdrawals: withdrawals,
l2Challenges: l2Challenges,
monitors: monitors,
extract: extract,
fetchBlockNumber: fetchBlockNumber,
fetchBlockHash: fetchBlockHash,
Expand All @@ -101,12 +86,10 @@ func (m *gameMonitor) monitorGames() error {
if err != nil {
return fmt.Errorf("failed to load games: %w", err)
}
m.resolutions(enrichedGames)
m.forecast(enrichedGames, ignored, failed)
m.bonds(enrichedGames)
m.claims(enrichedGames)
m.withdrawals(enrichedGames)
m.l2Challenges(enrichedGames)
for _, monitor := range m.monitors {
monitor(enrichedGames)
}
timeTaken := m.clock.Since(start)
m.metrics.RecordMonitorDuration(timeTaken)
m.logger.Info("Completed monitoring update", "blockNumber", blockNumber, "blockHash", blockHash, "duration", timeTaken, "games", len(enrichedGames), "ignored", ignored, "failed", failed)
Expand Down
Loading