core: keep Management cache always valid and up-to-date

pkg/core/native/management.go

@@ -174,21 +174,9 @@ 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
-}
 
 func getLimitedSlice(arg stackitem.Item, max int) ([]byte, error) {
 	_, isNull := arg.(stackitem.Null)
@@ -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`.
+	delete(cache.nep11, hash)
-	cache.contracts[h] = nil
+	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
 	}

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
+	// Call Persist, check c1 contract hash is returned
-	require.NoError(t, mgmt.PostPersist(&interop.Context{DAO: d}))
+	_, 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
+	// Call Persist, check c1Updated state is returned from lower.
-	require.NoError(t, mgmt.PostPersist(&interop.Context{DAO: d}))
+	_, err = private.Persist()
-	require.Equal(t, []util.Uint160{c2.Hash}, mgmt.GetNEP17Contracts(d))
+	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))
 }

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)