From 8f20a7096940a08336a5bd927649fbf4a91323a9 Mon Sep 17 00:00:00 2001 From: Evgenii Stratonikov Date: Mon, 15 Jun 2020 11:39:15 +0300 Subject: [PATCH 1/4] core: calculate a price of `System.Storage.Put` correctly It also depends on the item already stored by key. --- pkg/core/gas_price.go | 33 ++++------------------- pkg/core/gas_price_test.go | 54 -------------------------------------- pkg/core/interop_neo.go | 4 ++- pkg/core/interop_system.go | 11 ++++++-- 4 files changed, 17 insertions(+), 85 deletions(-) delete mode 100644 pkg/core/gas_price_test.go diff --git a/pkg/core/gas_price.go b/pkg/core/gas_price.go index b9130b0e1..e5b0fec0d 100644 --- a/pkg/core/gas_price.go +++ b/pkg/core/gas_price.go @@ -24,7 +24,11 @@ func getPrice(v *vm.VM, op opcode.Opcode, parameter []byte) util.Fixed8 { switch op { case opcode.SYSCALL: interopID := vm.GetInteropID(parameter) - return getSyscallPrice(v, interopID) + ifunc := v.GetInteropByID(interopID) + if ifunc != nil && ifunc.Price > 0 { + return toFixed8(int64(ifunc.Price)) + } + return toFixed8(1) default: return toFixed8(1) } @@ -33,30 +37,3 @@ func getPrice(v *vm.VM, op opcode.Opcode, parameter []byte) util.Fixed8 { func toFixed8(n int64) util.Fixed8 { return util.Fixed8(n * interopGasRatio) } - -// getSyscallPrice returns cost of executing syscall with provided id. -// Is SYSCALL is not found, cost is 1. -func getSyscallPrice(v *vm.VM, id uint32) util.Fixed8 { - ifunc := v.GetInteropByID(id) - if ifunc != nil && ifunc.Price > 0 { - return toFixed8(int64(ifunc.Price)) - } - - const ( - systemStoragePut = 0x84183fe6 // System.Storage.Put - systemStoragePutEx = 0x3a9be173 // System.Storage.PutEx - neoStoragePut = 0xf541a152 // Neo.Storage.Put - ) - - estack := v.Estack() - - switch id { - case systemStoragePut, systemStoragePutEx, neoStoragePut: - // price for storage PUT is 1 GAS per 1 KiB - keySize := len(estack.Peek(1).Bytes()) - valSize := len(estack.Peek(2).Bytes()) - return util.Fixed8FromInt64(int64((keySize+valSize-1)/1024 + 1)) - default: - return util.Fixed8FromInt64(1) - } -} diff --git a/pkg/core/gas_price_test.go b/pkg/core/gas_price_test.go deleted file mode 100644 index f618a949c..000000000 --- a/pkg/core/gas_price_test.go +++ /dev/null @@ -1,54 +0,0 @@ -package core - -import ( - "testing" - - "github.com/nspcc-dev/neo-go/pkg/core/dao" - "github.com/nspcc-dev/neo-go/pkg/core/storage" - "github.com/nspcc-dev/neo-go/pkg/smartcontract/trigger" - "github.com/nspcc-dev/neo-go/pkg/util" - "github.com/nspcc-dev/neo-go/pkg/vm" - "github.com/nspcc-dev/neo-go/pkg/vm/opcode" - "github.com/stretchr/testify/require" -) - -// These tests are taken from C# code -// https://github.com/neo-project/neo/blob/master-2.x/neo.UnitTests/UT_InteropPrices.cs#L245 -func TestGetPrice(t *testing.T) { - bc := newTestChain(t) - defer bc.Close() - sdao := dao.NewSimple(storage.NewMemoryStore()) - systemInterop := bc.newInteropContext(trigger.Application, sdao, nil, nil) - - v := SpawnVM(systemInterop) - v.SetPriceGetter(getPrice) - - t.Run("System.Storage.Put", func(t *testing.T) { - // System.Storage.Put: e63f1884 (requires push key and value) - v.Load([]byte{byte(opcode.PUSH3), byte(opcode.PUSH3), byte(opcode.PUSH0), - byte(opcode.SYSCALL), 0xe6, 0x3f, 0x18, 0x84}) - require.NoError(t, v.StepInto()) // push 03 (length 1) - require.NoError(t, v.StepInto()) // push 03 (length 1) - require.NoError(t, v.StepInto()) // push 00 - - checkGas(t, util.Fixed8FromInt64(1), v) - }) - - t.Run("System.Storage.PutEx", func(t *testing.T) { - // System.Storage.PutEx: 73e19b3a (requires push key and value) - v.Load([]byte{byte(opcode.PUSH3), byte(opcode.PUSH3), byte(opcode.PUSH0), - byte(opcode.SYSCALL), 0x73, 0xe1, 0x9b, 0x3a}) - require.NoError(t, v.StepInto()) // push 03 (length 1) - require.NoError(t, v.StepInto()) // push 03 (length 1) - require.NoError(t, v.StepInto()) // push 00 - - checkGas(t, util.Fixed8FromInt64(1), v) - }) -} - -func checkGas(t *testing.T, expected util.Fixed8, v *vm.VM) { - op, par, err := v.Context().Next() - - require.NoError(t, err) - require.Equal(t, expected, getPrice(v, op, par)) -} diff --git a/pkg/core/interop_neo.go b/pkg/core/interop_neo.go index c41258b7d..e27919ae5 100644 --- a/pkg/core/interop_neo.go +++ b/pkg/core/interop_neo.go @@ -26,6 +26,8 @@ const ( MaxContractStringLen = 252 ) +var errGasLimitExceeded = errors.New("gas limit exceeded") + // storageFind finds stored key-value pair. func storageFind(ic *interop.Context, v *vm.VM) error { stcInterface := v.Estack().Pop().Value() @@ -71,7 +73,7 @@ func createContractStateFromVM(ic *interop.Context, v *vm.VM) (*state.Contract, return nil, errors.New("manifest is too big") } if !v.AddGas(util.Fixed8(StoragePrice * (len(script) + len(manifestBytes)))) { - return nil, errors.New("gas limit exceeded") + return nil, errGasLimitExceeded } var m manifest.Manifest r := io.NewBinReaderFromBuf(manifestBytes) diff --git a/pkg/core/interop_system.go b/pkg/core/interop_system.go index da634e51d..2688fcc6c 100644 --- a/pkg/core/interop_system.go +++ b/pkg/core/interop_system.go @@ -332,7 +332,7 @@ func storageGetReadOnlyContext(ic *interop.Context, v *vm.VM) error { return nil } -func putWithContextAndFlags(ic *interop.Context, stc *StorageContext, key []byte, value []byte, isConst bool) error { +func putWithContextAndFlags(ic *interop.Context, v *vm.VM, stc *StorageContext, key []byte, value []byte, isConst bool) error { if len(key) > MaxStorageKeyLen { return errors.New("key is too big") } @@ -350,6 +350,13 @@ func putWithContextAndFlags(ic *interop.Context, stc *StorageContext, key []byte if si.IsConst { return errors.New("storage item exists and is read-only") } + sizeInc := 1 + if len(value) > len(si.Value) { + sizeInc = len(value) - len(si.Value) + } + if !v.AddGas(util.Fixed8(sizeInc) * StoragePrice) { + return errGasLimitExceeded + } si.Value = value si.IsConst = isConst return ic.DAO.PutStorageItem(stc.ScriptHash, key, si) @@ -368,7 +375,7 @@ func storagePutInternal(ic *interop.Context, v *vm.VM, getFlag bool) error { if getFlag { flag = int(v.Estack().Pop().BigInt().Int64()) } - return putWithContextAndFlags(ic, stc, key, value, flag == 1) + return putWithContextAndFlags(ic, v, stc, key, value, flag == 1) } // storagePut puts key-value pair into the storage. From fd2cd291e776be3bbda7e2de3c7edfbb131a5102 Mon Sep 17 00:00:00 2001 From: Evgenii Stratonikov Date: Mon, 15 Jun 2020 11:32:45 +0300 Subject: [PATCH 2/4] core: calculate opcode prices correctly --- pkg/core/gas_price.go | 11 ++--------- 1 file changed, 2 insertions(+), 9 deletions(-) diff --git a/pkg/core/gas_price.go b/pkg/core/gas_price.go index e5b0fec0d..e16ab85cd 100644 --- a/pkg/core/gas_price.go +++ b/pkg/core/gas_price.go @@ -17,21 +17,14 @@ const StoragePrice = 100000 // getPrice returns a price for executing op with the provided parameter. // Some SYSCALLs have variable price depending on their arguments. func getPrice(v *vm.VM, op opcode.Opcode, parameter []byte) util.Fixed8 { - if op <= opcode.NOP { - return 0 - } - - switch op { - case opcode.SYSCALL: + if op == opcode.SYSCALL { interopID := vm.GetInteropID(parameter) ifunc := v.GetInteropByID(interopID) if ifunc != nil && ifunc.Price > 0 { return toFixed8(int64(ifunc.Price)) } - return toFixed8(1) - default: - return toFixed8(1) } + return opcodePrice(op) } func toFixed8(n int64) util.Fixed8 { From 61bba1a39c956a1a291421950098268781a5fa24 Mon Sep 17 00:00:00 2001 From: Evgenii Stratonikov Date: Mon, 15 Jun 2020 11:53:40 +0300 Subject: [PATCH 3/4] core: store precise gas price in interop descriptions In NEO3 sycalls can have prices less than 10^5, we need to store them precisely. Adjusting actual prices is to be done in the following tasks. --- pkg/core/gas_price.go | 11 +---------- 1 file changed, 1 insertion(+), 10 deletions(-) diff --git a/pkg/core/gas_price.go b/pkg/core/gas_price.go index e16ab85cd..e066266e3 100644 --- a/pkg/core/gas_price.go +++ b/pkg/core/gas_price.go @@ -6,11 +6,6 @@ import ( "github.com/nspcc-dev/neo-go/pkg/vm/opcode" ) -// interopGasRatio is a multiplier by which a number returned from price getter -// and Fixed8 amount of GAS differ. Numbers defined in syscall tables are a multiple -// of 0.001 GAS = Fixed8(10^5). -const interopGasRatio = 100000 - // StoragePrice is a price for storing 1 byte of storage. const StoragePrice = 100000 @@ -21,12 +16,8 @@ func getPrice(v *vm.VM, op opcode.Opcode, parameter []byte) util.Fixed8 { interopID := vm.GetInteropID(parameter) ifunc := v.GetInteropByID(interopID) if ifunc != nil && ifunc.Price > 0 { - return toFixed8(int64(ifunc.Price)) + return util.Fixed8(ifunc.Price) } } return opcodePrice(op) } - -func toFixed8(n int64) util.Fixed8 { - return util.Fixed8(n * interopGasRatio) -} From 2e996ea57dae68d759f9ef512ff962a3d8ef70ba Mon Sep 17 00:00:00 2001 From: Evgenii Stratonikov Date: Mon, 15 Jun 2020 12:01:04 +0300 Subject: [PATCH 4/4] core: adjust `System.Storage.*` interop prices --- pkg/core/interops.go | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/pkg/core/interops.go b/pkg/core/interops.go index 8be1c16dc..d11f4e329 100644 --- a/pkg/core/interops.go +++ b/pkg/core/interops.go @@ -112,21 +112,21 @@ var systemInterops = []interop.Function{ {Name: "System.Runtime.Notify", Func: runtimeNotify, Price: 1, RequiredFlags: smartcontract.AllowNotify}, {Name: "System.Runtime.Platform", Func: runtimePlatform, Price: 1}, {Name: "System.Runtime.Serialize", Func: runtimeSerialize, Price: 1}, - {Name: "System.Storage.Delete", Func: storageDelete, Price: 100, + {Name: "System.Storage.Delete", Func: storageDelete, Price: StoragePrice, AllowedTriggers: trigger.Application, RequiredFlags: smartcontract.AllowModifyStates}, - {Name: "System.Storage.Find", Func: storageFind, Price: 1, + {Name: "System.Storage.Find", Func: storageFind, Price: 1000000, AllowedTriggers: trigger.Application, RequiredFlags: smartcontract.AllowStates}, - {Name: "System.Storage.Get", Func: storageGet, Price: 100, + {Name: "System.Storage.Get", Func: storageGet, Price: 1000000, AllowedTriggers: trigger.Application, RequiredFlags: smartcontract.AllowStates}, - {Name: "System.Storage.GetContext", Func: storageGetContext, Price: 1, + {Name: "System.Storage.GetContext", Func: storageGetContext, Price: 400, AllowedTriggers: trigger.Application, RequiredFlags: smartcontract.AllowStates}, - {Name: "System.Storage.GetReadOnlyContext", Func: storageGetReadOnlyContext, Price: 1, + {Name: "System.Storage.GetReadOnlyContext", Func: storageGetReadOnlyContext, Price: 400, AllowedTriggers: trigger.Application, RequiredFlags: smartcontract.AllowStates}, {Name: "System.Storage.Put", Func: storagePut, Price: 0, AllowedTriggers: trigger.Application, RequiredFlags: smartcontract.AllowModifyStates}, // These don't have static price in C# code. {Name: "System.Storage.PutEx", Func: storagePutEx, Price: 0, AllowedTriggers: trigger.Application, RequiredFlags: smartcontract.AllowModifyStates}, - {Name: "System.Storage.AsReadOnly", Func: storageContextAsReadOnly, Price: 1, + {Name: "System.Storage.AsReadOnly", Func: storageContextAsReadOnly, Price: 400, AllowedTriggers: trigger.Application, RequiredFlags: smartcontract.AllowStates}, }