From c0b490c7bf81b31aca6a5eda5e81cbe7faba8968 Mon Sep 17 00:00:00 2001 From: Anna Shaleva Date: Tue, 19 Apr 2022 17:12:03 +0300 Subject: [PATCH] core: keep Management cache always valid and up-to-date --- pkg/core/native/management.go | 56 +++++++------------ pkg/core/native/management_test.go | 36 +++++++++--- .../native/native_test/management_test.go | 39 +++++++++++++ 3 files changed, 85 insertions(+), 46 deletions(-) diff --git a/pkg/core/native/management.go b/pkg/core/native/management.go index 81b79d7bf..2d5cc7cfd 100644 --- a/pkg/core/native/management.go +++ b/pkg/core/native/management.go @@ -174,20 +174,8 @@ func (m *Management) GetContract(d *dao.Simple, hash util.Uint160) (*state.Contr cs, ok := cache.contracts[hash] if !ok { return nil, storage.ErrKeyNotFound - } else if cs != nil { - return cs, nil } - return m.getContractFromDAO(d, hash) -} - -func (m *Management) getContractFromDAO(d *dao.Simple, hash util.Uint160) (*state.Contract, error) { - contract := new(state.Contract) - key := MakeContractKey(hash) - err := getConvertibleFromDAO(m.ID, d, key, contract) - if err != nil { - return nil, err - } - return contract, nil + return cs, nil } func getLimitedSlice(arg stackitem.Item, max int) ([]byte, error) { @@ -283,10 +271,15 @@ func (m *Management) deployWithData(ic *interop.Context, args []stackitem.Item) return contractToStack(newcontract) } -func (m *Management) markUpdated(d *dao.Simple, h util.Uint160) { +func (m *Management) markUpdated(d *dao.Simple, hash util.Uint160, cs *state.Contract) { cache := d.Store.GetRWCache(m.ID).(*ManagementCache) - // Just set it to nil, to refresh cache in `PostPersist`. - cache.contracts[h] = nil + delete(cache.nep11, hash) + delete(cache.nep17, hash) + if cs == nil { + delete(cache.contracts, hash) + return + } + updateContractCache(cache, cs) } // Deploy creates contract's hash/ID and saves new contract into the given DAO. @@ -322,7 +315,6 @@ func (m *Management) Deploy(d *dao.Simple, sender util.Uint160, neff *nef.File, if err != nil { return nil, err } - m.markUpdated(d, newcontract.Hash) return newcontract, nil } @@ -362,7 +354,6 @@ func (m *Management) Update(d *dao.Simple, hash util.Uint160, neff *nef.File, ma contract = *oldcontract // Make a copy, don't ruin (potentially) cached contract. // if NEF was provided, update the contract script if neff != nil { - m.markUpdated(d, hash) contract.NEF = *neff } // if manifest was provided, update the contract manifest @@ -374,7 +365,6 @@ func (m *Management) Update(d *dao.Simple, hash util.Uint160, neff *nef.File, ma if err != nil { return nil, fmt.Errorf("invalid manifest: %w", err) } - m.markUpdated(d, hash) contract.Manifest = *manif } err = checkScriptAndMethods(contract.NEF.Script, contract.Manifest.ABI.Methods) @@ -415,7 +405,7 @@ func (m *Management) Destroy(d *dao.Simple, hash util.Uint160) error { d.DeleteStorageItem(contract.ID, k) return true }) - m.markUpdated(d, hash) + m.markUpdated(d, hash, nil) return nil } @@ -492,7 +482,7 @@ func (m *Management) OnPersist(ic *interop.Context) error { if err := native.Initialize(ic); err != nil { return fmt.Errorf("initializing %s native contract: %w", md.Name, err) } - err := m.PutContractState(ic.DAO, cs) + err := m.putContractState(ic.DAO, cs, false) // Perform cache update manually. if err != nil { return err } @@ -534,21 +524,6 @@ func (m *Management) InitializeCache(d *dao.Simple) error { // PostPersist implements Contract interface. func (m *Management) PostPersist(ic *interop.Context) error { - cache := ic.DAO.Store.GetRWCache(m.ID).(*ManagementCache) - for h, cs := range cache.contracts { - if cs != nil { - continue - } - delete(cache.nep11, h) - delete(cache.nep17, h) - newCs, err := m.getContractFromDAO(ic.DAO, h) - if err != nil { - // Contract was destroyed. - delete(cache.contracts, h) - continue - } - updateContractCache(cache, newCs) - } return nil } @@ -592,11 +567,18 @@ func (m *Management) Initialize(ic *interop.Context) error { // PutContractState saves given contract state into given DAO. func (m *Management) PutContractState(d *dao.Simple, cs *state.Contract) error { + return m.putContractState(d, cs, true) +} + +// putContractState is an internal PutContractState representation. +func (m *Management) putContractState(d *dao.Simple, cs *state.Contract, updateCache bool) error { key := MakeContractKey(cs.Hash) if err := putConvertibleToDAO(m.ID, d, key, cs); err != nil { return err } - m.markUpdated(d, cs.Hash) + if updateCache { + m.markUpdated(d, cs.Hash, cs) + } if cs.UpdateCounter != 0 { // Update. return nil } diff --git a/pkg/core/native/management_test.go b/pkg/core/native/management_test.go index 696a97a81..bb669a961 100644 --- a/pkg/core/native/management_test.go +++ b/pkg/core/native/management_test.go @@ -93,6 +93,7 @@ func TestManagement_GetNEP17Contracts(t *testing.T) { require.NoError(t, err) require.Empty(t, mgmt.GetNEP17Contracts(d)) + private := d.GetPrivate() // Deploy NEP-17 contract script := []byte{byte(opcode.RET)} @@ -106,29 +107,46 @@ func TestManagement_GetNEP17Contracts(t *testing.T) { Parameters: []manifest.Parameter{}, }) manif.SupportedStandards = []string{manifest.NEP17StandardName} - c1, err := mgmt.Deploy(d, sender, ne, manif) + c1, err := mgmt.Deploy(private, sender, ne, manif) require.NoError(t, err) - // PostPersist is not yet called, thus no NEP-17 contracts are expected + // c1 contract hash should be returned, as private DAO already contains changed cache. + require.Equal(t, []util.Uint160{c1.Hash}, mgmt.GetNEP17Contracts(private)) + + // Lower DAO still shouldn't contain c1, as no Persist was called. require.Empty(t, mgmt.GetNEP17Contracts(d)) - // Call PostPersist, check c1 contract hash is returned - require.NoError(t, mgmt.PostPersist(&interop.Context{DAO: d})) + // Call Persist, check c1 contract hash is returned + _, err = private.Persist() + require.NoError(t, err) require.Equal(t, []util.Uint160{c1.Hash}, mgmt.GetNEP17Contracts(d)) // Update contract + private = d.GetPrivate() manif.ABI.Methods = append(manif.ABI.Methods, manifest.Method{ Name: "dummy2", ReturnType: smartcontract.VoidType, Parameters: []manifest.Parameter{}, }) - c2, err := mgmt.Update(d, c1.Hash, ne, manif) + c1Updated, err := mgmt.Update(private, c1.Hash, ne, manif) require.NoError(t, err) + require.Equal(t, c1.Hash, c1Updated.Hash) - // No changes expected before PostPersist call. + // No changes expected in lower store. require.Equal(t, []util.Uint160{c1.Hash}, mgmt.GetNEP17Contracts(d)) + c1Lower, err := mgmt.GetContract(d, c1.Hash) + require.NoError(t, err) + require.Equal(t, 1, len(c1Lower.Manifest.ABI.Methods)) + require.Equal(t, []util.Uint160{c1Updated.Hash}, mgmt.GetNEP17Contracts(private)) + c1Upper, err := mgmt.GetContract(private, c1Updated.Hash) + require.NoError(t, err) + require.Equal(t, 2, len(c1Upper.Manifest.ABI.Methods)) - // Call PostPersist, check c2 contract hash is returned - require.NoError(t, mgmt.PostPersist(&interop.Context{DAO: d})) - require.Equal(t, []util.Uint160{c2.Hash}, mgmt.GetNEP17Contracts(d)) + // Call Persist, check c1Updated state is returned from lower. + _, err = private.Persist() + require.NoError(t, err) + require.Equal(t, []util.Uint160{c1.Hash}, mgmt.GetNEP17Contracts(d)) + c1Lower, err = mgmt.GetContract(d, c1.Hash) + require.NoError(t, err) + require.Equal(t, 2, len(c1Lower.Manifest.ABI.Methods)) } diff --git a/pkg/core/native/native_test/management_test.go b/pkg/core/native/native_test/management_test.go index c34b1c8d8..6ff49178a 100644 --- a/pkg/core/native/native_test/management_test.go +++ b/pkg/core/native/native_test/management_test.go @@ -20,6 +20,8 @@ import ( "github.com/nspcc-dev/neo-go/pkg/smartcontract/callflag" "github.com/nspcc-dev/neo-go/pkg/smartcontract/manifest" "github.com/nspcc-dev/neo-go/pkg/smartcontract/nef" + "github.com/nspcc-dev/neo-go/pkg/smartcontract/trigger" + "github.com/nspcc-dev/neo-go/pkg/vm" "github.com/nspcc-dev/neo-go/pkg/vm/emit" "github.com/nspcc-dev/neo-go/pkg/vm/opcode" "github.com/nspcc-dev/neo-go/pkg/vm/stackitem" @@ -34,6 +36,43 @@ func TestManagement_MinimumDeploymentFee(t *testing.T) { testGetSet(t, newManagementClient(t), "MinimumDeploymentFee", 10_00000000, 0, 0) } +func TestManagement_MinimumDeploymentFeeCache(t *testing.T) { + c := newManagementClient(t) + testGetSetCache(t, c, "MinimumDeploymentFee", 10_00000000) +} + +func TestManagement_ContractCache(t *testing.T) { + c := newManagementClient(t) + managementInvoker := c.WithSigners(c.Committee) + + cs1, _ := contracts.GetTestContractState(t, pathToInternalContracts, 1, 2, c.Committee.ScriptHash()) + manifestBytes, err := json.Marshal(cs1.Manifest) + require.NoError(t, err) + nefBytes, err := cs1.NEF.Bytes() + require.NoError(t, err) + + // Deploy contract, abort the transaction and check that Management cache wasn't persisted + // for FAULTed tx at the same block. + w := io.NewBufBinWriter() + emit.AppCall(w.BinWriter, managementInvoker.Hash, "deploy", callflag.All, nefBytes, manifestBytes) + emit.Opcodes(w.BinWriter, opcode.ABORT) + tx1 := managementInvoker.PrepareInvocation(t, w.Bytes(), managementInvoker.Signers) + tx2 := managementInvoker.PrepareInvoke(t, "getContract", cs1.Hash.BytesBE()) + managementInvoker.AddNewBlock(t, tx1, tx2) + managementInvoker.CheckFault(t, tx1.Hash(), "ABORT") + managementInvoker.CheckHalt(t, tx2.Hash(), stackitem.Null{}) + + // Deploy the contract and check that cache was persisted for HALTed transaction at the same block. + tx1 = managementInvoker.PrepareInvoke(t, "deploy", nefBytes, manifestBytes) + tx2 = managementInvoker.PrepareInvoke(t, "getContract", cs1.Hash.BytesBE()) + managementInvoker.AddNewBlock(t, tx1, tx2) + managementInvoker.CheckHalt(t, tx1.Hash()) + aer, err := managementInvoker.Chain.GetAppExecResults(tx2.Hash(), trigger.Application) + require.NoError(t, err) + require.Equal(t, vm.HaltState, aer[0].VMState, aer[0].FaultException) + require.NotEqual(t, stackitem.Null{}, aer[0].Stack) +} + func TestManagement_ContractDeploy(t *testing.T) { c := newManagementClient(t) managementInvoker := c.WithSigners(c.Committee)