diff --git a/cl/phase1/forkchoice/fork_graph/fork_graph_disk.go b/cl/phase1/forkchoice/fork_graph/fork_graph_disk.go index baefe9450e2c..44caf7ea62c2 100644 --- a/cl/phase1/forkchoice/fork_graph/fork_graph_disk.go +++ b/cl/phase1/forkchoice/fork_graph/fork_graph_disk.go @@ -151,9 +151,13 @@ type forkGraphDisk struct { envelopeMissing sync.Map // common.Hash -> struct{} invalidEnvelopes sync.Map // common.Hash -> struct{} - // reusable disk I/O buffers, each guarded by the mutex above it - stateDumpLock sync.Mutex - sszBuffer []byte + // stateDumpLock guards the reusable state buffer and the key of the last state dump + stateDumpLock sync.Mutex + sszBuffer []byte + lastDumpedRoot common.Hash + lastDumpedSlot uint64 + + // envelopeMu guards the reusable envelope buffer envelopeMu sync.Mutex envelopeSSZBuffer []byte diff --git a/cl/phase1/forkchoice/fork_graph/fork_graph_disk_fs.go b/cl/phase1/forkchoice/fork_graph/fork_graph_disk_fs.go index f22914f52cf8..fdbe1b087dd4 100644 --- a/cl/phase1/forkchoice/fork_graph/fork_graph_disk_fs.go +++ b/cl/phase1/forkchoice/fork_graph/fork_graph_disk_fs.go @@ -121,6 +121,12 @@ func (f *forkGraphDisk) DumpBeaconStateOnDisk(blockRoot common.Hash, bs *state.C } f.stateDumpLock.Lock() defer f.stateDumpLock.Unlock() + slot := bs.Slot() + if blockRoot == f.lastDumpedRoot && slot == f.lastDumpedSlot { + return nil + } + f.lastDumpedRoot, f.lastDumpedSlot = common.Hash{}, 0 + done := false // Truncate and then grow the buffer to the size of the state. f.sszBuffer, err = bs.EncodeSSZ(f.sszBuffer[:0]) if err != nil { @@ -134,6 +140,10 @@ func (f *forkGraphDisk) DumpBeaconStateOnDisk(blockRoot common.Hash, bs *state.C } defer func() { err = errors.Join(err, dumpedFile.Close()) + if done && err == nil { + f.lastDumpedRoot = blockRoot + f.lastDumpedSlot = slot + } }() sw := snappypool.Writer(dumpedFile) @@ -175,6 +185,7 @@ func (f *forkGraphDisk) DumpBeaconStateOnDisk(blockRoot common.Hash, bs *state.C return err } + done = true return } diff --git a/cl/phase1/forkchoice/fork_graph/fork_graph_test.go b/cl/phase1/forkchoice/fork_graph/fork_graph_test.go index 87af8df735a0..b0785bc267fd 100644 --- a/cl/phase1/forkchoice/fork_graph/fork_graph_test.go +++ b/cl/phase1/forkchoice/fork_graph/fork_graph_test.go @@ -286,6 +286,9 @@ func (f *envelopeBlockingRenameFs) Stat(name string) (os.FileInfo, error) { } func (f envelopeWriteFailureFile) Write(p []byte) (int, error) { + if f.stage == "panic" { + panic(errTestEnvelopeIO) + } if f.stage == "write" { return 0, errTestEnvelopeIO } @@ -417,14 +420,127 @@ func TestForkGraphDumpsDoNotRequireSync(t *testing.T) { func TestDumpBeaconStateReturnsCloseError(t *testing.T) { anchorState := state.New(&clparams.MainnetBeaconConfig) require.NoError(t, utils.DecodeSSZSnappy(anchorState, anchor, int(clparams.Phase0Version))) + wantHash, err := anchorState.HashSSZ() + require.NoError(t, err) root, err := anchorState.BlockRoot() require.NoError(t, err) + baseFs := afero.NewMemMapFs() var closes atomic.Int32 - fs := envelopeWriteFailureFs{Fs: afero.NewMemMapFs(), stage: "close", closes: &closes} - f := &forkGraphDisk{fs: fs, beaconCfg: &clparams.MainnetBeaconConfig} + f := &forkGraphDisk{ + fs: envelopeWriteFailureFs{Fs: baseFs, stage: "close", closes: &closes}, + beaconCfg: &clparams.MainnetBeaconConfig, + } require.ErrorIs(t, f.DumpBeaconStateOnDisk(root, anchorState, true), errTestEnvelopeIO) require.Equal(t, int32(1), closes.Load()) + f.fs = envelopeWriteFailureFs{Fs: baseFs, closes: &closes} + require.NoError(t, f.DumpBeaconStateOnDisk(root, anchorState, true)) + require.Equal(t, int32(2), closes.Load()) + persisted, err := f.readBeaconStateFromDisk(root) + require.NoError(t, err) + gotHash, err := persisted.HashSSZ() + require.NoError(t, err) + require.Equal(t, wantHash, gotHash) +} + +func TestDumpBeaconStateSkipsRepeatedRootAndSlot(t *testing.T) { + anchorState := state.New(&clparams.MainnetBeaconConfig) + require.NoError(t, utils.DecodeSSZSnappy(anchorState, anchor, int(clparams.Phase0Version))) + var closes atomic.Int32 + fs := envelopeWriteFailureFs{Fs: afero.NewMemMapFs(), closes: &closes} + f := &forkGraphDisk{fs: fs, beaconCfg: &clparams.MainnetBeaconConfig} + root := common.HexToHash("0x1234") + + require.NoError(t, f.DumpBeaconStateOnDisk(root, anchorState, true)) + require.NoError(t, f.DumpBeaconStateOnDisk(root, anchorState, true)) + require.Equal(t, int32(1), closes.Load()) +} + +func TestDumpBeaconStateWritesDifferentRoot(t *testing.T) { + anchorState := state.New(&clparams.MainnetBeaconConfig) + require.NoError(t, utils.DecodeSSZSnappy(anchorState, anchor, int(clparams.Phase0Version))) + var closes atomic.Int32 + fs := envelopeWriteFailureFs{Fs: afero.NewMemMapFs(), closes: &closes} + f := &forkGraphDisk{fs: fs, beaconCfg: &clparams.MainnetBeaconConfig} + + require.NoError(t, f.DumpBeaconStateOnDisk(common.HexToHash("0x1234"), anchorState, true)) + require.NoError(t, f.DumpBeaconStateOnDisk(common.HexToHash("0x5678"), anchorState, true)) + require.Equal(t, int32(2), closes.Load()) +} + +func TestDumpBeaconStateWritesSameRootAtDifferentSlot(t *testing.T) { + anchorState := state.New(&clparams.MainnetBeaconConfig) + require.NoError(t, utils.DecodeSSZSnappy(anchorState, anchor, int(clparams.Phase0Version))) + var closes atomic.Int32 + fs := envelopeWriteFailureFs{Fs: afero.NewMemMapFs(), closes: &closes} + f := &forkGraphDisk{fs: fs, beaconCfg: &clparams.MainnetBeaconConfig} + root := common.HexToHash("0x1234") + + require.NoError(t, f.DumpBeaconStateOnDisk(root, anchorState, true)) + require.NoError(t, anchorState.SetSlot(anchorState.Slot()+1)) + wantHash, err := anchorState.HashSSZ() + require.NoError(t, err) + require.NoError(t, f.DumpBeaconStateOnDisk(root, anchorState, true)) + require.Equal(t, int32(2), closes.Load()) + persisted, err := f.readBeaconStateFromDisk(root) + require.NoError(t, err) + require.Equal(t, anchorState.Slot(), persisted.Slot()) + gotHash, err := persisted.HashSSZ() + require.NoError(t, err) + require.Equal(t, wantHash, gotHash) +} + +func TestDumpBeaconStateRetriesEarlierSlotAfterLaterSlotFailure(t *testing.T) { + earlierState := state.New(&clparams.MainnetBeaconConfig) + require.NoError(t, utils.DecodeSSZSnappy(earlierState, anchor, int(clparams.Phase0Version))) + laterState, err := earlierState.Copy() + require.NoError(t, err) + wantHash, err := earlierState.HashSSZ() + require.NoError(t, err) + require.NoError(t, laterState.SetSlot(laterState.Slot()+1)) + baseFs := afero.NewMemMapFs() + var closes atomic.Int32 + fs := envelopeWriteFailureFs{Fs: baseFs, closes: &closes} + f := &forkGraphDisk{fs: fs, beaconCfg: &clparams.MainnetBeaconConfig} + root := common.HexToHash("0x1234") + + require.NoError(t, f.DumpBeaconStateOnDisk(root, earlierState, true)) + f.fs = envelopeWriteFailureFs{Fs: baseFs, stage: "write", closes: &closes} + require.ErrorIs(t, f.DumpBeaconStateOnDisk(root, laterState, true), errTestEnvelopeIO) + f.fs = envelopeWriteFailureFs{Fs: baseFs, closes: &closes} + require.NoError(t, f.DumpBeaconStateOnDisk(root, earlierState, true)) + require.Equal(t, int32(3), closes.Load()) + persisted, err := f.readBeaconStateFromDisk(root) + require.NoError(t, err) + gotHash, err := persisted.HashSSZ() + require.NoError(t, err) + require.Equal(t, wantHash, gotHash) +} + +func TestDumpBeaconStateRetriesAfterPanic(t *testing.T) { + anchorState := state.New(&clparams.MainnetBeaconConfig) + require.NoError(t, utils.DecodeSSZSnappy(anchorState, anchor, int(clparams.Phase0Version))) + wantHash, err := anchorState.HashSSZ() + require.NoError(t, err) + baseFs := afero.NewMemMapFs() + f := &forkGraphDisk{ + fs: envelopeWriteFailureFs{Fs: baseFs, stage: "panic"}, + beaconCfg: &clparams.MainnetBeaconConfig, + } + root := common.HexToHash("0x1234") + + require.PanicsWithValue(t, errTestEnvelopeIO, func() { + _ = f.DumpBeaconStateOnDisk(root, anchorState, true) + }) + var closes atomic.Int32 + f.fs = envelopeWriteFailureFs{Fs: baseFs, closes: &closes} + require.NoError(t, f.DumpBeaconStateOnDisk(root, anchorState, true)) + require.Equal(t, int32(1), closes.Load()) + persisted, err := f.readBeaconStateFromDisk(root) + require.NoError(t, err) + gotHash, err := persisted.HashSSZ() + require.NoError(t, err) + require.Equal(t, wantHash, gotHash) } func TestDumpEnvelopeErrorDoesNotPublishPartialFile(t *testing.T) {