Merge pull request #4783 from filecoin-project/asr/1.2.0

Small consensus-critical fixes in 1.2.0

chain/actors/params.go

@@ -3,6 +3,8 @@ package actors
 import (
 	"bytes"
 
+	"github.com/filecoin-project/go-state-types/exitcode"
+
 	"github.com/filecoin-project/lotus/chain/actors/aerrors"
 	cbg "github.com/whyrusleeping/cbor-gen"
 )
@@ -11,7 +13,7 @@ func SerializeParams(i cbg.CBORMarshaler) ([]byte, aerrors.ActorError) {
 	buf := new(bytes.Buffer)
 	if err := i.MarshalCBOR(buf); err != nil {
 		// TODO: shouldnt this be a fatal error?
-		return nil, aerrors.Absorb(err, 1, "failed to encode parameter")
+		return nil, aerrors.Absorb(err, exitcode.ErrSerialization, "failed to encode parameter")
 	}
 	return buf.Bytes(), nil
 }

chain/vm/invoker.go

@@ -6,6 +6,8 @@ import (
 	"fmt"
 	"reflect"
 
+	"github.com/filecoin-project/go-state-types/network"
+
 	"github.com/filecoin-project/lotus/chain/actors/builtin"
 
 	"github.com/ipfs/go-cid"
@@ -173,9 +175,14 @@ func (*ActorRegistry) transform(instance invokee) (nativeCode, error) {
 				paramT := meth.Type().In(1).Elem()
 				param := reflect.New(paramT)
 
+				rt := in[0].Interface().(*Runtime)
 				inBytes := in[1].Interface().([]byte)
 				if err := DecodeParams(inBytes, param.Interface()); err != nil {
-					aerr := aerrors.Absorb(err, 1, "failed to decode parameters")
+					ec := exitcode.ErrSerialization
+					if rt.NetworkVersion() < network.Version7 {
+						ec = 1
+					}
+					aerr := aerrors.Absorb(err, ec, "failed to decode parameters")
 					return []reflect.Value{
 						reflect.ValueOf([]byte{}),
 						// Below is a hack, fixed in Go 1.13
@@ -183,7 +190,6 @@ func (*ActorRegistry) transform(instance invokee) (nativeCode, error) {
 						reflect.ValueOf(&aerr).Elem(),
 					}
 				}
-				rt := in[0].Interface().(*Runtime)
 				rval, aerror := rt.shimCall(func() interface{} {
 					ret := meth.Call([]reflect.Value{
 						reflect.ValueOf(rt),

chain/vm/invoker_test.go

@@ -1,10 +1,13 @@
 package vm
 
 import (
+	"context"
 	"fmt"
 	"io"
 	"testing"
 
+	"github.com/filecoin-project/go-state-types/network"
+
 	cbor "github.com/ipfs/go-ipld-cbor"
 	"github.com/stretchr/testify/assert"
 	cbg "github.com/whyrusleeping/cbor-gen"
@@ -105,10 +108,27 @@ func TestInvokerBasic(t *testing.T) {
 		}
 	}
 
-	_, aerr := code[1](&Runtime{}, []byte{99})
+	{
+		_, aerr := code[1](&Runtime{
+			vm: &VM{ntwkVersion: func(ctx context.Context, epoch abi.ChainEpoch) network.Version {
+				return network.Version0
+			}},
+		}, []byte{99})
 		if aerrors.IsFatal(aerr) {
 			t.Fatal("err should not be fatal")
 		}
 		assert.Equal(t, exitcode.ExitCode(1), aerrors.RetCode(aerr), "return code should be 1")
-
+	}
+
+	{
+		_, aerr := code[1](&Runtime{
+			vm: &VM{ntwkVersion: func(ctx context.Context, epoch abi.ChainEpoch) network.Version {
+				return network.Version7
+			}},
+		}, []byte{99})
+		if aerrors.IsFatal(aerr) {
+			t.Fatal("err should not be fatal")
+		}
+		assert.Equal(t, exitcode.ErrSerialization, aerrors.RetCode(aerr), "return code should be %s", 1)
+	}
 }

chain/vm/runtime.go

@@ -244,20 +244,23 @@ func (rt *Runtime) NewActorAddress() address.Address {
 	return addr
 }
 
-func (rt *Runtime) CreateActor(codeID cid.Cid, address address.Address) {
+func (rt *Runtime) CreateActor(codeID cid.Cid, addr address.Address) {
+	if addr == address.Undef && rt.NetworkVersion() >= network.Version7 {
+		rt.Abortf(exitcode.SysErrorIllegalArgument, "CreateActor with Undef address")
+	}
 	act, aerr := rt.vm.areg.Create(codeID, rt)
 	if aerr != nil {
 		rt.Abortf(aerr.RetCode(), aerr.Error())
 	}
 
-	_, err := rt.state.GetActor(address)
+	_, err := rt.state.GetActor(addr)
 	if err == nil {
 		rt.Abortf(exitcode.SysErrorIllegalArgument, "Actor address already exists")
 	}
 
 	rt.chargeGas(rt.Pricelist().OnCreateActor())
 
-	err = rt.state.SetActor(address, act)
+	err = rt.state.SetActor(addr, act)
 	if err != nil {
 		panic(aerrors.Fatalf("creating actor entry: %v", err))
 	}
@@ -266,7 +269,7 @@ func (rt *Runtime) CreateActor(codeID cid.Cid, address address.Address) {
 
 // DeleteActor deletes the executing actor from the state tree, transferring
 // any balance to beneficiary.
-// Aborts if the beneficiary does not exist.
+// Aborts if the beneficiary does not exist or is the calling actor.
 // May only be called by the actor itself.
 func (rt *Runtime) DeleteActor(beneficiary address.Address) {
 	rt.chargeGas(rt.Pricelist().OnDeleteActor())
@@ -278,6 +281,9 @@ func (rt *Runtime) DeleteActor(beneficiary address.Address) {
 		panic(aerrors.Fatalf("failed to get actor: %s", err))
 	}
 	if !act.Balance.IsZero() {
+		if beneficiary == rt.Receiver() && rt.NetworkVersion() >= network.Version7 {
+			rt.Abortf(exitcode.SysErrorIllegalArgument, "benefactor cannot be beneficiary")
+		}
 		// Transfer the executing actor's balance to the beneficiary
 		if err := rt.vm.transfer(rt.Receiver(), beneficiary, act.Balance); err != nil {
 			panic(aerrors.Fatalf("failed to transfer balance to beneficiary actor: %s", err))

chain/vm/vm.go

@@ -138,6 +138,10 @@ func (vm *VM) makeRuntime(ctx context.Context, msg *types.Message, parent *Runti
 	}
 
 	if parent != nil {
+		// TODO: The version check here should be unnecessary, but we can wait to take it out
+		if !parent.allowInternal && rt.NetworkVersion() >= network.Version7 {
+			rt.Abortf(exitcode.SysErrForbidden, "internal calls currently disabled")
+		}
 		rt.gasUsed = parent.gasUsed
 		rt.origin = parent.origin
 		rt.originNonce = parent.originNonce