op-dispute-mon: Identify unclaimed credits based on the withdrawal request timestamp (#11488)

op-dispute-mon/mon/bonds/monitor.go

@@ -17,22 +17,19 @@ type RClock interface {
 type BondMetrics interface {
 	RecordCredit(expectation metrics.CreditExpectation, count int)
 	RecordBondCollateral(addr common.Address, required *big.Int, available *big.Int)
-	RecordHonestWithdrawableAmounts(map[common.Address]*big.Int)
 }
 
 type Bonds struct {
 	logger  log.Logger
 	clock   RClock
 	metrics BondMetrics
-	honestActors types.HonestActors
 }
 
-func NewBonds(logger log.Logger, metrics BondMetrics, honestActors types.HonestActors, clock RClock) *Bonds {
+func NewBonds(logger log.Logger, metrics BondMetrics, clock RClock) *Bonds {
 	return &Bonds{
 		logger:  logger,
 		clock:   clock,
 		metrics: metrics,
-		honestActors: honestActors,
 	}
 }
 
@@ -50,10 +47,6 @@ func (b *Bonds) CheckBonds(games []*types.EnrichedGameData) {
 
 func (b *Bonds) checkCredits(games []*types.EnrichedGameData) {
 	creditMetrics := make(map[metrics.CreditExpectation]int)
-	honestWithdrawableAmounts := make(map[common.Address]*big.Int)
-	for address := range b.honestActors {
-		honestWithdrawableAmounts[address] = big.NewInt(0)
-	}
 
 	for _, game := range games {
 		// Check if the max duration has been reached for this game
@@ -101,12 +94,6 @@ func (b *Bonds) checkCredits(games []*types.EnrichedGameData) {
 			}
 			comparison := actual.Cmp(expected)
 			if maxDurationReached {
-				if actual.Cmp(big.NewInt(0)) > 0 && b.honestActors.Contains(recipient) {
-					total := honestWithdrawableAmounts[recipient]
-					total = new(big.Int).Add(total, actual)
-					honestWithdrawableAmounts[recipient] = total
-					b.logger.Warn("Found unclaimed credit", "recipient", recipient, "game", game.Proxy, "amount", actual)
-				}
 				if comparison > 0 {
 					creditMetrics[metrics.CreditAboveWithdrawable] += 1
 					b.logger.Warn("Credit above expected amount", "recipient", recipient, "expected", expected, "actual", actual, "game", game.Proxy, "withdrawable", "withdrawable")
@@ -136,5 +123,4 @@ func (b *Bonds) checkCredits(games []*types.EnrichedGameData) {
 	b.metrics.RecordCredit(metrics.CreditBelowNonWithdrawable, creditMetrics[metrics.CreditBelowNonWithdrawable])
 	b.metrics.RecordCredit(metrics.CreditEqualNonWithdrawable, creditMetrics[metrics.CreditEqualNonWithdrawable])
 	b.metrics.RecordCredit(metrics.CreditAboveNonWithdrawable, creditMetrics[metrics.CreditAboveNonWithdrawable])
-	b.metrics.RecordHonestWithdrawableAmounts(honestWithdrawableAmounts)
 }

op-dispute-mon/mon/bonds/monitor_test.go

@@ -18,9 +18,6 @@ import (
 
 var (
 	frozen = time.Unix(int64(time.Hour.Seconds()), 0)
-	honestActor1 = common.Address{0x11, 0xaa}
-	honestActor2 = common.Address{0x22, 0xbb}
-	honestActor3 = common.Address{0x33, 0xcc}
 )
 
 func TestCheckBonds(t *testing.T) {
@@ -64,8 +61,8 @@ func TestCheckBonds(t *testing.T) {
 }
 
 func TestCheckRecipientCredit(t *testing.T) {
-	addr1 := honestActor1
-	addr2 := honestActor2
+	addr1 := common.Address{0x11, 0xaa}
+	addr2 := common.Address{0x22, 0xbb}
 	addr3 := common.Address{0x3c}
 	addr4 := common.Address{0x4d}
 	notRootPosition := types.NewPositionFromGIndex(big.NewInt(2))
@@ -349,14 +346,6 @@ func TestCheckRecipientCredit(t *testing.T) {
 	require.Equal(t, 2, m.credits[metrics.CreditEqualNonWithdrawable], "CreditEqualNonWithdrawable")
 	require.Equal(t, 2, m.credits[metrics.CreditAboveNonWithdrawable], "CreditAboveNonWithdrawable")
 
-	require.Len(t, m.honestWithdrawable, 3)
-	requireBigInt := func(name string, expected, actual *big.Int) {
-		require.Truef(t, expected.Cmp(actual) == 0, "Expected %v withdrawable to be %v but was %v", name, expected, actual)
-	}
-	requireBigInt("honest addr1", m.honestWithdrawable[addr1], big.NewInt(19))
-	requireBigInt("honest addr2", m.honestWithdrawable[addr2], big.NewInt(13))
-	requireBigInt("honest addr3", m.honestWithdrawable[honestActor3], big.NewInt(0))
-
 	// Logs from game1
 	// addr1 is correct so has no logs
 	// addr2 is below expected before max duration, so warn about early withdrawal
@@ -382,18 +371,8 @@ func TestCheckRecipientCredit(t *testing.T) {
 		testlog.NewAttributesFilter("withdrawable", "non_withdrawable")))
 
 	// Logs from game 2
-	// addr1 is below expected - no warning as withdrawals may now be possible, but has unclaimed credit
-	require.NotNil(t, logs.FindLog(
-		testlog.NewLevelFilter(log.LevelWarn),
-		testlog.NewMessageFilter("Found unclaimed credit"),
-		testlog.NewAttributesFilter("game", game2.Proxy.Hex()),
-		testlog.NewAttributesFilter("recipient", addr1.Hex())))
-	// addr2 is correct but has unclaimed credit
-	require.NotNil(t, logs.FindLog(
-		testlog.NewLevelFilter(log.LevelWarn),
-		testlog.NewMessageFilter("Found unclaimed credit"),
-		testlog.NewAttributesFilter("game", game2.Proxy.Hex()),
-		testlog.NewAttributesFilter("recipient", addr2.Hex())))
+	// addr1 is below expected - no warning as withdrawals may now be possible
+	// addr2 is correct
 	// addr3 is above expected - warn
 	require.NotNil(t, logs.FindLog(
 		testlog.NewLevelFilter(log.LevelWarn),
@@ -422,18 +401,8 @@ func TestCheckRecipientCredit(t *testing.T) {
 		testlog.NewAttributesFilter("withdrawable", "non_withdrawable")))
 
 	// Logs from game 4
-	// addr1 is correct but has unclaimed credit
-	require.NotNil(t, logs.FindLog(
-		testlog.NewLevelFilter(log.LevelWarn),
-		testlog.NewMessageFilter("Found unclaimed credit"),
-		testlog.NewAttributesFilter("game", game4.Proxy.Hex()),
-		testlog.NewAttributesFilter("recipient", addr1.Hex())))
-	// addr2 is below expected before max duration, no log because withdrawals may be possible but warn about unclaimed
-	require.NotNil(t, logs.FindLog(
-		testlog.NewLevelFilter(log.LevelWarn),
-		testlog.NewMessageFilter("Found unclaimed credit"),
-		testlog.NewAttributesFilter("game", game4.Proxy.Hex()),
-		testlog.NewAttributesFilter("recipient", addr2.Hex())))
+	// addr1 is correct
+	// addr2 is below expected before max duration, no log because withdrawals may be possible
 	// addr3 is not involved so no logs
 	// addr4 is above expected before max duration, so warn
 	require.NotNil(t, logs.FindLog(
@@ -450,19 +419,13 @@ func setupBondMetricsTest(t *testing.T) (*Bonds, *stubBondMetrics, *testlog.Capt
 		credits:  make(map[metrics.CreditExpectation]int),
 		recorded: make(map[common.Address]Collateral),
 	}
-	honestActors := monTypes.NewHonestActors([]common.Address{honestActor1, honestActor2, honestActor3})
-	bonds := NewBonds(logger, metrics, honestActors, clock.NewDeterministicClock(frozen))
+	bonds := NewBonds(logger, metrics, clock.NewDeterministicClock(frozen))
 	return bonds, metrics, logs
 }
 
 type stubBondMetrics struct {
 	credits  map[metrics.CreditExpectation]int
 	recorded map[common.Address]Collateral
-	honestWithdrawable map[common.Address]*big.Int
-}
-
-func (s *stubBondMetrics) RecordHonestWithdrawableAmounts(values map[common.Address]*big.Int) {
-	s.honestWithdrawable = values
 }
 
 func (s *stubBondMetrics) RecordBondCollateral(addr common.Address, required *big.Int, available *big.Int) {

op-dispute-mon/mon/service.go

@@ -116,7 +116,7 @@ func (s *Service) initResolutionMonitor() {
 }
 
 func (s *Service) initWithdrawalMonitor() {
-	s.withdrawals = NewWithdrawalMonitor(s.logger, s.metrics)
+	s.withdrawals = NewWithdrawalMonitor(s.logger, s.cl, s.metrics, s.honestActors)
 }
 
 func (s *Service) initGameCallerCreator() {
@@ -145,7 +145,7 @@ func (s *Service) initForecast(cfg *config.Config) {
 }
 
 func (s *Service) initBonds() {
-	s.bonds = bonds.NewBonds(s.logger, s.metrics, s.honestActors, s.cl)
+	s.bonds = bonds.NewBonds(s.logger, s.metrics, s.cl)
 }
 
 func (s *Service) initOutputRollupClient(ctx context.Context, cfg *config.Config) error {

op-dispute-mon/mon/withdrawals.go

 package mon
 
 import (
+	"math/big"
+	"time"
+
 	"github.com/ethereum-optimism/optimism/op-dispute-mon/mon/types"
 	"github.com/ethereum/go-ethereum/common"
 	"github.com/ethereum/go-ethereum/log"
@@ -8,25 +11,35 @@ import (
 
 type WithdrawalMetrics interface {
 	RecordWithdrawalRequests(delayedWeth common.Address, matches bool, count int)
+	RecordHonestWithdrawableAmounts(map[common.Address]*big.Int)
 }
 
 type WithdrawalMonitor struct {
 	logger       log.Logger
+	clock        RClock
 	metrics      WithdrawalMetrics
+	honestActors types.HonestActors
 }
 
-func NewWithdrawalMonitor(logger log.Logger, metrics WithdrawalMetrics) *WithdrawalMonitor {
+func NewWithdrawalMonitor(logger log.Logger, clock RClock, metrics WithdrawalMetrics, honestActors types.HonestActors) *WithdrawalMonitor {
 	return &WithdrawalMonitor{
 		logger:       logger,
+		clock:        clock,
 		metrics:      metrics,
+		honestActors: honestActors,
 	}
 }
 
 func (w *WithdrawalMonitor) CheckWithdrawals(games []*types.EnrichedGameData) {
+	now := w.clock.Now() // Use a consistent time for all checks
 	matching := make(map[common.Address]int)
 	divergent := make(map[common.Address]int)
+	honestWithdrawableAmounts := make(map[common.Address]*big.Int)
+	for address := range w.honestActors {
+		honestWithdrawableAmounts[address] = big.NewInt(0)
+	}
 	for _, game := range games {
-		matches, diverges := w.validateGameWithdrawals(game)
+		matches, diverges := w.validateGameWithdrawals(game, now, honestWithdrawableAmounts)
 		matching[game.WETHContract] += matches
 		divergent[game.WETHContract] += diverges
 	}
@@ -36,9 +49,10 @@ func (w *WithdrawalMonitor) CheckWithdrawals(games []*types.EnrichedGameData) {
 	for contract, count := range divergent {
 		w.metrics.RecordWithdrawalRequests(contract, false, count)
 	}
+	w.metrics.RecordHonestWithdrawableAmounts(honestWithdrawableAmounts)
 }
 
-func (w *WithdrawalMonitor) validateGameWithdrawals(game *types.EnrichedGameData) (int, int) {
+func (w *WithdrawalMonitor) validateGameWithdrawals(game *types.EnrichedGameData, now time.Time, honestWithdrawableAmounts map[common.Address]*big.Int) (int, int) {
 	matching := 0
 	divergent := 0
 	for recipient, withdrawalAmount := range game.WithdrawalRequests {
@@ -48,6 +62,16 @@ func (w *WithdrawalMonitor) validateGameWithdrawals(game *types.EnrichedGameData
 			divergent++
 			w.logger.Error("Withdrawal request amount does not match credit", "game", game.Proxy, "recipient", recipient, "credit", game.Credits[recipient], "withdrawal", game.WithdrawalRequests[recipient].Amount)
 		}
+
+		if withdrawalAmount.Amount.Cmp(big.NewInt(0)) > 0 && w.honestActors.Contains(recipient) {
+			if time.Unix(withdrawalAmount.Timestamp.Int64(), 0).Add(game.WETHDelay).Before(now) {
+				// Credits are withdrawable
+				total := honestWithdrawableAmounts[recipient]
+				total = new(big.Int).Add(total, withdrawalAmount.Amount)
+				honestWithdrawableAmounts[recipient] = total
+				w.logger.Warn("Found unclaimed credit", "recipient", recipient, "game", game.Proxy, "amount", withdrawalAmount.Amount)
+			}
+		}
 	}
 	return matching, divergent
 }

op-dispute-mon/mon/withdrawals_test.go

@@ -3,9 +3,12 @@ package mon
 import (
 	"math/big"
 	"testing"
+	"time"
 
 	"github.com/ethereum-optimism/optimism/op-challenger/game/fault/contracts"
+	"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/testlog"
 	"github.com/ethereum/go-ethereum/common"
 	"github.com/ethereum/go-ethereum/log"
@@ -15,58 +18,76 @@ import (
 var (
 	weth1 = common.Address{0x1a}
 	weth2 = common.Address{0x2b}
+
+	honestActor1    = common.Address{0x11, 0xaa}
+	honestActor2    = common.Address{0x22, 0xbb}
+	honestActor3    = common.Address{0x33, 0xcc}
+	dishonestActor4 = common.Address{0x44, 0xdd}
+
+	nowUnix = int64(10_000)
 )
 
 func makeGames() []*monTypes.EnrichedGameData {
 	weth1Balance := big.NewInt(4200)
 	weth2Balance := big.NewInt(6000)
+
 	game1 := &monTypes.EnrichedGameData{
+		GameMetadata: types.GameMetadata{Proxy: common.Address{0x11, 0x11, 0x11}},
 		Credits: map[common.Address]*big.Int{
-			common.Address{0x01}: big.NewInt(3),
-			common.Address{0x02}: big.NewInt(1),
+			honestActor1: big.NewInt(3),
+			honestActor2: big.NewInt(1),
 		},
 		WithdrawalRequests: map[common.Address]*contracts.WithdrawalRequest{
-			common.Address{0x01}: &contracts.WithdrawalRequest{Amount: big.NewInt(3)},
-			common.Address{0x02}: &contracts.WithdrawalRequest{Amount: big.NewInt(1)},
+			honestActor1: {Amount: big.NewInt(3), Timestamp: big.NewInt(nowUnix - 101)}, // Claimable
+			honestActor2: {Amount: big.NewInt(1), Timestamp: big.NewInt(nowUnix - 99)},  // Not claimable
 		},
 		WETHContract:  weth1,
 		ETHCollateral: weth1Balance,
+		WETHDelay:     100 * time.Second,
 	}
 	game2 := &monTypes.EnrichedGameData{
+		GameMetadata: types.GameMetadata{Proxy: common.Address{0x22, 0x22, 0x22}},
 		Credits: map[common.Address]*big.Int{
-			common.Address{0x01}: big.NewInt(46),
-			common.Address{0x02}: big.NewInt(1),
+			honestActor1: big.NewInt(46),
+			honestActor2: big.NewInt(1),
 		},
 		WithdrawalRequests: map[common.Address]*contracts.WithdrawalRequest{
-			common.Address{0x01}: &contracts.WithdrawalRequest{Amount: big.NewInt(3)},
-			common.Address{0x02}: &contracts.WithdrawalRequest{Amount: big.NewInt(1)},
+			honestActor1: {Amount: big.NewInt(3), Timestamp: big.NewInt(nowUnix - 501)}, // Claimable
+			honestActor2: {Amount: big.NewInt(1), Timestamp: big.NewInt(nowUnix)},       // Not claimable
 		},
 		WETHContract:  weth2,
 		ETHCollateral: weth2Balance,
+		WETHDelay:     500 * time.Second,
 	}
 	game3 := &monTypes.EnrichedGameData{
+		GameMetadata: types.GameMetadata{Proxy: common.Address{0x33, 0x33, 0x33}},
 		Credits: map[common.Address]*big.Int{
-			common.Address{0x03}: big.NewInt(2),
-			common.Address{0x04}: big.NewInt(4),
+			honestActor3:    big.NewInt(2),
+			dishonestActor4: big.NewInt(4),
 		},
 		WithdrawalRequests: map[common.Address]*contracts.WithdrawalRequest{
-			common.Address{0x03}: &contracts.WithdrawalRequest{Amount: big.NewInt(2)},
-			common.Address{0x04}: &contracts.WithdrawalRequest{Amount: big.NewInt(4)},
+			honestActor3:    {Amount: big.NewInt(2), Timestamp: big.NewInt(nowUnix - 1)}, // Claimable
+			dishonestActor4: {Amount: big.NewInt(4), Timestamp: big.NewInt(nowUnix - 5)}, // Claimable
 		},
 		WETHContract:  weth2,
 		ETHCollateral: weth2Balance,
+		WETHDelay:     0 * time.Second,
 	}
 	return []*monTypes.EnrichedGameData{game1, game2, game3}
 }
 
 func TestCheckWithdrawals(t *testing.T) {
-	logger := testlog.Logger(t, log.LvlInfo)
+	now := time.Unix(nowUnix, 0)
+	cl := clock.NewDeterministicClock(now)
+	logger, logs := testlog.CaptureLogger(t, log.LvlInfo)
 	metrics := &stubWithdrawalsMetrics{
 		matching:  make(map[common.Address]int),
 		divergent: make(map[common.Address]int),
 	}
-	withdrawals := NewWithdrawalMonitor(logger, metrics)
-	withdrawals.CheckWithdrawals(makeGames())
+	honestActors := monTypes.NewHonestActors([]common.Address{honestActor1, honestActor2, honestActor3})
+	withdrawals := NewWithdrawalMonitor(logger, cl, metrics, honestActors)
+	games := makeGames()
+	withdrawals.CheckWithdrawals(games)
 
 	require.Equal(t, metrics.matchCalls, 2)
 	require.Equal(t, metrics.divergeCalls, 2)
@@ -80,6 +101,47 @@ func TestCheckWithdrawals(t *testing.T) {
 	require.Equal(t, metrics.matching[weth2], 3)
 	require.Equal(t, metrics.divergent[weth1], 0)
 	require.Equal(t, metrics.divergent[weth2], 1)
+
+	require.Len(t, metrics.honestWithdrawable, 3)
+	requireBigInt := func(name string, expected, actual *big.Int) {
+		require.Truef(t, expected.Cmp(actual) == 0, "Expected %v withdrawable to be %v but was %v", name, expected, actual)
+	}
+	requireBigInt("honest addr1", big.NewInt(6), metrics.honestWithdrawable[honestActor1])
+	requireBigInt("honest addr2", big.NewInt(0), metrics.honestWithdrawable[honestActor2])
+	requireBigInt("honest addr3", big.NewInt(2), metrics.honestWithdrawable[honestActor3])
+	require.Nil(t, metrics.honestWithdrawable[dishonestActor4], "should only report withdrawable credits for honest actors")
+
+	findUnclaimedCreditWarning := func(game common.Address, actor common.Address) *testlog.CapturedRecord {
+		return logs.FindLog(
+			testlog.NewLevelFilter(log.LevelWarn),
+			testlog.NewMessageFilter("Found unclaimed credit"),
+			testlog.NewAttributesFilter("game", game.Hex()),
+			testlog.NewAttributesFilter("recipient", actor.Hex()))
+	}
+	requireUnclaimedWarning := func(game common.Address, actor common.Address) {
+		require.NotNil(t, findUnclaimedCreditWarning(game, actor))
+	}
+	noUnclaimedWarning := func(game common.Address, actor common.Address) {
+		require.Nil(t, findUnclaimedCreditWarning(game, actor))
+	}
+	// Game 1, unclaimed for honestActor1 only
+	requireUnclaimedWarning(games[0].Proxy, honestActor1)
+	noUnclaimedWarning(games[0].Proxy, honestActor2)
+	noUnclaimedWarning(games[0].Proxy, honestActor3)
+	noUnclaimedWarning(games[0].Proxy, dishonestActor4)
+
+	// Game 2, unclaimed for honestActor1 only
+	requireUnclaimedWarning(games[1].Proxy, honestActor1)
+	noUnclaimedWarning(games[1].Proxy, honestActor2)
+	noUnclaimedWarning(games[1].Proxy, honestActor3)
+	noUnclaimedWarning(games[1].Proxy, dishonestActor4)
+
+	// Game 3, unclaimed for honestActor3 only
+	// dishonestActor4 has unclaimed credits but we don't track them
+	noUnclaimedWarning(games[2].Proxy, honestActor1)
+	noUnclaimedWarning(games[2].Proxy, honestActor2)
+	requireUnclaimedWarning(games[2].Proxy, honestActor3)
+	noUnclaimedWarning(games[2].Proxy, dishonestActor4)
 }
 
 type stubWithdrawalsMetrics struct {
@@ -87,6 +149,11 @@ type stubWithdrawalsMetrics struct {
 	divergeCalls       int
 	matching           map[common.Address]int
 	divergent          map[common.Address]int
+	honestWithdrawable map[common.Address]*big.Int
+}
+
+func (s *stubWithdrawalsMetrics) RecordHonestWithdrawableAmounts(honestWithdrawable map[common.Address]*big.Int) {
+	s.honestWithdrawable = honestWithdrawable
 }
 
 func (s *stubWithdrawalsMetrics) RecordWithdrawalRequests(addr common.Address, matches bool, count int) {