From fddad0b47548c3d20d5cb5ad7de28c60b3eec5ff Mon Sep 17 00:00:00 2001 From: Anna Shaleva Date: Wed, 15 Jul 2020 14:39:20 +0300 Subject: [PATCH] core: adjust System.Contract.Create interop Part of #1055. It should check given scripthash against manifest groups and return the contract state as a struct (not interop interface). --- pkg/compiler/syscall.go | 2 +- pkg/core/interop_neo.go | 11 +++- pkg/core/interop_system_test.go | 60 ++++++++++++++++++--- pkg/interop/blockchain/blockchain.go | 14 ++--- pkg/interop/contract/contract.go | 9 +++- pkg/smartcontract/manifest/manifest.go | 14 +++++ pkg/smartcontract/manifest/manifest_test.go | 47 ++++++++++++++++ pkg/smartcontract/manifest/method.go | 7 +++ 8 files changed, 141 insertions(+), 23 deletions(-) diff --git a/pkg/compiler/syscall.go b/pkg/compiler/syscall.go index 6f96c5f79..cc2668c43 100644 --- a/pkg/compiler/syscall.go +++ b/pkg/compiler/syscall.go @@ -21,7 +21,7 @@ var syscalls = map[string]map[string]Syscall{ "GetTransactionHeight": {"System.Blockchain.GetTransactionHeight", false}, }, "contract": { - "Create": {"System.Contract.Create", false}, + "Create": {"System.Contract.Create", true}, "CreateStandardAccount": {"System.Contract.CreateStandardAccount", false}, "Destroy": {"System.Contract.Destroy", false}, "IsStandard": {"System.Contract.IsStandard", false}, diff --git a/pkg/core/interop_neo.go b/pkg/core/interop_neo.go index 47cff726f..26d652431 100644 --- a/pkg/core/interop_neo.go +++ b/pkg/core/interop_neo.go @@ -72,7 +72,7 @@ func createContractStateFromVM(ic *interop.Context, v *vm.VM) (*state.Contract, var m manifest.Manifest err := m.UnmarshalJSON(manifestBytes) if err != nil { - return nil, err + return nil, fmt.Errorf("unable to retrieve manifest from stack: %v", err) } return &state.Contract{ Script: script, @@ -95,10 +95,17 @@ func contractCreate(ic *interop.Context, v *vm.VM) error { return err } newcontract.ID = id + if !newcontract.Manifest.IsValid(newcontract.ScriptHash()) { + return errors.New("failed to check contract script hash against manifest") + } if err := ic.DAO.PutContractState(newcontract); err != nil { return err } - v.Estack().PushVal(stackitem.NewInterop(newcontract)) + cs, err := contractToStackItem(newcontract) + if err != nil { + return fmt.Errorf("cannot convert contract to stack item: %v", err) + } + v.Estack().PushVal(cs) return nil } diff --git a/pkg/core/interop_system_test.go b/pkg/core/interop_system_test.go index e5ecf893e..b8636ba2d 100644 --- a/pkg/core/interop_system_test.go +++ b/pkg/core/interop_system_test.go @@ -302,14 +302,8 @@ func TestBlockchainGetContractState(t *testing.T) { v.Estack().PushVal(cs.ScriptHash().BytesBE()) require.NoError(t, bcGetContract(ic, v)) - expectedManifest, err := cs.Manifest.MarshalJSON() - require.NoError(t, err) - actual := v.Estack().Pop().Array() - require.Equal(t, 4, len(actual)) - require.Equal(t, cs.Script, actual[0].Value().([]byte)) - require.Equal(t, expectedManifest, actual[1].Value().([]byte)) - require.Equal(t, cs.HasStorage(), actual[2].Bool()) - require.Equal(t, cs.IsPayable(), actual[3].Bool()) + actual := v.Estack().Pop().Item() + compareContractStates(t, cs, actual) }) t.Run("uncknown contract state", func(t *testing.T) { @@ -320,3 +314,53 @@ func TestBlockchainGetContractState(t *testing.T) { require.Equal(t, stackitem.Null{}, actual) }) } + +func TestContractCreate(t *testing.T) { + v, cs, ic, bc := createVMAndContractState(t) + v.GasLimit = -1 + defer bc.Close() + + putArgsOnStack := func() { + manifest, err := cs.Manifest.MarshalJSON() + require.NoError(t, err) + v.Estack().PushVal(manifest) + v.Estack().PushVal(cs.Script) + } + + t.Run("positive", func(t *testing.T) { + putArgsOnStack() + + require.NoError(t, contractCreate(ic, v)) + actual := v.Estack().Pop().Item() + compareContractStates(t, cs, actual) + }) + + t.Run("invalid scripthash", func(t *testing.T) { + cs.Script = append(cs.Script, 0x01) + putArgsOnStack() + + require.Error(t, contractCreate(ic, v)) + }) + + t.Run("contract already exists", func(t *testing.T) { + cs.Script = cs.Script[:len(cs.Script)-1] + require.NoError(t, ic.DAO.PutContractState(cs)) + putArgsOnStack() + + require.Error(t, contractCreate(ic, v)) + }) +} + +func compareContractStates(t *testing.T, expected *state.Contract, actual stackitem.Item) { + act, ok := actual.Value().([]stackitem.Item) + require.True(t, ok) + + expectedManifest, err := expected.Manifest.MarshalJSON() + require.NoError(t, err) + + require.Equal(t, 4, len(act)) + require.Equal(t, expected.Script, act[0].Value().([]byte)) + require.Equal(t, expectedManifest, act[1].Value().([]byte)) + require.Equal(t, expected.HasStorage(), act[2].Bool()) + require.Equal(t, expected.IsPayable(), act[3].Bool()) +} diff --git a/pkg/interop/blockchain/blockchain.go b/pkg/interop/blockchain/blockchain.go index c8e582a80..22ecb64d7 100644 --- a/pkg/interop/blockchain/blockchain.go +++ b/pkg/interop/blockchain/blockchain.go @@ -3,6 +3,8 @@ Package blockchain provides functions to access various blockchain data. */ package blockchain +import "github.com/nspcc-dev/neo-go/pkg/interop/contract" + // Transaction represents a NEO transaction. It's similar to Transaction class // in Neo .net framework. type Transaction struct { @@ -53,14 +55,6 @@ type Block struct { TransactionsLength int } -// Contract represents a Neo contract and is used in interop functions. -type Contract struct { - Script []byte - Manifest []byte - HasStorage bool - IsPayable bool -} - // GetHeight returns current block height (index of the last accepted block). // Note that when transaction is being run as a part of new block this block is // considered as not yet accepted (persisted) and thus you'll get an index of @@ -103,6 +97,6 @@ func GetTransactionHeight(hash []byte) int { // format represented as a slice of 20 bytes). Refer to the `contract` package // for details on how to use the returned structure. This function uses // `System.Blockchain.GetContract` syscall. -func GetContract(scriptHash []byte) Contract { - return Contract{} +func GetContract(scriptHash []byte) contract.Contract { + return contract.Contract{} } diff --git a/pkg/interop/contract/contract.go b/pkg/interop/contract/contract.go index 12e530bc9..4ef212b34 100644 --- a/pkg/interop/contract/contract.go +++ b/pkg/interop/contract/contract.go @@ -4,10 +4,15 @@ Package contract provides functions to work with contracts. package contract // Contract represents a Neo contract and is used in interop functions. It's -// an opaque data structure that you can manipulate with using functions from +// a data structure that you can manipulate with using functions from // this package. It's similar in function to the Contract class in the Neo .net // framework. -type Contract struct{} +type Contract struct { + Script []byte + Manifest []byte + HasStorage bool + IsPayable bool +} // Create creates a new contract using a set of input parameters: // script contract's bytecode (limited in length by 1M) diff --git a/pkg/smartcontract/manifest/manifest.go b/pkg/smartcontract/manifest/manifest.go index e5b355498..d37b0f8af 100644 --- a/pkg/smartcontract/manifest/manifest.go +++ b/pkg/smartcontract/manifest/manifest.go @@ -85,6 +85,20 @@ func (m *Manifest) CanCall(toCall *Manifest, method string) bool { return false } +// IsValid checks whether the given hash is the one specified in manifest and +// verifies it against all the keys in manifest groups. +func (m *Manifest) IsValid(hash util.Uint160) bool { + if m.ABI.Hash != hash { + return false + } + for _, g := range m.Groups { + if !g.IsValid(hash) { + return false + } + } + return true +} + // MarshalJSON implements json.Marshaler interface. func (m *Manifest) MarshalJSON() ([]byte, error) { features := make(map[string]bool) diff --git a/pkg/smartcontract/manifest/manifest_test.go b/pkg/smartcontract/manifest/manifest_test.go index f0869e946..be2e06217 100644 --- a/pkg/smartcontract/manifest/manifest_test.go +++ b/pkg/smartcontract/manifest/manifest_test.go @@ -119,3 +119,50 @@ func TestPermission_IsAllowed(t *testing.T) { require.False(t, perm.IsAllowed(manifest, "AAA")) }) } + +func TestIsValid(t *testing.T) { + contractHash := util.Uint160{1, 2, 3} + m := NewManifest(contractHash) + + t.Run("valid, no groups", func(t *testing.T) { + require.True(t, m.IsValid(contractHash)) + }) + + t.Run("invalid, no groups", func(t *testing.T) { + require.False(t, m.IsValid(util.Uint160{9, 8, 7})) + }) + + t.Run("with groups", func(t *testing.T) { + m.Groups = make([]Group, 3) + pks := make([]*keys.PrivateKey, 3) + for i := range pks { + pk, err := keys.NewPrivateKey() + require.NoError(t, err) + pks[i] = pk + m.Groups[i] = Group{ + PublicKey: pk.PublicKey(), + Signature: pk.Sign(contractHash.BytesBE()), + } + } + + t.Run("valid", func(t *testing.T) { + require.True(t, m.IsValid(contractHash)) + }) + + t.Run("invalid, wrong contract hash", func(t *testing.T) { + require.False(t, m.IsValid(util.Uint160{4, 5, 6})) + }) + + t.Run("invalid, wrong group signature", func(t *testing.T) { + pk, err := keys.NewPrivateKey() + require.NoError(t, err) + m.Groups = append(m.Groups, Group{ + PublicKey: pk.PublicKey(), + // actually, there shouldn't be such situation, as Signature is always the signature + // of the contract hash. + Signature: pk.Sign([]byte{1, 2, 3}), + }) + require.False(t, m.IsValid(contractHash)) + }) + }) +} diff --git a/pkg/smartcontract/manifest/method.go b/pkg/smartcontract/manifest/method.go index 4a3ada56a..6c761a4aa 100644 --- a/pkg/smartcontract/manifest/method.go +++ b/pkg/smartcontract/manifest/method.go @@ -4,8 +4,10 @@ import ( "encoding/hex" "encoding/json" + "github.com/nspcc-dev/neo-go/pkg/crypto/hash" "github.com/nspcc-dev/neo-go/pkg/crypto/keys" "github.com/nspcc-dev/neo-go/pkg/smartcontract" + "github.com/nspcc-dev/neo-go/pkg/util" ) // Parameter represents smartcontract's parameter's definition. @@ -60,6 +62,11 @@ func DefaultEntryPoint() *Method { } } +// IsValid checks whether group's signature corresponds to the given hash. +func (g *Group) IsValid(h util.Uint160) bool { + return g.PublicKey.Verify(g.Signature, hash.Sha256(h.BytesBE()).BytesBE()) +} + // MarshalJSON implements json.Marshaler interface. func (g *Group) MarshalJSON() ([]byte, error) { aux := &groupAux{