native: make newEpochNextValidators always contain non-empty value

If it's the end of epoch, then it contains the updated validators list recalculated
during the last block's PostPersist. If it's middle of the epoch, then it contains
previously calculated value (value for the previous completed epoch) that is equal
to the current nextValidators cache value.

Signed-off-by: Anna Shaleva <shaleva.ann@nspcc.ru>

pkg/consensus/consensus.go

@@ -683,16 +683,9 @@ func (s *service) getValidators(txes ...block.Transaction) []crypto.PublicKey {
 		pKeys, err = s.Chain.GetNextBlockValidators()
 	} else {
 		// getValidators with non-empty args is used by dbft to fill block's
-		// NextConsensus field, thus should return proper value for NextConsensus.
-		cfg := s.Chain.GetConfig().ProtocolConfiguration
-		if cfg.ShouldUpdateCommitteeAt(s.dbft.Context.BlockIndex) {
-			// Calculate NextConsensus based on the most fresh chain state.
-			pKeys = s.Chain.ComputeNextBlockValidators()
-		} else {
-			// Take the cached validators that are relevant for the current dBFT epoch
-			// to make NextConsensus.
-			pKeys, err = s.Chain.GetNextBlockValidators()
-		}
+		// NextConsensus field, ComputeNextBlockValidators will return proper
+		// value for NextConsensus wrt dBFT epoch start/end.
+		pKeys = s.Chain.ComputeNextBlockValidators()
 	}
 	if err != nil {
 		s.log.Error("error while trying to get validators", zap.Error(err))
@@ -734,20 +727,7 @@ func (s *service) newBlockFromContext(ctx *dbft.Context) block.Block {
 		block.PrevStateRoot = sr.Root
 	}
 
-	var validators keys.PublicKeys
-	var err error
-	cfg := s.Chain.GetConfig().ProtocolConfiguration
-	if cfg.ShouldUpdateCommitteeAt(ctx.BlockIndex) {
-		// Calculate NextConsensus based on the most fresh chain state.
-		validators = s.Chain.ComputeNextBlockValidators()
-	} else {
-		// Take the cached validators that are relevant for the current dBFT epoch
-		// to make NextConsensus.
-		validators, err = s.Chain.GetNextBlockValidators()
-	}
-	if err != nil {
-		s.log.Fatal(fmt.Sprintf("failed to get validators: %s", err.Error()))
-	}
+	var validators = s.Chain.ComputeNextBlockValidators()
 	script, err := smartcontract.CreateDefaultMultiSigRedeemScript(validators)
 	if err != nil {
 		s.log.Fatal(fmt.Sprintf("failed to create multisignature script: %s", err.Error()))

pkg/core/blockchain.go

@@ -2698,9 +2698,12 @@ func (bc *Blockchain) GetCommittee() (keys.PublicKeys, error) {
 }
 
 // ComputeNextBlockValidators returns current validators. Validators list
-// returned from this method may be updated every block (depending on
-// register/unregister/vote calls to NeoToken contract), not only once per
-// (committee size) number of blocks.
+// returned from this method is updated once per CommitteeSize number of blocks.
+// For the last block in the dBFT epoch this method returns the list of validators
+// recalculated from the latest relevant information about NEO votes; in this case
+// list of validators may differ from the one returned by GetNextBlockValidators.
+// For the not-last block of dBFT epoch this method returns the same list as
+// GetNextBlockValidators.
 func (bc *Blockchain) ComputeNextBlockValidators() []*keys.PublicKey {
 	return bc.contracts.NEO.ComputeNextBlockValidators(bc.dao)
 }

pkg/core/native/native_neo.go

@@ -139,9 +139,11 @@ func copyNeoCache(src, dst *NeoCache) {
 	// Can safely omit copying because the new array is created each time
 	// newEpochNextValidators list, nextValidators and committee are updated.
 	dst.nextValidators = src.nextValidators
-	dst.newEpochNextValidators = src.newEpochNextValidators
 	dst.committee = src.committee
 	dst.committeeHash = src.committeeHash
+	dst.newEpochNextValidators = src.newEpochNextValidators
+	dst.newEpochCommittee = src.newEpochCommittee
+	dst.newEpochCommitteeHash = src.newEpochCommitteeHash
 
 	dst.registerPrice = src.registerPrice
 
@@ -281,11 +283,6 @@ func (n *NEO) Initialize(ic *interop.Context) error {
 	cache := &NeoCache{
 		gasPerVoteCache: make(map[string]big.Int),
 		votesChanged:    true,
-		// Will be updated in the last epoch block's PostPersist right before the committee update block.
-		// NeoToken's deployment (and initialization) isn't intended to be performed not-in-genesis block anyway.
-		newEpochNextValidators: nil,
-		newEpochCommittee:      nil,
-		newEpochCommitteeHash:  util.Uint160{},
 	}
 
 	// We need cache to be present in DAO before the subsequent call to `mint`.
@@ -317,6 +314,11 @@ func (n *NEO) Initialize(ic *interop.Context) error {
 	setIntWithKey(n.ID, ic.DAO, []byte{prefixRegisterPrice}, DefaultRegisterPrice)
 	cache.registerPrice = int64(DefaultRegisterPrice)
 
+	var numOfCNs = n.cfg.GetNumOfCNs(ic.Block.Index + 1)
+	err = n.updateCachedNewEpochValues(ic.DAO, cache, ic.BlockHeight(), numOfCNs)
+	if err != nil {
+		return fmt.Errorf("failed to update next block newEpoch* cache: %w", err)
+	}
 	return nil
 }
 
@@ -328,13 +330,6 @@ func (n *NEO) InitializeCache(blockHeight uint32, d *dao.Simple) error {
 	cache := &NeoCache{
 		gasPerVoteCache: make(map[string]big.Int),
 		votesChanged:    true,
-		// If it's a block in the middle of dBFT epoch, then no one must access
-		// these NewEpoch* fields, and they will be updated during the PostPersist of the last
-		// block in the current epoch. If it's the last block of the current epoch,
-		// then update this cache manually below.
-		newEpochNextValidators: nil,
-		newEpochCommittee:      nil,
-		newEpochCommitteeHash:  util.Uint160{},
 	}
 
 	var committee = keysWithVotes{}
@@ -349,14 +344,21 @@ func (n *NEO) InitializeCache(blockHeight uint32, d *dao.Simple) error {
 	cache.gasPerBlock = n.getSortedGASRecordFromDAO(d)
 	cache.registerPrice = getIntWithKey(n.ID, d, []byte{prefixRegisterPrice})
 
-	// Update newEpoch* cache for external users if committee should be
-	// updated in the next block.
+	// Update newEpoch* cache for external users. It holds values for the previous
+	// dBFT epoch if the current one isn't yet finished.
 	if n.cfg.ShouldUpdateCommitteeAt(blockHeight + 1) {
 		var numOfCNs = n.cfg.GetNumOfCNs(blockHeight + 1)
 		err := n.updateCachedNewEpochValues(d, cache, blockHeight, numOfCNs)
 		if err != nil {
 			return fmt.Errorf("failed to update next block newEpoch* cache: %w", err)
 		}
+	} else {
+		// nextValidators, committee and committee hash are filled in by this moment
+		// via n.updateCache call.
+		cache.newEpochNextValidators = cache.nextValidators.Copy()
+		cache.newEpochCommittee = make(keysWithVotes, len(cache.committee))
+		copy(cache.newEpochCommittee, cache.committee)
+		cache.newEpochCommitteeHash = cache.committeeHash
 	}
 
 	d.SetCache(n.ID, cache)
@@ -418,15 +420,10 @@ func (n *NEO) OnPersist(ic *interop.Context) error {
 		cache := ic.DAO.GetRWCache(n.ID).(*NeoCache)
 		// Cached newEpoch* values always have proper value set (either by PostPersist
 		// during the last epoch block handling or by initialization code).
-		oldKeys := cache.nextValidators
-		oldCom := cache.committee
-		if n.cfg.GetNumOfCNs(ic.Block.Index) != len(oldKeys) ||
-			n.cfg.GetCommitteeSize(ic.Block.Index) != len(oldCom) {
-			cache.nextValidators = cache.newEpochNextValidators
-			cache.committee = cache.newEpochCommittee
-			cache.committeeHash = cache.newEpochCommitteeHash
-			cache.votesChanged = false
-		}
+		cache.nextValidators = cache.newEpochNextValidators
+		cache.committee = cache.newEpochCommittee
+		cache.committeeHash = cache.newEpochCommitteeHash
+		cache.votesChanged = false
 
 		// We need to put in storage anyway, as it affects dumps
 		ic.DAO.PutStorageItem(n.ID, prefixCommittee, cache.committee.Bytes(ic.DAO.GetItemCtx()))
@@ -490,7 +487,9 @@ func (n *NEO) PostPersist(ic *interop.Context) error {
 			h        = ic.Block.Index // consider persisting block as stored to get _next_ block newEpochNextValidators
 			numOfCNs = n.cfg.GetNumOfCNs(h + 1)
 		)
-		if cache.newEpochNextValidators == nil || numOfCNs != len(cache.newEpochNextValidators) {
+		if cache.votesChanged ||
+			numOfCNs != len(cache.newEpochNextValidators) ||
+			n.cfg.GetCommitteeSize(h+1) != len(cache.newEpochCommittee) {
 			if !isCacheRW {
 				cache = ic.DAO.GetRWCache(n.ID).(*NeoCache)
 			}
@@ -839,7 +838,9 @@ func (n *NEO) UnregisterCandidateInternal(ic *interop.Context, pub *keys.PublicK
 		return nil
 	}
 	cache := ic.DAO.GetRWCache(n.ID).(*NeoCache)
-	cache.newEpochNextValidators = nil
+	// Not only current committee/validators cache is interested in votesChanged, but also
+	// newEpoch cache, thus, modify votesChanged to update the latter.
+	cache.votesChanged = true
 	c := new(candidate).FromBytes(si)
 	emitEvent := c.Registered
 	c.Registered = false
@@ -1120,7 +1121,9 @@ func (n *NEO) ComputeNextBlockValidators(d *dao.Simple) keys.PublicKeys {
 		return vals.Copy()
 	}
 	// It's a caller's program error to call ComputeNextBlockValidators not having
-	// the right value in lower cache.
+	// the right value in lower cache. With the current scheme of handling
+	// newEpochNextValidators cache this code is expected to be unreachable, but
+	// let's have this panic here just in case.
 	panic("bug: unexpected external call to newEpochNextValidators cache")
 }