From c43cfae24c97fd5645d91b6eb6895419e431a038 Mon Sep 17 00:00:00 2001 From: Anna Shaleva Date: Fri, 17 May 2024 11:45:15 +0300 Subject: [PATCH 1/2] native: fix Policy's IsBlocked behaviour Account is blocked when it's in the Policy's storage, not when it's missing from the Policy storage. Introduced in bbbc6805a8261bca51428ae9e96340fc62704b61. This bug leads to the fact that during native Neo cache initialization at the last block in the dBFT epoch, all candidates accounts are "blocked", and thus, stand-by committee and validators are used in the subsequent new epoch. Close #3424. This bug may lead to the consequences described in #3273, but it needs to be confirmed. Signed-off-by: Anna Shaleva --- pkg/core/blockchain_neotest_test.go | 87 +++++++++++++++++++++++++++++ pkg/core/native/policy.go | 2 +- 2 files changed, 88 insertions(+), 1 deletion(-) diff --git a/pkg/core/blockchain_neotest_test.go b/pkg/core/blockchain_neotest_test.go index e40cc802d..5f70bdf37 100644 --- a/pkg/core/blockchain_neotest_test.go +++ b/pkg/core/blockchain_neotest_test.go @@ -6,6 +6,7 @@ import ( "fmt" "math/big" "path/filepath" + "sort" "strings" "testing" "time" @@ -316,6 +317,92 @@ func TestBlockchain_InitializeNeoCache_Bug3181(t *testing.T) { }) } +// TestBlockchain_InitializeNeoCache_Bug3424 ensures that Neo cache (new epoch +// committee and stand by validators) is properly initialized after node restart +// at the dBFT epoch boundary. +func TestBlockchain_InitializeNeoCache_Bug3424(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) + cfg := e.Chain.GetConfig() + committeeSize := cfg.GetCommitteeSize(0) + validatorsCount := cfg.GetNumOfCNs(0) + + // Stand by committee drives the chain. + standBySorted, err := keys.NewPublicKeysFromStrings(e.Chain.GetConfig().StandbyCommittee) + require.NoError(t, err) + standBySorted = standBySorted[:validatorsCount] + sort.Sort(standBySorted) + pubs := e.Chain.ComputeNextBlockValidators() + require.Equal(t, standBySorted, keys.PublicKeys(pubs)) + + // Move from stand by committee to the elected nodes. + e.ValidatorInvoker(e.NativeHash(t, nativenames.Gas)).Invoke(t, true, "transfer", e.Validator.ScriptHash(), e.CommitteeHash, 100_0000_0000, nil) + neoCommitteeInvoker := e.CommitteeInvoker(e.NativeHash(t, nativenames.Neo)) + neoValidatorsInvoker := neoCommitteeInvoker.WithSigners(neoCommitteeInvoker.Validator) + policyInvoker := neoCommitteeInvoker.CommitteeInvoker(neoCommitteeInvoker.NativeHash(t, nativenames.Policy)) + + advanceChain := func(t *testing.T) { + for int(e.Chain.BlockHeight())%committeeSize != 0 { + neoCommitteeInvoker.AddNewBlock(t) + } + } + advanceChainToEpochBoundary := func(t *testing.T) { + for int(e.Chain.BlockHeight()+1)%committeeSize != 0 { + neoCommitteeInvoker.AddNewBlock(t) + } + } + + // voters vote for candidates. + voters := make([]neotest.Signer, committeeSize+1) + candidates := make([]neotest.Signer, committeeSize+1) + for i := 0; i < committeeSize+1; i++ { + voters[i] = e.NewAccount(t, 10_0000_0000) + candidates[i] = e.NewAccount(t, 2000_0000_0000) // enough for one registration + } + txes := make([]*transaction.Transaction, 0, committeeSize*3) + for i := 0; i < committeeSize+1; i++ { + transferTx := neoValidatorsInvoker.PrepareInvoke(t, "transfer", e.Validator.ScriptHash(), voters[i].(neotest.SingleSigner).Account().PrivateKey().GetScriptHash(), int64(committeeSize+1-i)*1000000, nil) + txes = append(txes, transferTx) + registerTx := neoValidatorsInvoker.WithSigners(candidates[i]).PrepareInvoke(t, "registerCandidate", candidates[i].(neotest.SingleSigner).Account().PublicKey().Bytes()) + txes = append(txes, registerTx) + voteTx := neoValidatorsInvoker.WithSigners(voters[i]).PrepareInvoke(t, "vote", voters[i].(neotest.SingleSigner).Account().PrivateKey().GetScriptHash(), candidates[i].(neotest.SingleSigner).Account().PublicKey().Bytes()) + txes = append(txes, voteTx) + } + txes = append(txes, policyInvoker.PrepareInvoke(t, "blockAccount", candidates[len(candidates)-1].(neotest.SingleSigner).Account().ScriptHash())) + neoValidatorsInvoker.AddNewBlock(t, txes...) + for _, tx := range txes { + e.CheckHalt(t, tx.Hash(), stackitem.Make(true)) // luckily, both `transfer`, `registerCandidate` and `vote` return boolean values + } + + // Ensure validators are properly updated. + advanceChain(t) + pubs = e.Chain.ComputeNextBlockValidators() + sortedCandidates := make(keys.PublicKeys, validatorsCount) + for i := range candidates[:validatorsCount] { + sortedCandidates[i] = candidates[i].(neotest.SingleSigner).Account().PublicKey() + } + sort.Sort(sortedCandidates) + require.EqualValues(t, sortedCandidates, keys.PublicKeys(pubs)) + + // Move to the last block in the epoch and restart the node. + advanceChainToEpochBoundary(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 caldidates policies checks. + bc, _, _, err = chain.NewMultiWithCustomConfigAndStoreNoCheck(t, nil, ps) + require.NoError(t, err) + + pubs = bc.ComputeNextBlockValidators() + require.EqualValues(t, sortedCandidates, keys.PublicKeys(pubs)) +} + // 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. /* diff --git a/pkg/core/native/policy.go b/pkg/core/native/policy.go index 40133e961..0591fe7b4 100644 --- a/pkg/core/native/policy.go +++ b/pkg/core/native/policy.go @@ -319,7 +319,7 @@ func (p *Policy) IsBlocked(dao *dao.Simple, hash util.Uint160) bool { cache := dao.GetROCache(p.ID) if cache == nil { key := append([]byte{blockedAccountPrefix}, hash.BytesBE()...) - return dao.GetStorageItem(p.ID, key) == nil + return dao.GetStorageItem(p.ID, key) != nil } _, isBlocked := p.isBlockedInternal(cache.(*PolicyCache), hash) return isBlocked From 82a7c1bd9c08fae4a26fbe5fed1f81ea7b73ed23 Mon Sep 17 00:00:00 2001 From: Anna Shaleva Date: Fri, 17 May 2024 12:30:33 +0300 Subject: [PATCH 2/2] native: adjust test helper behaviour Supply the account with expected balance. Signed-off-by: Anna Shaleva --- pkg/core/native/native_test/neo_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/core/native/native_test/neo_test.go b/pkg/core/native/native_test/neo_test.go index c3ea676f7..7f9fb924a 100644 --- a/pkg/core/native/native_test/neo_test.go +++ b/pkg/core/native/native_test/neo_test.go @@ -39,7 +39,7 @@ func newNeoCommitteeClient(t *testing.T, expectedGASBalance int) *neotest.Contra e := neotest.NewExecutor(t, bc, validators, committee) if expectedGASBalance > 0 { - e.ValidatorInvoker(e.NativeHash(t, nativenames.Gas)).Invoke(t, true, "transfer", e.Validator.ScriptHash(), e.CommitteeHash, 100_0000_0000, nil) + e.ValidatorInvoker(e.NativeHash(t, nativenames.Gas)).Invoke(t, true, "transfer", e.Validator.ScriptHash(), e.CommitteeHash, expectedGASBalance, nil) } return e.CommitteeInvoker(e.NativeHash(t, nativenames.Neo))