From 574f08571561a06fb002ce5a5fa917a4130a782d Mon Sep 17 00:00:00 2001 From: Zsolt Felfoldi Date: Mon, 24 Jul 2023 16:47:32 +0200 Subject: [PATCH 01/19] beacon/light: add CommitteeChain --- beacon/light/checkpoint.go | 110 ++++++ beacon/light/committee_chain.go | 521 +++++++++++++++++++++++++++ beacon/light/committee_chain_test.go | 356 ++++++++++++++++++ beacon/light/range.go | 52 +++ beacon/light/test_helpers.go | 151 ++++++++ 5 files changed, 1190 insertions(+) create mode 100644 beacon/light/checkpoint.go create mode 100644 beacon/light/committee_chain.go create mode 100644 beacon/light/committee_chain_test.go create mode 100644 beacon/light/range.go create mode 100644 beacon/light/test_helpers.go diff --git a/beacon/light/checkpoint.go b/beacon/light/checkpoint.go new file mode 100644 index 000000000000..c9d4956dc519 --- /dev/null +++ b/beacon/light/checkpoint.go @@ -0,0 +1,110 @@ +// Copyright 2023 The go-ethereum Authors +// This file is part of the go-ethereum library. +// +// The go-ethereum library is free software: you can redistribute it and/or modify +// it under the terms of the GNU Lesser General Public License as published by +// the Free Software Foundation, either version 3 of the License, or +// (at your option) any later version. +// +// The go-ethereum library is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU Lesser General Public License for more details. +// +// You should have received a copy of the GNU Lesser General Public License +// along with the go-ethereum library. If not, see . + +package light + +import ( + "errors" + + "github.com/ethereum/go-ethereum/beacon/merkle" + "github.com/ethereum/go-ethereum/beacon/params" + "github.com/ethereum/go-ethereum/beacon/types" + "github.com/ethereum/go-ethereum/common" + "github.com/ethereum/go-ethereum/ethdb" + "github.com/ethereum/go-ethereum/log" + "github.com/ethereum/go-ethereum/rlp" +) + +var checkpointKey = []byte("checkpoint-") // block root -> RLP(CheckpointData) + +type CheckpointData struct { + Header types.Header + CommitteeRoot common.Hash + Committee *types.SerializedSyncCommittee `rlp:"-"` + CommitteeBranch merkle.Values +} + +func (c *CheckpointData) Validate() error { + if c.CommitteeRoot != c.Committee.Root() { + return errors.New("wrong committee root") + } + return merkle.VerifyProof(c.Header.StateRoot, params.StateIndexSyncCommittee, c.CommitteeBranch, merkle.Value(c.CommitteeRoot)) +} + +// expected to be validated already +func (c *CheckpointData) InitChain(chain *CommitteeChain) { + must := func(err error) { + if err != nil { + log.Crit("Error initializing committee chain with checkpoint", "error", err) + } + } + period := c.Header.SyncPeriod() + must(chain.DeleteFixedRootsFrom(period + 2)) + if chain.AddFixedRoot(period, c.CommitteeRoot) != nil { + chain.Reset() + must(chain.AddFixedRoot(period, c.CommitteeRoot)) + } + must(chain.AddFixedRoot(period+1, common.Hash(c.CommitteeBranch[0]))) + must(chain.AddCommittee(period, c.Committee)) +} + +type CheckpointStore struct { + chain *CommitteeChain + db ethdb.KeyValueStore +} + +func NewCheckpointStore(db ethdb.KeyValueStore, chain *CommitteeChain) *CheckpointStore { + return &CheckpointStore{ + db: db, + chain: chain, + } +} + +func getCheckpointKey(checkpoint common.Hash) []byte { + var ( + kl = len(checkpointKey) + key = make([]byte, kl+32) + ) + copy(key[:kl], checkpointKey) + copy(key[kl:], checkpoint[:]) + return key +} + +func (cs *CheckpointStore) Get(checkpoint common.Hash) *CheckpointData { + if enc, err := cs.db.Get(getCheckpointKey(checkpoint)); err == nil { + c := new(CheckpointData) + if err := rlp.DecodeBytes(enc, c); err != nil { + log.Error("Error decoding stored checkpoint", "error", err) + return nil + } + if committee := cs.chain.committees.get(c.Header.SyncPeriod()); committee != nil && committee.Root() == c.CommitteeRoot { + c.Committee = committee + return c + } + log.Error("Missing committee for stored checkpoint", "period", c.Header.SyncPeriod()) + } + return nil +} + +func (cs *CheckpointStore) Store(c *CheckpointData) { + enc, err := rlp.EncodeToBytes(c) + if err != nil { + log.Error("Error encoding checkpoint for storage", "error", err) + } + if err := cs.db.Put(getCheckpointKey(c.Header.Hash()), enc); err != nil { + log.Error("Error storing checkpoint in database", "error", err) + } +} diff --git a/beacon/light/committee_chain.go b/beacon/light/committee_chain.go new file mode 100644 index 000000000000..1aec863e4258 --- /dev/null +++ b/beacon/light/committee_chain.go @@ -0,0 +1,521 @@ +// Copyright 2023 The go-ethereum Authors +// This file is part of the go-ethereum library. +// +// The go-ethereum library is free software: you can redistribute it and/or modify +// it under the terms of the GNU Lesser General Public License as published by +// the Free Software Foundation, either version 3 of the License, or +// (at your option) any later version. +// +// The go-ethereum library is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU Lesser General Public License for more details. +// +// You should have received a copy of the GNU Lesser General Public License +// along with the go-ethereum library. If not, see . + +package light + +import ( + "encoding/binary" + "errors" + "sync" + "time" + + "github.com/ethereum/go-ethereum/beacon/params" + "github.com/ethereum/go-ethereum/beacon/types" + "github.com/ethereum/go-ethereum/common" + "github.com/ethereum/go-ethereum/common/lru" + "github.com/ethereum/go-ethereum/common/mclock" + "github.com/ethereum/go-ethereum/ethdb" + "github.com/ethereum/go-ethereum/log" + "github.com/ethereum/go-ethereum/rlp" +) + +var ( + ErrNotInitialized = errors.New("sync committee chain not initialized") + ErrNeedCommittee = errors.New("sync committee required") + ErrInvalidUpdate = errors.New("invalid committee update") + ErrInvalidPeriod = errors.New("invalid update period") + ErrWrongCommitteeRoot = errors.New("wrong committee root") + ErrCannotReorg = errors.New("can not reorg committee chain") +) + +var ( + bestUpdateKey = []byte("update-") // bigEndian64(syncPeriod) -> RLP(types.LightClientUpdate) (nextCommittee only referenced by root hash) + fixedRootKey = []byte("fixedRoot-") // bigEndian64(syncPeriod) -> committee root hash + syncCommitteeKey = []byte("committee-") // bigEndian64(syncPeriod) -> serialized committee +) + +// CommitteeChain maintains a chain of sync committee updates and a small +// set of best known signed heads. It is used in all client configurations +// operating on a beacon chain. It can sync its update chain and receive signed +// heads from either an ODR or beacon node API backend and propagate/serve this +// data to subscribed peers. Received signed heads are validated based on the +// known sync committee chain and added to the local set if valid or placed in a +// deferred queue if the committees are not synced up to the period of the new +// head yet. +// Sync committee chain is either initialized from a weak subjectivity checkpoint +// or controlled by a BeaconChain that is driven by a trusted source (beacon node API). +type CommitteeChain struct { + lock sync.RWMutex + db ethdb.KeyValueStore + sigVerifier committeeSigVerifier + clock mclock.Clock + updates *canonicalStore[*types.LightClientUpdate] + committees *canonicalStore[*types.SerializedSyncCommittee] + fixedRoots *canonicalStore[common.Hash] + syncCommitteeCache *lru.Cache[uint64, syncCommittee] // cache deserialized committees + unixNano func() int64 + + config *types.ChainConfig + signerThreshold int + minimumUpdateScore types.UpdateScore + enforceTime bool +} + +// NewCommitteeChain creates a new CommitteeChain +func NewCommitteeChain(db ethdb.KeyValueStore, config *types.ChainConfig, signerThreshold int, enforceTime bool, sigVerifier committeeSigVerifier, clock mclock.Clock, unixNano func() int64) *CommitteeChain { + s := &CommitteeChain{ + fixedRoots: newCanonicalStore[common.Hash](db, fixedRootKey, func(root common.Hash) ([]byte, error) { + return root[:], nil + }, func(enc []byte) (root common.Hash, err error) { + if len(enc) == len(root) { + copy(root[:], enc) + } else { + err = errors.New("Incorrect length for committee root entry in the database") + } + return + }), + committees: newCanonicalStore[*types.SerializedSyncCommittee](db, syncCommitteeKey, func(committee *types.SerializedSyncCommittee) ([]byte, error) { + return committee[:], nil + }, func(enc []byte) (*types.SerializedSyncCommittee, error) { + if len(enc) == types.SerializedSyncCommitteeSize { + committee := new(types.SerializedSyncCommittee) + copy(committee[:], enc) + return committee, nil + } + return nil, errors.New("Incorrect length for serialized committee entry in the database") + }), + updates: newCanonicalStore[*types.LightClientUpdate](db, bestUpdateKey, func(update *types.LightClientUpdate) ([]byte, error) { + return rlp.EncodeToBytes(update) + }, func(enc []byte) (*types.LightClientUpdate, error) { + update := new(types.LightClientUpdate) + if err := rlp.DecodeBytes(enc, update); err != nil { + return nil, err + } + return update, nil + }), + syncCommitteeCache: 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, + }, + } + + // check validity constraints + if !s.updates.IsEmpty() { + if s.fixedRoots.IsEmpty() || s.updates.First < s.fixedRoots.First || + s.updates.First >= s.fixedRoots.AfterLast { + log.Crit("Inconsistent database error: first update is not in the fixed roots range") + } + if s.committees.First > s.updates.First || s.committees.AfterLast <= s.updates.AfterLast { + log.Crit("Inconsistent database error: missing committees in update range") + } + } + if !s.committees.IsEmpty() { + if s.fixedRoots.IsEmpty() || s.committees.First < s.fixedRoots.First || + s.committees.First >= s.fixedRoots.AfterLast { + log.Crit("Inconsistent database error: first committee is not in the fixed roots range") + } + if s.committees.AfterLast > s.fixedRoots.AfterLast && s.committees.AfterLast > s.updates.AfterLast+1 { + log.Crit("Inconsistent database error: last committee is neither in the fixed roots range nor proven by updates") + } + log.Trace("Sync committee chain loaded", "first period", s.committees.First, "last period", s.committees.AfterLast-1) + } + // roll back invalid updates (might be necessary if forks have been changed since last time) + var batch ethdb.Batch + for !s.updates.IsEmpty() { + if update := s.updates.get(s.updates.AfterLast - 1); update == nil || s.verifyUpdate(update) { + if update == nil { + log.Crit("Sync committee update missing", "period", s.updates.AfterLast-1) + } + break + } + if batch == nil { + batch = s.db.NewBatch() + } + s.rollback(batch, s.updates.AfterLast) + } + if batch != nil { + if err := batch.Write(); err != nil { + log.Error("Error writing batch into chain database", "error", err) + } + } + return s +} + +func (s *CommitteeChain) Reset() { + s.lock.Lock() + defer s.lock.Unlock() + + batch := s.db.NewBatch() + s.rollback(batch, 0) + if err := batch.Write(); err != nil { + log.Error("Error writing batch into chain database", "error", err) + } +} + +func (s *CommitteeChain) AddFixedRoot(period uint64, root common.Hash) error { + s.lock.Lock() + defer s.lock.Unlock() + + batch := s.db.NewBatch() + oldRoot := s.getCommitteeRoot(period) + if !s.fixedRoots.CanExpand(period) { + if root != oldRoot { + return ErrInvalidPeriod + } + for p := s.fixedRoots.AfterLast; p <= period; p++ { + s.fixedRoots.add(batch, p, s.getCommitteeRoot(p)) + } + } + if oldRoot != (common.Hash{}) && (oldRoot != root) { + // existing old root was different, we have to reorg the chain + s.rollback(batch, period) + } + s.fixedRoots.add(batch, period, root) + if err := batch.Write(); err != nil { + log.Error("Error writing batch into chain database", "error", err) + return err + } + return nil +} + +func (s *CommitteeChain) DeleteFixedRootsFrom(period uint64) error { + s.lock.Lock() + defer s.lock.Unlock() + + if period >= s.fixedRoots.AfterLast { + return nil + } + batch := s.db.NewBatch() + s.fixedRoots.deleteFrom(batch, period) + if s.updates.IsEmpty() || period <= s.updates.First { + s.updates.deleteFrom(batch, period) + s.deleteCommitteesFrom(batch, period) + } else { + fromPeriod := s.updates.AfterLast + 1 + if period > fromPeriod { + fromPeriod = period + } + s.deleteCommitteesFrom(batch, fromPeriod) + } + if err := batch.Write(); err != nil { + log.Error("Error writing batch into chain database", "error", err) + return err + } + return nil +} + +func (s *CommitteeChain) deleteCommitteesFrom(batch ethdb.Batch, period uint64) { + deleted := s.committees.deleteFrom(batch, period) + for period := deleted.First; period < deleted.AfterLast; period++ { + s.syncCommitteeCache.Remove(period) + } +} + +func (s *CommitteeChain) GetCommittee(period uint64) *types.SerializedSyncCommittee { + return s.committees.get(period) +} + +func (s *CommitteeChain) AddCommittee(period uint64, committee *types.SerializedSyncCommittee) error { + s.lock.Lock() + defer s.lock.Unlock() + + if !s.committees.CanExpand(period) { + return ErrInvalidPeriod + } + root := s.getCommitteeRoot(period) + if root == (common.Hash{}) { + return ErrInvalidPeriod + } + if root != committee.Root() { + return ErrWrongCommitteeRoot + } + if !s.committees.Includes(period) { + s.committees.add(nil, period, committee) + s.syncCommitteeCache.Remove(period) + } + return nil +} + +func (s *CommitteeChain) GetUpdate(period uint64) *types.LightClientUpdate { + return s.updates.get(period) +} + +func (s *CommitteeChain) InsertUpdate(update *types.LightClientUpdate, nextCommittee *types.SerializedSyncCommittee) error { + s.lock.Lock() + defer s.lock.Unlock() + + period := update.AttestedHeader.Header.SyncPeriod() + if !s.updates.CanExpand(period) || !s.committees.Includes(period) { + return ErrInvalidPeriod + } + if s.minimumUpdateScore.BetterThan(update.Score()) { + return ErrInvalidUpdate + } + oldRoot := s.getCommitteeRoot(period + 1) + reorg := oldRoot != (common.Hash{}) && oldRoot != update.NextSyncCommitteeRoot + if oldUpdate := s.updates.get(period); oldUpdate != nil && !update.Score().BetterThan(oldUpdate.Score()) { + // a better or equal update already exists; no changes, only fail if new one tried to reorg + if reorg { + return ErrCannotReorg + } + return nil + } + if s.fixedRoots.Includes(period+1) && reorg { + return ErrCannotReorg + } + if !s.verifyUpdate(update) { + return ErrInvalidUpdate + } + addCommittee := !s.committees.Includes(period+1) || reorg + if addCommittee { + if nextCommittee == nil { + return ErrNeedCommittee + } + if nextCommittee.Root() != update.NextSyncCommitteeRoot { + return ErrWrongCommitteeRoot + } + } + batch := s.db.NewBatch() + if reorg { + s.rollback(batch, period+1) + } + if addCommittee { + s.committees.add(batch, period+1, nextCommittee) + s.syncCommitteeCache.Remove(period + 1) + } + s.updates.add(batch, period, update) + if err := batch.Write(); err != nil { + log.Error("Error writing batch into chain database", "error", err) + return err + } + log.Info("Inserted new committee update", "period", period, "next committee root", update.NextSyncCommitteeRoot) + return nil +} + +func (s *CommitteeChain) NextSyncPeriod() (uint64, bool) { + s.lock.RLock() + defer s.lock.RUnlock() + + if s.committees.IsEmpty() { + return 0, false + } + if !s.updates.IsEmpty() { + return s.updates.AfterLast, true + } + return s.committees.AfterLast - 1, true +} + +func (s *CommitteeChain) rollback(batch ethdb.Batch, period uint64) { + s.deleteCommitteesFrom(batch, period) + s.fixedRoots.deleteFrom(batch, period) + if period > 0 { + period-- + } + s.updates.deleteFrom(batch, period) +} + +func (s *CommitteeChain) getCommitteeRoot(period uint64) common.Hash { + if root := s.fixedRoots.get(period); root != (common.Hash{}) || period == 0 { + return root + } + if update := s.updates.get(period - 1); update != nil { + return update.NextSyncCommitteeRoot + } + return common.Hash{} +} + +// getSyncCommittee returns the deserialized sync committee at the given period +// of the current local committee chain (tracker mutex lock expected). +func (s *CommitteeChain) getSyncCommittee(period uint64) syncCommittee { + if c, ok := s.syncCommitteeCache.Get(period); ok { + return c + } + if sc := s.committees.get(period); sc != nil { + c, err := s.sigVerifier.deserializeSyncCommittee(sc) + if err != nil { + log.Error("Sync committee deserialization error", "error", err) + return nil + } + s.syncCommitteeCache.Add(period, c) + return c + } + log.Error("Missing serialized sync committee", "period", period) + return nil +} + +// VerifySignedHeader returns true if the given signed head has a valid signature +// according to the local committee chain. The caller should ensure that the +// committees advertised by the same source where the signed head came from are +// synced before verifying the signature. +// The age of the header is also returned (the time elapsed since the beginning +// of the given slot, according to the local system clock). If enforceTime is +// true then negative age (future) headers are rejected. +func (s *CommitteeChain) VerifySignedHeader(head types.SignedHeader) (bool, time.Duration) { + s.lock.RLock() + defer s.lock.RUnlock() + + return s.verifySignedHeader(head) +} + +// (rlock required) +func (s *CommitteeChain) verifySignedHeader(head types.SignedHeader) (bool, time.Duration) { + var ( + slotTime = int64(time.Second) * int64(s.config.GenesisTime+head.Header.Slot*12) + age = time.Duration(s.unixNano() - slotTime) + ) + if s.enforceTime && age < 0 { + return false, age + } + committee := s.getSyncCommittee(types.SyncPeriod(head.SignatureSlot)) + if committee == nil { + return false, age + } + if signingRoot, err := s.config.Forks.SigningRoot(head.Header); err == nil { + return s.sigVerifier.verifySignature(committee, signingRoot, &head.Signature), age + } + return false, age +} + +// verifyUpdate checks whether the header signature is correct and the update +// fits into the specified constraints (assumes that the update has been +// successfully validated previously) +// (rlock required) +func (s *CommitteeChain) verifyUpdate(update *types.LightClientUpdate) bool { + // Note: SignatureSlot determines the sync period of the committee used for signature + // verification. Though in reality SignatureSlot is always bigger than update.Header.Slot, + // setting them as equal here enforces the rule that they have to be in the same sync + // period in order for the light client update proof to be meaningful. + ok, age := s.verifySignedHeader(update.AttestedHeader) + if age < 0 { + log.Warn("Future committee update received", "age", age) + } + return ok +} + +type canonicalStore[T any] struct { + Range + db ethdb.KeyValueStore + keyPrefix []byte + cache *lru.Cache[uint64, T] + encode func(T) ([]byte, error) + decode func([]byte) (T, error) +} + +func newCanonicalStore[T any](db ethdb.KeyValueStore, keyPrefix []byte, + encode func(T) ([]byte, error), decode func([]byte) (T, error)) *canonicalStore[T] { + cs := &canonicalStore[T]{ + db: db, + keyPrefix: keyPrefix, + encode: encode, + decode: decode, + cache: lru.NewCache[uint64, T](100), + } + var ( + iter = db.NewIterator(keyPrefix, nil) + kl = len(keyPrefix) + ) + for iter.Next() { + period := binary.BigEndian.Uint64(iter.Key()[kl : kl+8]) + if cs.First == 0 { + cs.First = period + } else if cs.AfterLast != period { + if iter.Next() { + log.Error("Gap in the canonical chain database") + } + break // continuity guaranteed + } + cs.AfterLast = period + 1 + } + iter.Release() + return cs +} + +func (cs *canonicalStore[T]) getDbKey(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 +} + +func (cs *canonicalStore[T]) add(batch ethdb.Batch, period uint64, value T) { + if !cs.CanExpand(period) { + log.Error("Cannot expand canonical store", "range.first", cs.First, "range.afterLast", cs.AfterLast, "new period", period) + return + } + enc, err := cs.encode(value) + if err != nil { + log.Error("Error encoding canonical store value", "error", err) + return + } + key := cs.getDbKey(period) + if batch != nil { + err = batch.Put(key, enc) + } else { + err = cs.db.Put(key, enc) + } + if err != nil { + log.Error("Error writing into canonical store value database", "error", err) + } + cs.cache.Add(period, value) + cs.Expand(period) +} + +// should only be used in batch mode +func (cs *canonicalStore[T]) deleteFrom(batch ethdb.Batch, fromPeriod uint64) (deleted Range) { + if fromPeriod >= cs.AfterLast { + return + } + if fromPeriod < cs.First { + fromPeriod = cs.First + } + deleted = Range{First: fromPeriod, AfterLast: cs.AfterLast} + for period := fromPeriod; period < cs.AfterLast; period++ { + batch.Delete(cs.getDbKey(period)) + cs.cache.Remove(period) + } + if fromPeriod > cs.First { + cs.AfterLast = fromPeriod + } else { + cs.Range = Range{} + } + return +} + +func (cs *canonicalStore[T]) get(period uint64) T { + if value, ok := cs.cache.Get(period); ok { + return value + } + var value T + if enc, err := cs.db.Get(cs.getDbKey(period)); err == nil { + if v, err := cs.decode(enc); err == nil { + value = v + } else { + log.Error("Error decoding canonical store value", "error", err) + } + } + return value +} diff --git a/beacon/light/committee_chain_test.go b/beacon/light/committee_chain_test.go new file mode 100644 index 000000000000..ac073572e2d2 --- /dev/null +++ b/beacon/light/committee_chain_test.go @@ -0,0 +1,356 @@ +// Copyright 2022 The go-ethereum Authors +// This file is part of the go-ethereum library. +// +// The go-ethereum library is free software: you can redistribute it and/or modify +// it under the terms of the GNU Lesser General Public License as published by +// the Free Software Foundation, either version 3 of the License, or +// (at your option) any later version. +// +// The go-ethereum library is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU Lesser General Public License for more details. +// +// You should have received a copy of the GNU Lesser General Public License +// along with the go-ethereum library. If not, see . + +package light + +import ( + "math/rand" + "testing" + "time" + + "github.com/ethereum/go-ethereum/beacon/params" + "github.com/ethereum/go-ethereum/beacon/types" + "github.com/ethereum/go-ethereum/common/mclock" + "github.com/ethereum/go-ethereum/ethdb/memorydb" +) + +var ( + testGenesis = newTestGenesis() + testGenesis2 = newTestGenesis() + + tfBase = newTestForks(testGenesis, types.Forks{ + &types.Fork{Epoch: 0, Version: []byte{0}}, + }) + tfAlternative = newTestForks(testGenesis, types.Forks{ + &types.Fork{Epoch: 0, Version: []byte{0}}, + &types.Fork{Epoch: 0x700, Version: []byte{1}}, + }) + tfAnotherGenesis = newTestForks(testGenesis2, types.Forks{ + &types.Fork{Epoch: 0, Version: []byte{0}}, + }) + + tcBase = newTestCommitteeChain(nil, tfBase, true, 0, 10, 400, false) + tcBaseWithInvalidUpdates = newTestCommitteeChain(tcBase, tfBase, false, 5, 10, 200, false) // signer count too low + tcBaseWithBetterUpdates = newTestCommitteeChain(tcBase, tfBase, false, 5, 10, 440, false) + tcReorgWithWorseUpdates = newTestCommitteeChain(tcBase, tfBase, true, 5, 10, 400, false) + tcReorgWithWorseUpdates2 = newTestCommitteeChain(tcBase, tfBase, true, 5, 10, 380, false) + tcReorgWithBetterUpdates = newTestCommitteeChain(tcBase, tfBase, true, 5, 10, 420, false) + tcReorgWithFinalizedUpdates = newTestCommitteeChain(tcBase, tfBase, true, 5, 10, 400, true) + tcFork = newTestCommitteeChain(tcBase, tfAlternative, true, 7, 10, 400, false) + tcAnotherGenesis = newTestCommitteeChain(nil, tfAnotherGenesis, true, 0, 10, 400, false) +) + +func TestCommitteeChainFixedRoots(t *testing.T) { + for _, reload := range []bool{false, true} { + c := newCommitteeChainTest(t, tfBase, 300, true) + c.setClockPeriod(7) + c.addFixedRoot(tcBase, 4, nil) + c.addFixedRoot(tcBase, 5, nil) + c.addFixedRoot(tcBase, 6, nil) + c.addFixedRoot(tcBase, 8, ErrInvalidPeriod) // range has to be continuoous + c.addFixedRoot(tcBase, 3, nil) + c.addFixedRoot(tcBase, 2, nil) + if reload { + c.reloadChain() + } + c.addCommittee(tcBase, 4, nil) + c.addCommittee(tcBase, 6, ErrInvalidPeriod) // range has to be continuoous + c.addCommittee(tcBase, 5, nil) + c.addCommittee(tcBase, 6, nil) + c.addCommittee(tcAnotherGenesis, 3, ErrWrongCommitteeRoot) + c.addCommittee(tcBase, 3, nil) + if reload { + c.reloadChain() + } + c.verifyRange(tcBase, 3, 6) + } +} + +func TestCommitteeChainCheckpointSync(t *testing.T) { + for _, enforceTime := range []bool{false, true} { + for _, reload := range []bool{false, true} { + c := newCommitteeChainTest(t, tfBase, 300, enforceTime) + if enforceTime { + c.setClockPeriod(6) + } + c.insertUpdate(tcBase, 3, true, ErrInvalidPeriod) + c.addFixedRoot(tcBase, 3, nil) + c.addFixedRoot(tcBase, 4, nil) + c.insertUpdate(tcBase, 4, true, ErrInvalidPeriod) // still no committee + c.addCommittee(tcBase, 3, nil) + c.addCommittee(tcBase, 4, nil) + if reload { + c.reloadChain() + } + c.verifyRange(tcBase, 3, 4) + c.insertUpdate(tcBase, 3, false, nil) // update can be added without committee here + c.insertUpdate(tcBase, 4, false, ErrNeedCommittee) // but not here as committee 5 is not there yet + c.insertUpdate(tcBase, 4, true, nil) + c.verifyRange(tcBase, 3, 5) + c.insertUpdate(tcBaseWithInvalidUpdates, 5, true, ErrInvalidUpdate) // signer count too low + c.insertUpdate(tcBase, 5, true, nil) + if reload { + c.reloadChain() + } + if enforceTime { + c.insertUpdate(tcBase, 6, true, ErrInvalidUpdate) // future update rejected + c.setClockPeriod(7) + } + c.insertUpdate(tcBase, 6, true, nil) // when the time comes it's accepted + if reload { + c.reloadChain() + } + if enforceTime { + c.verifyRange(tcBase, 3, 6) // committee 7 is there but still in the future + c.setClockPeriod(8) + } + c.verifyRange(tcBase, 3, 7) // now period 7 can also be verified + // try reverse syncing an update + c.insertUpdate(tcBase, 2, false, ErrInvalidPeriod) // fixed committee is needed first + c.addFixedRoot(tcBase, 2, nil) + c.addCommittee(tcBase, 2, nil) + c.insertUpdate(tcBase, 2, false, nil) + c.verifyRange(tcBase, 2, 7) + } + } +} + +func TestCommitteeChainReorg(t *testing.T) { + for _, reload := range []bool{false, true} { + for _, addBetterUpdates := range []bool{false, true} { + c := newCommitteeChainTest(t, tfBase, 300, true) + c.setClockPeriod(11) + c.addFixedRoot(tcBase, 3, nil) + c.addFixedRoot(tcBase, 4, nil) + c.addCommittee(tcBase, 3, nil) + for period := uint64(3); period < 10; period++ { + c.insertUpdate(tcBase, period, true, nil) + } + if reload { + c.reloadChain() + } + c.verifyRange(tcBase, 3, 10) + c.insertUpdate(tcReorgWithWorseUpdates, 5, true, ErrCannotReorg) + c.insertUpdate(tcReorgWithWorseUpdates2, 5, true, ErrCannotReorg) + if addBetterUpdates { + // add better updates for the base chain and expect first reorg to fail + // (only add updates as committees should be the same) + for period := uint64(5); period < 10; period++ { + c.insertUpdate(tcBaseWithBetterUpdates, period, false, nil) + } + if reload { + c.reloadChain() + } + c.verifyRange(tcBase, 3, 10) // still on the same chain + c.insertUpdate(tcReorgWithBetterUpdates, 5, true, ErrCannotReorg) + } else { + // reorg with better updates + c.insertUpdate(tcReorgWithBetterUpdates, 5, false, ErrNeedCommittee) + c.verifyRange(tcBase, 3, 10) // no success yet, still on the base chain + c.verifyRange(tcReorgWithBetterUpdates, 3, 5) + c.insertUpdate(tcReorgWithBetterUpdates, 5, true, nil) + // successful reorg, base chain should only match before the reorg period + if reload { + c.reloadChain() + } + c.verifyRange(tcBase, 3, 5) + c.verifyRange(tcReorgWithBetterUpdates, 3, 6) + for period := uint64(6); period < 10; period++ { + c.insertUpdate(tcReorgWithBetterUpdates, period, true, nil) + } + c.verifyRange(tcReorgWithBetterUpdates, 3, 10) + } + // reorg with finalized updates; should succeed even if base chain updates + // have been improved because a finalized update beats everything else + c.insertUpdate(tcReorgWithFinalizedUpdates, 5, false, ErrNeedCommittee) + c.insertUpdate(tcReorgWithFinalizedUpdates, 5, true, nil) + if reload { + c.reloadChain() + } + c.verifyRange(tcReorgWithFinalizedUpdates, 3, 6) + for period := uint64(6); period < 10; period++ { + c.insertUpdate(tcReorgWithFinalizedUpdates, period, true, nil) + } + c.verifyRange(tcReorgWithFinalizedUpdates, 3, 10) + } + } +} + +func TestCommitteeChainFork(t *testing.T) { + c := newCommitteeChainTest(t, tfAlternative, 300, true) + c.setClockPeriod(11) + // trying to sync a chain on an alternative fork with the base chain data + c.addFixedRoot(tcBase, 0, nil) + c.addFixedRoot(tcBase, 1, nil) + c.addCommittee(tcBase, 0, nil) + // shared section should sync without errors + for period := uint64(0); period < 7; period++ { + c.insertUpdate(tcBase, period, true, nil) + } + c.insertUpdate(tcBase, 7, true, ErrInvalidUpdate) // wrong fork + // committee root #7 is still the same but signatures are already signed with + // a different fork id so period 7 should only verify on the alternative fork + c.verifyRange(tcBase, 0, 6) + c.verifyRange(tcFork, 0, 7) + for period := uint64(7); period < 10; period++ { + c.insertUpdate(tcFork, period, true, nil) + } + c.verifyRange(tcFork, 0, 10) + // reload the chain while switching to the base fork + c.config = tfBase + c.reloadChain() + // updates 7..9 should be rolled back now + c.verifyRange(tcFork, 0, 6) // again, period 7 only verifies on the right fork + c.verifyRange(tcBase, 0, 7) + c.insertUpdate(tcFork, 7, true, ErrInvalidUpdate) // wrong fork + for period := uint64(7); period < 10; period++ { + c.insertUpdate(tcBase, period, true, nil) + } + c.verifyRange(tcBase, 0, 10) +} + +type committeeChainTest struct { + t *testing.T + db *memorydb.Database + clock *mclock.Simulated + config types.ChainConfig + signerThreshold int + enforceTime bool + chain *CommitteeChain +} + +func newCommitteeChainTest(t *testing.T, config types.ChainConfig, signerThreshold int, enforceTime bool) *committeeChainTest { + c := &committeeChainTest{ + t: t, + db: memorydb.New(), + clock: &mclock.Simulated{}, + config: config, + signerThreshold: signerThreshold, + enforceTime: enforceTime, + } + c.chain = NewCommitteeChain(c.db, &config, signerThreshold, enforceTime, DummyVerifier{}, c.clock, func() int64 { return int64(c.clock.Now()) }) + return c +} + +func (c *committeeChainTest) reloadChain() { + c.chain = NewCommitteeChain(c.db, &c.config, c.signerThreshold, c.enforceTime, DummyVerifier{}, c.clock, func() int64 { return int64(c.clock.Now()) }) +} + +func (c *committeeChainTest) setClockPeriod(period float64) { + target := mclock.AbsTime(period * float64(time.Second*12*params.SyncPeriodLength)) + wait := time.Duration(target - c.clock.Now()) + if wait < 0 { + c.t.Fatalf("Invalid setClockPeriod") + } + c.clock.Run(wait) +} + +func (c *committeeChainTest) addFixedRoot(tc *testCommitteeChain, period uint64, expErr error) { + if err := c.chain.AddFixedRoot(period, tc.periods[period].committee.Root()); err != expErr { + c.t.Errorf("Incorrect error output from AddFixedRoot at period %d (expected %v, got %v)", period, expErr, err) + } +} + +func (c *committeeChainTest) addCommittee(tc *testCommitteeChain, period uint64, expErr error) { + if err := c.chain.AddCommittee(period, tc.periods[period].committee); err != expErr { + c.t.Errorf("Incorrect error output from AddCommittee at period %d (expected %v, got %v)", period, expErr, err) + } +} + +func (c *committeeChainTest) insertUpdate(tc *testCommitteeChain, period uint64, addCommittee bool, expErr error) { + var committee *types.SerializedSyncCommittee + if addCommittee { + committee = tc.periods[period+1].committee + } + if err := c.chain.InsertUpdate(tc.periods[period].update, committee); err != expErr { + c.t.Errorf("Incorrect error output from InsertUpdate at period %d (expected %v, got %v)", period, expErr, err) + } +} + +func (c *committeeChainTest) verifySignedHeader(tc *testCommitteeChain, period float64, expOk bool) { + slot := uint64(period * float64(params.SyncPeriodLength)) + signedHead := GenerateTestSignedHeader(types.Header{Slot: slot}, &tc.config, tc.periods[types.SyncPeriod(slot)].committee, slot+1, 400) + if ok, _ := c.chain.VerifySignedHeader(signedHead); ok != expOk { + c.t.Errorf("Incorrect output from VerifySignedHeader at period %f (expected %v, got %v)", period, expOk, ok) + } +} + +func (c *committeeChainTest) verifyRange(tc *testCommitteeChain, begin, end uint64) { + if begin > 0 { + c.verifySignedHeader(tc, float64(begin)-0.5, false) + } + for period := begin; period <= end; period++ { + c.verifySignedHeader(tc, float64(period)+0.5, true) + } + c.verifySignedHeader(tc, float64(end)+1.5, false) +} + +func newTestGenesis() types.ChainConfig { + var config types.ChainConfig + rand.Read(config.GenesisValidatorsRoot[:]) + return config +} + +func newTestForks(config types.ChainConfig, forks types.Forks) types.ChainConfig { + for _, fork := range forks { + config.AddFork(fork.Name, fork.Epoch, fork.Version) + } + return config +} + +func newTestCommitteeChain(parent *testCommitteeChain, config types.ChainConfig, newCommittees bool, begin, end int, signerCount int, finalizedHeader bool) *testCommitteeChain { + tc := &testCommitteeChain{ + config: config, + } + if parent != nil { + tc.periods = make([]testPeriod, len(parent.periods)) + copy(tc.periods, parent.periods) + } + if newCommittees { + if begin == 0 { + tc.fillCommittees(begin, end+1) + } else { + tc.fillCommittees(begin+1, end+1) + } + } + tc.fillUpdates(begin, end, signerCount, finalizedHeader) + return tc +} + +type testPeriod struct { + committee *types.SerializedSyncCommittee + update *types.LightClientUpdate +} + +type testCommitteeChain struct { + periods []testPeriod + config types.ChainConfig +} + +func (tc *testCommitteeChain) fillCommittees(begin, end int) { + if len(tc.periods) <= end { + tc.periods = append(tc.periods, make([]testPeriod, end+1-len(tc.periods))...) + } + for i := begin; i <= end; i++ { + tc.periods[i].committee = GenerateTestCommittee() + } +} + +func (tc *testCommitteeChain) fillUpdates(begin, end int, signerCount int, finalizedHeader bool) { + for i := begin; i <= end; i++ { + tc.periods[i].update = GenerateTestUpdate(&tc.config, uint64(i), tc.periods[i].committee, tc.periods[i+1].committee, signerCount, finalizedHeader) + } +} diff --git a/beacon/light/range.go b/beacon/light/range.go new file mode 100644 index 000000000000..ee01eb550ac7 --- /dev/null +++ b/beacon/light/range.go @@ -0,0 +1,52 @@ +// Copyright 2023 The go-ethereum Authors +// This file is part of the go-ethereum library. +// +// The go-ethereum library is free software: you can redistribute it and/or modify +// it under the terms of the GNU Lesser General Public License as published by +// the Free Software Foundation, either version 3 of the License, or +// (at your option) any later version. +// +// The go-ethereum library is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU Lesser General Public License for more details. +// +// You should have received a copy of the GNU Lesser General Public License +// along with the go-ethereum library. If not, see . + +package light + +type Range struct { + First uint64 + AfterLast uint64 +} + +func (a Range) IsEmpty() bool { + return a.AfterLast == a.First +} + +func (a Range) Includes(period uint64) bool { + return period >= a.First && period < a.AfterLast +} + +func (a Range) CanExpand(period uint64) bool { + return a.IsEmpty() || (period+1 >= a.First && period <= a.AfterLast) +} + +func (a *Range) Expand(period uint64) { + if a.IsEmpty() { + a.First, a.AfterLast = period, period+1 + return + } + if a.Includes(period) { + return + } + if a.First == period+1 { + a.First-- + return + } + if a.AfterLast == period { + a.AfterLast++ + return + } +} diff --git a/beacon/light/test_helpers.go b/beacon/light/test_helpers.go new file mode 100644 index 000000000000..7c77d640f2f1 --- /dev/null +++ b/beacon/light/test_helpers.go @@ -0,0 +1,151 @@ +// Copyright 2023 The go-ethereum Authors +// This file is part of the go-ethereum library. +// +// The go-ethereum library is free software: you can redistribute it and/or modify +// it under the terms of the GNU Lesser General Public License as published by +// the Free Software Foundation, either version 3 of the License, or +// (at your option) any later version. +// +// The go-ethereum library is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU Lesser General Public License for more details. +// +// You should have received a copy of the GNU Lesser General Public License +// along with the go-ethereum library. If not, see . + +package light + +import ( + "crypto/sha256" + "math/rand" + + "github.com/ethereum/go-ethereum/beacon/merkle" + "github.com/ethereum/go-ethereum/beacon/params" + "github.com/ethereum/go-ethereum/beacon/types" + "github.com/ethereum/go-ethereum/common" +) + +func GenerateTestCommittee() *types.SerializedSyncCommittee { + s := new(types.SerializedSyncCommittee) + rand.Read(s[:32]) + return s +} + +func GenerateTestUpdate(config *types.ChainConfig, period uint64, committee, nextCommittee *types.SerializedSyncCommittee, signerCount int, finalizedHeader bool) *types.LightClientUpdate { + update := new(types.LightClientUpdate) + update.NextSyncCommitteeRoot = nextCommittee.Root() + var attestedHeader types.Header + if finalizedHeader { + update.FinalizedHeader = new(types.Header) + *update.FinalizedHeader, update.NextSyncCommitteeBranch = makeTestHeaderWithMerkleProof(types.SyncPeriodStart(period)+100, params.StateIndexNextSyncCommittee, merkle.Value(update.NextSyncCommitteeRoot)) + attestedHeader, update.FinalityBranch = makeTestHeaderWithMerkleProof(types.SyncPeriodStart(period)+200, params.StateIndexFinalBlock, merkle.Value(update.FinalizedHeader.Hash())) + } else { + attestedHeader, update.NextSyncCommitteeBranch = makeTestHeaderWithMerkleProof(types.SyncPeriodStart(period)+2000, params.StateIndexNextSyncCommittee, merkle.Value(update.NextSyncCommitteeRoot)) + } + update.AttestedHeader = GenerateTestSignedHeader(attestedHeader, config, committee, attestedHeader.Slot+1, signerCount) + return update +} + +func GenerateTestSignedHeader(header types.Header, config *types.ChainConfig, committee *types.SerializedSyncCommittee, signatureSlot uint64, signerCount int) types.SignedHeader { + bitmask := makeBitmask(signerCount) + signingRoot, _ := config.Forks.SigningRoot(header) + c, _ := DummyVerifier{}.deserializeSyncCommittee(committee) + return types.SignedHeader{ + Header: header, + Signature: types.SyncAggregate{ + Signers: bitmask, + Signature: makeDummySignature(c.(dummySyncCommittee), signingRoot, bitmask), + }, + SignatureSlot: signatureSlot, + } +} + +func GenerateTestCheckpoint(period uint64, committee *types.SerializedSyncCommittee) *CheckpointData { + header, branch := makeTestHeaderWithMerkleProof(types.SyncPeriodStart(period)+200, params.StateIndexSyncCommittee, merkle.Value(committee.Root())) + return &CheckpointData{ + Header: header, + Committee: committee, + CommitteeRoot: committee.Root(), + CommitteeBranch: branch, + } +} + +func makeBitmask(signerCount int) (bitmask [params.SyncCommitteeBitmaskSize]byte) { + for i := 0; i < params.SyncCommitteeSize; i++ { + if rand.Intn(params.SyncCommitteeSize-i) < signerCount { + bitmask[i/8] += byte(1) << (i & 7) + signerCount-- + } + } + return +} + +func makeTestHeaderWithMerkleProof(slot, index uint64, value merkle.Value) (types.Header, merkle.Values) { + var branch merkle.Values + hasher := sha256.New() + for index > 1 { + var proofHash merkle.Value + rand.Read(proofHash[:]) + hasher.Reset() + if index&1 == 0 { + hasher.Write(value[:]) + hasher.Write(proofHash[:]) + } else { + hasher.Write(proofHash[:]) + hasher.Write(value[:]) + } + hasher.Sum(value[:0]) + index >>= 1 + branch = append(branch, proofHash) + } + return types.Header{Slot: slot, StateRoot: common.Hash(value)}, branch +} + +// syncCommittee holds either a blsSyncCommittee or a fake dummySyncCommittee used for testing +type syncCommittee interface{} + +// committeeSigVerifier verifies sync committee signatures (either proper BLS +// signatures or fake signatures used for testing) +type committeeSigVerifier interface { + deserializeSyncCommittee(s *types.SerializedSyncCommittee) (syncCommittee, error) + verifySignature(committee syncCommittee, signedRoot common.Hash, aggregate *types.SyncAggregate) bool +} + +// BLSVerifier implements committeeSigVerifier +type BLSVerifier struct{} + +// deserializeSyncCommittee implements committeeSigVerifier +func (BLSVerifier) deserializeSyncCommittee(s *types.SerializedSyncCommittee) (syncCommittee, error) { + return s.Deserialize() +} + +// verifySignature implements committeeSigVerifier +func (BLSVerifier) verifySignature(committee syncCommittee, signingRoot common.Hash, aggregate *types.SyncAggregate) bool { + return committee.(*types.SyncCommittee).VerifySignature(signingRoot, aggregate) +} + +type dummySyncCommittee [32]byte + +// DummyVerifier implements committeeSigVerifier +type DummyVerifier struct{} + +// deserializeSyncCommittee implements committeeSigVerifier +func (DummyVerifier) deserializeSyncCommittee(s *types.SerializedSyncCommittee) (syncCommittee, error) { + var sc dummySyncCommittee + copy(sc[:], s[:32]) + return sc, nil +} + +// verifySignature implements committeeSigVerifier +func (DummyVerifier) verifySignature(committee syncCommittee, signingRoot common.Hash, aggregate *types.SyncAggregate) bool { + return aggregate.Signature == makeDummySignature(committee.(dummySyncCommittee), signingRoot, aggregate.Signers) +} + +func makeDummySignature(committee dummySyncCommittee, signingRoot common.Hash, bitmask [params.SyncCommitteeBitmaskSize]byte) (sig [params.BLSSignatureSize]byte) { + for i, b := range committee[:] { + sig[i] = b ^ signingRoot[i] + } + copy(sig[32:], bitmask[:]) + return +} From 8ff38966c41f942d432238f3f85145f78e5f5ab2 Mon Sep 17 00:00:00 2001 From: Zsolt Felfoldi Date: Tue, 25 Jul 2023 14:37:07 +0200 Subject: [PATCH 02/19] beacon/light: added comments and removed unused code --- beacon/light/checkpoint.go | 59 +++-------------------- beacon/light/committee_chain.go | 70 +++++++++++++++++++++------- beacon/light/committee_chain_test.go | 2 +- beacon/light/range.go | 6 +++ beacon/light/test_helpers.go | 5 +- 5 files changed, 68 insertions(+), 74 deletions(-) diff --git a/beacon/light/checkpoint.go b/beacon/light/checkpoint.go index c9d4956dc519..25ec7fcc2b3e 100644 --- a/beacon/light/checkpoint.go +++ b/beacon/light/checkpoint.go @@ -23,13 +23,12 @@ import ( "github.com/ethereum/go-ethereum/beacon/params" "github.com/ethereum/go-ethereum/beacon/types" "github.com/ethereum/go-ethereum/common" - "github.com/ethereum/go-ethereum/ethdb" "github.com/ethereum/go-ethereum/log" - "github.com/ethereum/go-ethereum/rlp" ) -var checkpointKey = []byte("checkpoint-") // block root -> RLP(CheckpointData) - +// CheckpointData contains a sync committee where light sync can be started, +// together with a proof through a beacon header and corresponding state. +// Note: CheckpointData is fetched from a server based on a known checkpoint hash. type CheckpointData struct { Header types.Header CommitteeRoot common.Hash @@ -37,6 +36,7 @@ type CheckpointData struct { CommitteeBranch merkle.Values } +// Validate verifies the proof included in CheckpointData. func (c *CheckpointData) Validate() error { if c.CommitteeRoot != c.Committee.Root() { return errors.New("wrong committee root") @@ -44,7 +44,8 @@ func (c *CheckpointData) Validate() error { return merkle.VerifyProof(c.Header.StateRoot, params.StateIndexSyncCommittee, c.CommitteeBranch, merkle.Value(c.CommitteeRoot)) } -// expected to be validated already +// InitChain initializes a CommitteeChain based on the checkpoint. +// Note that the checkpoint is expected to be already validated. func (c *CheckpointData) InitChain(chain *CommitteeChain) { must := func(err error) { if err != nil { @@ -60,51 +61,3 @@ func (c *CheckpointData) InitChain(chain *CommitteeChain) { must(chain.AddFixedRoot(period+1, common.Hash(c.CommitteeBranch[0]))) must(chain.AddCommittee(period, c.Committee)) } - -type CheckpointStore struct { - chain *CommitteeChain - db ethdb.KeyValueStore -} - -func NewCheckpointStore(db ethdb.KeyValueStore, chain *CommitteeChain) *CheckpointStore { - return &CheckpointStore{ - db: db, - chain: chain, - } -} - -func getCheckpointKey(checkpoint common.Hash) []byte { - var ( - kl = len(checkpointKey) - key = make([]byte, kl+32) - ) - copy(key[:kl], checkpointKey) - copy(key[kl:], checkpoint[:]) - return key -} - -func (cs *CheckpointStore) Get(checkpoint common.Hash) *CheckpointData { - if enc, err := cs.db.Get(getCheckpointKey(checkpoint)); err == nil { - c := new(CheckpointData) - if err := rlp.DecodeBytes(enc, c); err != nil { - log.Error("Error decoding stored checkpoint", "error", err) - return nil - } - if committee := cs.chain.committees.get(c.Header.SyncPeriod()); committee != nil && committee.Root() == c.CommitteeRoot { - c.Committee = committee - return c - } - log.Error("Missing committee for stored checkpoint", "period", c.Header.SyncPeriod()) - } - return nil -} - -func (cs *CheckpointStore) Store(c *CheckpointData) { - enc, err := rlp.EncodeToBytes(c) - if err != nil { - log.Error("Error encoding checkpoint for storage", "error", err) - } - if err := cs.db.Put(getCheckpointKey(c.Header.Hash()), enc); err != nil { - log.Error("Error storing checkpoint in database", "error", err) - } -} diff --git a/beacon/light/committee_chain.go b/beacon/light/committee_chain.go index 1aec863e4258..929be66156eb 100644 --- a/beacon/light/committee_chain.go +++ b/beacon/light/committee_chain.go @@ -47,16 +47,25 @@ var ( syncCommitteeKey = []byte("committee-") // bigEndian64(syncPeriod) -> serialized committee ) -// CommitteeChain maintains a chain of sync committee updates and a small -// set of best known signed heads. It is used in all client configurations -// operating on a beacon chain. It can sync its update chain and receive signed -// heads from either an ODR or beacon node API backend and propagate/serve this -// data to subscribed peers. Received signed heads are validated based on the -// known sync committee chain and added to the local set if valid or placed in a -// deferred queue if the committees are not synced up to the period of the new -// head yet. -// Sync committee chain is either initialized from a weak subjectivity checkpoint -// or controlled by a BeaconChain that is driven by a trusted source (beacon node API). +// CommitteeChain is a passive data structure that can validate, hold and update +// a chain of beacon light sync committees and updates. It requires at least one +// externally set fixed committee root at the beginning of the chain which can +// be set either based on a CheckpointData or a trusted source (a local beacon +// full node). This makes the structure useful for both light client and light +// server setups. +// +// It always maintains the following consistency constraints: +// - a committee can only be present if its root hash matches an existing fixed +// root or if it is proven by an update at the previous period +// - an update can only be present if a committee is present at the same period +// and the update signature is valid and has enough participants. +// The committee at the next period (proven by the update) should also be +// present (note that this means they can only be added together if neither +// is present yet). If a fixed root is present at the next period then the +// update can only be present if it proves the same committee root. +// +// Once synced to the current sync period, CommitteeChain can also validate +// signed beacon headers. type CommitteeChain struct { lock sync.RWMutex db ethdb.KeyValueStore @@ -74,7 +83,7 @@ type CommitteeChain struct { enforceTime bool } -// NewCommitteeChain creates a new CommitteeChain +// NewCommitteeChain creates a new CommitteeChain. func NewCommitteeChain(db ethdb.KeyValueStore, config *types.ChainConfig, signerThreshold int, enforceTime bool, sigVerifier committeeSigVerifier, clock mclock.Clock, unixNano func() int64) *CommitteeChain { s := &CommitteeChain{ fixedRoots: newCanonicalStore[common.Hash](db, fixedRootKey, func(root common.Hash) ([]byte, error) { @@ -162,6 +171,7 @@ func NewCommitteeChain(db ethdb.KeyValueStore, config *types.ChainConfig, signer return s } +// Reset resets the committee chain. func (s *CommitteeChain) Reset() { s.lock.Lock() defer s.lock.Unlock() @@ -173,6 +183,9 @@ func (s *CommitteeChain) Reset() { } } +// AddFixedRoot sets a fixed committee root at the given period. +// Note that the period where the first committee is added has to have a fixed +// root which can either come from a CheckpointData or a trusted source. func (s *CommitteeChain) AddFixedRoot(period uint64, root common.Hash) error { s.lock.Lock() defer s.lock.Unlock() @@ -199,6 +212,9 @@ func (s *CommitteeChain) AddFixedRoot(period uint64, root common.Hash) error { return nil } +// DeleteFixedRootsFrom deletes fixed roots starting from the given period. +// It also maintains chain consistency, meaning that it also deletes updates and +// committees if they are no longer supported by a valid update chain. func (s *CommitteeChain) DeleteFixedRootsFrom(period uint64) error { s.lock.Lock() defer s.lock.Unlock() @@ -225,6 +241,7 @@ func (s *CommitteeChain) DeleteFixedRootsFrom(period uint64) error { return nil } +// deleteCommitteesFrom deletes committees starting from the given period. func (s *CommitteeChain) deleteCommitteesFrom(batch ethdb.Batch, period uint64) { deleted := s.committees.deleteFrom(batch, period) for period := deleted.First; period < deleted.AfterLast; period++ { @@ -232,10 +249,12 @@ func (s *CommitteeChain) deleteCommitteesFrom(batch ethdb.Batch, period uint64) } } +// GetCommittee returns the committee at the given period. func (s *CommitteeChain) GetCommittee(period uint64) *types.SerializedSyncCommittee { return s.committees.get(period) } +// AddCommittee adds a committee at the given period if possible. func (s *CommitteeChain) AddCommittee(period uint64, committee *types.SerializedSyncCommittee) error { s.lock.Lock() defer s.lock.Unlock() @@ -257,10 +276,12 @@ func (s *CommitteeChain) AddCommittee(period uint64, committee *types.Serialized return nil } +// GetUpdate returns the update at the given period. func (s *CommitteeChain) GetUpdate(period uint64) *types.LightClientUpdate { return s.updates.get(period) } +// InsertUpdate adds a new update if possible. func (s *CommitteeChain) InsertUpdate(update *types.LightClientUpdate, nextCommittee *types.SerializedSyncCommittee) error { s.lock.Lock() defer s.lock.Unlock() @@ -313,6 +334,8 @@ func (s *CommitteeChain) InsertUpdate(update *types.LightClientUpdate, nextCommi return nil } +// NextSyncPeriod returns the next period where an update can be added and also +// whether the chain is initialized at all. func (s *CommitteeChain) NextSyncPeriod() (uint64, bool) { s.lock.RLock() defer s.lock.RUnlock() @@ -326,6 +349,8 @@ func (s *CommitteeChain) NextSyncPeriod() (uint64, bool) { return s.committees.AfterLast - 1, true } +// rollback removes all committees and fixed roots from the given period and updates +// starting from the previous period. func (s *CommitteeChain) rollback(batch ethdb.Batch, period uint64) { s.deleteCommitteesFrom(batch, period) s.fixedRoots.deleteFrom(batch, period) @@ -335,6 +360,9 @@ func (s *CommitteeChain) rollback(batch ethdb.Batch, period uint64) { s.updates.deleteFrom(batch, period) } +// getCommitteeRoot returns the committee root at the given period, either fixed, +// proven by a previous update or both. It returns an empty hash if the committee +// root is unknown. func (s *CommitteeChain) getCommitteeRoot(period uint64) common.Hash { if root := s.fixedRoots.get(period); root != (common.Hash{}) || period == 0 { return root @@ -345,8 +373,7 @@ func (s *CommitteeChain) getCommitteeRoot(period uint64) common.Hash { return common.Hash{} } -// getSyncCommittee returns the deserialized sync committee at the given period -// of the current local committee chain (tracker mutex lock expected). +// getSyncCommittee returns the deserialized sync committee at the given period. func (s *CommitteeChain) getSyncCommittee(period uint64) syncCommittee { if c, ok := s.syncCommitteeCache.Get(period); ok { return c @@ -364,9 +391,9 @@ func (s *CommitteeChain) getSyncCommittee(period uint64) syncCommittee { return nil } -// VerifySignedHeader returns true if the given signed head has a valid signature +// VerifySignedHeader returns true if the given signed header has a valid signature // according to the local committee chain. The caller should ensure that the -// committees advertised by the same source where the signed head came from are +// committees advertised by the same source where the signed header came from are // synced before verifying the signature. // The age of the header is also returned (the time elapsed since the beginning // of the given slot, according to the local system clock). If enforceTime is @@ -378,7 +405,6 @@ func (s *CommitteeChain) VerifySignedHeader(head types.SignedHeader) (bool, time return s.verifySignedHeader(head) } -// (rlock required) func (s *CommitteeChain) verifySignedHeader(head types.SignedHeader) (bool, time.Duration) { var ( slotTime = int64(time.Second) * int64(s.config.GenesisTime+head.Header.Slot*12) @@ -400,7 +426,6 @@ func (s *CommitteeChain) verifySignedHeader(head types.SignedHeader) (bool, time // verifyUpdate checks whether the header signature is correct and the update // fits into the specified constraints (assumes that the update has been // successfully validated previously) -// (rlock required) func (s *CommitteeChain) verifyUpdate(update *types.LightClientUpdate) bool { // Note: SignatureSlot determines the sync period of the committee used for signature // verification. Though in reality SignatureSlot is always bigger than update.Header.Slot, @@ -413,6 +438,8 @@ func (s *CommitteeChain) verifyUpdate(update *types.LightClientUpdate) bool { return ok } +// canonicalStore stores instances of the given type in a database and caches +// them in memory, associated with a continuous range of period numbers. type canonicalStore[T any] struct { Range db ethdb.KeyValueStore @@ -422,6 +449,7 @@ type canonicalStore[T any] struct { decode func([]byte) (T, error) } +// newCanonicalStore creates a new canonicalStore. func newCanonicalStore[T any](db ethdb.KeyValueStore, keyPrefix []byte, encode func(T) ([]byte, error), decode func([]byte) (T, error)) *canonicalStore[T] { cs := &canonicalStore[T]{ @@ -451,6 +479,7 @@ func newCanonicalStore[T any](db ethdb.KeyValueStore, keyPrefix []byte, return cs } +// getDbKey returns the database key belonging to the given period. func (cs *canonicalStore[T]) getDbKey(period uint64) []byte { var ( kl = len(cs.keyPrefix) @@ -461,6 +490,8 @@ func (cs *canonicalStore[T]) getDbKey(period uint64) []byte { return key } +// add adds the given item to the database. It also ensures that the range remains +// continuous. Can be used both in batch mode and as a standalone operation. func (cs *canonicalStore[T]) add(batch ethdb.Batch, period uint64, value T) { if !cs.CanExpand(period) { log.Error("Cannot expand canonical store", "range.first", cs.First, "range.afterLast", cs.AfterLast, "new period", period) @@ -484,7 +515,8 @@ func (cs *canonicalStore[T]) add(batch ethdb.Batch, period uint64, value T) { cs.Expand(period) } -// should only be used in batch mode +// deleteFrom removes items starting from the given period. Can only be used in +// batch mode. func (cs *canonicalStore[T]) deleteFrom(batch ethdb.Batch, fromPeriod uint64) (deleted Range) { if fromPeriod >= cs.AfterLast { return @@ -505,6 +537,8 @@ func (cs *canonicalStore[T]) deleteFrom(batch ethdb.Batch, fromPeriod uint64) (d return } +// 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(period uint64) T { if value, ok := cs.cache.Get(period); ok { return value diff --git a/beacon/light/committee_chain_test.go b/beacon/light/committee_chain_test.go index ac073572e2d2..2cd5886866a0 100644 --- a/beacon/light/committee_chain_test.go +++ b/beacon/light/committee_chain_test.go @@ -17,7 +17,7 @@ package light import ( - "math/rand" + "crypto/rand" "testing" "time" diff --git a/beacon/light/range.go b/beacon/light/range.go index ee01eb550ac7..b4893bf515b2 100644 --- a/beacon/light/range.go +++ b/beacon/light/range.go @@ -16,23 +16,29 @@ package light +// Range represents a (possibly zero-length) range of integers (sync periods). type Range struct { First uint64 AfterLast uint64 } +// IsEmpty returns true if the length of the range is zero. func (a Range) IsEmpty() bool { return a.AfterLast == a.First } +// Includes returns true if the range includes the given period. func (a Range) Includes(period uint64) bool { return period >= a.First && period < a.AfterLast } +// CanExpand returns true if the range can be expanded with the given period +// (either the range is empty or the new period is right before or after the range). func (a Range) CanExpand(period uint64) bool { return a.IsEmpty() || (period+1 >= a.First && period <= a.AfterLast) } +// Expand expands the range with the given period (assumes that CanExpand returned true). func (a *Range) Expand(period uint64) { if a.IsEmpty() { a.First, a.AfterLast = period, period+1 diff --git a/beacon/light/test_helpers.go b/beacon/light/test_helpers.go index 7c77d640f2f1..bfecde591a20 100644 --- a/beacon/light/test_helpers.go +++ b/beacon/light/test_helpers.go @@ -17,8 +17,9 @@ package light import ( + "crypto/rand" "crypto/sha256" - "math/rand" + mrand "math/rand" "github.com/ethereum/go-ethereum/beacon/merkle" "github.com/ethereum/go-ethereum/beacon/params" @@ -73,7 +74,7 @@ func GenerateTestCheckpoint(period uint64, committee *types.SerializedSyncCommit func makeBitmask(signerCount int) (bitmask [params.SyncCommitteeBitmaskSize]byte) { for i := 0; i < params.SyncCommitteeSize; i++ { - if rand.Intn(params.SyncCommitteeSize-i) < signerCount { + if mrand.Intn(params.SyncCommitteeSize-i) < signerCount { bitmask[i/8] += byte(1) << (i & 7) signerCount-- } From 6eb5767b707b2c12448fb327144517ce9b8976ff Mon Sep 17 00:00:00 2001 From: Zsolt Felfoldi Date: Sun, 3 Sep 2023 09:07:52 +0200 Subject: [PATCH 03/19] beacon/light: comments and various minor changes --- beacon/light/checkpoint.go | 2 +- beacon/light/committee_chain.go | 274 ++++++++++++++++++-------------- beacon/light/range.go | 5 +- core/rawdb/schema.go | 4 + 4 files changed, 161 insertions(+), 124 deletions(-) diff --git a/beacon/light/checkpoint.go b/beacon/light/checkpoint.go index 25ec7fcc2b3e..cafe3d576ddb 100644 --- a/beacon/light/checkpoint.go +++ b/beacon/light/checkpoint.go @@ -49,7 +49,7 @@ func (c *CheckpointData) Validate() error { func (c *CheckpointData) InitChain(chain *CommitteeChain) { must := func(err error) { if err != nil { - log.Crit("Error initializing committee chain with checkpoint", "error", err) + log.Error("Error initializing committee chain with checkpoint", "error", err) } } period := c.Header.SyncPeriod() diff --git a/beacon/light/committee_chain.go b/beacon/light/committee_chain.go index 929be66156eb..cd87d7407880 100644 --- a/beacon/light/committee_chain.go +++ b/beacon/light/committee_chain.go @@ -19,6 +19,7 @@ package light import ( "encoding/binary" "errors" + "math" "sync" "time" @@ -27,13 +28,13 @@ import ( "github.com/ethereum/go-ethereum/common" "github.com/ethereum/go-ethereum/common/lru" "github.com/ethereum/go-ethereum/common/mclock" + "github.com/ethereum/go-ethereum/core/rawdb" "github.com/ethereum/go-ethereum/ethdb" "github.com/ethereum/go-ethereum/log" "github.com/ethereum/go-ethereum/rlp" ) var ( - ErrNotInitialized = errors.New("sync committee chain not initialized") ErrNeedCommittee = errors.New("sync committee required") ErrInvalidUpdate = errors.New("invalid committee update") ErrInvalidPeriod = errors.New("invalid update period") @@ -41,12 +42,6 @@ var ( ErrCannotReorg = errors.New("can not reorg committee chain") ) -var ( - bestUpdateKey = []byte("update-") // bigEndian64(syncPeriod) -> RLP(types.LightClientUpdate) (nextCommittee only referenced by root hash) - fixedRootKey = []byte("fixedRoot-") // bigEndian64(syncPeriod) -> committee root hash - syncCommitteeKey = []byte("committee-") // bigEndian64(syncPeriod) -> serialized committee -) - // CommitteeChain is a passive data structure that can validate, hold and update // a chain of beacon light sync committees and updates. It requires at least one // externally set fixed committee root at the beginning of the chain which can @@ -67,15 +62,16 @@ var ( // Once synced to the current sync period, CommitteeChain can also validate // signed beacon headers. type CommitteeChain struct { - lock sync.RWMutex + chainmu sync.RWMutex // locks database, cache and canonicalStore access db ethdb.KeyValueStore - sigVerifier committeeSigVerifier - clock mclock.Clock updates *canonicalStore[*types.LightClientUpdate] committees *canonicalStore[*types.SerializedSyncCommittee] fixedRoots *canonicalStore[common.Hash] syncCommitteeCache *lru.Cache[uint64, syncCommittee] // cache deserialized committees - unixNano func() int64 + + clock mclock.Clock // monotonic clock (simulated clock in tests) + unixNano func() int64 // system clock (simulated clock in tests) + sigVerifier committeeSigVerifier // BLS sig verifier (dummy verifier in tests) config *types.ChainConfig signerThreshold int @@ -86,17 +82,15 @@ type CommitteeChain struct { // NewCommitteeChain creates a new CommitteeChain. func NewCommitteeChain(db ethdb.KeyValueStore, config *types.ChainConfig, signerThreshold int, enforceTime bool, sigVerifier committeeSigVerifier, clock mclock.Clock, unixNano func() int64) *CommitteeChain { s := &CommitteeChain{ - fixedRoots: newCanonicalStore[common.Hash](db, fixedRootKey, func(root common.Hash) ([]byte, error) { + fixedRoots: newCanonicalStore[common.Hash](db, rawdb.FixedRootKey, func(root common.Hash) ([]byte, error) { return root[:], nil }, func(enc []byte) (root common.Hash, err error) { - if len(enc) == len(root) { - copy(root[:], enc) - } else { - err = errors.New("Incorrect length for committee root entry in the database") + if len(enc) != common.HashLength { + return common.Hash{}, errors.New("incorrect length for committee root entry in the database") } - return + return common.BytesToHash(enc), nil }), - committees: newCanonicalStore[*types.SerializedSyncCommittee](db, syncCommitteeKey, func(committee *types.SerializedSyncCommittee) ([]byte, error) { + committees: newCanonicalStore[*types.SerializedSyncCommittee](db, rawdb.SyncCommitteeKey, func(committee *types.SerializedSyncCommittee) ([]byte, error) { return committee[:], nil }, func(enc []byte) (*types.SerializedSyncCommittee, error) { if len(enc) == types.SerializedSyncCommitteeSize { @@ -104,9 +98,9 @@ func NewCommitteeChain(db ethdb.KeyValueStore, config *types.ChainConfig, signer copy(committee[:], enc) return committee, nil } - return nil, errors.New("Incorrect length for serialized committee entry in the database") + return nil, errors.New("incorrect length for serialized committee entry in the database") }), - updates: newCanonicalStore[*types.LightClientUpdate](db, bestUpdateKey, func(update *types.LightClientUpdate) ([]byte, error) { + updates: newCanonicalStore[*types.LightClientUpdate](db, rawdb.BestUpdateKey, func(update *types.LightClientUpdate) ([]byte, error) { return rlp.EncodeToBytes(update) }, func(enc []byte) (*types.LightClientUpdate, error) { update := new(types.LightClientUpdate) @@ -130,38 +124,38 @@ func NewCommitteeChain(db ethdb.KeyValueStore, config *types.ChainConfig, signer } // check validity constraints - if !s.updates.IsEmpty() { - if s.fixedRoots.IsEmpty() || s.updates.First < s.fixedRoots.First || - s.updates.First >= s.fixedRoots.AfterLast { - log.Crit("Inconsistent database error: first update is not in the fixed roots range") + if !s.updates.periods.IsEmpty() { + if s.fixedRoots.periods.IsEmpty() || s.updates.periods.First < s.fixedRoots.periods.First || + s.updates.periods.First >= s.fixedRoots.periods.AfterLast { + log.Error("Inconsistent database error: first update is not in the fixed roots range") } - if s.committees.First > s.updates.First || s.committees.AfterLast <= s.updates.AfterLast { - log.Crit("Inconsistent database error: missing committees in update range") + if s.committees.periods.First > s.updates.periods.First || s.committees.periods.AfterLast <= s.updates.periods.AfterLast { + log.Error("Inconsistent database error: missing committees in update range") } } - if !s.committees.IsEmpty() { - if s.fixedRoots.IsEmpty() || s.committees.First < s.fixedRoots.First || - s.committees.First >= s.fixedRoots.AfterLast { - log.Crit("Inconsistent database error: first committee is not in the fixed roots range") + if !s.committees.periods.IsEmpty() { + if s.fixedRoots.periods.IsEmpty() || s.committees.periods.First < s.fixedRoots.periods.First || + s.committees.periods.First >= s.fixedRoots.periods.AfterLast { + log.Error("Inconsistent database error: first committee is not in the fixed roots range") } - if s.committees.AfterLast > s.fixedRoots.AfterLast && s.committees.AfterLast > s.updates.AfterLast+1 { - log.Crit("Inconsistent database error: last committee is neither in the fixed roots range nor proven by updates") + if s.committees.periods.AfterLast > s.fixedRoots.periods.AfterLast && s.committees.periods.AfterLast > s.updates.periods.AfterLast+1 { + log.Error("Inconsistent database error: last committee is neither in the fixed roots range nor proven by updates") } - log.Trace("Sync committee chain loaded", "first period", s.committees.First, "last period", s.committees.AfterLast-1) + log.Trace("Sync committee chain loaded", "first period", s.committees.periods.First, "last period", s.committees.periods.AfterLast-1) } // roll back invalid updates (might be necessary if forks have been changed since last time) var batch ethdb.Batch - for !s.updates.IsEmpty() { - if update := s.updates.get(s.updates.AfterLast - 1); update == nil || s.verifyUpdate(update) { + for !s.updates.periods.IsEmpty() { + if update, ok := s.updates.get(s.updates.periods.AfterLast - 1); !ok || s.verifyUpdate(update) { if update == nil { - log.Crit("Sync committee update missing", "period", s.updates.AfterLast-1) + log.Error("Sync committee update missing", "period", s.updates.periods.AfterLast-1) } break } if batch == nil { batch = s.db.NewBatch() } - s.rollback(batch, s.updates.AfterLast) + s.rollback(batch, s.updates.periods.AfterLast) } if batch != nil { if err := batch.Write(); err != nil { @@ -173,8 +167,8 @@ func NewCommitteeChain(db ethdb.KeyValueStore, config *types.ChainConfig, signer // Reset resets the committee chain. func (s *CommitteeChain) Reset() { - s.lock.Lock() - defer s.lock.Unlock() + s.chainmu.Lock() + defer s.chainmu.Unlock() batch := s.db.NewBatch() s.rollback(batch, 0) @@ -187,24 +181,40 @@ func (s *CommitteeChain) Reset() { // Note that the period where the first committee is added has to have a fixed // root which can either come from a CheckpointData or a trusted source. func (s *CommitteeChain) AddFixedRoot(period uint64, root common.Hash) error { - s.lock.Lock() - defer s.lock.Unlock() + s.chainmu.Lock() + defer s.chainmu.Unlock() batch := s.db.NewBatch() oldRoot := s.getCommitteeRoot(period) - if !s.fixedRoots.CanExpand(period) { + if !s.fixedRoots.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 + // case when a root that is not adjacent to the already fixed ones can be + // fixed is when the same root has already been proven by an update chain. + // In this case the all roots in between can and should be fixed. + // This scenario makes sense when a new trusted checkpoint is added to an + // existing chain, ensuring that it will not be rolled back (might be + // important in case of low signer participation rate). if root != oldRoot { return ErrInvalidPeriod } - for p := s.fixedRoots.AfterLast; p <= period; p++ { - s.fixedRoots.add(batch, p, s.getCommitteeRoot(p)) + // if the old root exists and matches the new one then it is guaranteed + // that the given period is after the existing fixed range and the roots + // in between can also be fixed. + for p := s.fixedRoots.periods.AfterLast; p < period; p++ { + if err := s.fixedRoots.add(batch, p, s.getCommitteeRoot(p)); err != nil { + return err + } } } if oldRoot != (common.Hash{}) && (oldRoot != root) { // existing old root was different, we have to reorg the chain s.rollback(batch, period) } - s.fixedRoots.add(batch, period, root) + if err := s.fixedRoots.add(batch, period, root); err != nil { + return err + } if err := batch.Write(); err != nil { log.Error("Error writing batch into chain database", "error", err) return err @@ -216,21 +226,29 @@ func (s *CommitteeChain) AddFixedRoot(period uint64, root common.Hash) error { // It also maintains chain consistency, meaning that it also deletes updates and // committees if they are no longer supported by a valid update chain. func (s *CommitteeChain) DeleteFixedRootsFrom(period uint64) error { - s.lock.Lock() - defer s.lock.Unlock() + s.chainmu.Lock() + defer s.chainmu.Unlock() - if period >= s.fixedRoots.AfterLast { + if period >= s.fixedRoots.periods.AfterLast { return nil } batch := s.db.NewBatch() s.fixedRoots.deleteFrom(batch, period) - if s.updates.IsEmpty() || period <= s.updates.First { + if s.updates.periods.IsEmpty() || period <= s.updates.periods.First { + // 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 + // remaining fixed root range can stay. s.updates.deleteFrom(batch, period) s.deleteCommitteesFrom(batch, period) } else { - fromPeriod := s.updates.AfterLast + 1 + // The update chain stays intact, some previously fixed committee roots might + // get unfixed but are still proven by the update chain. If there were + // committees present after the range proven by updates, those should be + // removed if the belonging fixed roots are also removed. + fromPeriod := s.updates.periods.AfterLast + 1 // not proven by updates if period > fromPeriod { - fromPeriod = period + fromPeriod = period // also not justified by fixed roots } s.deleteCommitteesFrom(batch, fromPeriod) } @@ -250,16 +268,18 @@ func (s *CommitteeChain) deleteCommitteesFrom(batch ethdb.Batch, period uint64) } // GetCommittee returns the committee at the given period. +// Note: GetCommittee can be called either with locked or unlocked chain mutex. func (s *CommitteeChain) GetCommittee(period uint64) *types.SerializedSyncCommittee { - return s.committees.get(period) + committee, _ := s.committees.get(period) + return committee } // AddCommittee adds a committee at the given period if possible. func (s *CommitteeChain) AddCommittee(period uint64, committee *types.SerializedSyncCommittee) error { - s.lock.Lock() - defer s.lock.Unlock() + s.chainmu.Lock() + defer s.chainmu.Unlock() - if !s.committees.CanExpand(period) { + if !s.committees.periods.CanExpand(period) { return ErrInvalidPeriod } root := s.getCommitteeRoot(period) @@ -269,25 +289,29 @@ func (s *CommitteeChain) AddCommittee(period uint64, committee *types.Serialized if root != committee.Root() { return ErrWrongCommitteeRoot } - if !s.committees.Includes(period) { - s.committees.add(nil, period, committee) + if !s.committees.periods.Includes(period) { + if err := s.committees.add(s.db, period, committee); err != nil { + return err + } s.syncCommitteeCache.Remove(period) } return nil } // GetUpdate returns the update at the given period. +// Note: GetUpdate can be called either with locked or unlocked chain mutex. func (s *CommitteeChain) GetUpdate(period uint64) *types.LightClientUpdate { - return s.updates.get(period) + update, _ := s.updates.get(period) + return update } // InsertUpdate adds a new update if possible. func (s *CommitteeChain) InsertUpdate(update *types.LightClientUpdate, nextCommittee *types.SerializedSyncCommittee) error { - s.lock.Lock() - defer s.lock.Unlock() + s.chainmu.Lock() + defer s.chainmu.Unlock() period := update.AttestedHeader.Header.SyncPeriod() - if !s.updates.CanExpand(period) || !s.committees.Includes(period) { + if !s.updates.periods.CanExpand(period) || !s.committees.periods.Includes(period) { return ErrInvalidPeriod } if s.minimumUpdateScore.BetterThan(update.Score()) { @@ -295,20 +319,20 @@ func (s *CommitteeChain) InsertUpdate(update *types.LightClientUpdate, nextCommi } oldRoot := s.getCommitteeRoot(period + 1) reorg := oldRoot != (common.Hash{}) && oldRoot != update.NextSyncCommitteeRoot - if oldUpdate := s.updates.get(period); oldUpdate != nil && !update.Score().BetterThan(oldUpdate.Score()) { + if oldUpdate, ok := s.updates.get(period); ok && !update.Score().BetterThan(oldUpdate.Score()) { // a better or equal update already exists; no changes, only fail if new one tried to reorg if reorg { return ErrCannotReorg } return nil } - if s.fixedRoots.Includes(period+1) && reorg { + if s.fixedRoots.periods.Includes(period+1) && reorg { return ErrCannotReorg } if !s.verifyUpdate(update) { return ErrInvalidUpdate } - addCommittee := !s.committees.Includes(period+1) || reorg + addCommittee := !s.committees.periods.Includes(period+1) || reorg if addCommittee { if nextCommittee == nil { return ErrNeedCommittee @@ -322,10 +346,14 @@ func (s *CommitteeChain) InsertUpdate(update *types.LightClientUpdate, nextCommi s.rollback(batch, period+1) } if addCommittee { - s.committees.add(batch, period+1, nextCommittee) + if err := s.committees.add(batch, period+1, nextCommittee); err != nil { + return err + } s.syncCommitteeCache.Remove(period + 1) } - s.updates.add(batch, period, update) + if err := s.updates.add(batch, period, update); err != nil { + return err + } if err := batch.Write(); err != nil { log.Error("Error writing batch into chain database", "error", err) return err @@ -337,16 +365,16 @@ func (s *CommitteeChain) InsertUpdate(update *types.LightClientUpdate, nextCommi // NextSyncPeriod returns the next period where an update can be added and also // whether the chain is initialized at all. func (s *CommitteeChain) NextSyncPeriod() (uint64, bool) { - s.lock.RLock() - defer s.lock.RUnlock() + s.chainmu.RLock() + defer s.chainmu.RUnlock() - if s.committees.IsEmpty() { + if s.committees.periods.IsEmpty() { return 0, false } - if !s.updates.IsEmpty() { - return s.updates.AfterLast, true + if !s.updates.periods.IsEmpty() { + return s.updates.periods.AfterLast, true } - return s.committees.AfterLast - 1, true + return s.committees.periods.AfterLast - 1, true } // rollback removes all committees and fixed roots from the given period and updates @@ -364,10 +392,10 @@ func (s *CommitteeChain) rollback(batch ethdb.Batch, period uint64) { // proven by a previous update or both. It returns an empty hash if the committee // root is unknown. func (s *CommitteeChain) getCommitteeRoot(period uint64) common.Hash { - if root := s.fixedRoots.get(period); root != (common.Hash{}) || period == 0 { + if root, ok := s.fixedRoots.get(period); ok || period == 0 { return root } - if update := s.updates.get(period - 1); update != nil { + if update, ok := s.updates.get(period - 1); ok { return update.NextSyncCommitteeRoot } return common.Hash{} @@ -378,7 +406,7 @@ func (s *CommitteeChain) getSyncCommittee(period uint64) syncCommittee { if c, ok := s.syncCommitteeCache.Get(period); ok { return c } - if sc := s.committees.get(period); sc != nil { + if sc, ok := s.committees.get(period); ok { c, err := s.sigVerifier.deserializeSyncCommittee(sc) if err != nil { log.Error("Sync committee deserialization error", "error", err) @@ -399,17 +427,20 @@ func (s *CommitteeChain) getSyncCommittee(period uint64) syncCommittee { // of the given slot, according to the local system clock). If enforceTime is // true then negative age (future) headers are rejected. func (s *CommitteeChain) VerifySignedHeader(head types.SignedHeader) (bool, time.Duration) { - s.lock.RLock() - defer s.lock.RUnlock() + s.chainmu.RLock() + defer s.chainmu.RUnlock() return s.verifySignedHeader(head) } func (s *CommitteeChain) verifySignedHeader(head types.SignedHeader) (bool, time.Duration) { - var ( - slotTime = int64(time.Second) * int64(s.config.GenesisTime+head.Header.Slot*12) - age = time.Duration(s.unixNano() - slotTime) - ) + var age time.Duration + now := s.unixNano() + if head.Header.Slot < (uint64(now-math.MinInt64)/uint64(time.Second)-s.config.GenesisTime)/12 { + age = time.Duration(now - int64(time.Second)*int64(s.config.GenesisTime+head.Header.Slot*12)) + } else { + age = time.Duration(math.MinInt64) + } if s.enforceTime && age < 0 { return false, age } @@ -441,9 +472,9 @@ func (s *CommitteeChain) verifyUpdate(update *types.LightClientUpdate) bool { // canonicalStore stores instances of the given type in a database and caches // them in memory, associated with a continuous range of period numbers. type canonicalStore[T any] struct { - Range db ethdb.KeyValueStore keyPrefix []byte + periods Range cache *lru.Cache[uint64, T] encode func(T) ([]byte, error) decode func([]byte) (T, error) @@ -464,23 +495,27 @@ func newCanonicalStore[T any](db ethdb.KeyValueStore, keyPrefix []byte, kl = len(keyPrefix) ) for iter.Next() { + if len(iter.Key()) != kl+8 { + log.Error("Invalid key length in the canonical chain database") + continue + } period := binary.BigEndian.Uint64(iter.Key()[kl : kl+8]) - if cs.First == 0 { - cs.First = period - } else if cs.AfterLast != period { + if cs.periods.First == 0 { + cs.periods.First = period + } else if cs.periods.AfterLast != period { if iter.Next() { log.Error("Gap in the canonical chain database") } break // continuity guaranteed } - cs.AfterLast = period + 1 + cs.periods.AfterLast = period + 1 } iter.Release() return cs } -// getDbKey returns the database key belonging to the given period. -func (cs *canonicalStore[T]) getDbKey(period uint64) []byte { +// 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) @@ -491,65 +526,62 @@ func (cs *canonicalStore[T]) getDbKey(period uint64) []byte { } // add adds the given item to the database. It also ensures that the range remains -// continuous. Can be used both in batch mode and as a standalone operation. -func (cs *canonicalStore[T]) add(batch ethdb.Batch, period uint64, value T) { - if !cs.CanExpand(period) { - log.Error("Cannot expand canonical store", "range.first", cs.First, "range.afterLast", cs.AfterLast, "new period", period) - return +// 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) { + log.Error("Cannot expand canonical store", "range.first", cs.periods.First, "range.afterLast", cs.periods.AfterLast, "new period", period) + return errors.New("Cannot expand canonical store") } enc, err := cs.encode(value) if err != nil { log.Error("Error encoding canonical store value", "error", err) - return - } - key := cs.getDbKey(period) - if batch != nil { - err = batch.Put(key, enc) - } else { - err = cs.db.Put(key, enc) + return err } - if err != nil { + if err := backend.Put(cs.databaseKey(period), enc); err != nil { log.Error("Error writing into canonical store value database", "error", err) + return err } cs.cache.Add(period, value) - cs.Expand(period) + cs.periods.Expand(period) + return nil } -// deleteFrom removes items starting from the given period. Can only be used in -// batch mode. -func (cs *canonicalStore[T]) deleteFrom(batch ethdb.Batch, fromPeriod uint64) (deleted Range) { - if fromPeriod >= cs.AfterLast { +// deleteFrom removes items starting from the given period. Should be used with a +// batch backend. +func (cs *canonicalStore[T]) deleteFrom(backend ethdb.KeyValueWriter, fromPeriod uint64) (deleted Range) { + if fromPeriod >= cs.periods.AfterLast { return } - if fromPeriod < cs.First { - fromPeriod = cs.First + if fromPeriod < cs.periods.First { + fromPeriod = cs.periods.First } - deleted = Range{First: fromPeriod, AfterLast: cs.AfterLast} - for period := fromPeriod; period < cs.AfterLast; period++ { - batch.Delete(cs.getDbKey(period)) + deleted = Range{First: fromPeriod, AfterLast: cs.periods.AfterLast} + for period := fromPeriod; period < cs.periods.AfterLast; period++ { + backend.Delete(cs.databaseKey(period)) cs.cache.Remove(period) } - if fromPeriod > cs.First { - cs.AfterLast = fromPeriod + if fromPeriod > cs.periods.First { + cs.periods.AfterLast = fromPeriod } else { - cs.Range = Range{} + cs.periods = Range{} } return } // 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(period uint64) T { - if value, ok := cs.cache.Get(period); ok { - return value +// Note: get is thread safe in itself and therefore can be called either with +// locked or unlocked chain mutex. +func (cs *canonicalStore[T]) get(period uint64) (value T, ok bool) { + if value, ok = cs.cache.Get(period); ok { + return } - var value T - if enc, err := cs.db.Get(cs.getDbKey(period)); err == nil { + if enc, err := cs.db.Get(cs.databaseKey(period)); err == nil { if v, err := cs.decode(enc); err == nil { - value = v + value, ok = v, true } else { log.Error("Error decoding canonical store value", "error", err) } } - return value + return } diff --git a/beacon/light/range.go b/beacon/light/range.go index b4893bf515b2..2d58a6a51329 100644 --- a/beacon/light/range.go +++ b/beacon/light/range.go @@ -32,8 +32,9 @@ func (a Range) Includes(period uint64) bool { return period >= a.First && period < a.AfterLast } -// CanExpand returns true if the range can be expanded with the given period -// (either the range is empty or the new period is right before or after the range). +// 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.First && period <= a.AfterLast) } diff --git a/core/rawdb/schema.go b/core/rawdb/schema.go index 8e82459e8225..46cc7ecef2bd 100644 --- a/core/rawdb/schema.go +++ b/core/rawdb/schema.go @@ -134,6 +134,10 @@ var ( preimageCounter = metrics.NewRegisteredCounter("db/preimage/total", nil) preimageHitCounter = metrics.NewRegisteredCounter("db/preimage/hits", nil) + + BestUpdateKey = []byte("update-") // bigEndian64(syncPeriod) -> RLP(types.LightClientUpdate) (nextCommittee only referenced by root hash) + FixedRootKey = []byte("fixedRoot-") // bigEndian64(syncPeriod) -> committee root hash + SyncCommitteeKey = []byte("committee-") // bigEndian64(syncPeriod) -> serialized committee ) // LegacyTxLookupEntry is the legacy TxLookupEntry definition with some unnecessary From 46bdc031aff7fc77e3df47e7154adb51710f50ef Mon Sep 17 00:00:00 2001 From: Zsolt Felfoldi Date: Fri, 8 Sep 2023 01:26:48 +0200 Subject: [PATCH 04/19] beacon/light: more issues fixed --- beacon/light/committee_chain.go | 109 ++++++++++++++++---------------- 1 file changed, 56 insertions(+), 53 deletions(-) diff --git a/beacon/light/committee_chain.go b/beacon/light/committee_chain.go index cd87d7407880..cfac1daba27e 100644 --- a/beacon/light/committee_chain.go +++ b/beacon/light/committee_chain.go @@ -19,6 +19,7 @@ package light import ( "encoding/binary" "errors" + "fmt" "math" "sync" "time" @@ -62,12 +63,15 @@ var ( // Once synced to the current sync period, CommitteeChain can also validate // signed beacon headers. type CommitteeChain struct { - chainmu sync.RWMutex // locks database, cache and canonicalStore access - db ethdb.KeyValueStore - updates *canonicalStore[*types.LightClientUpdate] - committees *canonicalStore[*types.SerializedSyncCommittee] - fixedRoots *canonicalStore[common.Hash] - syncCommitteeCache *lru.Cache[uint64, syncCommittee] // cache deserialized committees + chainmu sync.RWMutex + // Note: chainmu avoids concurrent access to the canonicalStore structures + // (updates, committees, fixedRoots) and ensures that they stay consistent + // with each other and with committeeCache. + db ethdb.KeyValueStore + updates *canonicalStore[*types.LightClientUpdate] + committees *canonicalStore[*types.SerializedSyncCommittee] + fixedRoots *canonicalStore[common.Hash] + committeeCache *lru.Cache[uint64, syncCommittee] // cache deserialized committees clock mclock.Clock // monotonic clock (simulated clock in tests) unixNano func() int64 // system clock (simulated clock in tests) @@ -81,42 +85,50 @@ type CommitteeChain struct { // NewCommitteeChain creates a new CommitteeChain. func NewCommitteeChain(db ethdb.KeyValueStore, config *types.ChainConfig, signerThreshold int, enforceTime bool, sigVerifier committeeSigVerifier, clock mclock.Clock, unixNano func() int64) *CommitteeChain { - s := &CommitteeChain{ - fixedRoots: newCanonicalStore[common.Hash](db, rawdb.FixedRootKey, func(root common.Hash) ([]byte, error) { + var ( + fixedRootEncoder = func(root common.Hash) ([]byte, error) { return root[:], nil - }, func(enc []byte) (root common.Hash, err error) { + } + fixedRootDecoder = func(enc []byte) (root common.Hash, err error) { if len(enc) != common.HashLength { return common.Hash{}, errors.New("incorrect length for committee root entry in the database") } return common.BytesToHash(enc), nil - }), - committees: newCanonicalStore[*types.SerializedSyncCommittee](db, rawdb.SyncCommitteeKey, func(committee *types.SerializedSyncCommittee) ([]byte, error) { + } + committeeEncoder = func(committee *types.SerializedSyncCommittee) ([]byte, error) { return committee[:], nil - }, func(enc []byte) (*types.SerializedSyncCommittee, error) { + } + committeeDecoder = func(enc []byte) (*types.SerializedSyncCommittee, error) { if len(enc) == types.SerializedSyncCommitteeSize { committee := new(types.SerializedSyncCommittee) copy(committee[:], enc) return committee, nil } return nil, errors.New("incorrect length for serialized committee entry in the database") - }), - updates: newCanonicalStore[*types.LightClientUpdate](db, rawdb.BestUpdateKey, func(update *types.LightClientUpdate) ([]byte, error) { + } + updateEncoder = func(update *types.LightClientUpdate) ([]byte, error) { return rlp.EncodeToBytes(update) - }, func(enc []byte) (*types.LightClientUpdate, error) { + } + updateDecoder = func(enc []byte) (*types.LightClientUpdate, error) { update := new(types.LightClientUpdate) if err := rlp.DecodeBytes(enc, update); err != nil { return nil, err } return update, nil - }), - syncCommitteeCache: lru.NewCache[uint64, syncCommittee](10), - db: db, - sigVerifier: sigVerifier, - clock: clock, - unixNano: unixNano, - config: config, - signerThreshold: signerThreshold, - enforceTime: enforceTime, + } + ) + s := &CommitteeChain{ + fixedRoots: newCanonicalStore[common.Hash](db, rawdb.FixedRootKey, fixedRootEncoder, fixedRootDecoder), + 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, minimumUpdateScore: types.UpdateScore{ SignerCount: uint32(signerThreshold), SubPeriodIndex: params.SyncPeriodLength / 16, @@ -127,24 +139,24 @@ func NewCommitteeChain(db ethdb.KeyValueStore, config *types.ChainConfig, signer if !s.updates.periods.IsEmpty() { if s.fixedRoots.periods.IsEmpty() || s.updates.periods.First < s.fixedRoots.periods.First || s.updates.periods.First >= s.fixedRoots.periods.AfterLast { - log.Error("Inconsistent database error: first update is not in the fixed roots range") + log.Error("First update is not in the fixed roots range") } if s.committees.periods.First > s.updates.periods.First || s.committees.periods.AfterLast <= s.updates.periods.AfterLast { - log.Error("Inconsistent database error: missing committees in update range") + log.Error("Missing committees in update range") } } if !s.committees.periods.IsEmpty() { if s.fixedRoots.periods.IsEmpty() || s.committees.periods.First < s.fixedRoots.periods.First || s.committees.periods.First >= s.fixedRoots.periods.AfterLast { - log.Error("Inconsistent database error: first committee is not in the fixed roots range") + log.Error("First committee is not in the fixed roots range") } if s.committees.periods.AfterLast > s.fixedRoots.periods.AfterLast && s.committees.periods.AfterLast > s.updates.periods.AfterLast+1 { - log.Error("Inconsistent database error: last committee is neither in the fixed roots range nor proven by updates") + log.Error("Last committee is neither in the fixed roots range nor proven by updates") } log.Trace("Sync committee chain loaded", "first period", s.committees.periods.First, "last period", s.committees.periods.AfterLast-1) } // roll back invalid updates (might be necessary if forks have been changed since last time) - var batch ethdb.Batch + batch := s.db.NewBatch() for !s.updates.periods.IsEmpty() { if update, ok := s.updates.get(s.updates.periods.AfterLast - 1); !ok || s.verifyUpdate(update) { if update == nil { @@ -152,15 +164,10 @@ func NewCommitteeChain(db ethdb.KeyValueStore, config *types.ChainConfig, signer } break } - if batch == nil { - batch = s.db.NewBatch() - } s.rollback(batch, s.updates.periods.AfterLast) } - if batch != nil { - if err := batch.Write(); err != nil { - log.Error("Error writing batch into chain database", "error", err) - } + if err := batch.Write(); err != nil { + log.Error("Error writing batch into chain database", "error", err) } return s } @@ -263,7 +270,7 @@ func (s *CommitteeChain) DeleteFixedRootsFrom(period uint64) error { func (s *CommitteeChain) deleteCommitteesFrom(batch ethdb.Batch, period uint64) { deleted := s.committees.deleteFrom(batch, period) for period := deleted.First; period < deleted.AfterLast; period++ { - s.syncCommitteeCache.Remove(period) + s.committeeCache.Remove(period) } } @@ -293,7 +300,7 @@ func (s *CommitteeChain) AddCommittee(period uint64, committee *types.Serialized if err := s.committees.add(s.db, period, committee); err != nil { return err } - s.syncCommitteeCache.Remove(period) + s.committeeCache.Remove(period) } return nil } @@ -349,7 +356,7 @@ func (s *CommitteeChain) InsertUpdate(update *types.LightClientUpdate, nextCommi if err := s.committees.add(batch, period+1, nextCommittee); err != nil { return err } - s.syncCommitteeCache.Remove(period + 1) + s.committeeCache.Remove(period + 1) } if err := s.updates.add(batch, period, update); err != nil { return err @@ -403,7 +410,7 @@ func (s *CommitteeChain) getCommitteeRoot(period uint64) common.Hash { // getSyncCommittee returns the deserialized sync committee at the given period. func (s *CommitteeChain) getSyncCommittee(period uint64) syncCommittee { - if c, ok := s.syncCommitteeCache.Get(period); ok { + if c, ok := s.committeeCache.Get(period); ok { return c } if sc, ok := s.committees.get(period); ok { @@ -412,7 +419,7 @@ func (s *CommitteeChain) getSyncCommittee(period uint64) syncCommittee { log.Error("Sync committee deserialization error", "error", err) return nil } - s.syncCommitteeCache.Add(period, c) + s.committeeCache.Add(period, c) return c } log.Error("Missing serialized sync committee", "period", period) @@ -471,6 +478,8 @@ func (s *CommitteeChain) verifyUpdate(update *types.LightClientUpdate) bool { // canonicalStore stores instances of the given type in a database and caches // them in memory, associated with a continuous range of period numbers. +// Note: canonicalStore is not thread safe and it is the caller's responsibility +// to avoid concurrent access. type canonicalStore[T any] struct { db ethdb.KeyValueStore keyPrefix []byte @@ -496,16 +505,14 @@ func newCanonicalStore[T any](db ethdb.KeyValueStore, keyPrefix []byte, ) for iter.Next() { if len(iter.Key()) != kl+8 { - log.Error("Invalid key length in the canonical chain database") + log.Warn("Invalid key length in the canonical chain database", "key", fmt.Sprintf("%#x", iter.Key())) continue } period := binary.BigEndian.Uint64(iter.Key()[kl : kl+8]) if cs.periods.First == 0 { cs.periods.First = period } else if cs.periods.AfterLast != period { - if iter.Next() { - log.Error("Gap in the canonical chain database") - } + log.Warn("Gap in the canonical chain database") break // continuity guaranteed } cs.periods.AfterLast = period + 1 @@ -529,16 +536,13 @@ func (cs *canonicalStore[T]) databaseKey(period uint64) []byte { // 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) { - log.Error("Cannot expand canonical store", "range.first", cs.periods.First, "range.afterLast", cs.periods.AfterLast, "new period", period) - return errors.New("Cannot expand canonical store") + return fmt.Errorf("period expansion is not allowed, first: %d, next: %d, period: %d", cs.periods.First, cs.periods.AfterLast, period) } enc, err := cs.encode(value) if err != nil { - log.Error("Error encoding canonical store value", "error", err) return err } if err := backend.Put(cs.databaseKey(period), enc); err != nil { - log.Error("Error writing into canonical store value database", "error", err) return err } cs.cache.Add(period, value) @@ -546,9 +550,8 @@ func (cs *canonicalStore[T]) add(backend ethdb.KeyValueWriter, period uint64, va return nil } -// deleteFrom removes items starting from the given period. Should be used with a -// batch backend. -func (cs *canonicalStore[T]) deleteFrom(backend ethdb.KeyValueWriter, fromPeriod uint64) (deleted Range) { +// deleteFrom removes items starting from the given period. +func (cs *canonicalStore[T]) deleteFrom(batch ethdb.Batch, fromPeriod uint64) (deleted Range) { if fromPeriod >= cs.periods.AfterLast { return } @@ -557,7 +560,7 @@ func (cs *canonicalStore[T]) deleteFrom(backend ethdb.KeyValueWriter, fromPeriod } deleted = Range{First: fromPeriod, AfterLast: cs.periods.AfterLast} for period := fromPeriod; period < cs.periods.AfterLast; period++ { - backend.Delete(cs.databaseKey(period)) + batch.Delete(cs.databaseKey(period)) cs.cache.Remove(period) } if fromPeriod > cs.periods.First { From b039c2c1c07d5f0b5e4a4a9bfc83d730a9c12348 Mon Sep 17 00:00:00 2001 From: Zsolt Felfoldi Date: Fri, 8 Sep 2023 10:33:40 +0200 Subject: [PATCH 05/19] beacon/light: reset invalid chain --- beacon/light/committee_chain.go | 49 +++++++++++++++++++++------------ 1 file changed, 32 insertions(+), 17 deletions(-) diff --git a/beacon/light/committee_chain.go b/beacon/light/committee_chain.go index cfac1daba27e..685c96f346ad 100644 --- a/beacon/light/committee_chain.go +++ b/beacon/light/committee_chain.go @@ -135,41 +135,56 @@ func NewCommitteeChain(db ethdb.KeyValueStore, config *types.ChainConfig, signer }, } - // check validity constraints + if !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) + batch := s.db.NewBatch() + for !s.updates.periods.IsEmpty() { + if update, ok := s.updates.get(s.updates.periods.AfterLast - 1); !ok || s.verifyUpdate(update) { + if update == nil { + log.Error("Sync committee update missing", "period", s.updates.periods.AfterLast-1) + } + break + } + s.rollback(batch, s.updates.periods.AfterLast) + } + if err := batch.Write(); err != nil { + log.Error("Error writing batch into chain database", "error", err) + } + if !s.committees.periods.IsEmpty() { + log.Trace("Sync committee chain loaded", "first period", s.committees.periods.First, "last period", s.committees.periods.AfterLast-1) + } + return s +} + +// checkConstraints checks committee chain validity constraints +func (s *CommitteeChain) checkConstraints() bool { + valid := true if !s.updates.periods.IsEmpty() { if s.fixedRoots.periods.IsEmpty() || s.updates.periods.First < s.fixedRoots.periods.First || s.updates.periods.First >= s.fixedRoots.periods.AfterLast { log.Error("First update is not in the fixed roots range") + valid = false } if s.committees.periods.First > s.updates.periods.First || s.committees.periods.AfterLast <= s.updates.periods.AfterLast { log.Error("Missing committees in update range") + valid = false } } if !s.committees.periods.IsEmpty() { if s.fixedRoots.periods.IsEmpty() || s.committees.periods.First < s.fixedRoots.periods.First || s.committees.periods.First >= s.fixedRoots.periods.AfterLast { log.Error("First committee is not in the fixed roots range") + valid = false } if s.committees.periods.AfterLast > s.fixedRoots.periods.AfterLast && s.committees.periods.AfterLast > s.updates.periods.AfterLast+1 { log.Error("Last committee is neither in the fixed roots range nor proven by updates") + valid = false } - log.Trace("Sync committee chain loaded", "first period", s.committees.periods.First, "last period", s.committees.periods.AfterLast-1) - } - // roll back invalid updates (might be necessary if forks have been changed since last time) - batch := s.db.NewBatch() - for !s.updates.periods.IsEmpty() { - if update, ok := s.updates.get(s.updates.periods.AfterLast - 1); !ok || s.verifyUpdate(update) { - if update == nil { - log.Error("Sync committee update missing", "period", s.updates.periods.AfterLast-1) - } - break - } - s.rollback(batch, s.updates.periods.AfterLast) } - if err := batch.Write(); err != nil { - log.Error("Error writing batch into chain database", "error", err) - } - return s + return valid } // Reset resets the committee chain. From 2776aefab0c97f83013e3281801fe7d783f7456f Mon Sep 17 00:00:00 2001 From: Zsolt Felfoldi Date: Fri, 8 Sep 2023 13:30:03 +0200 Subject: [PATCH 06/19] beacon/light: rollback slot-by-slot instead of in a single batch --- beacon/light/committee_chain.go | 53 +++++++++++++++++++++------------ 1 file changed, 34 insertions(+), 19 deletions(-) diff --git a/beacon/light/committee_chain.go b/beacon/light/committee_chain.go index 685c96f346ad..170fdc199fc7 100644 --- a/beacon/light/committee_chain.go +++ b/beacon/light/committee_chain.go @@ -63,10 +63,10 @@ var ( // Once synced to the current sync period, CommitteeChain can also validate // signed beacon headers. type CommitteeChain struct { - chainmu sync.RWMutex - // Note: chainmu avoids concurrent access to the canonicalStore structures + // chainmu guards against concurrent access to the canonicalStore structures // (updates, committees, fixedRoots) and ensures that they stay consistent // with each other and with committeeCache. + chainmu sync.RWMutex db ethdb.KeyValueStore updates *canonicalStore[*types.LightClientUpdate] committees *canonicalStore[*types.SerializedSyncCommittee] @@ -140,7 +140,6 @@ func NewCommitteeChain(db ethdb.KeyValueStore, config *types.ChainConfig, signer s.Reset() } // roll back invalid updates (might be necessary if forks have been changed since last time) - batch := s.db.NewBatch() for !s.updates.periods.IsEmpty() { if update, ok := s.updates.get(s.updates.periods.AfterLast - 1); !ok || s.verifyUpdate(update) { if update == nil { @@ -148,10 +147,9 @@ func NewCommitteeChain(db ethdb.KeyValueStore, config *types.ChainConfig, signer } break } - s.rollback(batch, s.updates.periods.AfterLast) - } - if err := batch.Write(); err != nil { - log.Error("Error writing batch into chain database", "error", err) + if err := s.rollback(s.updates.periods.AfterLast); err != nil { + log.Error("Error writing batch into chain database", "error", err) + } } if !s.committees.periods.IsEmpty() { log.Trace("Sync committee chain loaded", "first period", s.committees.periods.First, "last period", s.committees.periods.AfterLast-1) @@ -192,9 +190,7 @@ func (s *CommitteeChain) Reset() { s.chainmu.Lock() defer s.chainmu.Unlock() - batch := s.db.NewBatch() - s.rollback(batch, 0) - if err := batch.Write(); err != nil { + if err := s.rollback(0); err != nil { log.Error("Error writing batch into chain database", "error", err) } } @@ -232,7 +228,9 @@ func (s *CommitteeChain) AddFixedRoot(period uint64, root common.Hash) error { } if oldRoot != (common.Hash{}) && (oldRoot != root) { // existing old root was different, we have to reorg the chain - s.rollback(batch, period) + if err := s.rollback(period); err != nil { + return err + } } if err := s.fixedRoots.add(batch, period, root); err != nil { return err @@ -363,10 +361,12 @@ func (s *CommitteeChain) InsertUpdate(update *types.LightClientUpdate, nextCommi return ErrWrongCommitteeRoot } } - batch := s.db.NewBatch() if reorg { - s.rollback(batch, period+1) + if err := s.rollback(period + 1); err != nil { + return err + } } + batch := s.db.NewBatch() if addCommittee { if err := s.committees.add(batch, period+1, nextCommittee); err != nil { return err @@ -401,13 +401,28 @@ func (s *CommitteeChain) NextSyncPeriod() (uint64, bool) { // rollback removes all committees and fixed roots from the given period and updates // starting from the previous period. -func (s *CommitteeChain) rollback(batch ethdb.Batch, period uint64) { - s.deleteCommitteesFrom(batch, period) - s.fixedRoots.deleteFrom(batch, period) - if period > 0 { - period-- +func (s *CommitteeChain) rollback(period uint64) error { + max := s.updates.periods.AfterLast + 1 + if s.committees.periods.AfterLast > max { + max = s.committees.periods.AfterLast + } + if s.fixedRoots.periods.AfterLast > max { + max = s.fixedRoots.periods.AfterLast + } + for max > period { + max-- + batch := s.db.NewBatch() + s.deleteCommitteesFrom(batch, max) + s.fixedRoots.deleteFrom(batch, max) + if max > 0 { + s.updates.deleteFrom(batch, max-1) + } + if err := batch.Write(); err != nil { + log.Error("Error writing batch into chain database", "error", err) + return err + } } - s.updates.deleteFrom(batch, period) + return nil } // getCommitteeRoot returns the committee root at the given period, either fixed, From 7128f9e3cbd83021e7d15118c80c19012416e4c9 Mon Sep 17 00:00:00 2001 From: Zsolt Felfoldi Date: Fri, 8 Sep 2023 14:42:54 +0200 Subject: [PATCH 07/19] beacon/light: changed AfterLast to Next --- beacon/light/committee_chain.go | 52 ++++++++++++++-------------- beacon/light/committee_chain_test.go | 4 +-- beacon/light/range.go | 15 ++++---- 3 files changed, 35 insertions(+), 36 deletions(-) diff --git a/beacon/light/committee_chain.go b/beacon/light/committee_chain.go index 170fdc199fc7..990af208defb 100644 --- a/beacon/light/committee_chain.go +++ b/beacon/light/committee_chain.go @@ -141,18 +141,18 @@ func NewCommitteeChain(db ethdb.KeyValueStore, config *types.ChainConfig, signer } // roll back invalid updates (might be necessary if forks have been changed since last time) for !s.updates.periods.IsEmpty() { - if update, ok := s.updates.get(s.updates.periods.AfterLast - 1); !ok || s.verifyUpdate(update) { + if update, ok := s.updates.get(s.updates.periods.Next - 1); !ok || s.verifyUpdate(update) { if update == nil { - log.Error("Sync committee update missing", "period", s.updates.periods.AfterLast-1) + log.Error("Sync committee update missing", "period", s.updates.periods.Next-1) } break } - if err := s.rollback(s.updates.periods.AfterLast); err != nil { + if err := s.rollback(s.updates.periods.Next); err != nil { log.Error("Error writing batch into chain database", "error", err) } } if !s.committees.periods.IsEmpty() { - log.Trace("Sync committee chain loaded", "first period", s.committees.periods.First, "last period", s.committees.periods.AfterLast-1) + log.Trace("Sync committee chain loaded", "first period", s.committees.periods.First, "last period", s.committees.periods.Next-1) } return s } @@ -162,22 +162,22 @@ func (s *CommitteeChain) checkConstraints() bool { valid := true if !s.updates.periods.IsEmpty() { if s.fixedRoots.periods.IsEmpty() || s.updates.periods.First < s.fixedRoots.periods.First || - s.updates.periods.First >= s.fixedRoots.periods.AfterLast { + s.updates.periods.First >= s.fixedRoots.periods.Next { log.Error("First update is not in the fixed roots range") valid = false } - if s.committees.periods.First > s.updates.periods.First || s.committees.periods.AfterLast <= s.updates.periods.AfterLast { + if s.committees.periods.First > s.updates.periods.First || s.committees.periods.Next <= s.updates.periods.Next { log.Error("Missing committees in update range") valid = false } } if !s.committees.periods.IsEmpty() { if s.fixedRoots.periods.IsEmpty() || s.committees.periods.First < s.fixedRoots.periods.First || - s.committees.periods.First >= s.fixedRoots.periods.AfterLast { + s.committees.periods.First >= s.fixedRoots.periods.Next { log.Error("First committee is not in the fixed roots range") valid = false } - if s.committees.periods.AfterLast > s.fixedRoots.periods.AfterLast && s.committees.periods.AfterLast > s.updates.periods.AfterLast+1 { + if s.committees.periods.Next > s.fixedRoots.periods.Next && s.committees.periods.Next > s.updates.periods.Next+1 { log.Error("Last committee is neither in the fixed roots range nor proven by updates") valid = false } @@ -220,7 +220,7 @@ func (s *CommitteeChain) AddFixedRoot(period uint64, root common.Hash) error { // if the old root exists and matches the new one then it is guaranteed // that the given period is after the existing fixed range and the roots // in between can also be fixed. - for p := s.fixedRoots.periods.AfterLast; p < period; p++ { + for p := s.fixedRoots.periods.Next; p < period; p++ { if err := s.fixedRoots.add(batch, p, s.getCommitteeRoot(p)); err != nil { return err } @@ -249,7 +249,7 @@ func (s *CommitteeChain) DeleteFixedRootsFrom(period uint64) error { s.chainmu.Lock() defer s.chainmu.Unlock() - if period >= s.fixedRoots.periods.AfterLast { + if period >= s.fixedRoots.periods.Next { return nil } batch := s.db.NewBatch() @@ -266,7 +266,7 @@ func (s *CommitteeChain) DeleteFixedRootsFrom(period uint64) error { // get unfixed but are still proven by the update chain. If there were // committees present after the range proven by updates, those should be // removed if the belonging fixed roots are also removed. - fromPeriod := s.updates.periods.AfterLast + 1 // not proven by updates + fromPeriod := s.updates.periods.Next + 1 // not proven by updates if period > fromPeriod { fromPeriod = period // also not justified by fixed roots } @@ -282,7 +282,7 @@ func (s *CommitteeChain) DeleteFixedRootsFrom(period uint64) error { // deleteCommitteesFrom deletes committees starting from the given period. func (s *CommitteeChain) deleteCommitteesFrom(batch ethdb.Batch, period uint64) { deleted := s.committees.deleteFrom(batch, period) - for period := deleted.First; period < deleted.AfterLast; period++ { + for period := deleted.First; period < deleted.Next; period++ { s.committeeCache.Remove(period) } } @@ -394,20 +394,20 @@ func (s *CommitteeChain) NextSyncPeriod() (uint64, bool) { return 0, false } if !s.updates.periods.IsEmpty() { - return s.updates.periods.AfterLast, true + return s.updates.periods.Next, true } - return s.committees.periods.AfterLast - 1, true + return s.committees.periods.Next - 1, true } // rollback removes all committees and fixed roots from the given period and updates // starting from the previous period. func (s *CommitteeChain) rollback(period uint64) error { - max := s.updates.periods.AfterLast + 1 - if s.committees.periods.AfterLast > max { - max = s.committees.periods.AfterLast + max := s.updates.periods.Next + 1 + if s.committees.periods.Next > max { + max = s.committees.periods.Next } - if s.fixedRoots.periods.AfterLast > max { - max = s.fixedRoots.periods.AfterLast + if s.fixedRoots.periods.Next > max { + max = s.fixedRoots.periods.Next } for max > period { max-- @@ -541,11 +541,11 @@ func newCanonicalStore[T any](db ethdb.KeyValueStore, keyPrefix []byte, period := binary.BigEndian.Uint64(iter.Key()[kl : kl+8]) if cs.periods.First == 0 { cs.periods.First = period - } else if cs.periods.AfterLast != period { + } else if cs.periods.Next != period { log.Warn("Gap in the canonical chain database") break // continuity guaranteed } - cs.periods.AfterLast = period + 1 + cs.periods.Next = period + 1 } iter.Release() return cs @@ -566,7 +566,7 @@ func (cs *canonicalStore[T]) databaseKey(period uint64) []byte { // 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) { - return fmt.Errorf("period expansion is not allowed, first: %d, next: %d, period: %d", cs.periods.First, cs.periods.AfterLast, period) + return fmt.Errorf("period expansion is not allowed, first: %d, next: %d, period: %d", cs.periods.First, cs.periods.Next, period) } enc, err := cs.encode(value) if err != nil { @@ -582,19 +582,19 @@ func (cs *canonicalStore[T]) add(backend ethdb.KeyValueWriter, period uint64, va // deleteFrom removes items starting from the given period. func (cs *canonicalStore[T]) deleteFrom(batch ethdb.Batch, fromPeriod uint64) (deleted Range) { - if fromPeriod >= cs.periods.AfterLast { + if fromPeriod >= cs.periods.Next { return } if fromPeriod < cs.periods.First { fromPeriod = cs.periods.First } - deleted = Range{First: fromPeriod, AfterLast: cs.periods.AfterLast} - for period := fromPeriod; period < cs.periods.AfterLast; period++ { + deleted = Range{First: fromPeriod, Next: cs.periods.Next} + for period := fromPeriod; period < cs.periods.Next; period++ { batch.Delete(cs.databaseKey(period)) cs.cache.Remove(period) } if fromPeriod > cs.periods.First { - cs.periods.AfterLast = fromPeriod + cs.periods.Next = fromPeriod } else { cs.periods = Range{} } diff --git a/beacon/light/committee_chain_test.go b/beacon/light/committee_chain_test.go index 2cd5886866a0..b41c30c030fc 100644 --- a/beacon/light/committee_chain_test.go +++ b/beacon/light/committee_chain_test.go @@ -60,14 +60,14 @@ func TestCommitteeChainFixedRoots(t *testing.T) { c.addFixedRoot(tcBase, 4, nil) c.addFixedRoot(tcBase, 5, nil) c.addFixedRoot(tcBase, 6, nil) - c.addFixedRoot(tcBase, 8, ErrInvalidPeriod) // range has to be continuoous + c.addFixedRoot(tcBase, 8, ErrInvalidPeriod) // range has to be continuous c.addFixedRoot(tcBase, 3, nil) c.addFixedRoot(tcBase, 2, nil) if reload { c.reloadChain() } c.addCommittee(tcBase, 4, nil) - c.addCommittee(tcBase, 6, ErrInvalidPeriod) // range has to be continuoous + c.addCommittee(tcBase, 6, ErrInvalidPeriod) // range has to be continuous c.addCommittee(tcBase, 5, nil) c.addCommittee(tcBase, 6, nil) c.addCommittee(tcAnotherGenesis, 3, ErrWrongCommitteeRoot) diff --git a/beacon/light/range.go b/beacon/light/range.go index 2d58a6a51329..903d0bcfe660 100644 --- a/beacon/light/range.go +++ b/beacon/light/range.go @@ -18,31 +18,30 @@ package light // Range represents a (possibly zero-length) range of integers (sync periods). type Range struct { - First uint64 - AfterLast uint64 + First, Next uint64 } // IsEmpty returns true if the length of the range is zero. func (a Range) IsEmpty() bool { - return a.AfterLast == a.First + return a.Next == a.First } // Includes returns true if the range includes the given period. func (a Range) Includes(period uint64) bool { - return period >= a.First && period < a.AfterLast + return period >= a.First && period < a.Next } // 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.First && period <= a.AfterLast) + return a.IsEmpty() || (period+1 >= a.First && period <= a.Next) } // Expand expands the range with the given period (assumes that CanExpand returned true). func (a *Range) Expand(period uint64) { if a.IsEmpty() { - a.First, a.AfterLast = period, period+1 + a.First, a.Next = period, period+1 return } if a.Includes(period) { @@ -52,8 +51,8 @@ func (a *Range) Expand(period uint64) { a.First-- return } - if a.AfterLast == period { - a.AfterLast++ + if a.Next == period { + a.Next++ return } } From 3a46f01060ed7e2cee1af35e39603ba9125a4192 Mon Sep 17 00:00:00 2001 From: Zsolt Felfoldi Date: Fri, 8 Sep 2023 15:59:43 +0200 Subject: [PATCH 08/19] beacon/light: bubble up getSyncCommittee errors --- beacon/light/committee_chain.go | 52 ++++++++++++++++------------ beacon/light/committee_chain_test.go | 2 +- 2 files changed, 31 insertions(+), 23 deletions(-) diff --git a/beacon/light/committee_chain.go b/beacon/light/committee_chain.go index 990af208defb..4481b980eb31 100644 --- a/beacon/light/committee_chain.go +++ b/beacon/light/committee_chain.go @@ -141,10 +141,15 @@ func NewCommitteeChain(db ethdb.KeyValueStore, config *types.ChainConfig, signer } // roll back invalid updates (might be necessary if forks have been changed since last time) for !s.updates.periods.IsEmpty() { - if update, ok := s.updates.get(s.updates.periods.Next - 1); !ok || s.verifyUpdate(update) { - if update == nil { - log.Error("Sync committee update missing", "period", s.updates.periods.Next-1) - } + update, ok := s.updates.get(s.updates.periods.Next - 1) + if !ok { + log.Error("Sync committee update missing", "period", s.updates.periods.Next-1) + s.Reset() + break + } + if valid, err := s.verifyUpdate(update); err != nil { + log.Error("Error validating update", "period", s.updates.periods.Next-1, "error", err) + } else if valid { break } if err := s.rollback(s.updates.periods.Next); err != nil { @@ -349,7 +354,9 @@ func (s *CommitteeChain) InsertUpdate(update *types.LightClientUpdate, nextCommi if s.fixedRoots.periods.Includes(period+1) && reorg { return ErrCannotReorg } - if !s.verifyUpdate(update) { + if ok, err := s.verifyUpdate(update); err != nil { + return err + } else if !ok { return ErrInvalidUpdate } addCommittee := !s.committees.periods.Includes(period+1) || reorg @@ -439,21 +446,19 @@ func (s *CommitteeChain) getCommitteeRoot(period uint64) common.Hash { } // getSyncCommittee returns the deserialized sync committee at the given period. -func (s *CommitteeChain) getSyncCommittee(period uint64) syncCommittee { +func (s *CommitteeChain) getSyncCommittee(period uint64) (syncCommittee, error) { if c, ok := s.committeeCache.Get(period); ok { - return c + return c, nil } if sc, ok := s.committees.get(period); ok { c, err := s.sigVerifier.deserializeSyncCommittee(sc) if err != nil { - log.Error("Sync committee deserialization error", "error", err) - return nil + return nil, fmt.Errorf("Sync committee #%d deserialization error: %v", period, err) } s.committeeCache.Add(period, c) - return c + return c, nil } - log.Error("Missing serialized sync committee", "period", period) - return nil + return nil, fmt.Errorf("Missing serialized sync committee #%d", period) } // VerifySignedHeader returns true if the given signed header has a valid signature @@ -463,14 +468,14 @@ func (s *CommitteeChain) getSyncCommittee(period uint64) syncCommittee { // The age of the header is also returned (the time elapsed since the beginning // of the given slot, according to the local system clock). If enforceTime is // true then negative age (future) headers are rejected. -func (s *CommitteeChain) VerifySignedHeader(head types.SignedHeader) (bool, time.Duration) { +func (s *CommitteeChain) VerifySignedHeader(head types.SignedHeader) (bool, time.Duration, error) { s.chainmu.RLock() defer s.chainmu.RUnlock() return s.verifySignedHeader(head) } -func (s *CommitteeChain) verifySignedHeader(head types.SignedHeader) (bool, time.Duration) { +func (s *CommitteeChain) verifySignedHeader(head types.SignedHeader) (bool, time.Duration, error) { var age time.Duration now := s.unixNano() if head.Header.Slot < (uint64(now-math.MinInt64)/uint64(time.Second)-s.config.GenesisTime)/12 { @@ -479,31 +484,34 @@ func (s *CommitteeChain) verifySignedHeader(head types.SignedHeader) (bool, time age = time.Duration(math.MinInt64) } if s.enforceTime && age < 0 { - return false, age + return false, age, nil + } + committee, err := s.getSyncCommittee(types.SyncPeriod(head.SignatureSlot)) + if err != nil { + return false, 0, err } - committee := s.getSyncCommittee(types.SyncPeriod(head.SignatureSlot)) if committee == nil { - return false, age + return false, age, nil } if signingRoot, err := s.config.Forks.SigningRoot(head.Header); err == nil { - return s.sigVerifier.verifySignature(committee, signingRoot, &head.Signature), age + return s.sigVerifier.verifySignature(committee, signingRoot, &head.Signature), age, nil } - return false, age + return false, age, nil } // verifyUpdate checks whether the header signature is correct and the update // fits into the specified constraints (assumes that the update has been // successfully validated previously) -func (s *CommitteeChain) verifyUpdate(update *types.LightClientUpdate) bool { +func (s *CommitteeChain) verifyUpdate(update *types.LightClientUpdate) (bool, error) { // Note: SignatureSlot determines the sync period of the committee used for signature // verification. Though in reality SignatureSlot is always bigger than update.Header.Slot, // setting them as equal here enforces the rule that they have to be in the same sync // period in order for the light client update proof to be meaningful. - ok, age := s.verifySignedHeader(update.AttestedHeader) + ok, age, err := s.verifySignedHeader(update.AttestedHeader) if age < 0 { log.Warn("Future committee update received", "age", age) } - return ok + return ok, err } // canonicalStore stores instances of the given type in a database and caches diff --git a/beacon/light/committee_chain_test.go b/beacon/light/committee_chain_test.go index b41c30c030fc..02d8710d3e2c 100644 --- a/beacon/light/committee_chain_test.go +++ b/beacon/light/committee_chain_test.go @@ -283,7 +283,7 @@ func (c *committeeChainTest) insertUpdate(tc *testCommitteeChain, period uint64, func (c *committeeChainTest) verifySignedHeader(tc *testCommitteeChain, period float64, expOk bool) { slot := uint64(period * float64(params.SyncPeriodLength)) signedHead := GenerateTestSignedHeader(types.Header{Slot: slot}, &tc.config, tc.periods[types.SyncPeriod(slot)].committee, slot+1, 400) - if ok, _ := c.chain.VerifySignedHeader(signedHead); ok != expOk { + if ok, _, _ := c.chain.VerifySignedHeader(signedHead); ok != expOk { c.t.Errorf("Incorrect output from VerifySignedHeader at period %f (expected %v, got %v)", period, expOk, ok) } } From 2baa1bf1d7ba5ec41087c8021db582453bd85b1e Mon Sep 17 00:00:00 2001 From: zsfelfoldi Date: Wed, 13 Sep 2023 13:52:50 +0200 Subject: [PATCH 09/19] beacon/light: moved canonicalStore to separate file --- beacon/light/canonical.go | 139 ++++++++++++++++++++++++++++++++ beacon/light/committee_chain.go | 114 -------------------------- 2 files changed, 139 insertions(+), 114 deletions(-) create mode 100644 beacon/light/canonical.go diff --git a/beacon/light/canonical.go b/beacon/light/canonical.go new file mode 100644 index 000000000000..6fa9de48bbb8 --- /dev/null +++ b/beacon/light/canonical.go @@ -0,0 +1,139 @@ +// Copyright 2023 The go-ethereum Authors +// This file is part of the go-ethereum library. +// +// The go-ethereum library is free software: you can redistribute it and/or modify +// it under the terms of the GNU Lesser General Public License as published by +// the Free Software Foundation, either version 3 of the License, or +// (at your option) any later version. +// +// The go-ethereum library is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU Lesser General Public License for more details. +// +// You should have received a copy of the GNU Lesser General Public License +// along with the go-ethereum library. If not, see . + +package light + +import ( + "encoding/binary" + "fmt" + + "github.com/ethereum/go-ethereum/common/lru" + "github.com/ethereum/go-ethereum/ethdb" + "github.com/ethereum/go-ethereum/log" +) + +// canonicalStore stores instances of the given type in a database and caches +// them in memory, associated with a continuous range of period numbers. +// Note: canonicalStore is not thread safe and it is the caller's responsibility +// to avoid concurrent access. +type canonicalStore[T any] struct { + db ethdb.KeyValueStore + keyPrefix []byte + periods Range + cache *lru.Cache[uint64, T] + encode func(T) ([]byte, error) + decode func([]byte) (T, error) +} + +// newCanonicalStore creates a new canonicalStore. +func newCanonicalStore[T any](db ethdb.KeyValueStore, keyPrefix []byte, + encode func(T) ([]byte, error), decode func([]byte) (T, error)) *canonicalStore[T] { + cs := &canonicalStore[T]{ + db: db, + keyPrefix: keyPrefix, + encode: encode, + decode: decode, + cache: lru.NewCache[uint64, T](100), + } + var ( + iter = db.NewIterator(keyPrefix, nil) + kl = len(keyPrefix) + ) + for iter.Next() { + if len(iter.Key()) != kl+8 { + log.Warn("Invalid key length in the canonical chain database", "key", fmt.Sprintf("%#x", iter.Key())) + continue + } + period := binary.BigEndian.Uint64(iter.Key()[kl : kl+8]) + if cs.periods.First == 0 { + cs.periods.First = period + } else if cs.periods.Next != period { + log.Warn("Gap in the canonical chain database") + break // continuity guaranteed + } + cs.periods.Next = period + 1 + } + iter.Release() + return cs +} + +// 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 +} + +// 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) { + return fmt.Errorf("period expansion is not allowed, first: %d, next: %d, period: %d", cs.periods.First, cs.periods.Next, period) + } + enc, err := cs.encode(value) + if err != nil { + return err + } + if err := backend.Put(cs.databaseKey(period), enc); err != nil { + return err + } + cs.cache.Add(period, value) + 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) { + if fromPeriod >= cs.periods.Next { + return + } + if fromPeriod < cs.periods.First { + fromPeriod = cs.periods.First + } + deleted = Range{First: fromPeriod, Next: cs.periods.Next} + for period := fromPeriod; period < cs.periods.Next; period++ { + batch.Delete(cs.databaseKey(period)) + cs.cache.Remove(period) + } + if fromPeriod > cs.periods.First { + cs.periods.Next = fromPeriod + } else { + cs.periods = Range{} + } + return +} + +// get returns the item at the given period or the null value of the given type +// if no item is present. +// Note: get is thread safe in itself and therefore can be called either with +// locked or unlocked chain mutex. +func (cs *canonicalStore[T]) get(period uint64) (value T, ok bool) { + if value, ok = cs.cache.Get(period); ok { + return + } + if enc, err := cs.db.Get(cs.databaseKey(period)); err == nil { + if v, err := cs.decode(enc); err == nil { + value, ok = v, true + } else { + log.Error("Error decoding canonical store value", "error", err) + } + } + return +} diff --git a/beacon/light/committee_chain.go b/beacon/light/committee_chain.go index 4481b980eb31..82a1830986b9 100644 --- a/beacon/light/committee_chain.go +++ b/beacon/light/committee_chain.go @@ -17,7 +17,6 @@ package light import ( - "encoding/binary" "errors" "fmt" "math" @@ -513,116 +512,3 @@ func (s *CommitteeChain) verifyUpdate(update *types.LightClientUpdate) (bool, er } return ok, err } - -// canonicalStore stores instances of the given type in a database and caches -// them in memory, associated with a continuous range of period numbers. -// Note: canonicalStore is not thread safe and it is the caller's responsibility -// to avoid concurrent access. -type canonicalStore[T any] struct { - db ethdb.KeyValueStore - keyPrefix []byte - periods Range - cache *lru.Cache[uint64, T] - encode func(T) ([]byte, error) - decode func([]byte) (T, error) -} - -// newCanonicalStore creates a new canonicalStore. -func newCanonicalStore[T any](db ethdb.KeyValueStore, keyPrefix []byte, - encode func(T) ([]byte, error), decode func([]byte) (T, error)) *canonicalStore[T] { - cs := &canonicalStore[T]{ - db: db, - keyPrefix: keyPrefix, - encode: encode, - decode: decode, - cache: lru.NewCache[uint64, T](100), - } - var ( - iter = db.NewIterator(keyPrefix, nil) - kl = len(keyPrefix) - ) - for iter.Next() { - if len(iter.Key()) != kl+8 { - log.Warn("Invalid key length in the canonical chain database", "key", fmt.Sprintf("%#x", iter.Key())) - continue - } - period := binary.BigEndian.Uint64(iter.Key()[kl : kl+8]) - if cs.periods.First == 0 { - cs.periods.First = period - } else if cs.periods.Next != period { - log.Warn("Gap in the canonical chain database") - break // continuity guaranteed - } - cs.periods.Next = period + 1 - } - iter.Release() - return cs -} - -// 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 -} - -// 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) { - return fmt.Errorf("period expansion is not allowed, first: %d, next: %d, period: %d", cs.periods.First, cs.periods.Next, period) - } - enc, err := cs.encode(value) - if err != nil { - return err - } - if err := backend.Put(cs.databaseKey(period), enc); err != nil { - return err - } - cs.cache.Add(period, value) - 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) { - if fromPeriod >= cs.periods.Next { - return - } - if fromPeriod < cs.periods.First { - fromPeriod = cs.periods.First - } - deleted = Range{First: fromPeriod, Next: cs.periods.Next} - for period := fromPeriod; period < cs.periods.Next; period++ { - batch.Delete(cs.databaseKey(period)) - cs.cache.Remove(period) - } - if fromPeriod > cs.periods.First { - cs.periods.Next = fromPeriod - } else { - cs.periods = Range{} - } - return -} - -// get returns the item at the given period or the null value of the given type -// if no item is present. -// Note: get is thread safe in itself and therefore can be called either with -// locked or unlocked chain mutex. -func (cs *canonicalStore[T]) get(period uint64) (value T, ok bool) { - if value, ok = cs.cache.Get(period); ok { - return - } - if enc, err := cs.db.Get(cs.databaseKey(period)); err == nil { - if v, err := cs.decode(enc); err == nil { - value, ok = v, true - } else { - log.Error("Error decoding canonical store value", "error", err) - } - } - return -} From 3d686626d7f643e25cf3706775d7734c1b6e59fd Mon Sep 17 00:00:00 2001 From: zsfelfoldi Date: Wed, 13 Sep 2023 14:22:21 +0200 Subject: [PATCH 10/19] beacon/light: multiple small changes --- beacon/light/canonical.go | 3 ++- beacon/light/committee_chain.go | 26 ++++++++++++++++++++------ beacon/light/committee_chain_test.go | 4 ++-- beacon/light/test_helpers.go | 18 +++++++++--------- core/rawdb/schema.go | 6 +++--- 5 files changed, 36 insertions(+), 21 deletions(-) diff --git a/beacon/light/canonical.go b/beacon/light/canonical.go index 6fa9de48bbb8..2d65ea0100d5 100644 --- a/beacon/light/canonical.go +++ b/beacon/light/canonical.go @@ -38,7 +38,8 @@ type canonicalStore[T any] struct { decode func([]byte) (T, error) } -// newCanonicalStore creates a new canonicalStore. +// 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] { cs := &canonicalStore[T]{ diff --git a/beacon/light/committee_chain.go b/beacon/light/committee_chain.go index 82a1830986b9..a0683878643d 100644 --- a/beacon/light/committee_chain.go +++ b/beacon/light/committee_chain.go @@ -79,11 +79,17 @@ type CommitteeChain struct { config *types.ChainConfig signerThreshold int minimumUpdateScore types.UpdateScore - enforceTime bool + enforceTime bool // enforceTime specifies whether the age of a signed header should be checked } // NewCommitteeChain creates a new CommitteeChain. -func NewCommitteeChain(db ethdb.KeyValueStore, config *types.ChainConfig, signerThreshold int, enforceTime bool, sigVerifier committeeSigVerifier, clock mclock.Clock, unixNano func() int64) *CommitteeChain { +func NewCommitteeChain(db ethdb.KeyValueStore, config *types.ChainConfig, signerThreshold int, enforceTime bool) *CommitteeChain { + return newCommitteeChain(db, config, signerThreshold, enforceTime, blsVerifier{}, &mclock.System{}, func() int64 { return time.Now().UnixNano() }) +} + +// newCommitteeChain creates a new CommitteeChain with the option of replacing the +// clock source and signature verification for testing purposes. +func newCommitteeChain(db ethdb.KeyValueStore, config *types.ChainConfig, signerThreshold int, enforceTime bool, sigVerifier committeeSigVerifier, clock mclock.Clock, unixNano func() int64) *CommitteeChain { var ( fixedRootEncoder = func(root common.Hash) ([]byte, error) { return root[:], nil @@ -163,10 +169,15 @@ func NewCommitteeChain(db ethdb.KeyValueStore, config *types.ChainConfig, signer // checkConstraints checks committee chain validity constraints func (s *CommitteeChain) checkConstraints() bool { + isNotInFixedRootRange := func(r Range) bool { + return s.fixedRoots.periods.IsEmpty() || + r.First < s.fixedRoots.periods.First || + r.First >= s.fixedRoots.periods.Next + } + valid := true if !s.updates.periods.IsEmpty() { - if s.fixedRoots.periods.IsEmpty() || s.updates.periods.First < s.fixedRoots.periods.First || - s.updates.periods.First >= s.fixedRoots.periods.Next { + if isNotInFixedRootRange(s.updates.periods) { log.Error("First update is not in the fixed roots range") valid = false } @@ -176,8 +187,7 @@ func (s *CommitteeChain) checkConstraints() bool { } } if !s.committees.periods.IsEmpty() { - if s.fixedRoots.periods.IsEmpty() || s.committees.periods.First < s.fixedRoots.periods.First || - s.committees.periods.First >= s.fixedRoots.periods.Next { + if isNotInFixedRootRange(s.committees.periods) { log.Error("First committee is not in the fixed roots range") valid = false } @@ -206,6 +216,10 @@ func (s *CommitteeChain) AddFixedRoot(period uint64, root common.Hash) error { s.chainmu.Lock() defer s.chainmu.Unlock() + if root == (common.Hash{}) { + return ErrWrongCommitteeRoot + } + batch := s.db.NewBatch() oldRoot := s.getCommitteeRoot(period) if !s.fixedRoots.periods.CanExpand(period) { diff --git a/beacon/light/committee_chain_test.go b/beacon/light/committee_chain_test.go index 02d8710d3e2c..8cb09d9ff0a1 100644 --- a/beacon/light/committee_chain_test.go +++ b/beacon/light/committee_chain_test.go @@ -241,12 +241,12 @@ func newCommitteeChainTest(t *testing.T, config types.ChainConfig, signerThresho signerThreshold: signerThreshold, enforceTime: enforceTime, } - c.chain = NewCommitteeChain(c.db, &config, signerThreshold, enforceTime, DummyVerifier{}, c.clock, func() int64 { return int64(c.clock.Now()) }) + c.chain = newCommitteeChain(c.db, &config, signerThreshold, enforceTime, dummyVerifier{}, c.clock, func() int64 { return int64(c.clock.Now()) }) return c } func (c *committeeChainTest) reloadChain() { - c.chain = NewCommitteeChain(c.db, &c.config, c.signerThreshold, c.enforceTime, DummyVerifier{}, c.clock, func() int64 { return int64(c.clock.Now()) }) + c.chain = newCommitteeChain(c.db, &c.config, c.signerThreshold, c.enforceTime, dummyVerifier{}, c.clock, func() int64 { return int64(c.clock.Now()) }) } func (c *committeeChainTest) setClockPeriod(period float64) { diff --git a/beacon/light/test_helpers.go b/beacon/light/test_helpers.go index bfecde591a20..8f7cc2bf61c7 100644 --- a/beacon/light/test_helpers.go +++ b/beacon/light/test_helpers.go @@ -51,7 +51,7 @@ func GenerateTestUpdate(config *types.ChainConfig, period uint64, committee, nex func GenerateTestSignedHeader(header types.Header, config *types.ChainConfig, committee *types.SerializedSyncCommittee, signatureSlot uint64, signerCount int) types.SignedHeader { bitmask := makeBitmask(signerCount) signingRoot, _ := config.Forks.SigningRoot(header) - c, _ := DummyVerifier{}.deserializeSyncCommittee(committee) + c, _ := dummyVerifier{}.deserializeSyncCommittee(committee) return types.SignedHeader{ Header: header, Signature: types.SyncAggregate{ @@ -113,33 +113,33 @@ type committeeSigVerifier interface { verifySignature(committee syncCommittee, signedRoot common.Hash, aggregate *types.SyncAggregate) bool } -// BLSVerifier implements committeeSigVerifier -type BLSVerifier struct{} +// blsVerifier implements committeeSigVerifier +type blsVerifier struct{} // deserializeSyncCommittee implements committeeSigVerifier -func (BLSVerifier) deserializeSyncCommittee(s *types.SerializedSyncCommittee) (syncCommittee, error) { +func (blsVerifier) deserializeSyncCommittee(s *types.SerializedSyncCommittee) (syncCommittee, error) { return s.Deserialize() } // verifySignature implements committeeSigVerifier -func (BLSVerifier) verifySignature(committee syncCommittee, signingRoot common.Hash, aggregate *types.SyncAggregate) bool { +func (blsVerifier) verifySignature(committee syncCommittee, signingRoot common.Hash, aggregate *types.SyncAggregate) bool { return committee.(*types.SyncCommittee).VerifySignature(signingRoot, aggregate) } type dummySyncCommittee [32]byte -// DummyVerifier implements committeeSigVerifier -type DummyVerifier struct{} +// dummyVerifier implements committeeSigVerifier +type dummyVerifier struct{} // deserializeSyncCommittee implements committeeSigVerifier -func (DummyVerifier) deserializeSyncCommittee(s *types.SerializedSyncCommittee) (syncCommittee, error) { +func (dummyVerifier) deserializeSyncCommittee(s *types.SerializedSyncCommittee) (syncCommittee, error) { var sc dummySyncCommittee copy(sc[:], s[:32]) return sc, nil } // verifySignature implements committeeSigVerifier -func (DummyVerifier) verifySignature(committee syncCommittee, signingRoot common.Hash, aggregate *types.SyncAggregate) bool { +func (dummyVerifier) verifySignature(committee syncCommittee, signingRoot common.Hash, aggregate *types.SyncAggregate) bool { return aggregate.Signature == makeDummySignature(committee.(dummySyncCommittee), signingRoot, aggregate.Signers) } diff --git a/core/rawdb/schema.go b/core/rawdb/schema.go index 46cc7ecef2bd..cd3651dffeec 100644 --- a/core/rawdb/schema.go +++ b/core/rawdb/schema.go @@ -132,12 +132,12 @@ var ( CliqueSnapshotPrefix = []byte("clique-") - preimageCounter = metrics.NewRegisteredCounter("db/preimage/total", nil) - preimageHitCounter = metrics.NewRegisteredCounter("db/preimage/hits", nil) - BestUpdateKey = []byte("update-") // bigEndian64(syncPeriod) -> RLP(types.LightClientUpdate) (nextCommittee only referenced by root hash) FixedRootKey = []byte("fixedRoot-") // bigEndian64(syncPeriod) -> committee root hash SyncCommitteeKey = []byte("committee-") // bigEndian64(syncPeriod) -> serialized committee + + preimageCounter = metrics.NewRegisteredCounter("db/preimage/total", nil) + preimageHitCounter = metrics.NewRegisteredCounter("db/preimage/hits", nil) ) // LegacyTxLookupEntry is the legacy TxLookupEntry definition with some unnecessary From 623d195b706c7519ee61de57ee5bcc105c6978b5 Mon Sep 17 00:00:00 2001 From: zsfelfoldi Date: Fri, 10 Nov 2023 13:37:54 +0100 Subject: [PATCH 11/19] beacon/light: changed First, Next to Start, End --- beacon/light/canonical.go | 24 ++++++++--------- beacon/light/committee_chain.go | 46 ++++++++++++++++----------------- beacon/light/range.go | 18 ++++++------- 3 files changed, 44 insertions(+), 44 deletions(-) diff --git a/beacon/light/canonical.go b/beacon/light/canonical.go index 2d65ea0100d5..50c70e1ffae4 100644 --- a/beacon/light/canonical.go +++ b/beacon/light/canonical.go @@ -59,13 +59,13 @@ func newCanonicalStore[T any](db ethdb.KeyValueStore, keyPrefix []byte, continue } period := binary.BigEndian.Uint64(iter.Key()[kl : kl+8]) - if cs.periods.First == 0 { - cs.periods.First = period - } else if cs.periods.Next != period { + if cs.periods.Start == 0 { + cs.periods.Start = period + } else if cs.periods.End != period { log.Warn("Gap in the canonical chain database") break // continuity guaranteed } - cs.periods.Next = period + 1 + cs.periods.End = period + 1 } iter.Release() return cs @@ -86,7 +86,7 @@ func (cs *canonicalStore[T]) databaseKey(period uint64) []byte { // 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) { - return fmt.Errorf("period expansion is not allowed, first: %d, next: %d, period: %d", cs.periods.First, cs.periods.Next, 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) if err != nil { @@ -102,19 +102,19 @@ func (cs *canonicalStore[T]) add(backend ethdb.KeyValueWriter, period uint64, va // deleteFrom removes items starting from the given period. func (cs *canonicalStore[T]) deleteFrom(batch ethdb.Batch, fromPeriod uint64) (deleted Range) { - if fromPeriod >= cs.periods.Next { + if fromPeriod >= cs.periods.End { return } - if fromPeriod < cs.periods.First { - fromPeriod = cs.periods.First + if fromPeriod < cs.periods.Start { + fromPeriod = cs.periods.Start } - deleted = Range{First: fromPeriod, Next: cs.periods.Next} - for period := fromPeriod; period < cs.periods.Next; period++ { + deleted = Range{Start: fromPeriod, End: cs.periods.End} + for period := fromPeriod; period < cs.periods.End; period++ { batch.Delete(cs.databaseKey(period)) cs.cache.Remove(period) } - if fromPeriod > cs.periods.First { - cs.periods.Next = fromPeriod + if fromPeriod > cs.periods.Start { + cs.periods.End = fromPeriod } else { cs.periods = Range{} } diff --git a/beacon/light/committee_chain.go b/beacon/light/committee_chain.go index a0683878643d..938b9974b355 100644 --- a/beacon/light/committee_chain.go +++ b/beacon/light/committee_chain.go @@ -146,23 +146,23 @@ func newCommitteeChain(db ethdb.KeyValueStore, config *types.ChainConfig, signer } // roll back invalid updates (might be necessary if forks have been changed since last time) for !s.updates.periods.IsEmpty() { - update, ok := s.updates.get(s.updates.periods.Next - 1) + update, ok := s.updates.get(s.updates.periods.End - 1) if !ok { - log.Error("Sync committee update missing", "period", s.updates.periods.Next-1) + log.Error("Sync committee update missing", "period", s.updates.periods.End-1) s.Reset() break } if valid, err := s.verifyUpdate(update); err != nil { - log.Error("Error validating update", "period", s.updates.periods.Next-1, "error", err) + log.Error("Error validating update", "period", s.updates.periods.End-1, "error", err) } else if valid { break } - if err := s.rollback(s.updates.periods.Next); err != nil { + if err := s.rollback(s.updates.periods.End); err != nil { log.Error("Error writing batch into chain database", "error", err) } } if !s.committees.periods.IsEmpty() { - log.Trace("Sync committee chain loaded", "first period", s.committees.periods.First, "last period", s.committees.periods.Next-1) + log.Trace("Sync committee chain loaded", "first period", s.committees.periods.Start, "last period", s.committees.periods.End-1) } return s } @@ -171,27 +171,27 @@ func newCommitteeChain(db ethdb.KeyValueStore, config *types.ChainConfig, signer func (s *CommitteeChain) checkConstraints() bool { isNotInFixedRootRange := func(r Range) bool { return s.fixedRoots.periods.IsEmpty() || - r.First < s.fixedRoots.periods.First || - r.First >= s.fixedRoots.periods.Next + r.Start < s.fixedRoots.periods.Start || + r.Start >= s.fixedRoots.periods.End } valid := true if !s.updates.periods.IsEmpty() { if isNotInFixedRootRange(s.updates.periods) { - log.Error("First update is not in the fixed roots range") + log.Error("Start update is not in the fixed roots range") valid = false } - if s.committees.periods.First > s.updates.periods.First || s.committees.periods.Next <= s.updates.periods.Next { + if s.committees.periods.Start > s.updates.periods.Start || s.committees.periods.End <= s.updates.periods.End { log.Error("Missing committees in update range") valid = false } } if !s.committees.periods.IsEmpty() { if isNotInFixedRootRange(s.committees.periods) { - log.Error("First committee is not in the fixed roots range") + log.Error("Start committee is not in the fixed roots range") valid = false } - if s.committees.periods.Next > s.fixedRoots.periods.Next && s.committees.periods.Next > s.updates.periods.Next+1 { + if s.committees.periods.End > s.fixedRoots.periods.End && s.committees.periods.End > s.updates.periods.End+1 { log.Error("Last committee is neither in the fixed roots range nor proven by updates") valid = false } @@ -238,7 +238,7 @@ func (s *CommitteeChain) AddFixedRoot(period uint64, root common.Hash) error { // if the old root exists and matches the new one then it is guaranteed // that the given period is after the existing fixed range and the roots // in between can also be fixed. - for p := s.fixedRoots.periods.Next; p < period; p++ { + for p := s.fixedRoots.periods.End; p < period; p++ { if err := s.fixedRoots.add(batch, p, s.getCommitteeRoot(p)); err != nil { return err } @@ -267,12 +267,12 @@ func (s *CommitteeChain) DeleteFixedRootsFrom(period uint64) error { s.chainmu.Lock() defer s.chainmu.Unlock() - if period >= s.fixedRoots.periods.Next { + if period >= s.fixedRoots.periods.End { return nil } batch := s.db.NewBatch() s.fixedRoots.deleteFrom(batch, period) - if s.updates.periods.IsEmpty() || period <= s.updates.periods.First { + 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 @@ -284,7 +284,7 @@ func (s *CommitteeChain) DeleteFixedRootsFrom(period uint64) error { // get unfixed but are still proven by the update chain. If there were // committees present after the range proven by updates, those should be // removed if the belonging fixed roots are also removed. - fromPeriod := s.updates.periods.Next + 1 // not proven by updates + fromPeriod := s.updates.periods.End + 1 // not proven by updates if period > fromPeriod { fromPeriod = period // also not justified by fixed roots } @@ -300,7 +300,7 @@ func (s *CommitteeChain) DeleteFixedRootsFrom(period uint64) error { // deleteCommitteesFrom deletes committees starting from the given period. func (s *CommitteeChain) deleteCommitteesFrom(batch ethdb.Batch, period uint64) { deleted := s.committees.deleteFrom(batch, period) - for period := deleted.First; period < deleted.Next; period++ { + for period := deleted.Start; period < deleted.End; period++ { s.committeeCache.Remove(period) } } @@ -414,20 +414,20 @@ func (s *CommitteeChain) NextSyncPeriod() (uint64, bool) { return 0, false } if !s.updates.periods.IsEmpty() { - return s.updates.periods.Next, true + return s.updates.periods.End, true } - return s.committees.periods.Next - 1, true + return s.committees.periods.End - 1, true } // rollback removes all committees and fixed roots from the given period and updates // starting from the previous period. func (s *CommitteeChain) rollback(period uint64) error { - max := s.updates.periods.Next + 1 - if s.committees.periods.Next > max { - max = s.committees.periods.Next + max := s.updates.periods.End + 1 + if s.committees.periods.End > max { + max = s.committees.periods.End } - if s.fixedRoots.periods.Next > max { - max = s.fixedRoots.periods.Next + if s.fixedRoots.periods.End > max { + max = s.fixedRoots.periods.End } for max > period { max-- diff --git a/beacon/light/range.go b/beacon/light/range.go index 903d0bcfe660..38cdb9b28c7b 100644 --- a/beacon/light/range.go +++ b/beacon/light/range.go @@ -18,41 +18,41 @@ package light // Range represents a (possibly zero-length) range of integers (sync periods). type Range struct { - First, Next uint64 + Start, End uint64 } // IsEmpty returns true if the length of the range is zero. func (a Range) IsEmpty() bool { - return a.Next == a.First + return a.End == a.Start } // Includes returns true if the range includes the given period. func (a Range) Includes(period uint64) bool { - return period >= a.First && period < a.Next + return period >= a.Start && period < a.End } // 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.First && period <= a.Next) + return a.IsEmpty() || (period+1 >= a.Start && period <= a.End) } // Expand expands the range with the given period (assumes that CanExpand returned true). func (a *Range) Expand(period uint64) { if a.IsEmpty() { - a.First, a.Next = period, period+1 + a.Start, a.End = period, period+1 return } if a.Includes(period) { return } - if a.First == period+1 { - a.First-- + if a.Start == period+1 { + a.Start-- return } - if a.Next == period { - a.Next++ + if a.End == period { + a.End++ return } } From 4d71502ab010932e7a7c28d7b73bfbde06606bab Mon Sep 17 00:00:00 2001 From: zsfelfoldi Date: Fri, 10 Nov 2023 13:56:06 +0100 Subject: [PATCH 12/19] beacon/light: more renames and cleanups --- beacon/light/checkpoint.go | 8 +-- beacon/light/committee_chain.go | 90 ++++++++++++++-------------- beacon/light/committee_chain_test.go | 34 +++++------ beacon/light/range.go | 12 ++-- core/rawdb/schema.go | 6 +- 5 files changed, 73 insertions(+), 77 deletions(-) diff --git a/beacon/light/checkpoint.go b/beacon/light/checkpoint.go index cafe3d576ddb..73fe97085ab8 100644 --- a/beacon/light/checkpoint.go +++ b/beacon/light/checkpoint.go @@ -53,11 +53,11 @@ func (c *CheckpointData) InitChain(chain *CommitteeChain) { } } period := c.Header.SyncPeriod() - must(chain.DeleteFixedRootsFrom(period + 2)) - if chain.AddFixedRoot(period, c.CommitteeRoot) != nil { + must(chain.DeleteFixedCommitteeRootsFrom(period + 2)) + if chain.AddFixedCommitteeRoot(period, c.CommitteeRoot) != nil { chain.Reset() - must(chain.AddFixedRoot(period, c.CommitteeRoot)) + must(chain.AddFixedCommitteeRoot(period, c.CommitteeRoot)) } - must(chain.AddFixedRoot(period+1, common.Hash(c.CommitteeBranch[0]))) + must(chain.AddFixedCommitteeRoot(period+1, common.Hash(c.CommitteeBranch[0]))) must(chain.AddCommittee(period, c.Committee)) } diff --git a/beacon/light/committee_chain.go b/beacon/light/committee_chain.go index 938b9974b355..5dcc4fd47860 100644 --- a/beacon/light/committee_chain.go +++ b/beacon/light/committee_chain.go @@ -63,14 +63,14 @@ var ( // signed beacon headers. type CommitteeChain struct { // chainmu guards against concurrent access to the canonicalStore structures - // (updates, committees, fixedRoots) and ensures that they stay consistent + // (updates, committees, fixedCommitteeRoots) and ensures that they stay consistent // with each other and with committeeCache. - chainmu sync.RWMutex - db ethdb.KeyValueStore - updates *canonicalStore[*types.LightClientUpdate] - committees *canonicalStore[*types.SerializedSyncCommittee] - fixedRoots *canonicalStore[common.Hash] - committeeCache *lru.Cache[uint64, syncCommittee] // cache deserialized committees + chainmu sync.RWMutex + db ethdb.KeyValueStore + updates *canonicalStore[*types.LightClientUpdate] + committees *canonicalStore[*types.SerializedSyncCommittee] + fixedCommitteeRoots *canonicalStore[common.Hash] + committeeCache *lru.Cache[uint64, syncCommittee] // cache deserialized committees clock mclock.Clock // monotonic clock (simulated clock in tests) unixNano func() int64 // system clock (simulated clock in tests) @@ -91,10 +91,10 @@ func NewCommitteeChain(db ethdb.KeyValueStore, config *types.ChainConfig, signer // clock source and signature verification for testing purposes. func newCommitteeChain(db ethdb.KeyValueStore, config *types.ChainConfig, signerThreshold int, enforceTime bool, sigVerifier committeeSigVerifier, clock mclock.Clock, unixNano func() int64) *CommitteeChain { var ( - fixedRootEncoder = func(root common.Hash) ([]byte, error) { + fixedCommitteeRootEncoder = func(root common.Hash) ([]byte, error) { return root[:], nil } - fixedRootDecoder = func(enc []byte) (root common.Hash, err error) { + fixedCommitteeRootDecoder = func(enc []byte) (root common.Hash, err error) { if len(enc) != common.HashLength { return common.Hash{}, errors.New("incorrect length for committee root entry in the database") } @@ -123,17 +123,17 @@ func newCommitteeChain(db ethdb.KeyValueStore, config *types.ChainConfig, signer } ) s := &CommitteeChain{ - fixedRoots: newCanonicalStore[common.Hash](db, rawdb.FixedRootKey, fixedRootEncoder, fixedRootDecoder), - 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, + 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, minimumUpdateScore: types.UpdateScore{ SignerCount: uint32(signerThreshold), SubPeriodIndex: params.SyncPeriodLength / 16, @@ -169,15 +169,15 @@ func newCommitteeChain(db ethdb.KeyValueStore, config *types.ChainConfig, signer // checkConstraints checks committee chain validity constraints func (s *CommitteeChain) checkConstraints() bool { - isNotInFixedRootRange := func(r Range) bool { - return s.fixedRoots.periods.IsEmpty() || - r.Start < s.fixedRoots.periods.Start || - r.Start >= s.fixedRoots.periods.End + isNotInFixedCommitteeRootRange := func(r Range) 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 isNotInFixedRootRange(s.updates.periods) { + if isNotInFixedCommitteeRootRange(s.updates.periods) { log.Error("Start update is not in the fixed roots range") valid = false } @@ -187,11 +187,11 @@ func (s *CommitteeChain) checkConstraints() bool { } } if !s.committees.periods.IsEmpty() { - if isNotInFixedRootRange(s.committees.periods) { + if isNotInFixedCommitteeRootRange(s.committees.periods) { log.Error("Start committee is not in the fixed roots range") valid = false } - if s.committees.periods.End > s.fixedRoots.periods.End && s.committees.periods.End > s.updates.periods.End+1 { + if s.committees.periods.End > s.fixedCommitteeRoots.periods.End && s.committees.periods.End > s.updates.periods.End+1 { log.Error("Last committee is neither in the fixed roots range nor proven by updates") valid = false } @@ -209,10 +209,10 @@ func (s *CommitteeChain) Reset() { } } -// AddFixedRoot sets a fixed committee root at the given period. +// AddFixedCommitteeRoot sets a fixed committee root at the given period. // Note that the period where the first committee is added has to have a fixed // root which can either come from a CheckpointData or a trusted source. -func (s *CommitteeChain) AddFixedRoot(period uint64, root common.Hash) error { +func (s *CommitteeChain) AddFixedCommitteeRoot(period uint64, root common.Hash) error { s.chainmu.Lock() defer s.chainmu.Unlock() @@ -222,7 +222,7 @@ func (s *CommitteeChain) AddFixedRoot(period uint64, root common.Hash) error { batch := s.db.NewBatch() oldRoot := s.getCommitteeRoot(period) - if !s.fixedRoots.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 @@ -238,8 +238,8 @@ func (s *CommitteeChain) AddFixedRoot(period uint64, root common.Hash) error { // if the old root exists and matches the new one then it is guaranteed // that the given period is after the existing fixed range and the roots // in between can also be fixed. - for p := s.fixedRoots.periods.End; p < period; p++ { - if err := s.fixedRoots.add(batch, p, s.getCommitteeRoot(p)); err != nil { + for p := s.fixedCommitteeRoots.periods.End; p < period; p++ { + if err := s.fixedCommitteeRoots.add(batch, p, s.getCommitteeRoot(p)); err != nil { return err } } @@ -250,7 +250,7 @@ func (s *CommitteeChain) AddFixedRoot(period uint64, root common.Hash) error { return err } } - if err := s.fixedRoots.add(batch, period, root); err != nil { + if err := s.fixedCommitteeRoots.add(batch, period, root); err != nil { return err } if err := batch.Write(); err != nil { @@ -260,18 +260,18 @@ func (s *CommitteeChain) AddFixedRoot(period uint64, root common.Hash) error { return nil } -// DeleteFixedRootsFrom deletes fixed roots starting from the given period. +// DeleteFixedCommitteeRootsFrom deletes fixed roots starting from the given period. // It also maintains chain consistency, meaning that it also deletes updates and // committees if they are no longer supported by a valid update chain. -func (s *CommitteeChain) DeleteFixedRootsFrom(period uint64) error { +func (s *CommitteeChain) DeleteFixedCommitteeRootsFrom(period uint64) error { s.chainmu.Lock() defer s.chainmu.Unlock() - if period >= s.fixedRoots.periods.End { + if period >= s.fixedCommitteeRoots.periods.End { return nil } batch := s.db.NewBatch() - s.fixedRoots.deleteFrom(batch, period) + s.fixedCommitteeRoots.deleteFrom(batch, period) 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 @@ -327,7 +327,7 @@ func (s *CommitteeChain) AddCommittee(period uint64, committee *types.Serialized if root != committee.Root() { return ErrWrongCommitteeRoot } - if !s.committees.periods.Includes(period) { + if !s.committees.periods.Contains(period) { if err := s.committees.add(s.db, period, committee); err != nil { return err } @@ -349,7 +349,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.Includes(period) { + if !s.updates.periods.CanExpand(period) || !s.committees.periods.Contains(period) { return ErrInvalidPeriod } if s.minimumUpdateScore.BetterThan(update.Score()) { @@ -364,7 +364,7 @@ func (s *CommitteeChain) InsertUpdate(update *types.LightClientUpdate, nextCommi } return nil } - if s.fixedRoots.periods.Includes(period+1) && reorg { + if s.fixedCommitteeRoots.periods.Contains(period+1) && reorg { return ErrCannotReorg } if ok, err := s.verifyUpdate(update); err != nil { @@ -372,7 +372,7 @@ func (s *CommitteeChain) InsertUpdate(update *types.LightClientUpdate, nextCommi } else if !ok { return ErrInvalidUpdate } - addCommittee := !s.committees.periods.Includes(period+1) || reorg + addCommittee := !s.committees.periods.Contains(period+1) || reorg if addCommittee { if nextCommittee == nil { return ErrNeedCommittee @@ -426,14 +426,14 @@ func (s *CommitteeChain) rollback(period uint64) error { if s.committees.periods.End > max { max = s.committees.periods.End } - if s.fixedRoots.periods.End > max { - max = s.fixedRoots.periods.End + if s.fixedCommitteeRoots.periods.End > max { + max = s.fixedCommitteeRoots.periods.End } for max > period { max-- batch := s.db.NewBatch() s.deleteCommitteesFrom(batch, max) - s.fixedRoots.deleteFrom(batch, max) + s.fixedCommitteeRoots.deleteFrom(batch, max) if max > 0 { s.updates.deleteFrom(batch, max-1) } @@ -449,7 +449,7 @@ func (s *CommitteeChain) rollback(period uint64) error { // proven by a previous update or both. It returns an empty hash if the committee // root is unknown. func (s *CommitteeChain) getCommitteeRoot(period uint64) common.Hash { - if root, ok := s.fixedRoots.get(period); ok || period == 0 { + if root, ok := s.fixedCommitteeRoots.get(period); ok || period == 0 { return root } if update, ok := s.updates.get(period - 1); ok { diff --git a/beacon/light/committee_chain_test.go b/beacon/light/committee_chain_test.go index 8cb09d9ff0a1..fd7c624f0367 100644 --- a/beacon/light/committee_chain_test.go +++ b/beacon/light/committee_chain_test.go @@ -53,16 +53,16 @@ var ( tcAnotherGenesis = newTestCommitteeChain(nil, tfAnotherGenesis, true, 0, 10, 400, false) ) -func TestCommitteeChainFixedRoots(t *testing.T) { +func TestCommitteeChainFixedCommitteeRoots(t *testing.T) { for _, reload := range []bool{false, true} { c := newCommitteeChainTest(t, tfBase, 300, true) c.setClockPeriod(7) - c.addFixedRoot(tcBase, 4, nil) - c.addFixedRoot(tcBase, 5, nil) - c.addFixedRoot(tcBase, 6, nil) - c.addFixedRoot(tcBase, 8, ErrInvalidPeriod) // range has to be continuous - c.addFixedRoot(tcBase, 3, nil) - c.addFixedRoot(tcBase, 2, nil) + c.addFixedCommitteeRoot(tcBase, 4, nil) + c.addFixedCommitteeRoot(tcBase, 5, nil) + c.addFixedCommitteeRoot(tcBase, 6, nil) + c.addFixedCommitteeRoot(tcBase, 8, ErrInvalidPeriod) // range has to be continuous + c.addFixedCommitteeRoot(tcBase, 3, nil) + c.addFixedCommitteeRoot(tcBase, 2, nil) if reload { c.reloadChain() } @@ -87,8 +87,8 @@ func TestCommitteeChainCheckpointSync(t *testing.T) { c.setClockPeriod(6) } c.insertUpdate(tcBase, 3, true, ErrInvalidPeriod) - c.addFixedRoot(tcBase, 3, nil) - c.addFixedRoot(tcBase, 4, nil) + c.addFixedCommitteeRoot(tcBase, 3, nil) + c.addFixedCommitteeRoot(tcBase, 4, nil) c.insertUpdate(tcBase, 4, true, ErrInvalidPeriod) // still no committee c.addCommittee(tcBase, 3, nil) c.addCommittee(tcBase, 4, nil) @@ -120,7 +120,7 @@ func TestCommitteeChainCheckpointSync(t *testing.T) { c.verifyRange(tcBase, 3, 7) // now period 7 can also be verified // try reverse syncing an update c.insertUpdate(tcBase, 2, false, ErrInvalidPeriod) // fixed committee is needed first - c.addFixedRoot(tcBase, 2, nil) + c.addFixedCommitteeRoot(tcBase, 2, nil) c.addCommittee(tcBase, 2, nil) c.insertUpdate(tcBase, 2, false, nil) c.verifyRange(tcBase, 2, 7) @@ -133,8 +133,8 @@ func TestCommitteeChainReorg(t *testing.T) { for _, addBetterUpdates := range []bool{false, true} { c := newCommitteeChainTest(t, tfBase, 300, true) c.setClockPeriod(11) - c.addFixedRoot(tcBase, 3, nil) - c.addFixedRoot(tcBase, 4, nil) + c.addFixedCommitteeRoot(tcBase, 3, nil) + c.addFixedCommitteeRoot(tcBase, 4, nil) c.addCommittee(tcBase, 3, nil) for period := uint64(3); period < 10; period++ { c.insertUpdate(tcBase, period, true, nil) @@ -193,8 +193,8 @@ func TestCommitteeChainFork(t *testing.T) { c := newCommitteeChainTest(t, tfAlternative, 300, true) c.setClockPeriod(11) // trying to sync a chain on an alternative fork with the base chain data - c.addFixedRoot(tcBase, 0, nil) - c.addFixedRoot(tcBase, 1, nil) + c.addFixedCommitteeRoot(tcBase, 0, nil) + c.addFixedCommitteeRoot(tcBase, 1, nil) c.addCommittee(tcBase, 0, nil) // shared section should sync without errors for period := uint64(0); period < 7; period++ { @@ -258,9 +258,9 @@ func (c *committeeChainTest) setClockPeriod(period float64) { c.clock.Run(wait) } -func (c *committeeChainTest) addFixedRoot(tc *testCommitteeChain, period uint64, expErr error) { - if err := c.chain.AddFixedRoot(period, tc.periods[period].committee.Root()); err != expErr { - c.t.Errorf("Incorrect error output from AddFixedRoot at period %d (expected %v, got %v)", period, expErr, err) +func (c *committeeChainTest) addFixedCommitteeRoot(tc *testCommitteeChain, period uint64, expErr error) { + if err := c.chain.AddFixedCommitteeRoot(period, tc.periods[period].committee.Root()); err != expErr { + c.t.Errorf("Incorrect error output from AddFixedCommitteeRoot at period %d (expected %v, got %v)", period, expErr, err) } } diff --git a/beacon/light/range.go b/beacon/light/range.go index 38cdb9b28c7b..e9427ed3826d 100644 --- a/beacon/light/range.go +++ b/beacon/light/range.go @@ -26,8 +26,8 @@ func (a Range) IsEmpty() bool { return a.End == a.Start } -// Includes returns true if the range includes the given period. -func (a Range) Includes(period uint64) bool { +// Contains returns true if the range includes the given period. +func (a Range) Contains(period uint64) bool { return period >= a.Start && period < a.End } @@ -38,21 +38,17 @@ func (a Range) CanExpand(period uint64) bool { return a.IsEmpty() || (period+1 >= a.Start && period <= a.End) } -// Expand expands the range with the given period (assumes that CanExpand returned true). +// 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() { a.Start, a.End = period, period+1 return } - if a.Includes(period) { - return - } if a.Start == period+1 { a.Start-- - return } if a.End == period { a.End++ - return } } diff --git a/core/rawdb/schema.go b/core/rawdb/schema.go index cd3651dffeec..be037235533a 100644 --- a/core/rawdb/schema.go +++ b/core/rawdb/schema.go @@ -132,9 +132,9 @@ var ( CliqueSnapshotPrefix = []byte("clique-") - BestUpdateKey = []byte("update-") // bigEndian64(syncPeriod) -> RLP(types.LightClientUpdate) (nextCommittee only referenced by root hash) - FixedRootKey = []byte("fixedRoot-") // bigEndian64(syncPeriod) -> committee root hash - SyncCommitteeKey = []byte("committee-") // bigEndian64(syncPeriod) -> serialized committee + BestUpdateKey = []byte("update-") // bigEndian64(syncPeriod) -> RLP(types.LightClientUpdate) (nextCommittee only referenced by root hash) + FixedCommitteeRootKey = []byte("fixedRoot-") // bigEndian64(syncPeriod) -> committee root hash + SyncCommitteeKey = []byte("committee-") // bigEndian64(syncPeriod) -> serialized committee preimageCounter = metrics.NewRegisteredCounter("db/preimage/total", nil) preimageHitCounter = metrics.NewRegisteredCounter("db/preimage/hits", nil) From 9fd49b01c988131d952f3f4fd1ea2162d596efee Mon Sep 17 00:00:00 2001 From: Zsolt Felfoldi Date: Wed, 29 Nov 2023 17:27:13 +0100 Subject: [PATCH 13/19] beacon/light: unexported fixed roots, moved checkpoint init into CommitteeChain --- beacon/light/checkpoint.go | 63 ---------------------- beacon/light/committee_chain.go | 65 +++++++++++++++-------- beacon/light/committee_chain_test.go | 8 +-- beacon/light/test_helpers.go | 4 +- beacon/types/{update.go => light_sync.go} | 18 +++++++ 5 files changed, 67 insertions(+), 91 deletions(-) delete mode 100644 beacon/light/checkpoint.go rename beacon/types/{update.go => light_sync.go} (88%) diff --git a/beacon/light/checkpoint.go b/beacon/light/checkpoint.go deleted file mode 100644 index 73fe97085ab8..000000000000 --- a/beacon/light/checkpoint.go +++ /dev/null @@ -1,63 +0,0 @@ -// Copyright 2023 The go-ethereum Authors -// This file is part of the go-ethereum library. -// -// The go-ethereum library is free software: you can redistribute it and/or modify -// it under the terms of the GNU Lesser General Public License as published by -// the Free Software Foundation, either version 3 of the License, or -// (at your option) any later version. -// -// The go-ethereum library is distributed in the hope that it will be useful, -// but WITHOUT ANY WARRANTY; without even the implied warranty of -// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the -// GNU Lesser General Public License for more details. -// -// You should have received a copy of the GNU Lesser General Public License -// along with the go-ethereum library. If not, see . - -package light - -import ( - "errors" - - "github.com/ethereum/go-ethereum/beacon/merkle" - "github.com/ethereum/go-ethereum/beacon/params" - "github.com/ethereum/go-ethereum/beacon/types" - "github.com/ethereum/go-ethereum/common" - "github.com/ethereum/go-ethereum/log" -) - -// CheckpointData contains a sync committee where light sync can be started, -// together with a proof through a beacon header and corresponding state. -// Note: CheckpointData is fetched from a server based on a known checkpoint hash. -type CheckpointData struct { - Header types.Header - CommitteeRoot common.Hash - Committee *types.SerializedSyncCommittee `rlp:"-"` - CommitteeBranch merkle.Values -} - -// Validate verifies the proof included in CheckpointData. -func (c *CheckpointData) Validate() error { - if c.CommitteeRoot != c.Committee.Root() { - return errors.New("wrong committee root") - } - return merkle.VerifyProof(c.Header.StateRoot, params.StateIndexSyncCommittee, c.CommitteeBranch, merkle.Value(c.CommitteeRoot)) -} - -// InitChain initializes a CommitteeChain based on the checkpoint. -// Note that the checkpoint is expected to be already validated. -func (c *CheckpointData) InitChain(chain *CommitteeChain) { - must := func(err error) { - if err != nil { - log.Error("Error initializing committee chain with checkpoint", "error", err) - } - } - period := c.Header.SyncPeriod() - must(chain.DeleteFixedCommitteeRootsFrom(period + 2)) - if chain.AddFixedCommitteeRoot(period, c.CommitteeRoot) != nil { - chain.Reset() - must(chain.AddFixedCommitteeRoot(period, c.CommitteeRoot)) - } - must(chain.AddFixedCommitteeRoot(period+1, common.Hash(c.CommitteeBranch[0]))) - must(chain.AddCommittee(period, c.Committee)) -} diff --git a/beacon/light/committee_chain.go b/beacon/light/committee_chain.go index 5dcc4fd47860..937453bff9d6 100644 --- a/beacon/light/committee_chain.go +++ b/beacon/light/committee_chain.go @@ -45,7 +45,7 @@ var ( // CommitteeChain is a passive data structure that can validate, hold and update // a chain of beacon light sync committees and updates. It requires at least one // externally set fixed committee root at the beginning of the chain which can -// be set either based on a CheckpointData or a trusted source (a local beacon +// be set either based on a BootstrapData or a trusted source (a local beacon // full node). This makes the structure useful for both light client and light // server setups. // @@ -209,10 +209,45 @@ func (s *CommitteeChain) Reset() { } } -// AddFixedCommitteeRoot sets a fixed committee root at the given period. +// CheckpointInit initializes a CommitteeChain based on the checkpoint. +// Note: if the chain is already initialized and the committees proven by the +// checkpoint do match the existing chain then the chain is retained and the +// new checkpoint becomes fixed. +func (s *CommitteeChain) CheckpointInit(bootstrap *types.BootstrapData) error { + s.chainmu.Lock() + defer s.chainmu.Unlock() + + if err := bootstrap.Validate(); err != nil { + return err + } + + period := bootstrap.Header.SyncPeriod() + if err := s.deleteFixedCommitteeRootsFrom(period + 2); err != nil { + s.Reset() + return err + } + if s.addFixedCommitteeRoot(period, bootstrap.CommitteeRoot) != nil { + s.Reset() + if err := s.addFixedCommitteeRoot(period, bootstrap.CommitteeRoot); err != nil { + s.Reset() + return err + } + } + if err := s.addFixedCommitteeRoot(period+1, common.Hash(bootstrap.CommitteeBranch[0])); err != nil { + s.Reset() + return err + } + if err := s.addCommittee(period, bootstrap.Committee); err != nil { + s.Reset() + return err + } + return nil +} + +// addFixedCommitteeRoot sets a fixed committee root at the given period. // Note that the period where the first committee is added has to have a fixed -// root which can either come from a CheckpointData or a trusted source. -func (s *CommitteeChain) AddFixedCommitteeRoot(period uint64, root common.Hash) error { +// root which can either come from a BootstrapData or a trusted source. +func (s *CommitteeChain) addFixedCommitteeRoot(period uint64, root common.Hash) error { s.chainmu.Lock() defer s.chainmu.Unlock() @@ -260,10 +295,10 @@ func (s *CommitteeChain) AddFixedCommitteeRoot(period uint64, root common.Hash) return nil } -// DeleteFixedCommitteeRootsFrom deletes fixed roots starting from the given period. +// deleteFixedCommitteeRootsFrom deletes fixed roots starting from the given period. // It also maintains chain consistency, meaning that it also deletes updates and // committees if they are no longer supported by a valid update chain. -func (s *CommitteeChain) DeleteFixedCommitteeRootsFrom(period uint64) error { +func (s *CommitteeChain) deleteFixedCommitteeRootsFrom(period uint64) error { s.chainmu.Lock() defer s.chainmu.Unlock() @@ -305,15 +340,8 @@ func (s *CommitteeChain) deleteCommitteesFrom(batch ethdb.Batch, period uint64) } } -// GetCommittee returns the committee at the given period. -// Note: GetCommittee can be called either with locked or unlocked chain mutex. -func (s *CommitteeChain) GetCommittee(period uint64) *types.SerializedSyncCommittee { - committee, _ := s.committees.get(period) - return committee -} - -// AddCommittee adds a committee at the given period if possible. -func (s *CommitteeChain) AddCommittee(period uint64, committee *types.SerializedSyncCommittee) error { +// addCommittee adds a committee at the given period if possible. +func (s *CommitteeChain) addCommittee(period uint64, committee *types.SerializedSyncCommittee) error { s.chainmu.Lock() defer s.chainmu.Unlock() @@ -336,13 +364,6 @@ func (s *CommitteeChain) AddCommittee(period uint64, committee *types.Serialized return nil } -// GetUpdate returns the update at the given period. -// Note: GetUpdate can be called either with locked or unlocked chain mutex. -func (s *CommitteeChain) GetUpdate(period uint64) *types.LightClientUpdate { - update, _ := s.updates.get(period) - return update -} - // InsertUpdate adds a new update if possible. func (s *CommitteeChain) InsertUpdate(update *types.LightClientUpdate, nextCommittee *types.SerializedSyncCommittee) error { s.chainmu.Lock() diff --git a/beacon/light/committee_chain_test.go b/beacon/light/committee_chain_test.go index fd7c624f0367..60ea2a0efdbf 100644 --- a/beacon/light/committee_chain_test.go +++ b/beacon/light/committee_chain_test.go @@ -259,14 +259,14 @@ func (c *committeeChainTest) setClockPeriod(period float64) { } func (c *committeeChainTest) addFixedCommitteeRoot(tc *testCommitteeChain, period uint64, expErr error) { - if err := c.chain.AddFixedCommitteeRoot(period, tc.periods[period].committee.Root()); err != expErr { - c.t.Errorf("Incorrect error output from AddFixedCommitteeRoot at period %d (expected %v, got %v)", period, expErr, err) + if err := c.chain.addFixedCommitteeRoot(period, tc.periods[period].committee.Root()); err != expErr { + c.t.Errorf("Incorrect error output from addFixedCommitteeRoot at period %d (expected %v, got %v)", period, expErr, err) } } func (c *committeeChainTest) addCommittee(tc *testCommitteeChain, period uint64, expErr error) { - if err := c.chain.AddCommittee(period, tc.periods[period].committee); err != expErr { - c.t.Errorf("Incorrect error output from AddCommittee at period %d (expected %v, got %v)", period, expErr, err) + if err := c.chain.addCommittee(period, tc.periods[period].committee); err != expErr { + c.t.Errorf("Incorrect error output from addCommittee at period %d (expected %v, got %v)", period, expErr, err) } } diff --git a/beacon/light/test_helpers.go b/beacon/light/test_helpers.go index 8f7cc2bf61c7..f537d963a667 100644 --- a/beacon/light/test_helpers.go +++ b/beacon/light/test_helpers.go @@ -62,9 +62,9 @@ func GenerateTestSignedHeader(header types.Header, config *types.ChainConfig, co } } -func GenerateTestCheckpoint(period uint64, committee *types.SerializedSyncCommittee) *CheckpointData { +func GenerateTestCheckpoint(period uint64, committee *types.SerializedSyncCommittee) *types.BootstrapData { header, branch := makeTestHeaderWithMerkleProof(types.SyncPeriodStart(period)+200, params.StateIndexSyncCommittee, merkle.Value(committee.Root())) - return &CheckpointData{ + return &types.BootstrapData{ Header: header, Committee: committee, CommitteeRoot: committee.Root(), diff --git a/beacon/types/update.go b/beacon/types/light_sync.go similarity index 88% rename from beacon/types/update.go rename to beacon/types/light_sync.go index 06c1b6179256..3284081e4d49 100644 --- a/beacon/types/update.go +++ b/beacon/types/light_sync.go @@ -25,6 +25,24 @@ import ( "github.com/ethereum/go-ethereum/common" ) +// BootstrapData contains a sync committee where light sync can be started, +// together with a proof through a beacon header and corresponding state. +// Note: BootstrapData is fetched from a server based on a known checkpoint hash. +type BootstrapData struct { + Header Header + CommitteeRoot common.Hash + Committee *SerializedSyncCommittee `rlp:"-"` + CommitteeBranch merkle.Values +} + +// Validate verifies the proof included in BootstrapData. +func (c *BootstrapData) Validate() error { + if c.CommitteeRoot != c.Committee.Root() { + return errors.New("wrong committee root") + } + return merkle.VerifyProof(c.Header.StateRoot, params.StateIndexSyncCommittee, c.CommitteeBranch, merkle.Value(c.CommitteeRoot)) +} + // LightClientUpdate is a proof of the next sync committee root based on a header // signed by the sync committee of the given period. Optionally, the update can // prove quasi-finality by the signed header referring to a previous, finalized From 0ee5336c52cf616ae4eea5758958f74631c0a18f Mon Sep 17 00:00:00 2001 From: zsfelfoldi Date: Thu, 30 Nov 2023 06:35:18 +0100 Subject: [PATCH 14/19] beacon/light: fixed some bugs --- beacon/light/canonical.go | 16 +++++++++++----- beacon/light/committee_chain.go | 9 --------- 2 files changed, 11 insertions(+), 14 deletions(-) diff --git a/beacon/light/canonical.go b/beacon/light/canonical.go index 50c70e1ffae4..bdd59324bda7 100644 --- a/beacon/light/canonical.go +++ b/beacon/light/canonical.go @@ -50,8 +50,9 @@ func newCanonicalStore[T any](db ethdb.KeyValueStore, keyPrefix []byte, cache: lru.NewCache[uint64, T](100), } var ( - iter = db.NewIterator(keyPrefix, nil) - kl = len(keyPrefix) + iter = db.NewIterator(keyPrefix, nil) + kl = len(keyPrefix) + first = true ) for iter.Next() { if len(iter.Key()) != kl+8 { @@ -59,12 +60,13 @@ func newCanonicalStore[T any](db ethdb.KeyValueStore, keyPrefix []byte, continue } period := binary.BigEndian.Uint64(iter.Key()[kl : kl+8]) - if cs.periods.Start == 0 { + if first { cs.periods.Start = period } else if cs.periods.End != period { log.Warn("Gap in the canonical chain database") break // continuity guaranteed } + first = false cs.periods.End = period + 1 } iter.Release() @@ -123,18 +125,22 @@ 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. -// Note: get is thread safe in itself and therefore can be called either with -// locked or unlocked chain mutex. func (cs *canonicalStore[T]) get(period uint64) (value T, ok bool) { + if !cs.periods.Contains(period) { + return + } if value, ok = cs.cache.Get(period); ok { return } if enc, err := cs.db.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 { + log.Error("Canonical store value not found", "period", period, "start", cs.periods.Start, "end", cs.periods.End) } return } diff --git a/beacon/light/committee_chain.go b/beacon/light/committee_chain.go index 937453bff9d6..96971859e6ab 100644 --- a/beacon/light/committee_chain.go +++ b/beacon/light/committee_chain.go @@ -248,9 +248,6 @@ func (s *CommitteeChain) CheckpointInit(bootstrap *types.BootstrapData) error { // Note that the period where the first committee is added has to have a fixed // root which can either come from a BootstrapData or a trusted source. func (s *CommitteeChain) addFixedCommitteeRoot(period uint64, root common.Hash) error { - s.chainmu.Lock() - defer s.chainmu.Unlock() - if root == (common.Hash{}) { return ErrWrongCommitteeRoot } @@ -299,9 +296,6 @@ func (s *CommitteeChain) addFixedCommitteeRoot(period uint64, root common.Hash) // It also maintains chain consistency, meaning that it also deletes updates and // committees if they are no longer supported by a valid update chain. func (s *CommitteeChain) deleteFixedCommitteeRootsFrom(period uint64) error { - s.chainmu.Lock() - defer s.chainmu.Unlock() - if period >= s.fixedCommitteeRoots.periods.End { return nil } @@ -342,9 +336,6 @@ 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 { - s.chainmu.Lock() - defer s.chainmu.Unlock() - if !s.committees.periods.CanExpand(period) { return ErrInvalidPeriod } From 94421cfb432fa3ee08fab21c41d4d5b3ef5e635b Mon Sep 17 00:00:00 2001 From: zsfelfoldi Date: Thu, 30 Nov 2023 06:46:05 +0100 Subject: [PATCH 15/19] beacon/light: always use external db parameter in canonicalStore --- beacon/light/canonical.go | 6 ++---- beacon/light/committee_chain.go | 10 +++++----- 2 files changed, 7 insertions(+), 9 deletions(-) diff --git a/beacon/light/canonical.go b/beacon/light/canonical.go index bdd59324bda7..c5b3c2ec1481 100644 --- a/beacon/light/canonical.go +++ b/beacon/light/canonical.go @@ -30,7 +30,6 @@ import ( // Note: canonicalStore is not thread safe and it is the caller's responsibility // to avoid concurrent access. type canonicalStore[T any] struct { - db ethdb.KeyValueStore keyPrefix []byte periods Range cache *lru.Cache[uint64, T] @@ -43,7 +42,6 @@ type canonicalStore[T any] struct { func newCanonicalStore[T any](db ethdb.KeyValueStore, keyPrefix []byte, encode func(T) ([]byte, error), decode func([]byte) (T, error)) *canonicalStore[T] { cs := &canonicalStore[T]{ - db: db, keyPrefix: keyPrefix, encode: encode, decode: decode, @@ -125,14 +123,14 @@ 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(period uint64) (value T, ok bool) { +func (cs *canonicalStore[T]) get(backend ethdb.KeyValueReader, period uint64) (value T, ok bool) { if !cs.periods.Contains(period) { return } if value, ok = cs.cache.Get(period); ok { return } - if enc, err := cs.db.Get(cs.databaseKey(period)); err == nil { + 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) diff --git a/beacon/light/committee_chain.go b/beacon/light/committee_chain.go index 96971859e6ab..2f833cb39210 100644 --- a/beacon/light/committee_chain.go +++ b/beacon/light/committee_chain.go @@ -146,7 +146,7 @@ func newCommitteeChain(db ethdb.KeyValueStore, config *types.ChainConfig, signer } // roll back invalid updates (might be necessary if forks have been changed since last time) for !s.updates.periods.IsEmpty() { - update, ok := s.updates.get(s.updates.periods.End - 1) + 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) s.Reset() @@ -369,7 +369,7 @@ func (s *CommitteeChain) InsertUpdate(update *types.LightClientUpdate, nextCommi } oldRoot := s.getCommitteeRoot(period + 1) reorg := oldRoot != (common.Hash{}) && oldRoot != update.NextSyncCommitteeRoot - if oldUpdate, ok := s.updates.get(period); ok && !update.Score().BetterThan(oldUpdate.Score()) { + if oldUpdate, ok := s.updates.get(s.db, period); ok && !update.Score().BetterThan(oldUpdate.Score()) { // a better or equal update already exists; no changes, only fail if new one tried to reorg if reorg { return ErrCannotReorg @@ -461,10 +461,10 @@ func (s *CommitteeChain) rollback(period uint64) error { // proven by a previous update or both. It returns an empty hash if the committee // root is unknown. func (s *CommitteeChain) getCommitteeRoot(period uint64) common.Hash { - if root, ok := s.fixedCommitteeRoots.get(period); ok || period == 0 { + if root, ok := s.fixedCommitteeRoots.get(s.db, period); ok || period == 0 { return root } - if update, ok := s.updates.get(period - 1); ok { + if update, ok := s.updates.get(s.db, period-1); ok { return update.NextSyncCommitteeRoot } return common.Hash{} @@ -475,7 +475,7 @@ func (s *CommitteeChain) getSyncCommittee(period uint64) (syncCommittee, error) if c, ok := s.committeeCache.Get(period); ok { return c, nil } - if sc, ok := s.committees.get(period); ok { + if sc, ok := s.committees.get(s.db, period); ok { c, err := s.sigVerifier.deserializeSyncCommittee(sc) if err != nil { return nil, fmt.Errorf("Sync committee #%d deserialization error: %v", period, err) From 8b1390e904bb7d0601b20d69cf6f30993d6461d3 Mon Sep 17 00:00:00 2001 From: zsfelfoldi Date: Thu, 30 Nov 2023 06:52:35 +0100 Subject: [PATCH 16/19] beacon/light: new Range methods, simplified deleteFrom --- beacon/light/canonical.go | 20 +++++--------------- beacon/light/range.go | 24 ++++++++++++++++++++++++ 2 files changed, 29 insertions(+), 15 deletions(-) diff --git a/beacon/light/canonical.go b/beacon/light/canonical.go index c5b3c2ec1481..381cfdfda17c 100644 --- a/beacon/light/canonical.go +++ b/beacon/light/canonical.go @@ -102,23 +102,13 @@ func (cs *canonicalStore[T]) add(backend ethdb.KeyValueWriter, period uint64, va // deleteFrom removes items starting from the given period. func (cs *canonicalStore[T]) deleteFrom(batch ethdb.Batch, fromPeriod uint64) (deleted Range) { - if fromPeriod >= cs.periods.End { - return - } - if fromPeriod < cs.periods.Start { - fromPeriod = cs.periods.Start - } - deleted = Range{Start: fromPeriod, End: cs.periods.End} - for period := fromPeriod; period < cs.periods.End; period++ { + keepRange, deleteRange := cs.periods.Split(fromPeriod) + deleteRange.Each(func(period uint64) { batch.Delete(cs.databaseKey(period)) cs.cache.Remove(period) - } - if fromPeriod > cs.periods.Start { - cs.periods.End = fromPeriod - } else { - cs.periods = Range{} - } - return + }) + cs.periods = keepRange + return deleteRange } // get returns the item at the given period or the null value of the given type diff --git a/beacon/light/range.go b/beacon/light/range.go index e9427ed3826d..71b140c0699d 100644 --- a/beacon/light/range.go +++ b/beacon/light/range.go @@ -52,3 +52,27 @@ func (a *Range) Expand(period uint64) { a.End++ } } + +// 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) { + if fromPeriod <= a.Start { + // First range empty, everything in second range, + return Range{}, *a + } + if fromPeriod >= a.End { + // Second range empty, everything in first range, + return *a, Range{} + } + x := Range{a.Start, fromPeriod} + y := Range{fromPeriod, a.End} + return x, y +} + +// Each invokes the supplied function fn once per period in range +func (a *Range) Each(fn func(uint64)) { + for p := a.Start; p < a.End; p++ { + fn(p) + } +} From 0bf9f8e4bf7cba4bf36bca8f80a27fb7646cf3f4 Mon Sep 17 00:00:00 2001 From: zsfelfoldi Date: Sat, 2 Dec 2023 03:17:07 +0100 Subject: [PATCH 17/19] beacon/light: addressed review comments --- beacon/light/canonical.go | 61 +++++++++++++++------------------ beacon/light/committee_chain.go | 61 ++++++++++++++++++--------------- beacon/light/range.go | 42 +++++++++++------------ 3 files changed, 83 insertions(+), 81 deletions(-) diff --git a/beacon/light/canonical.go b/beacon/light/canonical.go index 381cfdfda17c..06a0460d2e9c 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 2f833cb39210..bd8bde20ab8f 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 71b140c0699d..76ebe2381ae9 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) } From 3b38b3b8ac0e87a697902964e00bb254961c56a7 Mon Sep 17 00:00:00 2001 From: Zsolt Felfoldi Date: Tue, 5 Dec 2023 04:50:34 +0100 Subject: [PATCH 18/19] beacon/light: always use rlp encoding in canonicalStore --- beacon/light/canonical.go | 15 +++++-------- beacon/light/committee_chain.go | 39 +++------------------------------ 2 files changed, 8 insertions(+), 46 deletions(-) diff --git a/beacon/light/canonical.go b/beacon/light/canonical.go index 06a0460d2e9c..56b8865f9323 100644 --- a/beacon/light/canonical.go +++ b/beacon/light/canonical.go @@ -23,6 +23,7 @@ import ( "github.com/ethereum/go-ethereum/common/lru" "github.com/ethereum/go-ethereum/ethdb" "github.com/ethereum/go-ethereum/log" + "github.com/ethereum/go-ethereum/rlp" ) // canonicalStore stores instances of the given type in a database and caches @@ -33,18 +34,13 @@ type canonicalStore[T any] struct { keyPrefix []byte periods periodRange cache *lru.Cache[uint64, T] - encode func(T) ([]byte, error) - decode func([]byte) (T, error) } // 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.Iteratee, keyPrefix []byte, - encode func(T) ([]byte, error), decode func([]byte) (T, error)) (*canonicalStore[T], error) { +func newCanonicalStore[T any](db ethdb.Iteratee, keyPrefix []byte) (*canonicalStore[T], error) { cs := &canonicalStore[T]{ keyPrefix: keyPrefix, - encode: encode, - decode: decode, cache: lru.NewCache[uint64, T](100), } var ( @@ -81,7 +77,7 @@ func (cs *canonicalStore[T]) add(backend ethdb.KeyValueWriter, period uint64, va 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) + enc, err := rlp.EncodeToBytes(value) if err != nil { return err } @@ -107,7 +103,7 @@ func (cs *canonicalStore[T]) deleteFrom(db ethdb.KeyValueWriter, fromPeriod uint // 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) (T, bool) { - var null T + var null, value T if !cs.periods.contains(period) { return null, false } @@ -119,8 +115,7 @@ func (cs *canonicalStore[T]) get(backend ethdb.KeyValueReader, period uint64) (T log.Error("Canonical store value not found", "period", period, "start", cs.periods.Start, "end", cs.periods.End) return null, false } - value, err := cs.decode(enc) - if err != nil { + if err := rlp.DecodeBytes(enc, &value); err != nil { log.Error("Error decoding canonical store value", "error", err) return null, false } diff --git a/beacon/light/committee_chain.go b/beacon/light/committee_chain.go index bd8bde20ab8f..d707f8cc34da 100644 --- a/beacon/light/committee_chain.go +++ b/beacon/light/committee_chain.go @@ -31,7 +31,6 @@ import ( "github.com/ethereum/go-ethereum/core/rawdb" "github.com/ethereum/go-ethereum/ethdb" "github.com/ethereum/go-ethereum/log" - "github.com/ethereum/go-ethereum/rlp" ) var ( @@ -90,38 +89,6 @@ func NewCommitteeChain(db ethdb.KeyValueStore, config *types.ChainConfig, signer // newCommitteeChain creates a new CommitteeChain with the option of replacing the // clock source and signature verification for testing purposes. func newCommitteeChain(db ethdb.KeyValueStore, config *types.ChainConfig, signerThreshold int, enforceTime bool, sigVerifier committeeSigVerifier, clock mclock.Clock, unixNano func() int64) *CommitteeChain { - var ( - fixedCommitteeRootEncoder = func(root common.Hash) ([]byte, error) { - return root[:], nil - } - fixedCommitteeRootDecoder = func(enc []byte) (root common.Hash, err error) { - if len(enc) != common.HashLength { - return common.Hash{}, errors.New("incorrect length for committee root entry in the database") - } - return common.BytesToHash(enc), nil - } - committeeEncoder = func(committee *types.SerializedSyncCommittee) ([]byte, error) { - return committee[:], nil - } - committeeDecoder = func(enc []byte) (*types.SerializedSyncCommittee, error) { - if len(enc) == types.SerializedSyncCommitteeSize { - committee := new(types.SerializedSyncCommittee) - copy(committee[:], enc) - return committee, nil - } - return nil, errors.New("incorrect length for serialized committee entry in the database") - } - updateEncoder = func(update *types.LightClientUpdate) ([]byte, error) { - return rlp.EncodeToBytes(update) - } - updateDecoder = func(enc []byte) (*types.LightClientUpdate, error) { - update := new(types.LightClientUpdate) - if err := rlp.DecodeBytes(enc, update); err != nil { - return nil, err - } - return update, nil - } - ) s := &CommitteeChain{ committeeCache: lru.NewCache[uint64, syncCommittee](10), db: db, @@ -138,13 +105,13 @@ func newCommitteeChain(db ethdb.KeyValueStore, config *types.ChainConfig, signer } var err1, err2, err3 error - if s.fixedCommitteeRoots, err1 = newCanonicalStore[common.Hash](db, rawdb.FixedCommitteeRootKey, fixedCommitteeRootEncoder, fixedCommitteeRootDecoder); err1 != nil { + if s.fixedCommitteeRoots, err1 = newCanonicalStore[common.Hash](db, rawdb.FixedCommitteeRootKey); 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 { + if s.committees, err2 = newCanonicalStore[*types.SerializedSyncCommittee](db, rawdb.SyncCommitteeKey); err2 != nil { log.Error("Error creating committee store", "error", err2) } - if s.updates, err3 = newCanonicalStore[*types.LightClientUpdate](db, rawdb.BestUpdateKey, updateEncoder, updateDecoder); err3 != nil { + if s.updates, err3 = newCanonicalStore[*types.LightClientUpdate](db, rawdb.BestUpdateKey); err3 != nil { log.Error("Error creating update store", "error", err3) } if err1 != nil || err2 != nil || err3 != nil || !s.checkConstraints() { From 751dc7a8a6d7dcffc18c4b56331ef9b3bb7d350c Mon Sep 17 00:00:00 2001 From: Gary Rong Date: Fri, 8 Dec 2023 13:51:48 +0800 Subject: [PATCH 19/19] beacon/light: fix potential database iterator leaking --- beacon/light/canonical.go | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/beacon/light/canonical.go b/beacon/light/canonical.go index 56b8865f9323..b5371493b4c9 100644 --- a/beacon/light/canonical.go +++ b/beacon/light/canonical.go @@ -48,6 +48,8 @@ func newCanonicalStore[T any](db ethdb.Iteratee, keyPrefix []byte) (*canonicalSt kl = len(keyPrefix) first = true ) + defer iter.Release() + for iter.Next() { if len(iter.Key()) != kl+8 { log.Warn("Invalid key length in the canonical chain database", "key", fmt.Sprintf("%#x", iter.Key())) @@ -57,12 +59,11 @@ func newCanonicalStore[T any](db ethdb.Iteratee, keyPrefix []byte) (*canonicalSt if first { cs.periods.Start = period } else if cs.periods.End != period { - return nil, fmt.Errorf("Gap in the canonical chain database between periods %d and %d", cs.periods.End, period-1) + 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, nil }