From bbbc6805a8261bca51428ae9e96340fc62704b61 Mon Sep 17 00:00:00 2001 From: Anna Shaleva Date: Thu, 2 Nov 2023 17:16:56 +0300 Subject: [PATCH] native: fix NeoToken initialization process Refactored native NeoToken cache scheme introduced in #3110 sometimes requires validators list recalculation during native cache initialization process (when initializing with the existing storage from the block that is preceded each N-th block). To recalculate validators from candidates, native NeoToken needs an access to cached native Policy blocked accounts. By the moment of native Neo initialization, the cache of native Policy is not yet initialized, thus we need a direct DAO access for Policy to handle blocked account check. Close #3181. Signed-off-by: Anna Shaleva --- pkg/core/blockchain_neotest_test.go | 36 +++++++++++++++++++++++++++++ pkg/core/native/policy.go | 29 ++++++++++++++--------- 2 files changed, 54 insertions(+), 11 deletions(-) diff --git a/pkg/core/blockchain_neotest_test.go b/pkg/core/blockchain_neotest_test.go index f95d634b3..1598b7e86 100644 --- a/pkg/core/blockchain_neotest_test.go +++ b/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) { diff --git a/pkg/core/native/policy.go b/pkg/core/native/policy.go index d45972c73..6a044d4b8 100644 --- a/pkg/core/native/policy.go +++ b/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) } @@ -359,7 +366,7 @@ func (p *Policy) unblockAccount(ic *interop.Context, args []stackitem.Item) stac // fee limit. func (p *Policy) CheckPolicy(d *dao.Simple, tx *transaction.Transaction) error { for _, signer := range tx.Signers { - if _, isBlocked := p.isBlockedInternal(d, signer.Account); isBlocked { + if _, isBlocked := p.isBlockedInternal(d.GetROCache(p.ID).(*PolicyCache), signer.Account); isBlocked { return fmt.Errorf("account %s is blocked", signer.Account.StringLE()) } }