diff --git a/beacon/light/canonical.go b/beacon/light/canonical.go index 381cfdfda1..06a0460d2e 100644 --- a/beacon/light/canonical.go +++ b/beacon/light/canonical.go @@ -31,7 +31,7 @@ import ( // to avoid concurrent access. type canonicalStore[T any] struct { keyPrefix []byte - periods Range + periods periodRange cache *lru.Cache[uint64, T] encode func(T) ([]byte, error) decode func([]byte) (T, error) @@ -39,8 +39,8 @@ type canonicalStore[T any] struct { // newCanonicalStore creates a new canonicalStore and loads all keys associated // with the keyPrefix in order to determine the ranges available in the database. -func newCanonicalStore[T any](db ethdb.KeyValueStore, keyPrefix []byte, - encode func(T) ([]byte, error), decode func([]byte) (T, error)) *canonicalStore[T] { +func newCanonicalStore[T any](db ethdb.Iteratee, keyPrefix []byte, + encode func(T) ([]byte, error), decode func([]byte) (T, error)) (*canonicalStore[T], error) { cs := &canonicalStore[T]{ keyPrefix: keyPrefix, encode: encode, @@ -61,31 +61,24 @@ func newCanonicalStore[T any](db ethdb.KeyValueStore, keyPrefix []byte, if first { cs.periods.Start = period } else if cs.periods.End != period { - log.Warn("Gap in the canonical chain database") - break // continuity guaranteed + return nil, fmt.Errorf("Gap in the canonical chain database between periods %d and %d", cs.periods.End, period-1) } first = false cs.periods.End = period + 1 } iter.Release() - return cs + return cs, nil } // databaseKey returns the database key belonging to the given period. func (cs *canonicalStore[T]) databaseKey(period uint64) []byte { - var ( - kl = len(cs.keyPrefix) - key = make([]byte, kl+8) - ) - copy(key[:kl], cs.keyPrefix) - binary.BigEndian.PutUint64(key[kl:], period) - return key + return binary.BigEndian.AppendUint64(append([]byte{}, cs.keyPrefix...), period) } // add adds the given item to the database. It also ensures that the range remains // continuous. Can be used either with a batch or database backend. func (cs *canonicalStore[T]) add(backend ethdb.KeyValueWriter, period uint64, value T) error { - if !cs.periods.CanExpand(period) { + if !cs.periods.canExpand(period) { return fmt.Errorf("period expansion is not allowed, first: %d, next: %d, period: %d", cs.periods.Start, cs.periods.End, period) } enc, err := cs.encode(value) @@ -96,15 +89,15 @@ func (cs *canonicalStore[T]) add(backend ethdb.KeyValueWriter, period uint64, va return err } cs.cache.Add(period, value) - cs.periods.Expand(period) + cs.periods.expand(period) return nil } // deleteFrom removes items starting from the given period. -func (cs *canonicalStore[T]) deleteFrom(batch ethdb.Batch, fromPeriod uint64) (deleted Range) { - keepRange, deleteRange := cs.periods.Split(fromPeriod) - deleteRange.Each(func(period uint64) { - batch.Delete(cs.databaseKey(period)) +func (cs *canonicalStore[T]) deleteFrom(db ethdb.KeyValueWriter, fromPeriod uint64) (deleted periodRange) { + keepRange, deleteRange := cs.periods.split(fromPeriod) + deleteRange.each(func(period uint64) { + db.Delete(cs.databaseKey(period)) cs.cache.Remove(period) }) cs.periods = keepRange @@ -113,22 +106,24 @@ func (cs *canonicalStore[T]) deleteFrom(batch ethdb.Batch, fromPeriod uint64) (d // get returns the item at the given period or the null value of the given type // if no item is present. -func (cs *canonicalStore[T]) get(backend ethdb.KeyValueReader, period uint64) (value T, ok bool) { - if !cs.periods.Contains(period) { - return +func (cs *canonicalStore[T]) get(backend ethdb.KeyValueReader, period uint64) (T, bool) { + var null T + if !cs.periods.contains(period) { + return null, false } - if value, ok = cs.cache.Get(period); ok { - return + if value, ok := cs.cache.Get(period); ok { + return value, true } - if enc, err := backend.Get(cs.databaseKey(period)); err == nil { - if v, err := cs.decode(enc); err == nil { - value, ok = v, true - cs.cache.Add(period, value) - } else { - log.Error("Error decoding canonical store value", "error", err) - } - } else { + enc, err := backend.Get(cs.databaseKey(period)) + if err != nil { log.Error("Canonical store value not found", "period", period, "start", cs.periods.Start, "end", cs.periods.End) + return null, false } - return + value, err := cs.decode(enc) + if err != nil { + log.Error("Error decoding canonical store value", "error", err) + return null, false + } + cs.cache.Add(period, value) + return value, true } diff --git a/beacon/light/committee_chain.go b/beacon/light/committee_chain.go index 2f833cb392..bd8bde20ab 100644 --- a/beacon/light/committee_chain.go +++ b/beacon/light/committee_chain.go @@ -123,29 +123,36 @@ func newCommitteeChain(db ethdb.KeyValueStore, config *types.ChainConfig, signer } ) s := &CommitteeChain{ - fixedCommitteeRoots: newCanonicalStore[common.Hash](db, rawdb.FixedCommitteeRootKey, fixedCommitteeRootEncoder, fixedCommitteeRootDecoder), - committees: newCanonicalStore[*types.SerializedSyncCommittee](db, rawdb.SyncCommitteeKey, committeeEncoder, committeeDecoder), - updates: newCanonicalStore[*types.LightClientUpdate](db, rawdb.BestUpdateKey, updateEncoder, updateDecoder), - committeeCache: lru.NewCache[uint64, syncCommittee](10), - db: db, - sigVerifier: sigVerifier, - clock: clock, - unixNano: unixNano, - config: config, - signerThreshold: signerThreshold, - enforceTime: enforceTime, + committeeCache: lru.NewCache[uint64, syncCommittee](10), + db: db, + sigVerifier: sigVerifier, + clock: clock, + unixNano: unixNano, + config: config, + signerThreshold: signerThreshold, + enforceTime: enforceTime, minimumUpdateScore: types.UpdateScore{ SignerCount: uint32(signerThreshold), SubPeriodIndex: params.SyncPeriodLength / 16, }, } - if !s.checkConstraints() { + var err1, err2, err3 error + if s.fixedCommitteeRoots, err1 = newCanonicalStore[common.Hash](db, rawdb.FixedCommitteeRootKey, fixedCommitteeRootEncoder, fixedCommitteeRootDecoder); err1 != nil { + log.Error("Error creating fixed committee root store", "error", err1) + } + if s.committees, err2 = newCanonicalStore[*types.SerializedSyncCommittee](db, rawdb.SyncCommitteeKey, committeeEncoder, committeeDecoder); err2 != nil { + log.Error("Error creating committee store", "error", err2) + } + if s.updates, err3 = newCanonicalStore[*types.LightClientUpdate](db, rawdb.BestUpdateKey, updateEncoder, updateDecoder); err3 != nil { + log.Error("Error creating update store", "error", err3) + } + if err1 != nil || err2 != nil || err3 != nil || !s.checkConstraints() { log.Info("Resetting invalid committee chain") s.Reset() } // roll back invalid updates (might be necessary if forks have been changed since last time) - for !s.updates.periods.IsEmpty() { + for !s.updates.periods.isEmpty() { update, ok := s.updates.get(s.db, s.updates.periods.End-1) if !ok { log.Error("Sync committee update missing", "period", s.updates.periods.End-1) @@ -161,7 +168,7 @@ func newCommitteeChain(db ethdb.KeyValueStore, config *types.ChainConfig, signer log.Error("Error writing batch into chain database", "error", err) } } - if !s.committees.periods.IsEmpty() { + if !s.committees.periods.isEmpty() { log.Trace("Sync committee chain loaded", "first period", s.committees.periods.Start, "last period", s.committees.periods.End-1) } return s @@ -169,14 +176,14 @@ func newCommitteeChain(db ethdb.KeyValueStore, config *types.ChainConfig, signer // checkConstraints checks committee chain validity constraints func (s *CommitteeChain) checkConstraints() bool { - isNotInFixedCommitteeRootRange := func(r Range) bool { - return s.fixedCommitteeRoots.periods.IsEmpty() || + isNotInFixedCommitteeRootRange := func(r periodRange) bool { + return s.fixedCommitteeRoots.periods.isEmpty() || r.Start < s.fixedCommitteeRoots.periods.Start || r.Start >= s.fixedCommitteeRoots.periods.End } valid := true - if !s.updates.periods.IsEmpty() { + if !s.updates.periods.isEmpty() { if isNotInFixedCommitteeRootRange(s.updates.periods) { log.Error("Start update is not in the fixed roots range") valid = false @@ -186,7 +193,7 @@ func (s *CommitteeChain) checkConstraints() bool { valid = false } } - if !s.committees.periods.IsEmpty() { + if !s.committees.periods.isEmpty() { if isNotInFixedCommitteeRootRange(s.committees.periods) { log.Error("Start committee is not in the fixed roots range") valid = false @@ -254,7 +261,7 @@ func (s *CommitteeChain) addFixedCommitteeRoot(period uint64, root common.Hash) batch := s.db.NewBatch() oldRoot := s.getCommitteeRoot(period) - if !s.fixedCommitteeRoots.periods.CanExpand(period) { + if !s.fixedCommitteeRoots.periods.canExpand(period) { // Note: the fixed committee root range should always be continuous and // therefore the expected syncing method is to forward sync and optionally // backward sync periods one by one, starting from a checkpoint. The only @@ -301,7 +308,7 @@ func (s *CommitteeChain) deleteFixedCommitteeRootsFrom(period uint64) error { } batch := s.db.NewBatch() s.fixedCommitteeRoots.deleteFrom(batch, period) - if s.updates.periods.IsEmpty() || period <= s.updates.periods.Start { + if s.updates.periods.isEmpty() || period <= s.updates.periods.Start { // Note: the first period of the update chain should always be fixed so if // the fixed root at the first update is removed then the entire update chain // and the proven committees have to be removed. Earlier committees in the @@ -336,7 +343,7 @@ func (s *CommitteeChain) deleteCommitteesFrom(batch ethdb.Batch, period uint64) // addCommittee adds a committee at the given period if possible. func (s *CommitteeChain) addCommittee(period uint64, committee *types.SerializedSyncCommittee) error { - if !s.committees.periods.CanExpand(period) { + if !s.committees.periods.canExpand(period) { return ErrInvalidPeriod } root := s.getCommitteeRoot(period) @@ -346,7 +353,7 @@ func (s *CommitteeChain) addCommittee(period uint64, committee *types.Serialized if root != committee.Root() { return ErrWrongCommitteeRoot } - if !s.committees.periods.Contains(period) { + if !s.committees.periods.contains(period) { if err := s.committees.add(s.db, period, committee); err != nil { return err } @@ -361,7 +368,7 @@ func (s *CommitteeChain) InsertUpdate(update *types.LightClientUpdate, nextCommi defer s.chainmu.Unlock() period := update.AttestedHeader.Header.SyncPeriod() - if !s.updates.periods.CanExpand(period) || !s.committees.periods.Contains(period) { + if !s.updates.periods.canExpand(period) || !s.committees.periods.contains(period) { return ErrInvalidPeriod } if s.minimumUpdateScore.BetterThan(update.Score()) { @@ -376,7 +383,7 @@ func (s *CommitteeChain) InsertUpdate(update *types.LightClientUpdate, nextCommi } return nil } - if s.fixedCommitteeRoots.periods.Contains(period+1) && reorg { + if s.fixedCommitteeRoots.periods.contains(period+1) && reorg { return ErrCannotReorg } if ok, err := s.verifyUpdate(update); err != nil { @@ -384,7 +391,7 @@ func (s *CommitteeChain) InsertUpdate(update *types.LightClientUpdate, nextCommi } else if !ok { return ErrInvalidUpdate } - addCommittee := !s.committees.periods.Contains(period+1) || reorg + addCommittee := !s.committees.periods.contains(period+1) || reorg if addCommittee { if nextCommittee == nil { return ErrNeedCommittee @@ -422,10 +429,10 @@ func (s *CommitteeChain) NextSyncPeriod() (uint64, bool) { s.chainmu.RLock() defer s.chainmu.RUnlock() - if s.committees.periods.IsEmpty() { + if s.committees.periods.isEmpty() { return 0, false } - if !s.updates.periods.IsEmpty() { + if !s.updates.periods.isEmpty() { return s.updates.periods.End, true } return s.committees.periods.End - 1, true diff --git a/beacon/light/range.go b/beacon/light/range.go index 71b140c069..76ebe2381a 100644 --- a/beacon/light/range.go +++ b/beacon/light/range.go @@ -16,32 +16,32 @@ package light -// Range represents a (possibly zero-length) range of integers (sync periods). -type Range struct { +// periodRange represents a (possibly zero-length) range of integers (sync periods). +type periodRange struct { Start, End uint64 } -// IsEmpty returns true if the length of the range is zero. -func (a Range) IsEmpty() bool { +// isEmpty returns true if the length of the range is zero. +func (a periodRange) isEmpty() bool { return a.End == a.Start } -// Contains returns true if the range includes the given period. -func (a Range) Contains(period uint64) bool { +// contains returns true if the range includes the given period. +func (a periodRange) contains(period uint64) bool { return period >= a.Start && period < a.End } -// CanExpand returns true if the range includes or can be expanded with the given +// canExpand returns true if the range includes or can be expanded with the given // period (either the range is empty or the given period is inside, right before or // right after the range). -func (a Range) CanExpand(period uint64) bool { - return a.IsEmpty() || (period+1 >= a.Start && period <= a.End) +func (a periodRange) canExpand(period uint64) bool { + return a.isEmpty() || (period+1 >= a.Start && period <= a.End) } -// Expand expands the range with the given period. -// This method assumes that CanExpand returned true: otherwise this is a no-op. -func (a *Range) Expand(period uint64) { - if a.IsEmpty() { +// expand expands the range with the given period. +// This method assumes that canExpand returned true: otherwise this is a no-op. +func (a *periodRange) expand(period uint64) { + if a.isEmpty() { a.Start, a.End = period, period+1 return } @@ -53,25 +53,25 @@ func (a *Range) Expand(period uint64) { } } -// Split splits the range into two ranges. The 'fromPeriod' will be the first +// split splits the range into two ranges. The 'fromPeriod' will be the first // element in the second range (if present). // The original range is unchanged by this operation -func (a *Range) Split(fromPeriod uint64) (Range, Range) { +func (a *periodRange) split(fromPeriod uint64) (periodRange, periodRange) { if fromPeriod <= a.Start { // First range empty, everything in second range, - return Range{}, *a + return periodRange{}, *a } if fromPeriod >= a.End { // Second range empty, everything in first range, - return *a, Range{} + return *a, periodRange{} } - x := Range{a.Start, fromPeriod} - y := Range{fromPeriod, a.End} + x := periodRange{a.Start, fromPeriod} + y := periodRange{fromPeriod, a.End} return x, y } -// Each invokes the supplied function fn once per period in range -func (a *Range) Each(fn func(uint64)) { +// each invokes the supplied function fn once per period in range +func (a *periodRange) each(fn func(uint64)) { for p := a.Start; p < a.End; p++ { fn(p) }