Skip to content

Commit

Permalink
native: make newEpochNextValidators always contain non-empty value
Browse files Browse the repository at this point in the history
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 <[email protected]>
  • Loading branch information
AnnaShaleva committed Oct 9, 2023
1 parent d5126e2 commit c8d3fe1
Show file tree
Hide file tree
Showing 2 changed files with 34 additions and 51 deletions.
28 changes: 4 additions & 24 deletions pkg/consensus/consensus.go
Original file line number Diff line number Diff line change
Expand Up @@ -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))
Expand Down Expand Up @@ -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()))
Expand Down
57 changes: 30 additions & 27 deletions pkg/core/native/native_neo.go
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -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`.
Expand Down Expand Up @@ -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)
}

Check warning on line 321 in pkg/core/native/native_neo.go

View check run for this annotation

Codecov / codecov/patch

pkg/core/native/native_neo.go#L320-L321

Added lines #L320 - L321 were not covered by tests
return nil
}

Expand All @@ -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{}
Expand All @@ -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)
}

Check warning on line 354 in pkg/core/native/native_neo.go

View check run for this annotation

Codecov / codecov/patch

pkg/core/native/native_neo.go#L353-L354

Added lines #L353 - L354 were not covered by tests
} 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)
Expand Down Expand Up @@ -418,15 +420,10 @@ func (n *NEO) OnPersist(ic *interop.Context) error {
cache := ic.DAO.GetRWCache(n.ID).(*NeoCache)
// New epoch 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()))
Expand Down Expand Up @@ -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)
}
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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")

Check warning on line 1127 in pkg/core/native/native_neo.go

View check run for this annotation

Codecov / codecov/patch

pkg/core/native/native_neo.go#L1127

Added line #L1127 was not covered by tests
}

Expand Down

0 comments on commit c8d3fe1

Please sign in to comment.