From 33c971b0e4244685edb67bb447bda766f0e436fe Mon Sep 17 00:00:00 2001 From: Anna Shaleva Date: Wed, 26 Apr 2023 12:52:59 +0300 Subject: [PATCH 1/3] core: add InitializeCache method to Contract interface Make the contracts cache initialization unified. The order of cache iniitialization is not important and Nottary contract is added to the bc.contracts.Contracts wrt P2PSigExtensions setting, thus no functional changes, just refactoring for future applications. Signed-off-by: Anna Shaleva --- pkg/core/blockchain.go | 23 +++-------------------- pkg/core/interop/context.go | 5 +++++ pkg/core/native/crypto.go | 6 ++++++ pkg/core/native/designate.go | 2 +- pkg/core/native/ledger.go | 5 +++++ pkg/core/native/management.go | 2 +- pkg/core/native/management_test.go | 4 ++-- pkg/core/native/native_gas.go | 5 +++++ pkg/core/native/native_neo.go | 3 ++- pkg/core/native/notary.go | 2 +- pkg/core/native/oracle.go | 3 ++- pkg/core/native/policy.go | 2 +- pkg/core/native/std.go | 6 ++++++ 13 files changed, 40 insertions(+), 28 deletions(-) diff --git a/pkg/core/blockchain.go b/pkg/core/blockchain.go index 56ca5ed74..a70f7bb3a 100644 --- a/pkg/core/blockchain.go +++ b/pkg/core/blockchain.go @@ -1011,29 +1011,12 @@ func (bc *Blockchain) resetStateInternal(height uint32, stage stateChangeStage) } func (bc *Blockchain) initializeNativeCache(blockHeight uint32, d *dao.Simple) error { - err := bc.contracts.NEO.InitializeCache(blockHeight, d) - if err != nil { - return fmt.Errorf("can't init cache for NEO native contract: %w", err) - } - err = bc.contracts.Management.InitializeCache(d) - if err != nil { - return fmt.Errorf("can't init cache for Management native contract: %w", err) - } - err = bc.contracts.Designate.InitializeCache(d) - if err != nil { - return fmt.Errorf("can't init cache for Designation native contract: %w", err) - } - bc.contracts.Oracle.InitializeCache(d) - if bc.P2PSigExtensionsEnabled() { - err = bc.contracts.Notary.InitializeCache(d) + for _, c := range bc.contracts.Contracts { + err := c.InitializeCache(blockHeight, d) if err != nil { - return fmt.Errorf("can't init cache for Notary native contract: %w", err) + return fmt.Errorf("failed to initialize cache for %s: %w", c.Metadata().Name, err) } } - err = bc.contracts.Policy.InitializeCache(d) - if err != nil { - return fmt.Errorf("can't init cache for Policy native contract: %w", err) - } return nil } diff --git a/pkg/core/interop/context.go b/pkg/core/interop/context.go index 1d1448014..9a8270aab 100644 --- a/pkg/core/interop/context.go +++ b/pkg/core/interop/context.go @@ -153,6 +153,11 @@ type MethodAndPrice struct { // Contract is an interface for all native contracts. type Contract interface { Initialize(*Context) error + // InitializeCache aimed to initialize contract's cache when the contract has + // been deployed, but in-memory cached data were lost due to the node reset. + // It should be called each time after node restart iff the contract was + // deployed and no Initialize method was called. + InitializeCache(blockHeight uint32, d *dao.Simple) error Metadata() *ContractMD OnPersist(*Context) error PostPersist(*Context) error diff --git a/pkg/core/native/crypto.go b/pkg/core/native/crypto.go index 323a8ed21..2d8afc524 100644 --- a/pkg/core/native/crypto.go +++ b/pkg/core/native/crypto.go @@ -10,6 +10,7 @@ import ( bls12381 "github.com/consensys/gnark-crypto/ecc/bls12-381" "github.com/consensys/gnark-crypto/ecc/bls12-381/fr" "github.com/decred/dcrd/dcrec/secp256k1/v4" + "github.com/nspcc-dev/neo-go/pkg/core/dao" "github.com/nspcc-dev/neo-go/pkg/core/interop" "github.com/nspcc-dev/neo-go/pkg/core/native/nativenames" "github.com/nspcc-dev/neo-go/pkg/crypto/hash" @@ -467,6 +468,11 @@ func (c *Crypto) Initialize(ic *interop.Context) error { return nil } +// InitializeCache implements the Contract interface. +func (c *Crypto) InitializeCache(blockHeight uint32, d *dao.Simple) error { + return nil +} + // OnPersist implements the Contract interface. func (c *Crypto) OnPersist(ic *interop.Context) error { return nil diff --git a/pkg/core/native/designate.go b/pkg/core/native/designate.go index caf587ccf..12d8fd79a 100644 --- a/pkg/core/native/designate.go +++ b/pkg/core/native/designate.go @@ -132,7 +132,7 @@ func (s *Designate) Initialize(ic *interop.Context) error { // InitializeCache fills native Designate cache from DAO. It is called at non-zero height, thus // we can fetch the roles data right from the storage. -func (s *Designate) InitializeCache(d *dao.Simple) error { +func (s *Designate) InitializeCache(blockHeight uint32, d *dao.Simple) error { cache := &DesignationCache{} roles := []noderoles.Role{noderoles.Oracle, noderoles.NeoFSAlphabet, noderoles.StateValidator} if s.p2pSigExtensionsEnabled { diff --git a/pkg/core/native/ledger.go b/pkg/core/native/ledger.go index 51734c226..ba21a3487 100644 --- a/pkg/core/native/ledger.go +++ b/pkg/core/native/ledger.go @@ -85,6 +85,11 @@ func (l *Ledger) Initialize(ic *interop.Context) error { return nil } +// InitializeCache implements the Contract interface. +func (l *Ledger) InitializeCache(blockHeight uint32, d *dao.Simple) error { + return nil +} + // OnPersist implements the Contract interface. func (l *Ledger) OnPersist(ic *interop.Context) error { // Actual block/tx processing is done in Blockchain.storeBlock(). diff --git a/pkg/core/native/management.go b/pkg/core/native/management.go index e177edefb..f78d4080e 100644 --- a/pkg/core/native/management.go +++ b/pkg/core/native/management.go @@ -610,7 +610,7 @@ func (m *Management) OnPersist(ic *interop.Context) error { // InitializeCache initializes contract cache with the proper values from storage. // Cache initialization should be done apart from Initialize because Initialize is // called only when deploying native contracts. -func (m *Management) InitializeCache(d *dao.Simple) error { +func (m *Management) InitializeCache(blockHeight uint32, d *dao.Simple) error { cache := &ManagementCache{ contracts: make(map[util.Uint160]*state.Contract), nep11: make(map[util.Uint160]struct{}), diff --git a/pkg/core/native/management_test.go b/pkg/core/native/management_test.go index a4cb7bc87..cc8e1e1f8 100644 --- a/pkg/core/native/management_test.go +++ b/pkg/core/native/management_test.go @@ -82,7 +82,7 @@ func TestManagement_Initialize(t *testing.T) { t.Run("good", func(t *testing.T) { d := dao.NewSimple(storage.NewMemoryStore(), false, false) mgmt := newManagement() - require.NoError(t, mgmt.InitializeCache(d)) + require.NoError(t, mgmt.InitializeCache(0, d)) }) /* See #2801 t.Run("invalid contract state", func(t *testing.T) { @@ -101,7 +101,7 @@ func TestManagement_GetNEP17Contracts(t *testing.T) { err := mgmt.Initialize(&interop.Context{DAO: d}) require.NoError(t, err) require.NoError(t, mgmt.Policy.Initialize(&interop.Context{DAO: d})) - err = mgmt.InitializeCache(d) + err = mgmt.InitializeCache(0, d) require.NoError(t, err) require.Empty(t, mgmt.GetNEP17Contracts(d)) diff --git a/pkg/core/native/native_gas.go b/pkg/core/native/native_gas.go index e60f3f959..7276572b6 100644 --- a/pkg/core/native/native_gas.go +++ b/pkg/core/native/native_gas.go @@ -99,6 +99,11 @@ func (g *GAS) Initialize(ic *interop.Context) error { return nil } +// InitializeCache implements the Contract interface. +func (g *GAS) InitializeCache(blockHeight uint32, d *dao.Simple) error { + return nil +} + // OnPersist implements the Contract interface. func (g *GAS) OnPersist(ic *interop.Context) error { if len(ic.Block.Transactions) == 0 { diff --git a/pkg/core/native/native_neo.go b/pkg/core/native/native_neo.go index 844d27361..6c8742dbb 100644 --- a/pkg/core/native/native_neo.go +++ b/pkg/core/native/native_neo.go @@ -305,7 +305,8 @@ func (n *NEO) Initialize(ic *interop.Context) error { // InitializeCache initializes all NEO cache with the proper values from the storage. // Cache initialization should be done apart from Initialize because Initialize is -// called only when deploying native contracts. +// called only when deploying native contracts. InitializeCache implements the Contract +// interface. func (n *NEO) InitializeCache(blockHeight uint32, d *dao.Simple) error { cache := &NeoCache{ gasPerVoteCache: make(map[string]big.Int), diff --git a/pkg/core/native/notary.go b/pkg/core/native/notary.go index 93f2af788..f58e628a7 100644 --- a/pkg/core/native/notary.go +++ b/pkg/core/native/notary.go @@ -152,7 +152,7 @@ func (n *Notary) Initialize(ic *interop.Context) error { return nil } -func (n *Notary) InitializeCache(d *dao.Simple) error { +func (n *Notary) InitializeCache(blockHeight uint32, d *dao.Simple) error { cache := &NotaryCache{ maxNotValidBeforeDelta: uint32(getIntWithKey(n.ID, d, maxNotValidBeforeDeltaKey)), notaryServiceFeePerKey: getIntWithKey(n.ID, d, notaryServiceFeeKey), diff --git a/pkg/core/native/oracle.go b/pkg/core/native/oracle.go index 0189f4fd3..ef943966e 100644 --- a/pkg/core/native/oracle.go +++ b/pkg/core/native/oracle.go @@ -251,10 +251,11 @@ func (o *Oracle) Initialize(ic *interop.Context) error { return nil } -func (o *Oracle) InitializeCache(d *dao.Simple) { +func (o *Oracle) InitializeCache(blockHeight uint32, d *dao.Simple) error { cache := &OracleCache{} cache.requestPrice = getIntWithKey(o.ID, d, prefixRequestPrice) d.SetCache(o.ID, cache) + return nil } func getResponse(tx *transaction.Transaction) *transaction.OracleResponse { diff --git a/pkg/core/native/policy.go b/pkg/core/native/policy.go index 570dfc40a..d45972c73 100644 --- a/pkg/core/native/policy.go +++ b/pkg/core/native/policy.go @@ -153,7 +153,7 @@ func (p *Policy) Initialize(ic *interop.Context) error { return nil } -func (p *Policy) InitializeCache(d *dao.Simple) error { +func (p *Policy) InitializeCache(blockHeight uint32, d *dao.Simple) error { cache := &PolicyCache{} err := p.fillCacheFromDAO(cache, d) if err != nil { diff --git a/pkg/core/native/std.go b/pkg/core/native/std.go index 733fd7338..18dabccc5 100644 --- a/pkg/core/native/std.go +++ b/pkg/core/native/std.go @@ -9,6 +9,7 @@ import ( "strings" "github.com/mr-tron/base58" + "github.com/nspcc-dev/neo-go/pkg/core/dao" "github.com/nspcc-dev/neo-go/pkg/core/interop" "github.com/nspcc-dev/neo-go/pkg/core/native/nativenames" base58neogo "github.com/nspcc-dev/neo-go/pkg/encoding/base58" @@ -429,6 +430,11 @@ func (s *Std) Initialize(ic *interop.Context) error { return nil } +// InitializeCache implements the Contract interface. +func (s *Std) InitializeCache(blockHeight uint32, d *dao.Simple) error { + return nil +} + // OnPersist implements the Contract interface. func (s *Std) OnPersist(ic *interop.Context) error { return nil From edb2d46d5b7dfdd05597f90f5da91fa7fe966253 Mon Sep 17 00:00:00 2001 From: Anna Shaleva Date: Wed, 26 Apr 2023 13:22:13 +0300 Subject: [PATCH 2/3] core: initialize natives cache wrt NativeActivations If the contract was deployed then cache must be initialized after in-memory data reset. If the contract isn't active yet, then no cache will be initialized on deploy (i.e. on call to Initialize() method by native Management). Close #2984. Signed-off-by: Anna Shaleva --- pkg/core/blockchain.go | 11 +++-- pkg/core/blockchain_neotest_test.go | 65 +++++++++++++++++++++++++++++ 2 files changed, 73 insertions(+), 3 deletions(-) diff --git a/pkg/core/blockchain.go b/pkg/core/blockchain.go index a70f7bb3a..064453ca5 100644 --- a/pkg/core/blockchain.go +++ b/pkg/core/blockchain.go @@ -1012,9 +1012,14 @@ func (bc *Blockchain) resetStateInternal(height uint32, stage stateChangeStage) func (bc *Blockchain) initializeNativeCache(blockHeight uint32, d *dao.Simple) error { for _, c := range bc.contracts.Contracts { - err := c.InitializeCache(blockHeight, d) - if err != nil { - return fmt.Errorf("failed to initialize cache for %s: %w", c.Metadata().Name, err) + for _, h := range c.Metadata().UpdateHistory { + if blockHeight >= h { // check that contract was deployed. + err := c.InitializeCache(blockHeight, d) + if err != nil { + return fmt.Errorf("failed to initialize cache for %s: %w", c.Metadata().Name, err) + } + break + } } } return nil diff --git a/pkg/core/blockchain_neotest_test.go b/pkg/core/blockchain_neotest_test.go index 8a99ad04d..f5e2edcbf 100644 --- a/pkg/core/blockchain_neotest_test.go +++ b/pkg/core/blockchain_neotest_test.go @@ -299,6 +299,71 @@ func TestBlockchain_StartFromExistingDB(t *testing.T) { }) } +// 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) { + const notaryEnabledHeight = 3 + ps, path := newLevelDBForTestingWithPath(t, "") + customConfig := func(c *config.Blockchain) { + c.P2PSigExtensions = true + c.NativeUpdateHistories = make(map[string][]uint32) + for _, n := range []string{ + nativenames.Neo, + nativenames.Gas, + nativenames.Designation, + nativenames.Management, + nativenames.CryptoLib, + nativenames.Ledger, + nativenames.Management, + nativenames.Oracle, + nativenames.Policy, + nativenames.StdLib, + nativenames.Notary, + } { + if n == nativenames.Notary { + c.NativeUpdateHistories[n] = []uint32{notaryEnabledHeight} + } else { + c.NativeUpdateHistories[n] = []uint32{0} + } + } + } + bc, validators, committee, err := chain.NewMultiWithCustomConfigAndStoreNoCheck(t, customConfig, ps) + require.NoError(t, err) + go bc.Run() + e := neotest.NewExecutor(t, bc, validators, committee) + e.AddNewBlock(t) + bc.Close() // Ensure persist is done and persistent store is properly closed. + + ps, _ = newLevelDBForTestingWithPath(t, path) + + // If NativeActivations are not taken into account during native cache initialization, + // bs.init() will panic on Notary cache initialization as it's not deployed yet. + require.NotPanics(t, func() { + bc, _, _, err = chain.NewMultiWithCustomConfigAndStoreNoCheck(t, customConfig, ps) + require.NoError(t, err) + }) + go bc.Run() + defer bc.Close() + e = neotest.NewExecutor(t, bc, validators, committee) + h := e.Chain.BlockHeight() + + // Notary isn't initialized yet, so accessing Notary cache should panic. + require.Panics(t, func() { + _ = e.Chain.GetMaxNotValidBeforeDelta() + }) + + // Ensure Notary will be properly initialized and accessing Notary cache works + // as expected. + for i := 0; i < notaryEnabledHeight; i++ { + require.NotPanics(t, func() { + e.AddNewBlock(t) + }, h+uint32(i)+1) + } + require.NotPanics(t, func() { + _ = e.Chain.GetMaxNotValidBeforeDelta() + }) +} + func TestBlockchain_AddHeaders(t *testing.T) { bc, acc := chain.NewSingleWithCustomConfig(t, func(c *config.Blockchain) { c.StateRootInHeader = true From 67d4d891ef847a8979fce36a37073ff4d74316b6 Mon Sep 17 00:00:00 2001 From: Anna Shaleva Date: Wed, 26 Apr 2023 14:10:12 +0300 Subject: [PATCH 3/3] core: prevent direct access to Notary contract if not active Otherwise it will cause panic, which isn't expected behaviour. Signed-off-by: Anna Shaleva --- pkg/core/blockchain.go | 14 ++++++++++---- pkg/core/blockchain_neotest_test.go | 20 +++++++++++--------- 2 files changed, 21 insertions(+), 13 deletions(-) diff --git a/pkg/core/blockchain.go b/pkg/core/blockchain.go index 064453ca5..67c6f8b9e 100644 --- a/pkg/core/blockchain.go +++ b/pkg/core/blockchain.go @@ -2567,7 +2567,10 @@ func (bc *Blockchain) verifyTxAttributes(d *dao.Simple, tx *transaction.Transact nvb := tx.Attributes[i].Value.(*transaction.NotValidBefore).Height curHeight := bc.BlockHeight() if isPartialTx { - maxNVBDelta := bc.contracts.Notary.GetMaxNotValidBeforeDelta(bc.dao) + maxNVBDelta, err := bc.GetMaxNotValidBeforeDelta() + if err != nil { + return fmt.Errorf("%w: failed to retrieve MaxNotValidBeforeDelta value from native Notary contract: %v", ErrInvalidAttribute, err) + } if curHeight+maxNVBDelta < nvb { return fmt.Errorf("%w: NotValidBefore (%d) bigger than MaxNVBDelta (%d) allows at height %d", ErrInvalidAttribute, nvb, maxNVBDelta, curHeight) } @@ -2972,11 +2975,14 @@ func (bc *Blockchain) GetMaxVerificationGAS() int64 { } // GetMaxNotValidBeforeDelta returns maximum NotValidBeforeDelta Notary limit. -func (bc *Blockchain) GetMaxNotValidBeforeDelta() uint32 { +func (bc *Blockchain) GetMaxNotValidBeforeDelta() (uint32, error) { if !bc.config.P2PSigExtensions { - panic("disallowed call to Notary") + panic("disallowed call to Notary") // critical error, thus panic. } - return bc.contracts.Notary.GetMaxNotValidBeforeDelta(bc.dao) + if bc.contracts.Notary.Metadata().UpdateHistory[0] > bc.BlockHeight() { + return 0, fmt.Errorf("native Notary is active starting from %d", bc.contracts.Notary.Metadata().UpdateHistory[0]) + } + return bc.contracts.Notary.GetMaxNotValidBeforeDelta(bc.dao), nil } // GetStoragePrice returns current storage price. diff --git a/pkg/core/blockchain_neotest_test.go b/pkg/core/blockchain_neotest_test.go index f5e2edcbf..3e64fadd5 100644 --- a/pkg/core/blockchain_neotest_test.go +++ b/pkg/core/blockchain_neotest_test.go @@ -347,10 +347,9 @@ func TestBlockchain_InitializeNativeCacheWrtNativeActivations(t *testing.T) { e = neotest.NewExecutor(t, bc, validators, committee) h := e.Chain.BlockHeight() - // Notary isn't initialized yet, so accessing Notary cache should panic. - require.Panics(t, func() { - _ = e.Chain.GetMaxNotValidBeforeDelta() - }) + // Notary isn't initialized yet, so accessing Notary cache should return error. + _, err = e.Chain.GetMaxNotValidBeforeDelta() + require.Error(t, err) // Ensure Notary will be properly initialized and accessing Notary cache works // as expected. @@ -359,9 +358,8 @@ func TestBlockchain_InitializeNativeCacheWrtNativeActivations(t *testing.T) { e.AddNewBlock(t) }, h+uint32(i)+1) } - require.NotPanics(t, func() { - _ = e.Chain.GetMaxNotValidBeforeDelta() - }) + _, err = e.Chain.GetMaxNotValidBeforeDelta() + require.NoError(t, err) } func TestBlockchain_AddHeaders(t *testing.T) { @@ -1949,11 +1947,15 @@ func TestBlockchain_VerifyTx(t *testing.T) { require.Error(t, bc.PoolTxWithData(tx, 5, mp, bc, verificationF)) }) t.Run("bad NVB: too big", func(t *testing.T) { - tx := getPartiallyFilledTx(bc.BlockHeight()+bc.GetMaxNotValidBeforeDelta()+1, bc.BlockHeight()+1) + maxNVB, err := bc.GetMaxNotValidBeforeDelta() + require.NoError(t, err) + tx := getPartiallyFilledTx(bc.BlockHeight()+maxNVB+1, bc.BlockHeight()+1) require.True(t, errors.Is(bc.PoolTxWithData(tx, 5, mp, bc, verificationF), core.ErrInvalidAttribute)) }) t.Run("bad ValidUntilBlock: too small", func(t *testing.T) { - tx := getPartiallyFilledTx(bc.BlockHeight(), bc.BlockHeight()+bc.GetMaxNotValidBeforeDelta()+1) + maxNVB, err := bc.GetMaxNotValidBeforeDelta() + require.NoError(t, err) + tx := getPartiallyFilledTx(bc.BlockHeight(), bc.BlockHeight()+maxNVB+1) require.True(t, errors.Is(bc.PoolTxWithData(tx, 5, mp, bc, verificationF), core.ErrInvalidAttribute)) }) t.Run("good", func(t *testing.T) {