core: allow Null in System.Contract.Update

Null means absense of script or manifest, empty
byte-slice is an error.

Related #1459.
This commit is contained in:
Evgenii Stratonikov 2020-10-05 16:12:08 +03:00
parent 0e82d4cbd1
commit 0b76f875c7
3 changed files with 61 additions and 24 deletions

View file

@ -112,6 +112,17 @@ func contractCreate(ic *interop.Context) error {
return nil
}
func checkNonEmpty(b []byte, max int) error {
if b != nil {
if l := len(b); l == 0 {
return errors.New("empty")
} else if l > max {
return fmt.Errorf("len is %d (max %d)", l, max)
}
}
return nil
}
// contractUpdate migrates a contract. This method assumes that Manifest and Script
// of the contract can be updated independently.
func contractUpdate(ic *interop.Context) error {
@ -119,22 +130,22 @@ func contractUpdate(ic *interop.Context) error {
if contract == nil {
return errors.New("contract doesn't exist")
}
script := ic.VM.Estack().Pop().Bytes()
if len(script) > MaxContractScriptSize {
return errors.New("the script is too big")
script := ic.VM.Estack().Pop().BytesOrNil()
manifestBytes := ic.VM.Estack().Pop().BytesOrNil()
if script == nil && manifestBytes == nil {
return errors.New("both script and manifest are nil")
}
manifestBytes := ic.VM.Estack().Pop().Bytes()
if len(manifestBytes) > manifest.MaxManifestSize {
return errors.New("manifest is too big")
if err := checkNonEmpty(script, MaxContractScriptSize); err != nil {
return fmt.Errorf("invalid script size: %w", err)
}
if err := checkNonEmpty(manifestBytes, manifest.MaxManifestSize); err != nil {
return fmt.Errorf("invalid manifest size: %w", err)
}
if !ic.VM.AddGas(int64(StoragePrice * (len(script) + len(manifestBytes)))) {
return errGasLimitExceeded
}
// if script was provided, update the old contract script and Manifest.ABI hash
if l := len(script); l > 0 {
if l > MaxContractScriptSize {
return errors.New("invalid script len")
}
if script != nil {
newHash := hash.Hash160(script)
if newHash.Equals(contract.ScriptHash()) {
return errors.New("the script is the same")
@ -158,7 +169,7 @@ func contractUpdate(ic *interop.Context) error {
}
// if manifest was provided, update the old contract manifest and check associated
// storage items if needed
if len(manifestBytes) > 0 {
if manifestBytes != nil {
var newManifest manifest.Manifest
err := newManifest.UnmarshalJSON(manifestBytes)
if err != nil {

View file

@ -627,7 +627,7 @@ func TestContractUpdate(t *testing.T) {
defer bc.Close()
v.GasLimit = -1
putArgsOnStack := func(script, manifest []byte) {
putArgsOnStack := func(script, manifest interface{}) {
v.Estack().PushVal(manifest)
v.Estack().PushVal(script)
}
@ -635,8 +635,8 @@ func TestContractUpdate(t *testing.T) {
t.Run("no args", func(t *testing.T) {
require.NoError(t, ic.DAO.PutContractState(cs))
v.LoadScriptWithHash([]byte{byte(opcode.RET)}, cs.ScriptHash(), smartcontract.All)
putArgsOnStack(nil, nil)
require.NoError(t, contractUpdate(ic))
putArgsOnStack(stackitem.Null{}, stackitem.Null{})
require.Error(t, contractUpdate(ic))
})
t.Run("no contract", func(t *testing.T) {
@ -648,14 +648,14 @@ func TestContractUpdate(t *testing.T) {
t.Run("too large script", func(t *testing.T) {
require.NoError(t, ic.DAO.PutContractState(cs))
v.LoadScriptWithHash([]byte{byte(opcode.RET)}, cs.ScriptHash(), smartcontract.All)
putArgsOnStack(make([]byte, MaxContractScriptSize+1), nil)
putArgsOnStack(make([]byte, MaxContractScriptSize+1), stackitem.Null{})
require.Error(t, contractUpdate(ic))
})
t.Run("too large manifest", func(t *testing.T) {
require.NoError(t, ic.DAO.PutContractState(cs))
v.LoadScriptWithHash([]byte{byte(opcode.RET)}, cs.ScriptHash(), smartcontract.All)
putArgsOnStack(nil, make([]byte, manifest.MaxManifestSize+1))
putArgsOnStack(stackitem.Null{}, make([]byte, manifest.MaxManifestSize+1))
require.Error(t, contractUpdate(ic))
})
@ -671,7 +671,7 @@ func TestContractUpdate(t *testing.T) {
require.NoError(t, ic.DAO.PutContractState(cs))
v.GasLimit = -1
v.LoadScriptWithHash([]byte{byte(opcode.RET)}, cs.ScriptHash(), smartcontract.All)
putArgsOnStack(cs.Script, nil)
putArgsOnStack(cs.Script, stackitem.Null{})
require.Error(t, contractUpdate(ic))
})
@ -689,16 +689,23 @@ func TestContractUpdate(t *testing.T) {
},
}))
v.LoadScriptWithHash([]byte{byte(opcode.RET)}, cs.ScriptHash(), smartcontract.All)
putArgsOnStack(duplicateScript, nil)
putArgsOnStack(duplicateScript, stackitem.Null{})
require.Error(t, contractUpdate(ic))
})
t.Run("update script, positive", func(t *testing.T) {
require.NoError(t, ic.DAO.PutContractState(cs))
t.Run("empty manifest", func(t *testing.T) {
v.LoadScriptWithHash([]byte{byte(opcode.RET)}, cs.ScriptHash(), smartcontract.All)
newScript := []byte{9, 8, 7, 6, 5}
putArgsOnStack(newScript, []byte{})
require.Error(t, contractUpdate(ic))
})
v.LoadScriptWithHash([]byte{byte(opcode.RET)}, cs.ScriptHash(), smartcontract.All)
newScript := []byte{9, 8, 7, 6, 5}
putArgsOnStack(newScript, nil)
putArgsOnStack(newScript, stackitem.Null{})
require.NoError(t, contractUpdate(ic))
@ -722,7 +729,7 @@ func TestContractUpdate(t *testing.T) {
t.Run("update manifest, bad manifest", func(t *testing.T) {
require.NoError(t, ic.DAO.PutContractState(cs))
v.LoadScriptWithHash([]byte{byte(opcode.RET)}, cs.ScriptHash(), smartcontract.All)
putArgsOnStack(nil, []byte{1, 2, 3})
putArgsOnStack(stackitem.Null{}, []byte{1, 2, 3})
require.Error(t, contractUpdate(ic))
})
@ -737,7 +744,7 @@ func TestContractUpdate(t *testing.T) {
}
manifestBytes, err := manifest.MarshalJSON()
require.NoError(t, err)
putArgsOnStack(nil, manifestBytes)
putArgsOnStack(stackitem.Null{}, manifestBytes)
require.Error(t, contractUpdate(ic))
})
@ -757,7 +764,7 @@ func TestContractUpdate(t *testing.T) {
}
manifestBytes, err := manifest.MarshalJSON()
require.NoError(t, err)
putArgsOnStack(nil, manifestBytes)
putArgsOnStack(stackitem.Null{}, manifestBytes)
require.Error(t, contractUpdate(ic))
})
@ -765,7 +772,6 @@ func TestContractUpdate(t *testing.T) {
t.Run("update manifest, positive", func(t *testing.T) {
cs.Manifest.Features = smartcontract.NoProperties
require.NoError(t, ic.DAO.PutContractState(cs))
v.LoadScriptWithHash([]byte{byte(opcode.RET)}, cs.ScriptHash(), smartcontract.All)
manifest := &manifest.Manifest{
ABI: manifest.ABI{
Hash: cs.ScriptHash(),
@ -774,8 +780,15 @@ func TestContractUpdate(t *testing.T) {
}
manifestBytes, err := manifest.MarshalJSON()
require.NoError(t, err)
putArgsOnStack(nil, manifestBytes)
t.Run("empty script", func(t *testing.T) {
v.LoadScriptWithHash([]byte{byte(opcode.RET)}, cs.ScriptHash(), smartcontract.All)
putArgsOnStack([]byte{}, manifestBytes)
require.Error(t, contractUpdate(ic))
})
v.LoadScriptWithHash([]byte{byte(opcode.RET)}, cs.ScriptHash(), smartcontract.All)
putArgsOnStack(stackitem.Null{}, manifestBytes)
require.NoError(t, contractUpdate(ic))
// updated contract should have new scripthash

View file

@ -100,6 +100,19 @@ func (e *Element) Bytes() []byte {
return bs
}
// BytesOrNil attempts to get the underlying value of the element as a byte array or nil.
// Will panic if the assertion failed which will be caught by the VM.
func (e *Element) BytesOrNil() []byte {
if _, ok := e.value.(stackitem.Null); ok {
return nil
}
bs, err := e.value.TryBytes()
if err != nil {
panic(err)
}
return bs
}
// String attempts to get string from the element value.
// It is assumed to be use in interops and panics if string is not a valid UTF-8 byte sequence.
func (e *Element) String() string {