Merge pull request #2432 from nspcc-dev/fix-fees

core: use proper BaseExecFee and StoragePrice for interop context

pkg/compiler/interop_test.go

@@ -12,6 +12,7 @@ import (
 	"github.com/nspcc-dev/neo-go/pkg/core"
 	"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"
 	"github.com/nspcc-dev/neo-go/pkg/core/state"
 	"github.com/nspcc-dev/neo-go/pkg/core/storage"
 	"github.com/nspcc-dev/neo-go/pkg/crypto/hash"
@@ -200,7 +201,7 @@ func TestAppCall(t *testing.T) {
 	}
 
 	fc := fakechain.NewFakeChain()
-	ic := interop.NewContext(trigger.Application, fc, dao.NewSimple(storage.NewMemoryStore(), false, false), contractGetter, nil, nil, nil, zaptest.NewLogger(t))
+	ic := interop.NewContext(trigger.Application, fc, dao.NewSimple(storage.NewMemoryStore(), false, false), interop.DefaultBaseExecFee, native.DefaultStoragePrice, contractGetter, nil, nil, nil, zaptest.NewLogger(t))
 
 	t.Run("valid script", func(t *testing.T) {
 		src := getAppCallScript(fmt.Sprintf("%#v", ih.BytesBE()))

pkg/consensus/consensus.go

@@ -56,6 +56,7 @@ type Ledger interface {
 	PoolTx(t *transaction.Transaction, pools ...*mempool.Pool) error
 	SubscribeForBlocks(ch chan<- *coreb.Block)
 	UnsubscribeFromBlocks(ch chan<- *coreb.Block)
+	GetBaseExecFee() int64
 	interop.Ledger
 	mempool.Feer
 }

pkg/core/blockchain.go

@@ -2304,7 +2304,19 @@ func (bc *Blockchain) ManagementContractHash() util.Uint160 {
 }
 
 func (bc *Blockchain) newInteropContext(trigger trigger.Type, d *dao.Simple, block *block.Block, tx *transaction.Transaction) *interop.Context {
-	ic := interop.NewContext(trigger, bc, d, bc.contracts.Management.GetContract, bc.contracts.Contracts, block, tx, bc.log)
+	baseExecFee := int64(interop.DefaultBaseExecFee)
+	if block == nil || block.Index != 0 {
+		// Use provided dao instead of Blockchain's one to fetch possible ExecFeeFactor
+		// changes that were not yet persisted to Blockchain's dao.
+		baseExecFee = bc.contracts.Policy.GetExecFeeFactorInternal(d)
+	}
+	baseStorageFee := int64(native.DefaultStoragePrice)
+	if block == nil || block.Index != 0 {
+		// Use provided dao instead of Blockchain's one to fetch possible StoragePrice
+		// changes that were not yet persisted to Blockchain's dao.
+		baseStorageFee = bc.contracts.Policy.GetStoragePriceInternal(d)
+	}
+	ic := interop.NewContext(trigger, bc, d, baseExecFee, baseStorageFee, bc.contracts.Management.GetContract, bc.contracts.Contracts, block, tx, bc.log)
 	ic.Functions = systemInterops
 	switch {
 	case tx != nil:
@@ -2329,6 +2341,9 @@ func (bc *Blockchain) RegisterPostBlock(f func(func(*transaction.Transaction, *m
 
 // GetBaseExecFee return execution price for `NOP`.
 func (bc *Blockchain) GetBaseExecFee() int64 {
+	if bc.BlockHeight() == 0 {
+		return interop.DefaultBaseExecFee
+	}
 	return bc.contracts.Policy.GetExecFeeFactorInternal(bc.dao)
 }
 

pkg/core/blockchain_core_test.go

@@ -11,10 +11,12 @@ import (
 	"github.com/nspcc-dev/neo-go/internal/testchain"
 	"github.com/nspcc-dev/neo-go/pkg/config"
 	"github.com/nspcc-dev/neo-go/pkg/core/block"
+	"github.com/nspcc-dev/neo-go/pkg/core/dao"
 	"github.com/nspcc-dev/neo-go/pkg/core/state"
 	"github.com/nspcc-dev/neo-go/pkg/core/storage"
 	"github.com/nspcc-dev/neo-go/pkg/core/transaction"
 	"github.com/nspcc-dev/neo-go/pkg/smartcontract"
+	"github.com/nspcc-dev/neo-go/pkg/smartcontract/trigger"
 	"github.com/nspcc-dev/neo-go/pkg/util"
 	"github.com/nspcc-dev/neo-go/pkg/util/slice"
 	"github.com/nspcc-dev/neo-go/pkg/vm/opcode"
@@ -320,3 +322,22 @@ func setSigner(tx *transaction.Transaction, h util.Uint160) {
 		Scopes:  transaction.Global,
 	}}
 }
+
+// This test checks that value of BaseExecFee returned from corresponding Blockchain's method matches
+// the one provided to the constructor of new interop context.
+func TestBlockchain_BaseExecFeeBaseStoragePrice_Compat(t *testing.T) {
+	bc := newTestChain(t)
+
+	check := func(t *testing.T) {
+		ic := bc.newInteropContext(trigger.Application, dao.NewSimple(storage.NewMemoryStore(), bc.config.StateRootInHeader, bc.config.P2PSigExtensions), bc.topBlock.Load().(*block.Block), nil)
+		require.Equal(t, bc.GetBaseExecFee(), ic.BaseExecFee())
+		require.Equal(t, bc.GetStoragePrice(), ic.BaseStorageFee())
+	}
+	t.Run("zero block", func(t *testing.T) {
+		check(t)
+	})
+	t.Run("non-zero block", func(t *testing.T) {
+		require.NoError(t, bc.AddBlock(bc.newBlock()))
+		check(t)
+	})
+}

pkg/core/interop/context.go

@@ -37,11 +37,9 @@ const (
 type Ledger interface {
 	BlockHeight() uint32
 	CurrentBlockHash() util.Uint256
-	GetBaseExecFee() int64
 	GetBlock(hash util.Uint256) (*block.Block, error)
 	GetConfig() config.ProtocolConfiguration
 	GetHeaderHash(int) util.Uint256
-	GetStoragePrice() int64
 }
 
 // Context represents context in which interops are executed.
@@ -63,19 +61,15 @@ type Context struct {
 	cancelFuncs    []context.CancelFunc
 	getContract    func(*dao.Simple, util.Uint160) (*state.Contract, error)
 	baseExecFee    int64
+	baseStorageFee int64
 	signers        []transaction.Signer
 }
 
 // NewContext returns new interop context.
-func NewContext(trigger trigger.Type, bc Ledger, d *dao.Simple,
+func NewContext(trigger trigger.Type, bc Ledger, d *dao.Simple, baseExecFee, baseStorageFee int64,
 	getContract func(*dao.Simple, util.Uint160) (*state.Contract, error), natives []Contract,
 	block *block.Block, tx *transaction.Transaction, log *zap.Logger) *Context {
-	baseExecFee := int64(DefaultBaseExecFee)
 	dao := d.GetPrivate()
-
-	if bc != nil && (block == nil || block.Index != 0) {
-		baseExecFee = bc.GetBaseExecFee()
-	}
 	return &Context{
 		Chain:          bc,
 		Network:        uint32(bc.GetConfig().Magic),
@@ -88,6 +82,7 @@ func NewContext(trigger trigger.Type, bc Ledger, d *dao.Simple,
 		Invocations:    make(map[util.Uint160]int),
 		getContract:    getContract,
 		baseExecFee:    baseExecFee,
+		baseStorageFee: baseStorageFee,
 	}
 }
 
@@ -293,6 +288,11 @@ func (ic *Context) BaseExecFee() int64 {
 	return ic.baseExecFee
 }
 
+// BaseStorageFee represents price for storing one byte of data in the contract storage.
+func (ic *Context) BaseStorageFee() int64 {
+	return ic.baseStorageFee
+}
+
 // SyscallHandler handles syscall with id.
 func (ic *Context) SyscallHandler(_ *vm.VM, id uint32) error {
 	f := ic.GetFunction(id)

pkg/core/interop/crypto/ecdsa_test.go

@@ -10,6 +10,7 @@ import (
 	"github.com/nspcc-dev/neo-go/pkg/core/dao"
 	"github.com/nspcc-dev/neo-go/pkg/core/fee"
 	"github.com/nspcc-dev/neo-go/pkg/core/interop"
+	"github.com/nspcc-dev/neo-go/pkg/core/native"
 	"github.com/nspcc-dev/neo-go/pkg/core/storage"
 	"github.com/nspcc-dev/neo-go/pkg/core/transaction"
 	"github.com/nspcc-dev/neo-go/pkg/crypto/hash"
@@ -72,7 +73,7 @@ func initCheckMultisigVMNoArgs(container *transaction.Transaction) *vm.VM {
 		trigger.Verification,
 		fakechain.NewFakeChain(),
 		dao.NewSimple(storage.NewMemoryStore(), false, false),
-		nil, nil, nil,
+		interop.DefaultBaseExecFee, native.DefaultStoragePrice, nil, nil, nil,
 		container,
 		nil)
 	ic.Container = container

pkg/core/interop_system.go

@@ -126,7 +126,7 @@ func putWithContext(ic *interop.Context, stc *StorageContext, key []byte, value
 			sizeInc = (len(si)-1)/4 + 1 + len(value) - len(si)
 		}
 	}
-	if !ic.VM.AddGas(int64(sizeInc) * ic.Chain.GetStoragePrice()) {
+	if !ic.VM.AddGas(int64(sizeInc) * ic.BaseStorageFee()) {
 		return errGasLimitExceeded
 	}
 	ic.DAO.PutStorageItem(stc.ID, key, value)

pkg/core/native/interop.go

@@ -42,8 +42,8 @@ func Call(ic *interop.Context) error {
 		return fmt.Errorf("missing call flags for native %d `%s` operation call: %05b vs %05b",
 			version, m.MD.Name, ic.VM.Context().GetCallFlags(), m.RequiredFlags)
 	}
-	invokeFee := m.CPUFee*ic.Chain.GetBaseExecFee() +
+	invokeFee := m.CPUFee*ic.BaseExecFee() +
-		m.StorageFee*ic.Chain.GetStoragePrice()
+		m.StorageFee*ic.BaseStorageFee()
 	if !ic.VM.AddGas(invokeFee) {
 		return errors.New("gas limit exceeded")
 	}

pkg/core/native/management.go

@@ -198,7 +198,7 @@ func (m *Management) getNefAndManifestFromItems(ic *interop.Context, args []stac
 		return nil, nil, fmt.Errorf("invalid manifest: %w", err)
 	}
 
-	gas := ic.Chain.GetStoragePrice() * int64(len(nefBytes)+len(manifestBytes))
+	gas := ic.BaseStorageFee() * int64(len(nefBytes)+len(manifestBytes))
 	if isDeploy {
 		fee := m.minimumDeploymentFee(ic.DAO)
 		if fee > gas {

pkg/core/native/native_test/common_test.go

@@ -55,16 +55,25 @@ func testGetSet(t *testing.T, c *neotest.ContractInvoker, name string, defaultVa
 		c.AddNewBlock(t, txSet, txGet)
 		c.CheckHalt(t, txSet.Hash(), stackitem.Null{})
 
-		if name != "GasPerBlock" { // GasPerBlock is set on the next block
+		switch name {
-			c.CheckHalt(t, txGet.Hash(), stackitem.Make(defaultValue+1))
+		case "GasPerBlock":
-		} else {
+			// GasPerBlock is set on the next block
 			c.CheckHalt(t, txGet.Hash(), stackitem.Make(defaultValue))
 			c.AddNewBlock(t)
 			randomInvoker.Invoke(t, defaultValue+1, getName)
-		}
+		case "ExecFeeFactor":
-
+			// ExecFeeFactor was risen, so the second transaction will fail because
+			// of gas limit exceeding (its fees are out-of-date).
+			c.CheckFault(t, txGet.Hash(), "gas limit exceeded")
+			// Set in a separate block.
+			committeeInvoker.Invoke(t, stackitem.Null{}, setName, defaultValue+1)
 			// Get in the next block.
 			randomInvoker.Invoke(t, defaultValue+1, getName)
+		default:
+			c.CheckHalt(t, txGet.Hash(), stackitem.Make(defaultValue+1))
+			// Get in the next block.
+			randomInvoker.Invoke(t, defaultValue+1, getName)
+		}
 	})
 }