reintroduce Refactor send command for better testability

api/api_full.go

@@ -32,6 +32,8 @@ import (
 	"github.com/filecoin-project/lotus/node/modules/dtypes"
 )
 
+//go:generate go run github.com/golang/mock/mockgen -destination=mocks/mock_full.go -package=mocks . FullNode
+
 // FullNode API is a low-level interface to the Filecoin network full node
 type FullNode interface {
 	Common

build/tools.go

@@ -3,6 +3,7 @@
 package build
 
 import (
+	_ "github.com/golang/mock/mockgen"
 	_ "github.com/GeertJohan/go.rice/rice"
 	_ "github.com/whyrusleeping/bencher"
 )

cli/cmd.go

@@ -207,6 +207,19 @@ func GetFullNodeAPI(ctx *cli.Context) (api.FullNode, jsonrpc.ClientCloser, error
 	return client.NewFullNodeRPC(ctx.Context, addr, headers)
 }
 
+func GetFullNodeServices(ctx *cli.Context) (ServicesAPI, error) {
+	if tn, ok := ctx.App.Metadata["test-services"]; ok {
+		return tn.(ServicesAPI), nil
+	}
+
+	api, c, err := GetFullNodeAPI(ctx)
+	if err != nil {
+		return nil, err
+	}
+
+	return &ServicesImpl{api: api, closer: c}, nil
+}
+
 type GetStorageMinerOptions struct {
 	PreferHttp bool
 }

cli/send.go

@@ -1,22 +1,17 @@
 package cli
 
 import (
-	"bytes"
-	"context"
 	"encoding/hex"
-	"encoding/json"
+	"errors"
 	"fmt"
-	"reflect"
 
 	"github.com/urfave/cli/v2"
-	cbg "github.com/whyrusleeping/cbor-gen"
+	"golang.org/x/xerrors"
 
 	"github.com/filecoin-project/go-address"
 	"github.com/filecoin-project/go-state-types/abi"
 
-	"github.com/filecoin-project/lotus/api"
 	"github.com/filecoin-project/lotus/chain/actors/builtin"
-	"github.com/filecoin-project/lotus/chain/stmgr"
 	"github.com/filecoin-project/lotus/chain/types"
 )
 
@@ -72,15 +67,16 @@ var sendCmd = &cli.Command{
 			return ShowHelp(cctx, fmt.Errorf("'send' expects two arguments, target and amount"))
 		}
 
-		api, closer, err := GetFullNodeAPI(cctx)
+		srv, err := GetFullNodeServices(cctx)
 		if err != nil {
 			return err
 		}
-		defer closer()
+		defer srv.Close() //nolint:errcheck
 
 		ctx := ReqContext(cctx)
+		var params SendParams
 
-		toAddr, err := address.NewFromString(cctx.Args().Get(0))
+		params.To, err = address.NewFromString(cctx.Args().Get(0))
 		if err != nil {
 			return ShowHelp(cctx, fmt.Errorf("failed to parse target address: %w", err))
 		}
@@ -89,123 +85,75 @@ var sendCmd = &cli.Command{
 		if err != nil {
 			return ShowHelp(cctx, fmt.Errorf("failed to parse amount: %w", err))
 		}
+		params.Val = abi.TokenAmount(val)
 
-		var fromAddr address.Address
-		if from := cctx.String("from"); from == "" {
-			defaddr, err := api.WalletDefaultAddress(ctx)
+		if from := cctx.String("from"); from != "" {
+			addr, err := address.NewFromString(from)
 			if err != nil {
 				return err
 			}
 
-			fromAddr = defaddr
-		} else {
-			addr, err := address.NewFromString(from)
+			params.From = addr
+		}
+
+		if cctx.IsSet("gas-premium") {
+			gp, err := types.BigFromString(cctx.String("gas-premium"))
 			if err != nil {
 				return err
 			}
-
-			fromAddr = addr
+			params.GasPremium = &gp
 		}
 
-		gp, err := types.BigFromString(cctx.String("gas-premium"))
-		if err != nil {
-			return err
+		if cctx.IsSet("gas-feecap") {
+			gfc, err := types.BigFromString(cctx.String("gas-feecap"))
+			if err != nil {
+				return err
+			}
+			params.GasFeeCap = &gfc
 		}
-		gfc, err := types.BigFromString(cctx.String("gas-feecap"))
-		if err != nil {
-			return err
+
+		if cctx.IsSet("gas-limit") {
+			limit := cctx.Int64("gas-limit")
+			params.GasLimit = &limit
 		}
 
-		method := abi.MethodNum(cctx.Uint64("method"))
+		params.Method = abi.MethodNum(cctx.Uint64("method"))
 
-		var params []byte
 		if cctx.IsSet("params-json") {
-			decparams, err := decodeTypedParams(ctx, api, toAddr, method, cctx.String("params-json"))
+			decparams, err := srv.DecodeTypedParamsFromJSON(ctx, params.To, params.Method, cctx.String("params-json"))
 			if err != nil {
 				return fmt.Errorf("failed to decode json params: %w", err)
 			}
-			params = decparams
+			params.Params = decparams
 		}
 		if cctx.IsSet("params-hex") {
-			if params != nil {
+			if params.Params != nil {
 				return fmt.Errorf("can only specify one of 'params-json' and 'params-hex'")
 			}
 			decparams, err := hex.DecodeString(cctx.String("params-hex"))
 			if err != nil {
 				return fmt.Errorf("failed to decode hex params: %w", err)
 			}
-			params = decparams
+			params.Params = decparams
 		}
 
-		msg := &types.Message{
-			From:       fromAddr,
-			To:         toAddr,
-			Value:      types.BigInt(val),
-			GasPremium: gp,
-			GasFeeCap:  gfc,
-			GasLimit:   cctx.Int64("gas-limit"),
-			Method:     method,
-			Params:     params,
-		}
-
-		if !cctx.Bool("force") {
-			// Funds insufficient check
-			fromBalance, err := api.WalletBalance(ctx, msg.From)
-			if err != nil {
-				return err
-			}
-			totalCost := types.BigAdd(types.BigMul(msg.GasFeeCap, types.NewInt(uint64(msg.GasLimit))), msg.Value)
+		params.Force = cctx.Bool("force")
 
-			if fromBalance.LessThan(totalCost) {
-				fmt.Printf("WARNING: From balance %s less than total cost %s\n", types.FIL(fromBalance), types.FIL(totalCost))
-				return fmt.Errorf("--force must be specified for this action to have an effect; you have been warned")
-			}
+		if cctx.IsSet("nonce") {
+			n := cctx.Uint64("nonce")
+			params.Nonce = &n
 		}
 
-		if cctx.IsSet("nonce") {
-			msg.Nonce = cctx.Uint64("nonce")
-			sm, err := api.WalletSignMessage(ctx, fromAddr, msg)
-			if err != nil {
-				return err
-			}
+		msgCid, err := srv.Send(ctx, params)
 
-			_, err = api.MpoolPush(ctx, sm)
-			if err != nil {
-				return err
-			}
-			fmt.Println(sm.Cid())
-		} else {
-			sm, err := api.MpoolPushMessage(ctx, msg, nil)
-			if err != nil {
-				return err
+		if err != nil {
+			if errors.Is(err, ErrSendBalanceTooLow) {
+				return fmt.Errorf("--force must be specified for this action to have an effect; you have been warned: %w", err)
 			}
-			fmt.Println(sm.Cid())
+			return xerrors.Errorf("executing send: %w", err)
 		}
 
+		fmt.Fprintf(cctx.App.Writer, "%s\n", msgCid)
 		return nil
 	},
 }
-
-func decodeTypedParams(ctx context.Context, fapi api.FullNode, to address.Address, method abi.MethodNum, paramstr string) ([]byte, error) {
-	act, err := fapi.StateGetActor(ctx, to, types.EmptyTSK)
-	if err != nil {
-		return nil, err
-	}
-
-	methodMeta, found := stmgr.MethodsMap[act.Code][method]
-	if !found {
-		return nil, fmt.Errorf("method %d not found on actor %s", method, act.Code)
-	}
-
-	p := reflect.New(methodMeta.Params.Elem()).Interface().(cbg.CBORMarshaler)
-
-	if err := json.Unmarshal([]byte(paramstr), p); err != nil {
-		return nil, fmt.Errorf("unmarshaling input into params type: %w", err)
-	}
-
-	buf := new(bytes.Buffer)
-	if err := p.MarshalCBOR(buf); err != nil {
-		return nil, err
-	}
-	return buf.Bytes(), nil
-}

cli/send_test.go

@@ -0,0 +1,128 @@
+package cli
+
+import (
+	"bytes"
+	"errors"
+	"testing"
+
+	"github.com/filecoin-project/go-address"
+	"github.com/filecoin-project/go-state-types/abi"
+	types "github.com/filecoin-project/lotus/chain/types"
+	gomock "github.com/golang/mock/gomock"
+	cid "github.com/ipfs/go-cid"
+	"github.com/stretchr/testify/assert"
+	ucli "github.com/urfave/cli/v2"
+)
+
+var arbtCid = (&types.Message{
+	From:  mustAddr(address.NewIDAddress(2)),
+	To:    mustAddr(address.NewIDAddress(1)),
+	Value: types.NewInt(1000),
+}).Cid()
+
+func mustAddr(a address.Address, err error) address.Address {
+	if err != nil {
+		panic(err)
+	}
+	return a
+}
+
+func newMockApp(t *testing.T, cmd *ucli.Command) (*ucli.App, *MockServicesAPI, *bytes.Buffer, func()) {
+	app := ucli.NewApp()
+	app.Commands = ucli.Commands{cmd}
+	app.Setup()
+
+	mockCtrl := gomock.NewController(t)
+	mockSrvcs := NewMockServicesAPI(mockCtrl)
+	app.Metadata["test-services"] = mockSrvcs
+
+	buf := &bytes.Buffer{}
+	app.Writer = buf
+
+	return app, mockSrvcs, buf, mockCtrl.Finish
+}
+
+func TestSendCLI(t *testing.T) {
+	oneFil := abi.TokenAmount(types.MustParseFIL("1"))
+
+	t.Run("simple", func(t *testing.T) {
+		app, mockSrvcs, buf, done := newMockApp(t, sendCmd)
+		defer done()
+
+		gomock.InOrder(
+			mockSrvcs.EXPECT().Send(gomock.Any(), SendParams{
+				To:  mustAddr(address.NewIDAddress(1)),
+				Val: oneFil,
+			}).Return(arbtCid, nil),
+			mockSrvcs.EXPECT().Close(),
+		)
+		err := app.Run([]string{"lotus", "send", "t01", "1"})
+		assert.NoError(t, err)
+		assert.EqualValues(t, arbtCid.String()+"\n", buf.String())
+	})
+	t.Run("ErrSendBalanceTooLow", func(t *testing.T) {
+		app, mockSrvcs, _, done := newMockApp(t, sendCmd)
+		defer done()
+
+		gomock.InOrder(
+			mockSrvcs.EXPECT().Send(gomock.Any(), SendParams{
+				To:  mustAddr(address.NewIDAddress(1)),
+				Val: oneFil,
+			}).Return(cid.Undef, ErrSendBalanceTooLow),
+			mockSrvcs.EXPECT().Close(),
+		)
+		err := app.Run([]string{"lotus", "send", "t01", "1"})
+		assert.ErrorIs(t, err, ErrSendBalanceTooLow)
+	})
+	t.Run("generic-err-is-forwarded", func(t *testing.T) {
+		app, mockSrvcs, _, done := newMockApp(t, sendCmd)
+		defer done()
+
+		errMark := errors.New("something")
+		gomock.InOrder(
+			mockSrvcs.EXPECT().Send(gomock.Any(), SendParams{
+				To:  mustAddr(address.NewIDAddress(1)),
+				Val: oneFil,
+			}).Return(cid.Undef, errMark),
+			mockSrvcs.EXPECT().Close(),
+		)
+		err := app.Run([]string{"lotus", "send", "t01", "1"})
+		assert.ErrorIs(t, err, errMark)
+	})
+
+	t.Run("from-specific", func(t *testing.T) {
+		app, mockSrvcs, buf, done := newMockApp(t, sendCmd)
+		defer done()
+
+		gomock.InOrder(
+			mockSrvcs.EXPECT().Send(gomock.Any(), SendParams{
+				To:   mustAddr(address.NewIDAddress(1)),
+				From: mustAddr(address.NewIDAddress(2)),
+				Val:  oneFil,
+			}).Return(arbtCid, nil),
+			mockSrvcs.EXPECT().Close(),
+		)
+		err := app.Run([]string{"lotus", "send", "--from=t02", "t01", "1"})
+		assert.NoError(t, err)
+		assert.EqualValues(t, arbtCid.String()+"\n", buf.String())
+	})
+
+	t.Run("nonce-specific", func(t *testing.T) {
+		app, mockSrvcs, buf, done := newMockApp(t, sendCmd)
+		defer done()
+		zero := uint64(0)
+
+		gomock.InOrder(
+			mockSrvcs.EXPECT().Send(gomock.Any(), SendParams{
+				To:    mustAddr(address.NewIDAddress(1)),
+				Nonce: &zero,
+				Val:   oneFil,
+			}).Return(arbtCid, nil),
+			mockSrvcs.EXPECT().Close(),
+		)
+		err := app.Run([]string{"lotus", "send", "--nonce=0", "t01", "1"})
+		assert.NoError(t, err)
+		assert.EqualValues(t, arbtCid.String()+"\n", buf.String())
+	})
+
+}