From 27880ece2bd864770213a91df85c014784d7dc59 Mon Sep 17 00:00:00 2001 From: Aayush Date: Mon, 18 Jul 2022 12:36:51 -0400 Subject: [PATCH] feat: support typed errors over RPC --- api/api_errors.go | 29 +++++++++++++ api/client/client.go | 13 +++--- build/params_shared_vals.go | 5 ++- chain/messagepool/repub.go | 2 +- chain/messagepool/selection.go | 4 +- chain/store/basefee_test.go | 2 +- .../simulation/blockbuilder/blockbuilder.go | 9 ++-- cmd/lotus-wallet/main.go | 2 +- cmd/lotus-worker/sealworker/rpc.go | 2 +- cmd/lotus/daemon.go | 5 ++- gateway/handler.go | 2 +- itests/api_test.go | 43 +++++++++++++++++++ itests/paych_cli_test.go | 16 +++++++ node/impl/full/gas.go | 6 ++- node/impl/full/state.go | 7 ++- node/rpc.go | 4 +- 16 files changed, 127 insertions(+), 24 deletions(-) create mode 100644 api/api_errors.go diff --git a/api/api_errors.go b/api/api_errors.go new file mode 100644 index 000000000..953cf63bf --- /dev/null +++ b/api/api_errors.go @@ -0,0 +1,29 @@ +package api + +import ( + "github.com/filecoin-project/go-jsonrpc" +) + +const ( + EOutOfGas = iota + jsonrpc.FirstUserCode + EActorNotFound +) + +type ErrOutOfGas struct{} + +func (e ErrOutOfGas) Error() string { + return "call ran out of gas" +} + +type ErrActorNotFound struct{} + +func (e ErrActorNotFound) Error() string { + return "actor not found" +} + +var RPCErrors = jsonrpc.NewErrors() + +func init() { + RPCErrors.Register(EOutOfGas, new(ErrOutOfGas)) + RPCErrors.Register(EActorNotFound, new(ErrActorNotFound)) +} diff --git a/api/client/client.go b/api/client/client.go index 669c58f27..32583097e 100644 --- a/api/client/client.go +++ b/api/client/client.go @@ -19,7 +19,7 @@ import ( func NewCommonRPCV0(ctx context.Context, addr string, requestHeader http.Header) (api.CommonNet, jsonrpc.ClientCloser, error) { var res v0api.CommonNetStruct closer, err := jsonrpc.NewMergeClient(ctx, addr, "Filecoin", - api.GetInternalStructs(&res), requestHeader) + api.GetInternalStructs(&res), requestHeader, jsonrpc.WithErrors(api.RPCErrors)) return &res, closer, err } @@ -29,7 +29,7 @@ func NewFullNodeRPCV0(ctx context.Context, addr string, requestHeader http.Heade var res v0api.FullNodeStruct closer, err := jsonrpc.NewMergeClient(ctx, addr, "Filecoin", - api.GetInternalStructs(&res), requestHeader) + api.GetInternalStructs(&res), requestHeader, jsonrpc.WithErrors(api.RPCErrors)) return &res, closer, err } @@ -38,7 +38,7 @@ func NewFullNodeRPCV0(ctx context.Context, addr string, requestHeader http.Heade func NewFullNodeRPCV1(ctx context.Context, addr string, requestHeader http.Header) (api.FullNode, jsonrpc.ClientCloser, error) { var res v1api.FullNodeStruct closer, err := jsonrpc.NewMergeClient(ctx, addr, "Filecoin", - api.GetInternalStructs(&res), requestHeader) + api.GetInternalStructs(&res), requestHeader, jsonrpc.WithErrors(api.RPCErrors)) return &res, closer, err } @@ -72,6 +72,7 @@ func NewStorageMinerRPCV0(ctx context.Context, addr string, requestHeader http.H api.GetInternalStructs(&res), requestHeader, append([]jsonrpc.Option{ rpcenc.ReaderParamEncoder(pushUrl), + jsonrpc.WithErrors(api.RPCErrors), }, opts...)...) return &res, closer, err @@ -90,6 +91,7 @@ func NewWorkerRPCV0(ctx context.Context, addr string, requestHeader http.Header) rpcenc.ReaderParamEncoder(pushUrl), jsonrpc.WithNoReconnect(), jsonrpc.WithTimeout(30*time.Second), + jsonrpc.WithErrors(api.RPCErrors), ) return &res, closer, err @@ -101,7 +103,7 @@ func NewGatewayRPCV1(ctx context.Context, addr string, requestHeader http.Header closer, err := jsonrpc.NewMergeClient(ctx, addr, "Filecoin", api.GetInternalStructs(&res), requestHeader, - opts..., + append(opts, jsonrpc.WithErrors(api.RPCErrors))..., ) return &res, closer, err @@ -113,7 +115,7 @@ func NewGatewayRPCV0(ctx context.Context, addr string, requestHeader http.Header closer, err := jsonrpc.NewMergeClient(ctx, addr, "Filecoin", api.GetInternalStructs(&res), requestHeader, - opts..., + append(opts, jsonrpc.WithErrors(api.RPCErrors))..., ) return &res, closer, err @@ -124,6 +126,7 @@ func NewWalletRPCV0(ctx context.Context, addr string, requestHeader http.Header) closer, err := jsonrpc.NewMergeClient(ctx, addr, "Filecoin", api.GetInternalStructs(&res), requestHeader, + jsonrpc.WithErrors(api.RPCErrors), ) return &res, closer, err diff --git a/build/params_shared_vals.go b/build/params_shared_vals.go index a77feb9d8..f7cc2374e 100644 --- a/build/params_shared_vals.go +++ b/build/params_shared_vals.go @@ -118,8 +118,9 @@ const VerifSigCacheSize = 32000 // TODO: If this is gonna stay, it should move to specs-actors const BlockMessageLimit = 10000 -const BlockGasLimit = 10_000_000_000 -const BlockGasTarget = BlockGasLimit / 2 +var BlockGasLimit = int64(10_000_000_000) +var BlockGasTarget = BlockGasLimit / 2 + const BaseFeeMaxChangeDenom = 8 // 12.5% const InitialBaseFee = 100e6 const MinimumBaseFee = 100 diff --git a/chain/messagepool/repub.go b/chain/messagepool/repub.go index c4f1b26e3..9a1e19b60 100644 --- a/chain/messagepool/repub.go +++ b/chain/messagepool/repub.go @@ -79,7 +79,7 @@ func (mp *MessagePool) republishPendingMessages(ctx context.Context) error { return chains[i].Before(chains[j]) }) - gasLimit := int64(build.BlockGasLimit) + gasLimit := build.BlockGasLimit minGas := int64(gasguess.MinGas) var msgs []*types.SignedMessage loop: diff --git a/chain/messagepool/selection.go b/chain/messagepool/selection.go index 0d93f2af9..e84962869 100644 --- a/chain/messagepool/selection.go +++ b/chain/messagepool/selection.go @@ -258,7 +258,7 @@ func (mp *MessagePool) selectMessagesOptimal(ctx context.Context, curTs, ts *typ nextChain := 0 partitions := make([][]*msgChain, MaxBlocks) for i := 0; i < MaxBlocks && nextChain < len(chains); i++ { - gasLimit := int64(build.BlockGasLimit) + gasLimit := build.BlockGasLimit msgLimit := build.BlockMessageLimit for nextChain < len(chains) { chain := chains[nextChain] @@ -590,7 +590,7 @@ func (mp *MessagePool) selectPriorityMessages(ctx context.Context, pending map[a mpCfg := mp.getConfig() result := &selectedMessages{ msgs: make([]*types.SignedMessage, 0, mpCfg.SizeLimitLow), - gasLimit: int64(build.BlockGasLimit), + gasLimit: build.BlockGasLimit, blsLimit: cbg.MaxLength, secpLimit: cbg.MaxLength, } diff --git a/chain/store/basefee_test.go b/chain/store/basefee_test.go index ea45a7673..8dd61f709 100644 --- a/chain/store/basefee_test.go +++ b/chain/store/basefee_test.go @@ -25,7 +25,7 @@ func TestBaseFee(t *testing.T) { {100e6, build.BlockGasTarget, 1, 103.125e6, 100e6}, {100e6, build.BlockGasTarget * 2, 2, 103.125e6, 100e6}, {100e6, build.BlockGasLimit * 2, 2, 112.5e6, 112.5e6}, - {100e6, build.BlockGasLimit * 1.5, 2, 110937500, 106.250e6}, + {100e6, (build.BlockGasLimit * 15) / 10, 2, 110937500, 106.250e6}, } for _, test := range tests { diff --git a/cmd/lotus-sim/simulation/blockbuilder/blockbuilder.go b/cmd/lotus-sim/simulation/blockbuilder/blockbuilder.go index d9a32481c..f984c993d 100644 --- a/cmd/lotus-sim/simulation/blockbuilder/blockbuilder.go +++ b/cmd/lotus-sim/simulation/blockbuilder/blockbuilder.go @@ -31,12 +31,13 @@ const ( // has. // 5 per tipset, but we effectively get 4 blocks worth of messages. expectedBlocks = 4 - // TODO: This will produce invalid blocks but it will accurately model the amount of gas - // we're willing to use per-tipset. - // A more correct approach would be to produce 5 blocks. We can do that later. - targetGas = build.BlockGasTarget * expectedBlocks ) +// TODO: This will produce invalid blocks but it will accurately model the amount of gas +// we're willing to use per-tipset. +// A more correct approach would be to produce 5 blocks. We can do that later. +var targetGas = build.BlockGasTarget * expectedBlocks + type BlockBuilder struct { ctx context.Context logger *zap.SugaredLogger diff --git a/cmd/lotus-wallet/main.go b/cmd/lotus-wallet/main.go index f15b9e84d..75d31da56 100644 --- a/cmd/lotus-wallet/main.go +++ b/cmd/lotus-wallet/main.go @@ -209,7 +209,7 @@ var runCmd = &cli.Command{ rpcApi = api.PermissionedWalletAPI(rpcApi) } - rpcServer := jsonrpc.NewServer() + rpcServer := jsonrpc.NewServer(jsonrpc.WithServerErrors(api.RPCErrors)) rpcServer.Register("Filecoin", rpcApi) mux.Handle("/rpc/v0", rpcServer) diff --git a/cmd/lotus-worker/sealworker/rpc.go b/cmd/lotus-worker/sealworker/rpc.go index 1b98ccb83..7d84b5c8b 100644 --- a/cmd/lotus-worker/sealworker/rpc.go +++ b/cmd/lotus-worker/sealworker/rpc.go @@ -29,7 +29,7 @@ var log = logging.Logger("sealworker") func WorkerHandler(authv func(ctx context.Context, token string) ([]auth.Permission, error), remote http.HandlerFunc, a api.Worker, permissioned bool) http.Handler { mux := mux.NewRouter() readerHandler, readerServerOpt := rpcenc.ReaderParamDecoder() - rpcServer := jsonrpc.NewServer(readerServerOpt) + rpcServer := jsonrpc.NewServer(jsonrpc.WithServerErrors(api.RPCErrors), readerServerOpt) wapi := proxy.MetricedWorkerAPI(a) if permissioned { diff --git a/cmd/lotus/daemon.go b/cmd/lotus/daemon.go index 33c579e3a..8086ab2d1 100644 --- a/cmd/lotus/daemon.go +++ b/cmd/lotus/daemon.go @@ -31,6 +31,7 @@ import ( "github.com/filecoin-project/go-paramfetch" "github.com/filecoin-project/lotus/api" + lapi "github.com/filecoin-project/lotus/api" "github.com/filecoin-project/lotus/build" "github.com/filecoin-project/lotus/chain/consensus/filcns" "github.com/filecoin-project/lotus/chain/stmgr" @@ -303,7 +304,7 @@ var DaemonCmd = &cli.Command{ } defer closer() - liteModeDeps = node.Override(new(api.Gateway), gapi) + liteModeDeps = node.Override(new(lapi.Gateway), gapi) } // some libraries like ipfs/go-ds-measure and ipfs/go-ipfs-blockstore @@ -360,7 +361,7 @@ var DaemonCmd = &cli.Command{ // ---- // Populate JSON-RPC options. - serverOptions := make([]jsonrpc.ServerOption, 0) + serverOptions := []jsonrpc.ServerOption{jsonrpc.WithServerErrors(lapi.RPCErrors)} if maxRequestSize := cctx.Int("api-max-req-size"); maxRequestSize != 0 { serverOptions = append(serverOptions, jsonrpc.WithMaxRequestSize(int64(maxRequestSize))) } diff --git a/gateway/handler.go b/gateway/handler.go index 648065a44..be824b430 100644 --- a/gateway/handler.go +++ b/gateway/handler.go @@ -30,7 +30,7 @@ func Handler(gwapi lapi.Gateway, api lapi.FullNode, rateLimit int64, connPerMinu m := mux.NewRouter() serveRpc := func(path string, hnd interface{}) { - rpcServer := jsonrpc.NewServer(opts...) + rpcServer := jsonrpc.NewServer(append(opts, jsonrpc.WithServerErrors(lapi.RPCErrors))...) rpcServer.Register("Filecoin", hnd) rpcServer.AliasMethod("rpc.discover", "Filecoin.Discover") diff --git a/itests/api_test.go b/itests/api_test.go index 7eac4e8a3..36b495c61 100644 --- a/itests/api_test.go +++ b/itests/api_test.go @@ -10,6 +10,7 @@ import ( logging "github.com/ipfs/go-log/v2" "github.com/libp2p/go-libp2p/core/peer" "github.com/stretchr/testify/require" + "golang.org/x/xerrors" "github.com/filecoin-project/go-address" "github.com/filecoin-project/go-state-types/big" @@ -51,6 +52,8 @@ func runAPITest(t *testing.T, opts ...interface{}) { t.Run("testMiningReal", ts.testMiningReal) t.Run("testSlowNotify", ts.testSlowNotify) t.Run("testSearchMsg", ts.testSearchMsg) + t.Run("testOutOfGasError", ts.testOutOfGasError) + t.Run("testLookupNotFoundError", ts.testLookupNotFoundError) t.Run("testNonGenesisMiner", ts.testNonGenesisMiner) } @@ -149,6 +152,46 @@ func (ts *apiSuite) testSearchMsg(t *testing.T) { require.Equalf(t, res.TipSet, searchRes.TipSet, "search ts: %s, different from wait ts: %s", searchRes.TipSet, res.TipSet) } +func (ts *apiSuite) testOutOfGasError(t *testing.T) { + ctx := context.Background() + + full, _, _ := kit.EnsembleMinimal(t, ts.opts...) + + senderAddr, err := full.WalletDefaultAddress(ctx) + require.NoError(t, err) + + // the gas estimator API executes the message with gasLimit = BlockGasLimit + // Lowering it to 2 will cause it to run out of gas, testing the failure case we want + originalLimit := build.BlockGasLimit + build.BlockGasLimit = 2 + defer func() { + build.BlockGasLimit = originalLimit + }() + + msg := &types.Message{ + From: senderAddr, + To: senderAddr, + Value: big.Zero(), + } + + _, err = full.GasEstimateMessageGas(ctx, msg, nil, types.EmptyTSK) + require.Error(t, err, "should have failed") + require.True(t, xerrors.Is(err, lapi.ErrOutOfGas{})) +} + +func (ts *apiSuite) testLookupNotFoundError(t *testing.T) { + ctx := context.Background() + + full, _, _ := kit.EnsembleMinimal(t, ts.opts...) + + addr, err := full.WalletNew(ctx, types.KTSecp256k1) + require.NoError(t, err) + + _, err = full.StateLookupID(ctx, addr, types.EmptyTSK) + require.Error(t, err) + require.True(t, xerrors.Is(err, lapi.ErrActorNotFound{})) +} + func (ts *apiSuite) testMining(t *testing.T) { ctx := context.Background() diff --git a/itests/paych_cli_test.go b/itests/paych_cli_test.go index d73f96905..0075a1204 100644 --- a/itests/paych_cli_test.go +++ b/itests/paych_cli_test.go @@ -13,6 +13,7 @@ import ( cbor "github.com/ipfs/go-ipld-cbor" "github.com/stretchr/testify/require" + "golang.org/x/xerrors" "github.com/filecoin-project/go-address" "github.com/filecoin-project/go-state-types/abi" @@ -66,6 +67,21 @@ func TestPaymentChannelsBasic(t *testing.T) { vamt := strconv.Itoa(voucherAmt) voucher := creatorCLI.RunCmd("paych", "voucher", "create", chAddr.String(), vamt) + // DEFLAKE: We have observed this test flakily failing when the receiver node hasn't seen the paych create message + // This makes us wait as much as 10 epochs before giving up and failing + retry := 0 + _, err = paymentReceiver.StateLookupID(ctx, chAddr, types.EmptyTSK) + for err != nil && xerrors.Is(err, api.ErrActorNotFound{}) { + time.Sleep(blocktime) + _, err = paymentReceiver.StateLookupID(ctx, chAddr, types.EmptyTSK) + retry++ + if retry > 10 { + break + } + } + + require.NoError(t, err) + // receiver: paych voucher add receiverCLI.RunCmd("paych", "voucher", "add", chAddr.String(), voucher) diff --git a/node/impl/full/gas.go b/node/impl/full/gas.go index c179c27d6..4b7ebc4a0 100644 --- a/node/impl/full/gas.go +++ b/node/impl/full/gas.go @@ -290,6 +290,10 @@ func gasEstimateGasLimit( if err != nil { return -1, xerrors.Errorf("CallWithGas failed: %w", err) } + if res.MsgRct.ExitCode == exitcode.SysErrOutOfGas { + return -1, api.ErrOutOfGas{} + } + if res.MsgRct.ExitCode != exitcode.Ok { return -1, xerrors.Errorf("message execution failed: exit %s, reason: %s", res.MsgRct.ExitCode, res.Error) } @@ -356,7 +360,7 @@ func (m *GasModule) GasEstimateMessageGas(ctx context.Context, msg *types.Messag if msg.GasLimit == 0 { gasLimit, err := m.GasEstimateGasLimit(ctx, msg, types.EmptyTSK) if err != nil { - return nil, xerrors.Errorf("estimating gas used: %w", err) + return nil, err } msg.GasLimit = int64(float64(gasLimit) * m.Mpool.GetConfig().GasLimitOverestimation) } diff --git a/node/impl/full/state.go b/node/impl/full/state.go index 5247b9fb5..d11037d49 100644 --- a/node/impl/full/state.go +++ b/node/impl/full/state.go @@ -482,7 +482,12 @@ func (m *StateModule) StateLookupID(ctx context.Context, addr address.Address, t return address.Undef, xerrors.Errorf("loading tipset %s: %w", tsk, err) } - return m.StateManager.LookupID(ctx, addr, ts) + ret, err := m.StateManager.LookupID(ctx, addr, ts) + if err != nil && xerrors.Is(err, types.ErrActorNotFound) { + return address.Undef, api.ErrActorNotFound{} + } + + return ret, err } func (a *StateAPI) StateLookupRobustAddress(ctx context.Context, addr address.Address, tsk types.TipSetKey) (address.Address, error) { diff --git a/node/rpc.go b/node/rpc.go index bee540500..2c85c71be 100644 --- a/node/rpc.go +++ b/node/rpc.go @@ -69,7 +69,7 @@ func FullNodeHandler(a v1api.FullNode, permissioned bool, opts ...jsonrpc.Server m := mux.NewRouter() serveRpc := func(path string, hnd interface{}) { - rpcServer := jsonrpc.NewServer(opts...) + rpcServer := jsonrpc.NewServer(append(opts, jsonrpc.WithServerErrors(api.RPCErrors))...) rpcServer.Register("Filecoin", hnd) rpcServer.AliasMethod("rpc.discover", "Filecoin.Discover") @@ -130,7 +130,7 @@ func MinerHandler(a api.StorageMiner, permissioned bool) (http.Handler, error) { } readerHandler, readerServerOpt := rpcenc.ReaderParamDecoder() - rpcServer := jsonrpc.NewServer(readerServerOpt) + rpcServer := jsonrpc.NewServer(jsonrpc.WithServerErrors(api.RPCErrors), readerServerOpt) rpcServer.Register("Filecoin", mapi) rpcServer.AliasMethod("rpc.discover", "Filecoin.Discover")