From 9d183632e4d14b88866548df81135fb10e246ccc Mon Sep 17 00:00:00 2001 From: Giulio rebuffo Date: Sun, 19 Jan 2025 16:56:03 +0100 Subject: [PATCH] Caplin: fix occassional loss of sync during `ForwardSync` (#13495) --- cl/cltypes/eth1_data.go | 7 +++- .../forkchoice/fork_graph/fork_graph_disk.go | 3 ++ .../fork_graph/fork_graph_disk_fs.go | 42 ++++++++----------- cl/transition/machine/block.go | 11 +---- 4 files changed, 27 insertions(+), 36 deletions(-) diff --git a/cl/cltypes/eth1_data.go b/cl/cltypes/eth1_data.go index b87c19d3445..18507ec9e24 100644 --- a/cl/cltypes/eth1_data.go +++ b/cl/cltypes/eth1_data.go @@ -35,8 +35,11 @@ func NewEth1Data() *Eth1Data { } func (e *Eth1Data) Copy() *Eth1Data { - copied := *e - return &copied + return &Eth1Data{ + Root: e.Root, + DepositCount: e.DepositCount, + BlockHash: e.BlockHash, + } } func (e *Eth1Data) Equal(b *Eth1Data) bool { diff --git a/cl/phase1/forkchoice/fork_graph/fork_graph_disk.go b/cl/phase1/forkchoice/fork_graph/fork_graph_disk.go index d12500e4cff..51ab1b82c7b 100644 --- a/cl/phase1/forkchoice/fork_graph/fork_graph_disk.go +++ b/cl/phase1/forkchoice/fork_graph/fork_graph_disk.go @@ -395,6 +395,9 @@ func (f *forkGraphDisk) getState(blockRoot libcommon.Hash, alwaysCopy bool, addC // Traverse the blocks from top to bottom. for i := len(blocksInTheWay) - 1; i >= 0; i-- { if err := transition.TransitionState(copyReferencedState, blocksInTheWay[i], nil, false); err != nil { + if addChainSegment { + f.currentState = nil // reset the state if it fails here. + } return nil, err } } 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 7f32c315129..f6f4d10a5d2 100644 --- a/cl/phase1/forkchoice/fork_graph/fork_graph_disk_fs.go +++ b/cl/phase1/forkchoice/fork_graph/fork_graph_disk_fs.go @@ -112,12 +112,7 @@ func (f *forkGraphDisk) DumpBeaconStateOnDisk(blockRoot libcommon.Hash, bs *stat return } f.stateDumpLock.Lock() - unlockOnDefer := true - defer func() { - if unlockOnDefer { - f.stateDumpLock.Unlock() - } - }() + defer f.stateDumpLock.Unlock() // Truncate and then grow the buffer to the size of the state. f.sszBuffer, err = bs.EncodeSSZ(f.sszBuffer[:0]) if err != nil { @@ -170,25 +165,22 @@ func (f *forkGraphDisk) DumpBeaconStateOnDisk(blockRoot libcommon.Hash, bs *stat log.Error("failed to sync dumped file", "err", err) return } - unlockOnDefer = false - go func() { - cacheFile, err := f.fs.OpenFile(getBeaconStateCacheFilename(blockRoot), os.O_TRUNC|os.O_CREATE|os.O_RDWR, 0o755) - if err != nil { - log.Error("failed to open cache file", "err", err) - return - } - defer cacheFile.Close() - defer f.stateDumpLock.Unlock() - - if _, err = cacheFile.Write(b.Bytes()); err != nil { - log.Error("failed to write cache file", "err", err) - return - } - if err = cacheFile.Sync(); err != nil { - log.Error("failed to sync cache file", "err", err) - return - } - }() + + cacheFile, err := f.fs.OpenFile(getBeaconStateCacheFilename(blockRoot), os.O_TRUNC|os.O_CREATE|os.O_RDWR, 0o755) + if err != nil { + log.Error("failed to open cache file", "err", err) + return + } + defer cacheFile.Close() + + if _, err = cacheFile.Write(b.Bytes()); err != nil { + log.Error("failed to write cache file", "err", err) + return + } + if err = cacheFile.Sync(); err != nil { + log.Error("failed to sync cache file", "err", err) + return + } return } diff --git a/cl/transition/machine/block.go b/cl/transition/machine/block.go index 6f464fa8e3c..f94cbe0d0d6 100644 --- a/cl/transition/machine/block.go +++ b/cl/transition/machine/block.go @@ -117,8 +117,9 @@ func ProcessBlock(impl BlockProcessor, s abstract.BeaconState, block cltypes.Gen // ProcessOperations is called by ProcessBlock and processes the block body operations func ProcessOperations(impl BlockOperationProcessor, s abstract.BeaconState, blockBody cltypes.GenericBeaconBody) (signatures [][]byte, messages [][]byte, publicKeys [][]byte, err error) { + maxDepositsAllowed := int(min(s.BeaconConfig().MaxDeposits, s.Eth1Data().DepositCount-s.Eth1DepositIndex())) if s.Version() <= clparams.DenebVersion { - if blockBody.GetDeposits().Len() != int(maximumDeposits(s)) { + if blockBody.GetDeposits().Len() != maxDepositsAllowed { return nil, nil, nil, errors.New("outstanding deposits do not match maximum deposits") } } else if s.Version() >= clparams.ElectraVersion { @@ -345,11 +346,3 @@ func processBlsToExecutionChanges(impl BlockOperationProcessor, s abstract.Beaco return } - -func maximumDeposits(s abstract.BeaconState) (maxDeposits uint64) { - maxDeposits = s.Eth1Data().DepositCount - s.Eth1DepositIndex() - if maxDeposits > s.BeaconConfig().MaxDeposits { - maxDeposits = s.BeaconConfig().MaxDeposits - } - return -}