op-dispute-mon: Consider chess clocks of descendant claims when determining if...

op-dispute-mon: Consider chess clocks of descendant claims when determining if a claim is resolvable (#10653)

op-dispute-mon/metrics/metrics.go

@@ -4,6 +4,7 @@ import (
 	"fmt"
 	"io"
 	"math/big"
+	"strings"
 	"time"
 
 	contractMetrics "github.com/ethereum-optimism/optimism/op-challenger/game/fault/contracts/metrics"
@@ -66,32 +67,76 @@ const (
 	DisagreeChallengerWins
 )
 
-type ClaimStatus uint8
+type ClaimStatus struct {
+	resolved     bool
+	clockExpired bool
+	firstHalf    bool
+	resolvable   bool
+}
 
-const (
-	// Claims where the game is in the first half
-	FirstHalfExpiredResolved ClaimStatus = iota
-	FirstHalfExpiredUnresolved
-	FirstHalfNotExpiredResolved
-	FirstHalfNotExpiredUnresolved
-
-	// Claims where the game is in the second half
-	SecondHalfExpiredResolved
-	SecondHalfExpiredUnresolved
-	SecondHalfNotExpiredResolved
-	SecondHalfNotExpiredUnresolved
-)
+func (s ClaimStatus) AsLabels() []string {
+	labels := make([]string, 4)
+	if s.resolved {
+		labels[0] = "resolved"
+	} else {
+		labels[0] = "unresolved"
+	}
+	if s.clockExpired {
+		labels[1] = "expired"
+	} else {
+		labels[1] = "not_expired"
+	}
+	if s.firstHalf {
+		labels[2] = "first_half"
+	} else {
+		labels[2] = "second_half"
+	}
+	if s.resolvable {
+		labels[3] = "resolvable"
+	} else {
+		labels[3] = "unresolvable"
+	}
+	return labels
+}
+
+func (s ClaimStatus) String() string {
+	return strings.Join(s.AsLabels(), ", ")
+}
 
-func ZeroClaimStatuses() map[ClaimStatus]int {
-	return map[ClaimStatus]int{
-		FirstHalfExpiredResolved:       0,
-		FirstHalfExpiredUnresolved:     0,
-		FirstHalfNotExpiredResolved:    0,
-		FirstHalfNotExpiredUnresolved:  0,
-		SecondHalfExpiredResolved:      0,
-		SecondHalfExpiredUnresolved:    0,
-		SecondHalfNotExpiredResolved:   0,
-		SecondHalfNotExpiredUnresolved: 0,
+type ClaimStatuses struct {
+	statuses map[ClaimStatus]int
+}
+
+func (c *ClaimStatuses) RecordClaim(firstHalf bool, clockExpired bool, resolvable bool, resolved bool) {
+	if c.statuses == nil {
+		c.statuses = make(map[ClaimStatus]int)
+	}
+	c.statuses[NewClaimStatus(firstHalf, clockExpired, resolvable, resolved)]++
+}
+
+// ForEachStatus iterates through all possible statuses and calls the callback function with the status and count of
+// claims. This ensures that statuses that have no claims counted against them are still considered to have 0 claims.
+func (c *ClaimStatuses) ForEachStatus(callback func(status ClaimStatus, count int)) {
+	allBools := []bool{true, false}
+	for _, firstHalf := range allBools {
+		for _, clockExpired := range allBools {
+			for _, resolvable := range allBools {
+				for _, resolved := range allBools {
+					status := NewClaimStatus(firstHalf, clockExpired, resolvable, resolved)
+					count := c.statuses[status]
+					callback(status, count)
+				}
+			}
+		}
+	}
+}
+
+func NewClaimStatus(firstHalf bool, clockExpired bool, resolvable bool, resolved bool) ClaimStatus {
+	return ClaimStatus{
+		firstHalf:    firstHalf,
+		clockExpired: clockExpired,
+		resolvable:   resolvable,
+		resolved:     resolved,
 	}
 }
 
@@ -118,7 +163,7 @@ type Metricer interface {
 
 	RecordCredit(expectation CreditExpectation, count int)
 
-	RecordClaims(status ClaimStatus, count int)
+	RecordClaims(statuses *ClaimStatuses)
 
 	RecordWithdrawalRequests(delayedWeth common.Address, matches bool, count int)
 
@@ -258,6 +303,7 @@ func NewMetrics() *Metrics {
 			"resolved",
 			"clock",
 			"game_time_period",
+			"resolvable",
 		}),
 		withdrawalRequests: *factory.NewGaugeVec(prometheus.GaugeOpts{
 			Namespace: Namespace,
@@ -407,30 +453,10 @@ func (m *Metrics) RecordCredit(expectation CreditExpectation, count int) {
 	m.credits.WithLabelValues(asLabels(expectation)...).Set(float64(count))
 }
 
-func (m *Metrics) RecordClaims(status ClaimStatus, count int) {
-	asLabels := func(status ClaimStatus) []string {
-		switch status {
-		case FirstHalfExpiredResolved:
-			return []string{"resolved", "expired", "first_half"}
-		case FirstHalfExpiredUnresolved:
-			return []string{"unresolved", "expired", "first_half"}
-		case FirstHalfNotExpiredResolved:
-			return []string{"resolved", "not_expired", "first_half"}
-		case FirstHalfNotExpiredUnresolved:
-			return []string{"unresolved", "not_expired", "first_half"}
-		case SecondHalfExpiredResolved:
-			return []string{"resolved", "expired", "second_half"}
-		case SecondHalfExpiredUnresolved:
-			return []string{"unresolved", "expired", "second_half"}
-		case SecondHalfNotExpiredResolved:
-			return []string{"resolved", "not_expired", "second_half"}
-		case SecondHalfNotExpiredUnresolved:
-			return []string{"unresolved", "not_expired", "second_half"}
-		default:
-			panic(fmt.Errorf("unknown claim status: %v", status))
-		}
-	}
-	m.claims.WithLabelValues(asLabels(status)...).Set(float64(count))
+func (m *Metrics) RecordClaims(statuses *ClaimStatuses) {
+	statuses.ForEachStatus(func(status ClaimStatus, count int) {
+		m.claims.WithLabelValues(status.AsLabels()...).Set(float64(count))
+	})
 }
 
 func (m *Metrics) RecordWithdrawalRequests(delayedWeth common.Address, matches bool, count int) {

op-dispute-mon/metrics/noop.go

@@ -28,7 +28,7 @@ func (*NoopMetricsImpl) RecordGameResolutionStatus(_ ResolutionStatus, _ int) {}
 
 func (*NoopMetricsImpl) RecordCredit(_ CreditExpectation, _ int) {}
 
-func (*NoopMetricsImpl) RecordClaims(_ ClaimStatus, _ int) {}
+func (*NoopMetricsImpl) RecordClaims(_ *ClaimStatuses) {}
 
 func (*NoopMetricsImpl) RecordWithdrawalRequests(_ common.Address, _ bool, _ int) {}
 

op-dispute-mon/mon/claims.go

@@ -18,7 +18,7 @@ type RClock interface {
 }
 
 type ClaimMetrics interface {
-	RecordClaims(status metrics.ClaimStatus, count int)
+	RecordClaims(statuses *metrics.ClaimStatuses)
 	RecordHonestActorClaims(address common.Address, data *metrics.HonestActorData)
 }
 
@@ -38,7 +38,7 @@ func NewClaimMonitor(logger log.Logger, clock RClock, honestActors []common.Addr
 }
 
 func (c *ClaimMonitor) CheckClaims(games []*types.EnrichedGameData) {
-	claimStatus := metrics.ZeroClaimStatuses()
+	claimStatuses := &metrics.ClaimStatuses{}
 	honest := make(map[common.Address]*metrics.HonestActorData)
 	for actor := range c.honestActors {
 		honest[actor] = &metrics.HonestActorData{
@@ -48,11 +48,9 @@ func (c *ClaimMonitor) CheckClaims(games []*types.EnrichedGameData) {
 		}
 	}
 	for _, game := range games {
-		c.checkGameClaims(game, claimStatus, honest)
-	}
-	for status, count := range claimStatus {
-		c.metrics.RecordClaims(status, count)
+		c.checkGameClaims(game, claimStatuses, honest)
 	}
+	c.metrics.RecordClaims(claimStatuses)
 	for actor := range c.honestActors {
 		c.metrics.RecordHonestActorClaims(actor, honest[actor])
 	}
@@ -84,20 +82,25 @@ func (c *ClaimMonitor) checkUpdateHonestActorStats(proxy common.Address, claim *
 
 func (c *ClaimMonitor) checkGameClaims(
 	game *types.EnrichedGameData,
-	claimStatus map[metrics.ClaimStatus]int,
+	claimStatuses *metrics.ClaimStatuses,
 	honest map[common.Address]*metrics.HonestActorData,
 ) {
 	// Check if the game is in the first half
-	duration := uint64(c.clock.Now().Unix()) - game.Timestamp
+	now := c.clock.Now()
+	duration := uint64(now.Unix()) - game.Timestamp
 	firstHalf := duration <= game.MaxClockDuration
 
+	minDescendantAccumulatedTimeByIndex := make(map[int]time.Duration)
+
 	// Iterate over the game's claims
-	for _, claim := range game.Claims {
+	// Reverse order so we can track whether the claim has unresolvable children
+	for i := len(game.Claims) - 1; i >= 0; i-- {
+		claim := game.Claims[i]
 		c.checkUpdateHonestActorStats(game.Proxy, &claim, honest)
 
 		// Check if the clock has expired
 		if firstHalf && claim.Resolved {
-			c.logger.Error("Claim resolved in the first half of the game duration", "game", game.Proxy, "claimContractIndex", claim.ContractIndex, "id", claim.ID(), "clock", duration)
+			c.logger.Error("Claim resolved in the first half of the game duration", "game", game.Proxy, "claimContractIndex", claim.ContractIndex, "clock", duration)
 		}
 
 		maxChessTime := time.Duration(game.MaxClockDuration) * time.Second
@@ -105,42 +108,31 @@ func (c *ClaimMonitor) checkGameClaims(
 		if !claim.IsRoot() {
 			parent = game.Claims[claim.ParentContractIndex].Claim
 		}
-		accumulatedTime := faultTypes.ChessClock(c.clock.Now(), claim.Claim, parent)
-		clockExpired := accumulatedTime >= maxChessTime
+		accumulatedTime := faultTypes.ChessClock(now, claim.Claim, parent)
 
-		if claim.Resolved {
-			if clockExpired {
-				if firstHalf {
-					claimStatus[metrics.FirstHalfExpiredResolved]++
-				} else {
-					claimStatus[metrics.SecondHalfExpiredResolved]++
-				}
-			} else {
-				if firstHalf {
-					claimStatus[metrics.FirstHalfNotExpiredResolved]++
-				} else {
-					claimStatus[metrics.SecondHalfNotExpiredResolved]++
+		// Calculate the minimum accumulated time of this claim or any of its descendants
+		minAccumulatedTime, ok := minDescendantAccumulatedTimeByIndex[claim.ContractIndex]
+		if !ok || accumulatedTime < minAccumulatedTime {
+			minAccumulatedTime = accumulatedTime
 		}
+		// Update the minimum accumulated time for the parent claim to include this claim's time.
+		curr, ok := minDescendantAccumulatedTimeByIndex[claim.ParentContractIndex]
+		if !ok || minAccumulatedTime < curr {
+			minDescendantAccumulatedTimeByIndex[claim.ParentContractIndex] = minAccumulatedTime
 		}
-		} else {
-			if clockExpired {
-				// SAFETY: accumulatedTime must be larger than or equal to maxChessTime since clockExpired
-				overflow := accumulatedTime - maxChessTime
+
+		// Our clock is expired based on this claim accumulated time (can any more counter claims be posted)
+		clockExpired := accumulatedTime >= maxChessTime
+		// This claim is only resolvable if it and all it's descendants have expired clocks
+		resolvable := minAccumulatedTime >= maxChessTime
+
+		claimStatuses.RecordClaim(firstHalf, clockExpired, resolvable, claim.Resolved)
+		if !claim.Resolved && resolvable {
+			// SAFETY: minAccumulatedTime must be larger than or equal to maxChessTime since the claim is resolvable
+			overflow := minAccumulatedTime - maxChessTime
 			if overflow >= MaximumResolutionResponseBuffer {
 				c.logger.Warn("Claim unresolved after clock expiration", "game", game.Proxy, "claimContractIndex", claim.ContractIndex, "delay", overflow)
 			}
-				if firstHalf {
-					claimStatus[metrics.FirstHalfExpiredUnresolved]++
-				} else {
-					claimStatus[metrics.SecondHalfExpiredUnresolved]++
-				}
-			} else {
-				if firstHalf {
-					claimStatus[metrics.FirstHalfNotExpiredUnresolved]++
-				} else {
-					claimStatus[metrics.SecondHalfNotExpiredUnresolved]++
-				}
-			}
 		}
 	}
 }

op-dispute-mon/mon/claims_test.go

 package mon
 
 import (
+	"fmt"
 	"math/big"
 	"testing"
 	"time"
 
 	faultTypes "github.com/ethereum-optimism/optimism/op-challenger/game/fault/types"
+	gameTypes "github.com/ethereum-optimism/optimism/op-challenger/game/types"
 	"github.com/ethereum-optimism/optimism/op-dispute-mon/metrics"
 	"github.com/ethereum-optimism/optimism/op-dispute-mon/mon/types"
 	"github.com/ethereum-optimism/optimism/op-service/clock"
@@ -19,39 +21,147 @@ var frozen = time.Unix(int64(time.Hour.Seconds()), 0)
 
 func TestClaimMonitor_CheckClaims(t *testing.T) {
 	t.Run("RecordsClaims", func(t *testing.T) {
-		monitor, cl, cMetrics := newTestClaimMonitor(t)
+		monitor, cl, cMetrics, _ := newTestClaimMonitor(t)
 		games := makeMultipleTestGames(uint64(cl.Now().Unix()))
 		monitor.CheckClaims(games)
 
-		require.Equal(t, 2, cMetrics.calls[metrics.FirstHalfExpiredResolved])
-		require.Equal(t, 1, cMetrics.calls[metrics.FirstHalfExpiredUnresolved])
-		require.Equal(t, 1, cMetrics.calls[metrics.FirstHalfNotExpiredResolved])
-		require.Equal(t, 1, cMetrics.calls[metrics.FirstHalfNotExpiredUnresolved])
+		for status, count := range cMetrics.calls {
+			fmt.Printf("%v: %v \n", status, count)
+		}
+
+		// Test data is a bit weird and has unresolvable claims that have been resolved
+		require.Equal(t, 2, cMetrics.calls[metrics.NewClaimStatus(true, true, false, true)])
+		require.Equal(t, 1, cMetrics.calls[metrics.NewClaimStatus(true, true, true, false)])
+		require.Equal(t, 1, cMetrics.calls[metrics.NewClaimStatus(true, false, false, true)])
+		require.Equal(t, 1, cMetrics.calls[metrics.NewClaimStatus(true, false, false, false)])
 
-		require.Equal(t, 2, cMetrics.calls[metrics.SecondHalfExpiredResolved])
-		require.Equal(t, 1, cMetrics.calls[metrics.SecondHalfExpiredUnresolved])
-		require.Equal(t, 1, cMetrics.calls[metrics.SecondHalfNotExpiredResolved])
-		require.Equal(t, 1, cMetrics.calls[metrics.SecondHalfNotExpiredUnresolved])
+		// Test data is a bit weird and has unresolvable claims that have been resolved
+		require.Equal(t, 2, cMetrics.calls[metrics.NewClaimStatus(false, true, false, true)])
+		require.Equal(t, 1, cMetrics.calls[metrics.NewClaimStatus(false, true, true, false)])
+		require.Equal(t, 1, cMetrics.calls[metrics.NewClaimStatus(false, false, false, true)])
+		require.Equal(t, 1, cMetrics.calls[metrics.NewClaimStatus(false, false, false, false)])
 	})
 
 	t.Run("ZeroRecordsClaims", func(t *testing.T) {
-		monitor, _, cMetrics := newTestClaimMonitor(t)
+		monitor, _, cMetrics, _ := newTestClaimMonitor(t)
 		var games []*types.EnrichedGameData
 		monitor.CheckClaims(games)
-		// Check we zero'd out any categories that didn't have games in them (otherwise they retain their previous value)
-		require.Contains(t, cMetrics.calls, metrics.FirstHalfExpiredResolved)
-		require.Contains(t, cMetrics.calls, metrics.FirstHalfExpiredUnresolved)
-		require.Contains(t, cMetrics.calls, metrics.FirstHalfNotExpiredResolved)
-		require.Contains(t, cMetrics.calls, metrics.FirstHalfNotExpiredUnresolved)
-
-		require.Contains(t, cMetrics.calls, metrics.SecondHalfExpiredResolved)
-		require.Contains(t, cMetrics.calls, metrics.SecondHalfExpiredUnresolved)
-		require.Contains(t, cMetrics.calls, metrics.SecondHalfNotExpiredResolved)
-		require.Contains(t, cMetrics.calls, metrics.SecondHalfNotExpiredUnresolved)
+		// Should record 0 values for true and false variants of the four fields in ClaimStatus
+		require.Len(t, cMetrics.calls, 2*2*2*2)
+	})
+
+	t.Run("ConsiderChildResolvability", func(t *testing.T) {
+		monitor, _, cMetrics, logs := newTestClaimMonitor(t)
+		chessClockDuration := 10 * time.Minute
+		// Game started long enough ago that the root chess clock has now expired
+		gameStart := frozen.Add(-chessClockDuration - 15*time.Minute)
+		games := []*types.EnrichedGameData{
+			{
+				MaxClockDuration: uint64(chessClockDuration.Seconds()),
+				GameMetadata: gameTypes.GameMetadata{
+					Proxy:     common.Address{0xaa},
+					Timestamp: 50,
+				},
+				Claims: []types.EnrichedClaim{
+					{
+						Claim: faultTypes.Claim{
+							ContractIndex: 0,
+							ClaimData: faultTypes.ClaimData{
+								Position: faultTypes.RootPosition,
+							},
+							Clock: faultTypes.NewClock(time.Duration(0), gameStart),
+						},
+						Resolved: false,
+					},
+					{
+						Claim: faultTypes.Claim{ // Fast challenge, clock has expired
+							ContractIndex:       1,
+							ParentContractIndex: 0,
+							ClaimData: faultTypes.ClaimData{
+								Position: faultTypes.RootPosition,
+							},
+							Clock: faultTypes.NewClock(1*time.Minute, gameStart.Add(1*time.Minute)),
+						},
+						Resolved: false,
+					},
+					{
+						Claim: faultTypes.Claim{ // Fast counter to fast challenge, clock has expired, resolved
+							ContractIndex:       2,
+							ParentContractIndex: 1,
+							ClaimData: faultTypes.ClaimData{
+								Position: faultTypes.RootPosition,
+							},
+							Clock: faultTypes.NewClock(1*time.Minute, gameStart.Add((1+1)*time.Minute)),
+						},
+						Resolved: true,
+					},
+					{
+						Claim: faultTypes.Claim{ // Second fast counter to fast challenge, clock has expired, not resolved
+							ContractIndex:       3,
+							ParentContractIndex: 1,
+							ClaimData: faultTypes.ClaimData{
+								Position: faultTypes.RootPosition,
+							},
+							Clock: faultTypes.NewClock(1*time.Minute, gameStart.Add((1+1)*time.Minute)),
+						},
+						Resolved: false,
+					},
+					{
+						Claim: faultTypes.Claim{ // Challenge, clock has not yet expired
+							ContractIndex:       4,
+							ParentContractIndex: 0,
+							ClaimData: faultTypes.ClaimData{
+								Position: faultTypes.RootPosition,
+							},
+							Clock: faultTypes.NewClock(20*time.Minute, gameStart.Add(20*time.Minute)),
+						},
+						Resolved: false,
+					},
+					{
+						Claim: faultTypes.Claim{ // Counter to challenge, clock hasn't expired yet
+							ContractIndex:       5,
+							ParentContractIndex: 4,
+							ClaimData: faultTypes.ClaimData{
+								Position: faultTypes.RootPosition,
+							},
+							Clock: faultTypes.NewClock(1*time.Minute, gameStart.Add((20+1)*time.Minute)),
+						},
+						Resolved: false,
+					},
+				},
+			},
+		}
+		monitor.CheckClaims(games)
+		expected := &metrics.ClaimStatuses{}
+		// Root claim - clock expired, but not resolvable because of child claims
+		expected.RecordClaim(false, true, false, false)
+		// Claim 1 - clock expired, resolvable as both children are resolvable even though only one is resolved
+		expected.RecordClaim(false, true, true, false)
+		// Claim 2 - clock expired, resolvable and resolved
+		expected.RecordClaim(false, true, true, true)
+		// Claim 3 - clock expired, resolvable but not resolved
+		expected.RecordClaim(false, true, true, false)
+		// Claim 4 - clock not expired
+		expected.RecordClaim(false, false, false, false)
+		// Claim 5 - clock not expired
+		expected.RecordClaim(false, false, false, false)
+
+		expected.ForEachStatus(func(status metrics.ClaimStatus, count int) {
+			require.Equalf(t, count, cMetrics.calls[status], "status %v", status)
+		})
+
+		unresolvedClaimMsg := testlog.NewMessageFilter("Claim unresolved after clock expiration")
+		claim1Warn := logs.FindLog(unresolvedClaimMsg, testlog.NewAttributesFilter("claimContractIndex", "1"))
+		require.NotNil(t, claim1Warn, "Should warn about claim 1 being unresolved")
+		claim3Warn := logs.FindLog(unresolvedClaimMsg, testlog.NewAttributesFilter("claimContractIndex", "3"))
+		require.NotNil(t, claim3Warn, "Should warn about claim 3 being unresolved")
+
+		require.Equal(t, claim3Warn.AttrValue("delay"), claim1Warn.AttrValue("delay"),
+			"Claim 1 should have same delay as claim 3 as it could not be resolved before claim 3 clock expired")
 	})
 
 	t.Run("RecordsUnexpectedClaimResolution", func(t *testing.T) {
-		monitor, cl, cMetrics := newTestClaimMonitor(t)
+		monitor, cl, cMetrics, _ := newTestClaimMonitor(t)
 		games := makeMultipleTestGames(uint64(cl.Now().Unix()))
 		monitor.CheckClaims(games)
 
@@ -80,15 +190,16 @@ func TestClaimMonitor_CheckClaims(t *testing.T) {
 	})
 }
 
-func newTestClaimMonitor(t *testing.T) (*ClaimMonitor, *clock.DeterministicClock, *stubClaimMetrics) {
-	logger := testlog.Logger(t, log.LvlInfo)
+func newTestClaimMonitor(t *testing.T) (*ClaimMonitor, *clock.DeterministicClock, *stubClaimMetrics, *testlog.CapturingHandler) {
+	logger, handler := testlog.CaptureLogger(t, log.LvlInfo)
 	cl := clock.NewDeterministicClock(frozen)
 	metrics := &stubClaimMetrics{}
 	honestActors := []common.Address{
 		{0x01},
 		{0x02},
 	}
-	return NewClaimMonitor(logger, cl, honestActors, metrics), cl, metrics
+	monitor := NewClaimMonitor(logger, cl, honestActors, metrics)
+	return monitor, cl, metrics, handler
 }
 
 type stubClaimMetrics struct {
@@ -96,11 +207,13 @@ type stubClaimMetrics struct {
 	honest map[common.Address]metrics.HonestActorData
 }
 
-func (s *stubClaimMetrics) RecordClaims(status metrics.ClaimStatus, count int) {
+func (s *stubClaimMetrics) RecordClaims(statuses *metrics.ClaimStatuses) {
 	if s.calls == nil {
 		s.calls = make(map[metrics.ClaimStatus]int)
 	}
-	s.calls[status] += count
+	statuses.ForEachStatus(func(status metrics.ClaimStatus, count int) {
+		s.calls[status] = count
+	})
 }
 
 func (s *stubClaimMetrics) RecordHonestActorClaims(address common.Address, data *metrics.HonestActorData) {