Commit d7ac3e97 authored by Adrian Sutton's avatar Adrian Sutton Committed by GitHub

Manage game data directories at the multi-game level (#6968)

* op-challenger: Manage allocation of game data directories in multi-game code.

* op-challenger: Delete all directories except required games
parent ba1a5942
......@@ -50,7 +50,7 @@ type CannonTraceProvider struct {
lastProof *proofData
}
func NewTraceProvider(ctx context.Context, logger log.Logger, cfg *config.Config, l1Client bind.ContractCaller, gameAddr common.Address) (*CannonTraceProvider, error) {
func NewTraceProvider(ctx context.Context, logger log.Logger, cfg *config.Config, l1Client bind.ContractCaller, dir string, gameAddr common.Address) (*CannonTraceProvider, error) {
l2Client, err := ethclient.DialContext(ctx, cfg.CannonL2)
if err != nil {
return nil, fmt.Errorf("dial l2 client %v: %w", cfg.CannonL2, err)
......@@ -64,11 +64,10 @@ func NewTraceProvider(ctx context.Context, logger log.Logger, cfg *config.Config
if err != nil {
return nil, fmt.Errorf("fetch local game inputs: %w", err)
}
return NewTraceProviderFromInputs(logger, cfg, gameAddr.Hex(), localInputs), nil
return NewTraceProviderFromInputs(logger, cfg, localInputs, dir), nil
}
func NewTraceProviderFromInputs(logger log.Logger, cfg *config.Config, gameDirName string, localInputs LocalGameInputs) *CannonTraceProvider {
dir := filepath.Join(cfg.Datadir, gameDirName)
func NewTraceProviderFromInputs(logger log.Logger, cfg *config.Config, localInputs LocalGameInputs, dir string) *CannonTraceProvider {
return &CannonTraceProvider{
logger: logger,
dir: dir,
......@@ -118,10 +117,6 @@ func (p *CannonTraceProvider) AbsolutePreState(ctx context.Context) ([]byte, err
return state.EncodeWitness(), nil
}
func (p *CannonTraceProvider) Cleanup() error {
return os.RemoveAll(p.dir)
}
// loadProof will attempt to load or generate the proof data at the specified index
// If the requested index is beyond the end of the actual trace it is extended with no-op instructions.
func (p *CannonTraceProvider) loadProof(ctx context.Context, i uint64) (*proofData, error) {
......
......@@ -11,7 +11,6 @@ import (
"testing"
"github.com/ethereum-optimism/optimism/cannon/mipsevm"
"github.com/ethereum-optimism/optimism/op-challenger/config"
"github.com/ethereum-optimism/optimism/op-challenger/fault/types"
"github.com/ethereum-optimism/optimism/op-node/testlog"
"github.com/ethereum/go-ethereum/common"
......@@ -189,29 +188,6 @@ func TestAbsolutePreState(t *testing.T) {
})
}
func TestUseGameSpecificSubdir(t *testing.T) {
tempDir := t.TempDir()
dataDir := filepath.Join(tempDir, "data")
setupPreState(t, tempDir, "state.json")
logger := testlog.Logger(t, log.LvlInfo)
cfg := &config.Config{
CannonAbsolutePreState: filepath.Join(tempDir, "state.json"),
Datadir: dataDir,
}
gameDirName := "gameSubdir"
localInputs := LocalGameInputs{}
provider := NewTraceProviderFromInputs(logger, cfg, gameDirName, localInputs)
require.Equal(t, filepath.Join(dataDir, gameDirName), provider.dir, "should use game specific subdir")
}
func TestCleanup(t *testing.T) {
dataDir, prestate := setupTestData(t)
provider, _ := setupWithTestData(t, dataDir, prestate)
require.NoError(t, provider.Cleanup())
_, err := os.Stat(dataDir)
require.ErrorIs(t, err, os.ErrNotExist)
}
func setupPreState(t *testing.T, dataDir string, filename string) {
srcDir := filepath.Join("test_data")
path := filepath.Join(srcDir, filename)
......
package fault
import (
"fmt"
"os"
"path/filepath"
"strings"
"github.com/ethereum/go-ethereum/common"
"github.com/hashicorp/go-multierror"
"golang.org/x/exp/slices"
)
const gameDirPrefix = "game-"
// diskManager coordinates
type diskManager struct {
datadir string
}
func newDiskManager(dir string) *diskManager {
return &diskManager{datadir: dir}
}
func (d *diskManager) DirForGame(addr common.Address) string {
return filepath.Join(d.datadir, gameDirPrefix+addr.Hex())
}
func (d *diskManager) RemoveAllExcept(keep []common.Address) error {
entries, err := os.ReadDir(d.datadir)
if err != nil {
return fmt.Errorf("failed to list directory: %w", err)
}
var result error
for _, entry := range entries {
if !entry.IsDir() || !strings.HasPrefix(entry.Name(), gameDirPrefix) {
// Skip files and directories that don't have the game directory prefix.
// While random content shouldn't be in our datadir, we want to avoid
// deleting things like OS generated files.
continue
}
name := entry.Name()[len(gameDirPrefix):]
addr := common.HexToAddress(name)
if addr == (common.Address{}) {
// Couldn't parse the directory name to an address so mustn't be a game directory
continue
}
if slices.Contains(keep, addr) {
// We need to preserve this data
continue
}
if err := os.RemoveAll(filepath.Join(d.datadir, entry.Name())); err != nil {
result = multierror.Append(result, err)
}
}
return result
}
package fault
import (
"os"
"path/filepath"
"testing"
"github.com/ethereum/go-ethereum/common"
"github.com/stretchr/testify/require"
)
func TestDiskManager_DirForGame(t *testing.T) {
baseDir := t.TempDir()
addr := common.Address{0x53}
disk := newDiskManager(baseDir)
result := disk.DirForGame(addr)
require.Equal(t, filepath.Join(baseDir, gameDirPrefix+addr.Hex()), result)
}
func TestDiskManager_RemoveAllExcept(t *testing.T) {
baseDir := t.TempDir()
keep := common.Address{0x53}
delete := common.Address{0xaa}
disk := newDiskManager(baseDir)
keepDir := disk.DirForGame(keep)
deleteDir := disk.DirForGame(delete)
unexpectedFile := filepath.Join(baseDir, "file.txt")
require.NoError(t, os.WriteFile(unexpectedFile, []byte("test"), 0644))
unexpectedDir := filepath.Join(baseDir, "notagame")
require.NoError(t, os.MkdirAll(unexpectedDir, 0777))
invalidHexDir := filepath.Join(baseDir, gameDirPrefix+"0xNOPE")
require.NoError(t, os.MkdirAll(invalidHexDir, 0777))
populateDir := func(dir string) []string {
require.NoError(t, os.MkdirAll(dir, 0777))
file1 := filepath.Join(dir, "test.txt")
require.NoError(t, os.WriteFile(file1, []byte("foo"), 0644))
nestedDirs := filepath.Join(dir, "subdir", "deep")
require.NoError(t, os.MkdirAll(nestedDirs, 0777))
file2 := filepath.Join(nestedDirs, ".foo.txt")
require.NoError(t, os.WriteFile(file2, []byte("foo"), 0644))
return []string{file1, file2}
}
keepFiles := populateDir(keepDir)
populateDir(deleteDir)
require.NoError(t, disk.RemoveAllExcept([]common.Address{keep}))
require.NoDirExists(t, deleteDir, "should have deleted directory")
for _, file := range keepFiles {
require.FileExists(t, file, "should have kept file for active game")
}
require.FileExists(t, unexpectedFile, "should not delete unexpected file")
require.DirExists(t, unexpectedDir, "should not delete unexpected dir")
require.DirExists(t, invalidHexDir, "should not delete dir with invalid address")
}
......@@ -13,10 +13,9 @@ import (
type gamePlayer interface {
ProgressGame(ctx context.Context) bool
Cleanup() error
}
type playerCreator func(address common.Address) (gamePlayer, error)
type playerCreator func(address common.Address, dir string) (gamePlayer, error)
type blockNumberFetcher func(ctx context.Context) (uint64, error)
// gameSource loads information about the games available to play
......@@ -24,9 +23,15 @@ type gameSource interface {
FetchAllGamesAtBlock(ctx context.Context, earliest uint64, blockNumber *big.Int) ([]FaultDisputeGame, error)
}
type gameDiskAllocator interface {
DirForGame(common.Address) string
RemoveAllExcept([]common.Address) error
}
type gameMonitor struct {
logger log.Logger
clock clock.Clock
diskManager gameDiskAllocator
source gameSource
gameWindow time.Duration
createPlayer playerCreator
......@@ -35,10 +40,20 @@ type gameMonitor struct {
players map[common.Address]gamePlayer
}
func newGameMonitor(logger log.Logger, gameWindow time.Duration, cl clock.Clock, fetchBlockNumber blockNumberFetcher, allowedGames []common.Address, source gameSource, createGame playerCreator) *gameMonitor {
func newGameMonitor(
logger log.Logger,
gameWindow time.Duration,
cl clock.Clock,
disk gameDiskAllocator,
fetchBlockNumber blockNumberFetcher,
allowedGames []common.Address,
source gameSource,
createGame playerCreator,
) *gameMonitor {
return &gameMonitor{
logger: logger,
clock: cl,
diskManager: disk,
source: source,
gameWindow: gameWindow,
createPlayer: createGame,
......@@ -82,6 +97,7 @@ func (m *gameMonitor) progressGames(ctx context.Context) error {
return fmt.Errorf("failed to load games: %w", err)
}
requiredGames := make(map[common.Address]bool)
var keepGameData []common.Address
for _, game := range games {
if !m.allowedGame(game.Proxy) {
m.logger.Debug("Skipping game not on allow list", "game", game.Proxy)
......@@ -94,15 +110,17 @@ func (m *gameMonitor) progressGames(ctx context.Context) error {
continue
}
done := player.ProgressGame(ctx)
if done {
// Remove resources on disk as soon as the game is complete to save disk space.
if !done {
// We only keep resources on disk for games that are incomplete.
// Games that are complete have their data removed as soon as possible to save disk space.
// We keep the player in memory to avoid recreating it on every update but will no longer
// need the resources on disk because there are no further actions required on the game.
if err := player.Cleanup(); err != nil {
m.logger.Error("Unable to cleanup player data", "err", err)
}
keepGameData = append(keepGameData, game.Proxy)
}
}
if err := m.diskManager.RemoveAllExcept(keepGameData); err != nil {
m.logger.Error("Unable to cleanup game data", "err", err)
}
// Remove the player for any game that's no longer being returned from the list of active games
for addr := range m.players {
if _, ok := requiredGames[addr]; ok {
......@@ -118,7 +136,7 @@ func (m *gameMonitor) fetchOrCreateGamePlayer(gameData FaultDisputeGame) (gamePl
if player, ok := m.players[gameData.Proxy]; ok {
return player, nil
}
player, err := m.createPlayer(gameData.Proxy)
player, err := m.createPlayer(gameData.Proxy, m.diskManager.DirForGame(gameData.Proxy))
if err != nil {
return nil, fmt.Errorf("failed to create game player %v: %w", gameData.Proxy, err)
}
......
......@@ -10,6 +10,7 @@ import (
"github.com/ethereum/go-ethereum/common"
"github.com/ethereum/go-ethereum/log"
"github.com/stretchr/testify/require"
"golang.org/x/exp/slices"
"github.com/ethereum-optimism/optimism/op-node/testlog"
)
......@@ -18,20 +19,20 @@ func TestMonitorMinGameTimestamp(t *testing.T) {
t.Parallel()
t.Run("zero game window returns zero", func(t *testing.T) {
monitor, _, _ := setupMonitorTest(t, []common.Address{})
monitor, _, _, _ := setupMonitorTest(t, []common.Address{})
monitor.gameWindow = time.Duration(0)
require.Equal(t, monitor.minGameTimestamp(), uint64(0))
})
t.Run("non-zero game window with zero clock", func(t *testing.T) {
monitor, _, _ := setupMonitorTest(t, []common.Address{})
monitor, _, _, _ := setupMonitorTest(t, []common.Address{})
monitor.gameWindow = time.Minute
monitor.clock = clock.NewDeterministicClock(time.Unix(0, 0))
require.Equal(t, monitor.minGameTimestamp(), uint64(0))
})
t.Run("minimum computed correctly", func(t *testing.T) {
monitor, _, _ := setupMonitorTest(t, []common.Address{})
monitor, _, _, _ := setupMonitorTest(t, []common.Address{})
monitor.gameWindow = time.Minute
frozen := time.Unix(int64(time.Hour.Seconds()), 0)
monitor.clock = clock.NewDeterministicClock(frozen)
......@@ -41,7 +42,7 @@ func TestMonitorMinGameTimestamp(t *testing.T) {
}
func TestMonitorExitsWhenContextDone(t *testing.T) {
monitor, _, _ := setupMonitorTest(t, []common.Address{common.Address{}})
monitor, _, _, _ := setupMonitorTest(t, []common.Address{{}})
ctx, cancel := context.WithCancel(context.Background())
cancel()
err := monitor.MonitorGames(ctx)
......@@ -49,7 +50,7 @@ func TestMonitorExitsWhenContextDone(t *testing.T) {
}
func TestMonitorCreateAndProgressGameAgents(t *testing.T) {
monitor, source, games := setupMonitorTest(t, []common.Address{})
monitor, source, games, _ := setupMonitorTest(t, []common.Address{})
addr1 := common.Address{0xaa}
addr2 := common.Address{0xbb}
......@@ -82,7 +83,7 @@ func TestMonitorCreateAndProgressGameAgents(t *testing.T) {
func TestMonitorOnlyCreateSpecifiedGame(t *testing.T) {
addr1 := common.Address{0xaa}
addr2 := common.Address{0xbb}
monitor, source, games := setupMonitorTest(t, []common.Address{addr2})
monitor, source, games, _ := setupMonitorTest(t, []common.Address{addr2})
source.games = []FaultDisputeGame{
{
......@@ -107,7 +108,7 @@ func TestMonitorOnlyCreateSpecifiedGame(t *testing.T) {
func TestDeletePlayersWhenNoLongerInListOfGames(t *testing.T) {
addr1 := common.Address{0xaa}
addr2 := common.Address{0xbb}
monitor, source, games := setupMonitorTest(t, nil)
monitor, source, games, _ := setupMonitorTest(t, nil)
allGames := []FaultDisputeGame{
{
......@@ -147,13 +148,19 @@ func TestDeletePlayersWhenNoLongerInListOfGames(t *testing.T) {
}
func TestCleanupResourcesOfCompletedGames(t *testing.T) {
monitor, source, games := setupMonitorTest(t, []common.Address{})
games.createCompleted = true
addr1 := common.Address{0xaa}
addr2 := common.Address{0xbb}
monitor, source, games, disk := setupMonitorTest(t, []common.Address{})
games.createCompleted = addr1
source.games = []FaultDisputeGame{
{
Proxy: addr1,
Timestamp: 1999,
},
{
Proxy: addr2,
Timestamp: 9999,
},
}
......@@ -161,13 +168,19 @@ func TestCleanupResourcesOfCompletedGames(t *testing.T) {
err := monitor.progressGames(context.Background())
require.NoError(t, err)
require.Len(t, games.created, 1, "should create game agents")
require.Len(t, games.created, 2, "should create game agents")
require.Contains(t, games.created, addr1)
require.Contains(t, games.created, addr2)
require.Equal(t, 1, games.created[addr1].progressCount)
require.Equal(t, 1, games.created[addr1].cleanupCount, "should clean up once game is done")
require.Equal(t, 1, games.created[addr2].progressCount)
require.Contains(t, disk.gameDirExists, addr1, "should have allocated a game dir for game 1")
require.False(t, disk.gameDirExists[addr1], "should have then deleted the game 1 dir")
require.Contains(t, disk.gameDirExists, addr2, "should have allocated a game dir for game 2")
require.True(t, disk.gameDirExists[addr2], "should not have deleted the game 2 dir")
}
func setupMonitorTest(t *testing.T, allowedGames []common.Address) (*gameMonitor, *stubGameSource, *createdGames) {
func setupMonitorTest(t *testing.T, allowedGames []common.Address) (*gameMonitor, *stubGameSource, *createdGames, *stubDiskManager) {
logger := testlog.Logger(t, log.LvlDebug)
source := &stubGameSource{}
games := &createdGames{
......@@ -177,8 +190,11 @@ func setupMonitorTest(t *testing.T, allowedGames []common.Address) (*gameMonitor
fetchBlockNum := func(ctx context.Context) (uint64, error) {
return 1234, nil
}
monitor := newGameMonitor(logger, time.Duration(0), clock.SystemClock, fetchBlockNum, allowedGames, source, games.CreateGame)
return monitor, source, games
disk := &stubDiskManager{
gameDirExists: make(map[common.Address]bool),
}
monitor := newGameMonitor(logger, time.Duration(0), clock.SystemClock, disk, fetchBlockNum, allowedGames, source, games.CreateGame)
return monitor, source, games, disk
}
type stubGameSource struct {
......@@ -192,8 +208,8 @@ func (s *stubGameSource) FetchAllGamesAtBlock(ctx context.Context, earliest uint
type stubGame struct {
addr common.Address
progressCount int
cleanupCount int
done bool
dir string
}
func (g *stubGame) ProgressGame(ctx context.Context) bool {
......@@ -201,22 +217,37 @@ func (g *stubGame) ProgressGame(ctx context.Context) bool {
return g.done
}
func (g *stubGame) Cleanup() error {
g.cleanupCount++
return nil
}
type createdGames struct {
t *testing.T
createCompleted bool
createCompleted common.Address
created map[common.Address]*stubGame
}
func (c *createdGames) CreateGame(addr common.Address) (gamePlayer, error) {
func (c *createdGames) CreateGame(addr common.Address, dir string) (gamePlayer, error) {
if _, exists := c.created[addr]; exists {
c.t.Fatalf("game %v already exists", addr)
}
game := &stubGame{addr: addr, done: c.createCompleted}
game := &stubGame{
addr: addr,
done: addr == c.createCompleted,
dir: dir,
}
c.created[addr] = game
return game, nil
}
type stubDiskManager struct {
gameDirExists map[common.Address]bool
}
func (s *stubDiskManager) DirForGame(addr common.Address) string {
s.gameDirExists[addr] = true
return addr.Hex()
}
func (s *stubDiskManager) RemoveAllExcept(addrs []common.Address) error {
for address := range s.gameDirExists {
s.gameDirExists[address] = slices.Contains(addrs, address)
}
return nil
}
......@@ -29,7 +29,6 @@ type GamePlayer struct {
agreeWithProposedOutput bool
caller GameInfo
logger log.Logger
cleanup func() error
completed bool
}
......@@ -38,6 +37,7 @@ func NewGamePlayer(
ctx context.Context,
logger log.Logger,
cfg *config.Config,
dir string,
addr common.Address,
txMgr txmgr.TxManager,
client bind.ContractCaller,
......@@ -57,14 +57,12 @@ func NewGamePlayer(
var provider types.TraceProvider
var updater types.OracleUpdater
var cleanup func() error
switch cfg.TraceType {
case config.TraceTypeCannon:
cannonProvider, err := cannon.NewTraceProvider(ctx, logger, cfg, client, addr)
cannonProvider, err := cannon.NewTraceProvider(ctx, logger, cfg, client, dir, addr)
if err != nil {
return nil, fmt.Errorf("create cannon trace provider: %w", err)
}
cleanup = cannonProvider.Cleanup
provider = cannonProvider
updater, err = cannon.NewOracleUpdater(ctx, logger, txMgr, addr, client)
if err != nil {
......@@ -72,7 +70,6 @@ func NewGamePlayer(
}
case config.TraceTypeAlphabet:
provider = alphabet.NewTraceProvider(cfg.AlphabetTrace, gameDepth)
cleanup = func() error { return nil }
updater = alphabet.NewOracleUpdater(logger)
default:
return nil, fmt.Errorf("unsupported trace type: %v", cfg.TraceType)
......@@ -97,7 +94,6 @@ func NewGamePlayer(
agreeWithProposedOutput: cfg.AgreeWithProposedOutput,
caller: caller,
logger: logger,
cleanup: cleanup,
}, nil
}
......@@ -143,7 +139,3 @@ func (g *GamePlayer) logGameStatus(ctx context.Context, status types.GameStatus)
g.logger.Error("Game lost", "status", status)
}
}
func (g *GamePlayer) Cleanup() error {
return g.cleanup()
}
......@@ -27,24 +27,6 @@ func TestProgressGame_LogErrorFromAct(t *testing.T) {
require.Equal(t, uint64(1), msg.GetContextValue("claims"))
}
func TestCleanup(t *testing.T) {
t.Run("default cleanup", func(t *testing.T) {
game := &GamePlayer{
cleanup: func() error { return nil },
}
require.NoError(t, game.Cleanup())
})
t.Run("cleanup bubbles up error", func(t *testing.T) {
err := errors.New("wassie")
game := &GamePlayer{
cleanup: func() error { return err },
}
require.Error(t, err, game.Cleanup())
})
}
func TestProgressGame_LogGameStatus(t *testing.T) {
tests := []struct {
name string
......
......@@ -73,9 +73,18 @@ func NewService(ctx context.Context, logger log.Logger, cfg *config.Config) (*se
}
loader := NewGameLoader(factory)
monitor := newGameMonitor(logger, cfg.GameWindow, cl, client.BlockNumber, cfg.GameAllowlist, loader, func(addr common.Address) (gamePlayer, error) {
return NewGamePlayer(ctx, logger, cfg, addr, txMgr, client)
})
disk := newDiskManager(cfg.Datadir)
monitor := newGameMonitor(
logger,
cfg.GameWindow,
cl,
disk,
client.BlockNumber,
cfg.GameAllowlist,
loader,
func(addr common.Address, dir string) (gamePlayer, error) {
return NewGamePlayer(ctx, logger, cfg, dir, addr, txMgr, client)
})
m.RecordInfo(version.SimpleWithMeta)
m.RecordUp()
......
......@@ -2,6 +2,7 @@ package disputegame
import (
"context"
"path/filepath"
"github.com/ethereum-optimism/optimism/op-challenger/fault/cannon"
"github.com/ethereum-optimism/optimism/op-e2e/e2eutils/challenger"
......@@ -38,7 +39,8 @@ func (g *CannonGameHelper) CreateHonestActor(ctx context.Context, rollupCfg *rol
}
opts = append(opts, options...)
cfg := challenger.NewChallengerConfig(g.t, l1Endpoint, opts...)
provider, err := cannon.NewTraceProvider(ctx, testlog.Logger(g.t, log.LvlInfo).New("role", "CorrectTrace"), cfg, l1Client, g.addr)
logger := testlog.Logger(g.t, log.LvlInfo).New("role", "CorrectTrace")
provider, err := cannon.NewTraceProvider(ctx, logger, cfg, l1Client, filepath.Join(cfg.Datadir, "honest"), g.addr)
g.require.NoError(err, "create cannon trace provider")
return &HonestHelper{
......
......@@ -175,7 +175,7 @@ func (h *FactoryHelper) StartCannonGameWithCorrectRoot(ctx context.Context, roll
L2Claim: challengedOutput.OutputRoot,
L2BlockNumber: challengedOutput.L2BlockNumber,
}
provider := cannon.NewTraceProviderFromInputs(testlog.Logger(h.t, log.LvlInfo).New("role", "CorrectTrace"), cfg, "correct", inputs)
provider := cannon.NewTraceProviderFromInputs(testlog.Logger(h.t, log.LvlInfo).New("role", "CorrectTrace"), cfg, inputs, cfg.Datadir)
rootClaim, err := provider.Get(ctx, math.MaxUint64)
h.require.NoError(err, "Compute correct root hash")
......
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