Merge pull request #3187 from nspcc-dev/fix-neo-init

Fix native Neo cache initialization

pkg/core/blockchain.go

@@ -468,6 +468,8 @@ func (bc *Blockchain) init() error {
 	}
 	bc.blockHeight = bHeight
 	bc.persistedHeight = bHeight
+
+	bc.log.Debug("initializing caches", zap.Uint32("blockHeight", bHeight))
 	if err = bc.stateRoot.Init(bHeight); err != nil {
 		return fmt.Errorf("can't init MPT at height %d: %w", bHeight, err)
 	}

pkg/core/blockchain_neotest_test.go

@@ -301,6 +301,42 @@ func TestBlockchain_StartFromExistingDB(t *testing.T) {
 	})
 }
 
+// TestBlockchain_InitializeNeoCache_Bug3181 is aimed to reproduce and check situation
+// when panic occures on native Neo cache initialization due to access to native Policy
+// cache when it's not yet initialized to recalculate candidates.
+func TestBlockchain_InitializeNeoCache_Bug3181(t *testing.T) {
+	ps, path := newLevelDBForTestingWithPath(t, "")
+	bc, validators, committee, err := chain.NewMultiWithCustomConfigAndStoreNoCheck(t, nil, ps)
+	require.NoError(t, err)
+	go bc.Run()
+	e := neotest.NewExecutor(t, bc, validators, committee)
+
+	// Add at least one registered candidate to enable candidates Policy check.
+	acc := e.NewAccount(t, 10000_0000_0000) // block #1
+	neo := e.NewInvoker(e.NativeHash(t, nativenames.Neo), acc)
+	neo.Invoke(t, true, "registerCandidate", acc.(neotest.SingleSigner).Account().PublicKey().Bytes()) // block #2
+
+	// Put some empty blocks to reach N-1 block height, so that newEpoch cached
+	// values of native Neo contract require an update on the subsequent cache
+	// initialization.
+	for i := 0; i < len(bc.GetConfig().StandbyCommittee)-1-2; i++ {
+		e.AddNewBlock(t)
+	}
+	bc.Close() // Ensure persist is done and persistent store is properly closed.
+
+	ps, _ = newLevelDBForTestingWithPath(t, path)
+	t.Cleanup(func() { require.NoError(t, ps.Close()) })
+
+	// Start chain from the existing database that should trigger an update of native
+	// Neo newEpoch* cached values during initializaition. This update requires candidates
+	// list recalculation and policies checks, thus, access to native Policy cache
+	// that is not yet initialized by that moment.
+	require.NotPanics(t, func() {
+		_, _, _, err = chain.NewMultiWithCustomConfigAndStoreNoCheck(t, nil, ps)
+		require.NoError(t, err)
+	})
+}
+
 // This test enables Notary native contract at non-zero height and checks that no
 // Notary cache initialization is performed before that height on node restart.
 func TestBlockchain_InitializeNativeCacheWrtNativeActivations(t *testing.T) {

pkg/core/dao/dao.go

@@ -970,7 +970,7 @@ func (dao *Simple) GetRWCache(id int32) NativeContractCache {
 func (dao *Simple) getCache(k int32, ro bool) NativeContractCache {
 	if itm, ok := dao.nativeCache[k]; ok {
 		// Don't need to create itm copy, because its value was already copied
-		// the first time it was retrieved from loser ps.
+		// the first time it was retrieved from lower ps.
 		return itm
 	}
 

pkg/core/native/policy.go

@@ -241,26 +241,33 @@ func (p *Policy) setExecFeeFactor(ic *interop.Context, args []stackitem.Item) st
 // isBlocked is Policy contract method that checks whether provided account is blocked.
 func (p *Policy) isBlocked(ic *interop.Context, args []stackitem.Item) stackitem.Item {
 	hash := toUint160(args[0])
-	_, blocked := p.isBlockedInternal(ic.DAO, hash)
+	_, blocked := p.isBlockedInternal(ic.DAO.GetROCache(p.ID).(*PolicyCache), hash)
 	return stackitem.NewBool(blocked)
 }
 
-// IsBlocked checks whether provided account is blocked.
+// IsBlocked checks whether provided account is blocked. Normally it uses Policy
+// cache, falling back to the DB queries when Policy cache is not available yet
+// (the only case is native cache initialization pipeline, where native Neo cache
+// is being initialized before the Policy's one).
 func (p *Policy) IsBlocked(dao *dao.Simple, hash util.Uint160) bool {
-	_, isBlocked := p.isBlockedInternal(dao, hash)
+	cache := dao.GetROCache(p.ID)
+	if cache == nil {
+		key := append([]byte{blockedAccountPrefix}, hash.BytesBE()...)
+		return dao.GetStorageItem(p.ID, key) == nil
+	}
+	_, isBlocked := p.isBlockedInternal(cache.(*PolicyCache), hash)
 	return isBlocked
 }
 
 // isBlockedInternal checks whether provided account is blocked. It returns position
 // of the blocked account in the blocked accounts list (or the position it should be
 // put at).
-func (p *Policy) isBlockedInternal(dao *dao.Simple, hash util.Uint160) (int, bool) {
-	cache := dao.GetROCache(p.ID).(*PolicyCache)
-	length := len(cache.blockedAccounts)
+func (p *Policy) isBlockedInternal(roCache *PolicyCache, hash util.Uint160) (int, bool) {
+	length := len(roCache.blockedAccounts)
 	i := sort.Search(length, func(i int) bool {
-		return !cache.blockedAccounts[i].Less(hash)
+		return !roCache.blockedAccounts[i].Less(hash)
 	})
-	if length != 0 && i != length && cache.blockedAccounts[i].Equals(hash) {
+	if length != 0 && i != length && roCache.blockedAccounts[i].Equals(hash) {
 		return i, true
 	}
 	return i, false
@@ -320,7 +327,7 @@ func (p *Policy) blockAccount(ic *interop.Context, args []stackitem.Item) stacki
 	return stackitem.NewBool(p.blockAccountInternal(ic.DAO, hash))
 }
 func (p *Policy) blockAccountInternal(d *dao.Simple, hash util.Uint160) bool {
-	i, blocked := p.isBlockedInternal(d, hash)
+	i, blocked := p.isBlockedInternal(d.GetROCache(p.ID).(*PolicyCache), hash)
 	if blocked {
 		return false
 	}
@@ -343,7 +350,7 @@ func (p *Policy) unblockAccount(ic *interop.Context, args []stackitem.Item) stac
 		panic("invalid committee signature")
 	}
 	hash := toUint160(args[0])
-	i, blocked := p.isBlockedInternal(ic.DAO, hash)
+	i, blocked := p.isBlockedInternal(ic.DAO.GetROCache(p.ID).(*PolicyCache), hash)
 	if !blocked {
 		return stackitem.NewBool(false)
 	}
@@ -358,8 +365,9 @@ func (p *Policy) unblockAccount(ic *interop.Context, args []stackitem.Item) stac
 // like not being signed by a blocked account or not exceeding the block-level system
 // fee limit.
 func (p *Policy) CheckPolicy(d *dao.Simple, tx *transaction.Transaction) error {
+	cache := d.GetROCache(p.ID).(*PolicyCache)
 	for _, signer := range tx.Signers {
-		if _, isBlocked := p.isBlockedInternal(d, signer.Account); isBlocked {
+		if _, isBlocked := p.isBlockedInternal(cache, signer.Account); isBlocked {
 			return fmt.Errorf("account %s is blocked", signer.Account.StringLE())
 		}
 	}