From 910d53b27b58a610a51e7953eb682f958a6ee5e4 Mon Sep 17 00:00:00 2001 From: Anna Shaleva Date: Wed, 22 Nov 2023 20:15:43 +0300 Subject: [PATCH] core: do not check manifest size on deploy/update Manifest will be a part of the state.Contract which will be checked on its way to the storage. Tiny optimisation which allows not to serialize manifest twice. Ref. https://github.com/nspcc-dev/neo-go/pull/3218#discussion_r1402374232. Signed-off-by: Anna Shaleva --- cli/smartcontract/manifest.go | 2 +- pkg/compiler/compiler.go | 2 +- pkg/core/native/management.go | 4 +-- pkg/smartcontract/manifest/manifest.go | 5 ++- pkg/smartcontract/manifest/manifest_test.go | 34 ++++++++++----------- 5 files changed, 25 insertions(+), 22 deletions(-) diff --git a/cli/smartcontract/manifest.go b/cli/smartcontract/manifest.go index 3964ae1a9..76c7fdc68 100644 --- a/cli/smartcontract/manifest.go +++ b/cli/smartcontract/manifest.go @@ -109,7 +109,7 @@ func readManifest(filename string, hash util.Uint160) (*manifest.Manifest, []byt if err != nil { return nil, nil, err } - if err := m.IsValid(hash); err != nil { + if err := m.IsValid(hash, true); err != nil { return nil, nil, fmt.Errorf("manifest is invalid: %w", err) } return m, manifestBytes, nil diff --git a/pkg/compiler/compiler.go b/pkg/compiler/compiler.go index a7826a552..47e2a43dd 100644 --- a/pkg/compiler/compiler.go +++ b/pkg/compiler/compiler.go @@ -447,7 +447,7 @@ func CreateManifest(di *DebugInfo, o *Options) (*manifest.Manifest, error) { return m, fmt.Errorf("method %s is marked as safe but missing from manifest", name) } } - err = m.IsValid(util.Uint160{}) // Check as much as possible without hash. + err = m.IsValid(util.Uint160{}, true) // Check as much as possible without hash. if err != nil { return m, fmt.Errorf("manifest is invalid: %w", err) } diff --git a/pkg/core/native/management.go b/pkg/core/native/management.go index ed118a22d..233513164 100644 --- a/pkg/core/native/management.go +++ b/pkg/core/native/management.go @@ -389,7 +389,7 @@ func (m *Management) Deploy(ic *interop.Context, sender util.Uint160, neff *nef. if err != nil { return nil, err } - err = manif.IsValid(h) + err = manif.IsValid(h, false) // do not check manifest size, the whole state.Contract will be checked later. if err != nil { return nil, fmt.Errorf("invalid manifest: %w", err) } @@ -458,7 +458,7 @@ func (m *Management) Update(ic *interop.Context, hash util.Uint160, neff *nef.Fi if manif.Name != contract.Manifest.Name { return nil, errors.New("contract name can't be changed") } - err = manif.IsValid(contract.Hash) + err = manif.IsValid(contract.Hash, false) // do not check manifest size, the whole state.Contract will be checked later. if err != nil { return nil, fmt.Errorf("invalid manifest: %w", err) } diff --git a/pkg/smartcontract/manifest/manifest.go b/pkg/smartcontract/manifest/manifest.go index 900701cef..3f5ac8417 100644 --- a/pkg/smartcontract/manifest/manifest.go +++ b/pkg/smartcontract/manifest/manifest.go @@ -84,7 +84,7 @@ func (m *Manifest) CanCall(hash util.Uint160, toCall *Manifest, method string) b // IsValid checks manifest internal consistency and correctness, one of the // checks is for group signature correctness, contract hash is passed for it. // If hash is empty, then hash-related checks are omitted. -func (m *Manifest) IsValid(hash util.Uint160) error { +func (m *Manifest) IsValid(hash util.Uint160, checkSize bool) error { var err error if m.Name == "" { @@ -122,6 +122,9 @@ func (m *Manifest) IsValid(hash util.Uint160) error { if err != nil { return err } + if !checkSize { + return nil + } si, err := m.ToStackItem() if err != nil { return fmt.Errorf("failed to check manifest serialisation: %w", err) diff --git a/pkg/smartcontract/manifest/manifest_test.go b/pkg/smartcontract/manifest/manifest_test.go index 47ad9805e..7fd940f81 100644 --- a/pkg/smartcontract/manifest/manifest_test.go +++ b/pkg/smartcontract/manifest/manifest_test.go @@ -125,13 +125,13 @@ func TestIsValid(t *testing.T) { m := &Manifest{} t.Run("invalid, no name", func(t *testing.T) { - require.Error(t, m.IsValid(contractHash)) + require.Error(t, m.IsValid(contractHash, true)) }) m = NewManifest("Test") t.Run("invalid, no ABI methods", func(t *testing.T) { - require.Error(t, m.IsValid(contractHash)) + require.Error(t, m.IsValid(contractHash, true)) }) m.ABI.Methods = append(m.ABI.Methods, Method{ @@ -141,7 +141,7 @@ func TestIsValid(t *testing.T) { }) t.Run("valid, no groups/events", func(t *testing.T) { - require.NoError(t, m.IsValid(contractHash)) + require.NoError(t, m.IsValid(contractHash, true)) }) m.ABI.Events = append(m.ABI.Events, Event{ @@ -150,7 +150,7 @@ func TestIsValid(t *testing.T) { }) t.Run("valid, with events", func(t *testing.T) { - require.NoError(t, m.IsValid(contractHash)) + require.NoError(t, m.IsValid(contractHash, true)) }) m.ABI.Events = append(m.ABI.Events, Event{ @@ -162,52 +162,52 @@ func TestIsValid(t *testing.T) { }) t.Run("invalid, bad event", func(t *testing.T) { - require.Error(t, m.IsValid(contractHash)) + require.Error(t, m.IsValid(contractHash, true)) }) m.ABI.Events = m.ABI.Events[:1] m.Permissions = append(m.Permissions, *NewPermission(PermissionHash, util.Uint160{1, 2, 3})) t.Run("valid, with permissions", func(t *testing.T) { - require.NoError(t, m.IsValid(contractHash)) + require.NoError(t, m.IsValid(contractHash, true)) }) m.Permissions = append(m.Permissions, *NewPermission(PermissionHash, util.Uint160{1, 2, 3})) t.Run("invalid, with permissions", func(t *testing.T) { - require.Error(t, m.IsValid(contractHash)) + require.Error(t, m.IsValid(contractHash, true)) }) m.Permissions = m.Permissions[:1] m.SupportedStandards = append(m.SupportedStandards, "NEP-17") t.Run("valid, with standards", func(t *testing.T) { - require.NoError(t, m.IsValid(contractHash)) + require.NoError(t, m.IsValid(contractHash, true)) }) m.SupportedStandards = append(m.SupportedStandards, "") t.Run("invalid, with nameless standard", func(t *testing.T) { - require.Error(t, m.IsValid(contractHash)) + require.Error(t, m.IsValid(contractHash, true)) }) m.SupportedStandards = m.SupportedStandards[:1] m.SupportedStandards = append(m.SupportedStandards, "NEP-17") t.Run("invalid, with duplicate standards", func(t *testing.T) { - require.Error(t, m.IsValid(contractHash)) + require.Error(t, m.IsValid(contractHash, true)) }) m.SupportedStandards = m.SupportedStandards[:1] d := PermissionDesc{Type: PermissionHash, Value: util.Uint160{1, 2, 3}} m.Trusts.Add(d) t.Run("valid, with trust", func(t *testing.T) { - require.NoError(t, m.IsValid(contractHash)) + require.NoError(t, m.IsValid(contractHash, true)) }) m.Trusts.Add(PermissionDesc{Type: PermissionHash, Value: util.Uint160{3, 2, 1}}) t.Run("valid, with trusts", func(t *testing.T) { - require.NoError(t, m.IsValid(contractHash)) + require.NoError(t, m.IsValid(contractHash, true)) }) m.Trusts.Add(d) t.Run("invalid, with trusts", func(t *testing.T) { - require.Error(t, m.IsValid(contractHash)) + require.Error(t, m.IsValid(contractHash, true)) }) m.Trusts.Restrict() @@ -225,11 +225,11 @@ func TestIsValid(t *testing.T) { } t.Run("valid", func(t *testing.T) { - require.NoError(t, m.IsValid(contractHash)) + require.NoError(t, m.IsValid(contractHash, true)) }) t.Run("invalid, wrong contract hash", func(t *testing.T) { - require.Error(t, m.IsValid(util.Uint160{4, 5, 6})) + require.Error(t, m.IsValid(util.Uint160{4, 5, 6}, true)) }) t.Run("invalid, wrong group signature", func(t *testing.T) { @@ -241,7 +241,7 @@ func TestIsValid(t *testing.T) { // of the contract hash. Signature: pk.Sign([]byte{1, 2, 3}), }) - require.Error(t, m.IsValid(contractHash)) + require.Error(t, m.IsValid(contractHash, true)) }) }) m.Groups = m.Groups[:0] @@ -253,7 +253,7 @@ func TestIsValid(t *testing.T) { Parameters: []Parameter{}, }) } - err := m.IsValid(contractHash) + err := m.IsValid(contractHash, true) require.ErrorIs(t, err, stackitem.ErrTooBig) }) }