From 624f193f94de4295cddbce5ca0c591ee8c48fb34 Mon Sep 17 00:00:00 2001 From: Anna Shaleva Date: Tue, 8 Aug 2023 18:52:14 +0300 Subject: [PATCH] vm: move JNumbers parsing precision under HFBasilisk Signed-off-by: Anna Shaleva --- docs/node-configuration.md | 2 +- pkg/config/hardfork.go | 3 ++- pkg/core/native/std.go | 5 ++-- pkg/vm/stackitem/json.go | 32 ++++++++++++++++++------ pkg/vm/stackitem/json_test.go | 47 +++++++++++++++++++++++++---------- 5 files changed, 64 insertions(+), 25 deletions(-) diff --git a/docs/node-configuration.md b/docs/node-configuration.md index 425b15b8c6..1049732d44 100644 --- a/docs/node-configuration.md +++ b/docs/node-configuration.md @@ -343,7 +343,7 @@ protocol-related settings described in the table below. | --- | --- | --- | --- | --- | | CommitteeHistory | map[uint32]uint32 | none | Number of committee members after the given height, for example `{0: 1, 20: 4}` sets up a chain with one committee member since the genesis and then changes the setting to 4 committee members at the height of 20. `StandbyCommittee` committee setting must have the number of keys equal or exceeding the highest value in this option. Blocks numbers where the change happens must be divisible by the old and by the new values simultaneously. If not set, committee size is derived from the `StandbyCommittee` setting and never changes. | | GarbageCollectionPeriod | `uint32` | 10000 | Controls MPT garbage collection interval (in blocks) for configurations with `RemoveUntraceableBlocks` enabled and `KeepOnlyLatestState` disabled. In this mode the node stores a number of MPT trees (corresponding to `MaxTraceableBlocks` and `StateSyncInterval`), but the DB needs to be clean from old entries from time to time. Doing it too often will cause too much processing overhead, doing it too rarely will leave more useless data in the DB. This setting is deprecated in favor of the same setting in the ApplicationConfiguration and will be removed in future node versions. If both settings are used, ApplicationConfiguration is prioritized over this one. | -| Hardforks | `map[string]uint32` | [] | The set of incompatible changes that affect node behaviour starting from the specified height. The default value is an empty set which should be interpreted as "each known hard-fork is applied from the zero blockchain height". The list of valid hard-fork names:
• `Aspidochelone` represents hard-fork introduced in [#2469](https://github.com/nspcc-dev/neo-go/pull/2469) (ported from the [reference](https://github.com/neo-project/neo/pull/2712)). It adjusts the prices of `System.Contract.CreateStandardAccount` and `System.Contract.CreateMultisigAccount` interops so that the resulting prices are in accordance with `sha256` method of native `CryptoLib` contract. It also includes [#2519](https://github.com/nspcc-dev/neo-go/pull/2519) (ported from the [reference](https://github.com/neo-project/neo/pull/2749)) that adjusts the price of `System.Runtime.GetRandom` interop and fixes its vulnerability. A special NeoGo-specific change is included as well for ContractManagement's update/deploy call flags behaviour to be compatible with pre-0.99.0 behaviour that was changed because of the [3.2.0 protocol change](https://github.com/neo-project/neo/pull/2653).
• `Basilisk` represents hard-fork introduced in [#3056](https://github.com/nspcc-dev/neo-go/pull/3056) (ported from the [reference](https://github.com/neo-project/neo/pull/2881)). It enables strict smart contract script check against a set of JMP instructions and against method boundaries enabled on contract deploy or update. | +| Hardforks | `map[string]uint32` | [] | The set of incompatible changes that affect node behaviour starting from the specified height. The default value is an empty set which should be interpreted as "each known hard-fork is applied from the zero blockchain height". The list of valid hard-fork names:
• `Aspidochelone` represents hard-fork introduced in [#2469](https://github.com/nspcc-dev/neo-go/pull/2469) (ported from the [reference](https://github.com/neo-project/neo/pull/2712)). It adjusts the prices of `System.Contract.CreateStandardAccount` and `System.Contract.CreateMultisigAccount` interops so that the resulting prices are in accordance with `sha256` method of native `CryptoLib` contract. It also includes [#2519](https://github.com/nspcc-dev/neo-go/pull/2519) (ported from the [reference](https://github.com/neo-project/neo/pull/2749)) that adjusts the price of `System.Runtime.GetRandom` interop and fixes its vulnerability. A special NeoGo-specific change is included as well for ContractManagement's update/deploy call flags behaviour to be compatible with pre-0.99.0 behaviour that was changed because of the [3.2.0 protocol change](https://github.com/neo-project/neo/pull/2653).
• `Basilisk` represents hard-fork introduced in [#3056](https://github.com/nspcc-dev/neo-go/pull/3056) (ported from the [reference](https://github.com/neo-project/neo/pull/2881)). It enables strict smart contract script check against a set of JMP instructions and against method boundaries enabled on contract deploy or update. It also includes [#3080](https://github.com/nspcc-dev/neo-go/pull/3080) (ported from the [reference](https://github.com/neo-project/neo/pull/2883)) that increases `stackitem.Integer` JSON parsing precision up to the maximum value supported by the NeoVM.| | KeepOnlyLatestState | `bool` | `false` | Specifies if MPT should only store the latest state (or a set of latest states, see `P2PStateExcangeExtensions` section for details). If true, DB size will be smaller, but older roots won't be accessible. This value should remain the same for the same database. | This setting is deprecated in favor of the same setting in the ApplicationConfiguration and will be removed in future node versions. If both settings are used, setting any of them to true enables the function. | | Magic | `uint32` | `0` | Magic number which uniquely identifies Neo network. | | MaxBlockSize | `uint32` | `262144` | Maximum block size in bytes. | diff --git a/pkg/config/hardfork.go b/pkg/config/hardfork.go index be83acfcdf..21409b60d8 100644 --- a/pkg/config/hardfork.go +++ b/pkg/config/hardfork.go @@ -11,7 +11,8 @@ const ( // https://github.com/neo-project/neo/pull/2749). HFAspidochelone Hardfork = 1 << iota // Aspidochelone // HFBasilisk represents hard-fork introduced in #3056 (ported from - // https://github.com/neo-project/neo/pull/2881). + // https://github.com/neo-project/neo/pull/2881) and #3080 (ported from + // https://github.com/neo-project/neo/pull/2883). HFBasilisk // Basilisk ) diff --git a/pkg/core/native/std.go b/pkg/core/native/std.go index 18dabccc5d..92f8988ecf 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/config" "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" @@ -199,13 +200,13 @@ func (s *Std) jsonSerialize(_ *interop.Context, args []stackitem.Item) stackitem return stackitem.NewByteArray(data) } -func (s *Std) jsonDeserialize(_ *interop.Context, args []stackitem.Item) stackitem.Item { +func (s *Std) jsonDeserialize(ic *interop.Context, args []stackitem.Item) stackitem.Item { data, err := args[0].TryBytes() if err != nil { panic(err) } - item, err := stackitem.FromJSON(data, stackitem.MaxDeserialized) + item, err := stackitem.FromJSON(data, stackitem.MaxDeserialized, ic.IsHardforkEnabled(config.HFBasilisk)) if err != nil { panic(err) } diff --git a/pkg/vm/stackitem/json.go b/pkg/vm/stackitem/json.go index 482ff27ae4..77e17f0564 100644 --- a/pkg/vm/stackitem/json.go +++ b/pkg/vm/stackitem/json.go @@ -18,6 +18,11 @@ type decoder struct { count int depth int + // bestIntPrecision denotes whether maximum allowed integer precision should + // be used to parse big.Int items. If false, then default NeoC# value will be + // used which doesn't allow to precisely parse big values. This behaviour is + // managed by the config.HFBasilisk. + bestIntPrecision bool } // MaxAllowedInteger is the maximum integer allowed to be encoded. @@ -26,10 +31,16 @@ const MaxAllowedInteger = 2<<53 - 1 // MaxJSONDepth is the maximum allowed nesting level of an encoded/decoded JSON. const MaxJSONDepth = 10 -// MaxIntegerPrec is the maximum precision allowed for big.Integer parsing. -// It allows to properly parse integer numbers that our 256-bit VM is able to -// handle. -const MaxIntegerPrec = 1<<8 + 1 +const ( + // MaxIntegerPrec is the maximum precision allowed for big.Integer parsing. + // It allows to properly parse integer numbers that our 256-bit VM is able to + // handle. + MaxIntegerPrec = 1<<8 + 1 + // CompatIntegerPrec is the maximum precision allowed for big.Integer parsing + // by the C# node before the Basilisk hardfork. It doesn't allow to precisely + // parse big numbers, see the https://github.com/neo-project/neo/issues/2879. + CompatIntegerPrec = 53 +) // ErrInvalidValue is returned when an item value doesn't fit some constraints // during serialization or deserialization. @@ -166,10 +177,11 @@ func itemToJSONString(it Item) ([]byte, error) { // null -> Null // array -> Array // map -> Map, keys are UTF-8 -func FromJSON(data []byte, maxCount int) (Item, error) { +func FromJSON(data []byte, maxCount int, bestIntPrecision bool) (Item, error) { d := decoder{ - Decoder: *json.NewDecoder(bytes.NewReader(data)), - count: maxCount, + Decoder: *json.NewDecoder(bytes.NewReader(data)), + count: maxCount, + bestIntPrecision: bestIntPrecision, } d.UseNumber() if item, err := d.decode(); err != nil { @@ -226,7 +238,11 @@ func (d *decoder) decode() (Item, error) { if isScientific { // As a special case numbers like 2.8e+22 are allowed (SetString rejects them). // That's the way how C# code works. - f, _, err := big.ParseFloat(ts, 10, MaxIntegerPrec, big.ToNearestEven) + var prec uint = CompatIntegerPrec + if d.bestIntPrecision { + prec = MaxIntegerPrec + } + f, _, err := big.ParseFloat(ts, 10, prec, big.ToNearestEven) if err != nil { return nil, fmt.Errorf("%w (malformed exp value for int)", ErrInvalidValue) } diff --git a/pkg/vm/stackitem/json_test.go b/pkg/vm/stackitem/json_test.go index 0895b283db..aa30318dc2 100644 --- a/pkg/vm/stackitem/json_test.go +++ b/pkg/vm/stackitem/json_test.go @@ -14,7 +14,7 @@ func getTestDecodeFunc(js string, expected ...interface{}) func(t *testing.T) { func getTestDecodeEncodeFunc(js string, needEncode bool, expected ...interface{}) func(t *testing.T) { return func(t *testing.T) { - actual, err := FromJSON([]byte(js), 20) + actual, err := FromJSON([]byte(js), 20, true) if expected[0] == nil { require.Error(t, err) return @@ -59,10 +59,10 @@ func TestFromToJSON(t *testing.T) { NewArray([]Item{NewArray([]Item{}), NewArray([]Item{NewMap(), Null{}})}))) t.Run("ManyElements", func(t *testing.T) { js := `[1, 2, 3]` // 3 elements + array itself - _, err := FromJSON([]byte(js), 4) + _, err := FromJSON([]byte(js), 4, true) require.NoError(t, err) - _, err = FromJSON([]byte(js), 3) + _, err = FromJSON([]byte(js), 3, true) require.ErrorIs(t, err, errTooBigElements) }) }) @@ -82,10 +82,10 @@ func TestFromToJSON(t *testing.T) { t.Run("ManyElements", func(t *testing.T) { js := `{"a":1,"b":3}` // 4 elements + map itself - _, err := FromJSON([]byte(js), 5) + _, err := FromJSON([]byte(js), 5, true) require.NoError(t, err) - _, err = FromJSON([]byte(js), 4) + _, err = FromJSON([]byte(js), 4, true) require.ErrorIs(t, err, errTooBigElements) }) }) @@ -133,17 +133,38 @@ func TestFromToJSON(t *testing.T) { // TestFromJSON_CompatBigInt ensures that maximum BigInt parsing precision matches // the C# one, ref. https://github.com/neo-project/neo/issues/2879. func TestFromJSON_CompatBigInt(t *testing.T) { - tcs := map[string]string{ - `9.05e+28`: "90500000000000000000000000000", - `1.871e+21`: "1871000000000000000000", - `3.0366e+32`: "303660000000000000000000000000000", - `1e+30`: "1000000000000000000000000000000", + tcs := map[string]struct { + bestPrec string + compatPrec string + }{ + `9.05e+28`: { + bestPrec: "90500000000000000000000000000", + compatPrec: "90499999999999993918259200000", + }, + `1.871e+21`: { + bestPrec: "1871000000000000000000", + compatPrec: "1871000000000000000000", + }, + `3.0366e+32`: { + bestPrec: "303660000000000000000000000000000", + compatPrec: "303660000000000004445016810323968", + }, + `1e+30`: { + bestPrec: "1000000000000000000000000000000", + compatPrec: "1000000000000000019884624838656", + }, } for in, expected := range tcs { t.Run(in, func(t *testing.T) { - actual, err := FromJSON([]byte(in), 5) + // Best precision. + actual, err := FromJSON([]byte(in), 5, true) + require.NoError(t, err) + require.Equal(t, expected.bestPrec, actual.Value().(*big.Int).String()) + + // Compatible precision. + actual, err = FromJSON([]byte(in), 5, false) require.NoError(t, err) - require.Equal(t, expected, actual.Value().(*big.Int).String()) + require.Equal(t, expected.compatPrec, actual.Value().(*big.Int).String()) }) } } @@ -156,7 +177,7 @@ func testToJSON(t *testing.T, expectedErr error, item Item) { } require.NoError(t, err) - actual, err := FromJSON(data, 1024) + actual, err := FromJSON(data, 1024, true) require.NoError(t, err) require.Equal(t, item, actual) }