From 89f0213796298dc9c54b9c1ef3f50e96edbc0188 Mon Sep 17 00:00:00 2001 From: Hans Moog <3293976+hmoog@users.noreply.github.com> Date: Sun, 3 Dec 2023 02:14:32 +0100 Subject: [PATCH] Refactor: fixed typo + race condition --- pkg/protocol/chain.go | 12 ++++++------ pkg/protocol/commitment_verifier.go | 9 +++++++++ pkg/protocol/protocol_warp_sync.go | 4 ++-- 3 files changed, 17 insertions(+), 8 deletions(-) diff --git a/pkg/protocol/chain.go b/pkg/protocol/chain.go index b03b7a2d0..86b1d1d30 100644 --- a/pkg/protocol/chain.go +++ b/pkg/protocol/chain.go @@ -217,8 +217,8 @@ func (c *Chain) initDerivedProperties() (shutdown func()) { return latestProducedCommitment.cumulativeWeight() }, c.LatestProducedCommitment)), - c.WarpSyncModeEnabled.DeriveValueFrom(reactive.NewDerivedVariable3(func(warpSyncMode bool, latestFullyBookedCommitment *Commitment, latestSeenSlot iotago.SlotIndex, outOfSyncThreshold iotago.SlotIndex) bool { - return warpSyncModeEnabled(warpSyncMode, latestFullyBookedCommitment, latestSeenSlot, outOfSyncThreshold) + c.WarpSyncModeEnabled.DeriveValueFrom(reactive.NewDerivedVariable3(func(enabled bool, latestFullyBookedCommitment *Commitment, latestSeenSlot iotago.SlotIndex, outOfSyncThreshold iotago.SlotIndex) bool { + return warpSyncModeEnabled(enabled, latestFullyBookedCommitment, latestSeenSlot, outOfSyncThreshold) }, c.LatestFullyBookedCommitment, c.chains.LatestSeenSlot, c.OutOfSyncThreshold, c.WarpSyncModeEnabled.Get())), c.LatestAttestedCommitment.WithNonEmptyValue(func(latestAttestedCommitment *Commitment) (shutdown func()) { @@ -347,18 +347,18 @@ func outOfSyncThreshold(engineInstance *engine.Engine, latestSeenSlot iotago.Slo } // warpSyncModeEnabled determines whether warp sync mode should be enabled or not. -func warpSyncModeEnabled(enabled bool, latestProducedCommitment *Commitment, latestSeenSlot iotago.SlotIndex, outOfSyncThreshold iotago.SlotIndex) bool { +func warpSyncModeEnabled(enabled bool, latestFullyBookedCommitment *Commitment, latestSeenSlot iotago.SlotIndex, outOfSyncThreshold iotago.SlotIndex) bool { // latest produced commitment is nil if we have not produced any commitment yet (intermediary state during // startup) - if latestProducedCommitment == nil { + if latestFullyBookedCommitment == nil { return enabled } // if warp sync mode is enabled, keep it enabled until we are no longer below the warp sync threshold if enabled { - return latestProducedCommitment.ID().Slot() < latestSeenSlot + return latestFullyBookedCommitment.ID().Slot() < latestSeenSlot } // if warp sync mode is disabled, enable it only if we fall below the out of sync threshold - return latestProducedCommitment.ID().Slot() < outOfSyncThreshold + return latestFullyBookedCommitment.ID().Slot() < outOfSyncThreshold } diff --git a/pkg/protocol/commitment_verifier.go b/pkg/protocol/commitment_verifier.go index ab600fbc9..3ee2b4cc8 100644 --- a/pkg/protocol/commitment_verifier.go +++ b/pkg/protocol/commitment_verifier.go @@ -5,6 +5,7 @@ import ( "github.com/iotaledger/hive.go/ds" "github.com/iotaledger/hive.go/ierrors" "github.com/iotaledger/hive.go/kvstore/mapdb" + "github.com/iotaledger/hive.go/runtime/syncutils" "github.com/iotaledger/iota-core/pkg/model" "github.com/iotaledger/iota-core/pkg/protocol/engine" "github.com/iotaledger/iota-core/pkg/protocol/engine/accounts" @@ -22,6 +23,9 @@ type CommitmentVerifier struct { // validatorAccountsData is the accounts data of the validators for the current epoch as known at lastCommonSlotBeforeFork. // Initially, it is set to the accounts data of the validators for the epoch of the last common commitment before the fork. validatorAccountsData map[iotago.AccountID]*accounts.AccountData + + // mutex is used to synchronize access to validatorAccountsData and epoch. + mutex syncutils.RWMutex } func newCommitmentVerifier(mainEngine *engine.Engine, lastCommonCommitmentBeforeFork *model.Commitment) (*CommitmentVerifier, error) { @@ -76,6 +80,7 @@ func (c *CommitmentVerifier) verifyCommitment(commitment *Commitment, attestatio // This is necessary because the committee might have rotated at the epoch boundary and different validators might be part of it. // In case anything goes wrong we keep using previously known accounts data (initially set to the accounts data // of the validators for the epoch of the last common commitment before the fork). + c.mutex.Lock() apiForSlot := c.engine.APIForSlot(commitment.Slot()) commitmentEpoch := apiForSlot.TimeProvider().EpochFromSlot(commitment.Slot()) if commitmentEpoch > c.epoch { @@ -92,6 +97,7 @@ func (c *CommitmentVerifier) verifyCommitment(commitment *Commitment, attestatio } } } + c.mutex.Unlock() // 3. Verify attestations. blockIDs, seatCount, err := c.verifyAttestations(attestations) @@ -107,6 +113,9 @@ func (c *CommitmentVerifier) verifyCommitment(commitment *Commitment, attestatio } func (c *CommitmentVerifier) verifyAttestations(attestations []*iotago.Attestation) (iotago.BlockIDs, uint64, error) { + c.mutex.RLock() + defer c.mutex.RUnlock() + visitedIdentities := ds.NewSet[iotago.AccountID]() var blockIDs iotago.BlockIDs var seatCount uint64 diff --git a/pkg/protocol/protocol_warp_sync.go b/pkg/protocol/protocol_warp_sync.go index 0b23ea6e3..374aaa3c6 100644 --- a/pkg/protocol/protocol_warp_sync.go +++ b/pkg/protocol/protocol_warp_sync.go @@ -47,8 +47,8 @@ func newWarpSyncProtocol(protocol *Protocol) *WarpSyncProtocol { protocol.Constructed.OnTrigger(func() { protocol.Chains.WithInitializedEngines(func(chain *Chain, engine *engine.Engine) (shutdown func()) { - return chain.WarpSyncModeEnabled.OnUpdate(func(_ bool, warpSyncMode bool) { - if warpSyncMode { + return chain.WarpSyncModeEnabled.OnUpdate(func(_ bool, warpSyncModeEnabled bool) { + if warpSyncModeEnabled { engine.Workers.WaitChildren() engine.Reset() }