From e6326438013bfe0c30cc1f86c04f398b3c681dd0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C5=81ukasz=20Magiera?= Date: Thu, 17 Sep 2020 12:22:56 +0200 Subject: [PATCH] api: Test return types --- api/api_full.go | 2 +- api/api_test.go | 69 +++++++++++++++++++ api/apistruct/struct.go | 4 +- cli/wallet.go | 6 +- .../sector-storage/ffiwrapper/sealer_test.go | 12 ++++ node/impl/full/wallet.go | 4 +- 6 files changed, 91 insertions(+), 6 deletions(-) diff --git a/api/api_full.go b/api/api_full.go index 1adcc5c19..778b458a8 100644 --- a/api/api_full.go +++ b/api/api_full.go @@ -230,7 +230,7 @@ type FullNode interface { WalletSignMessage(context.Context, address.Address, *types.Message) (*types.SignedMessage, error) // WalletVerify takes an address, a signature, and some bytes, and indicates whether the signature is valid. // The address does not have to be in the wallet. - WalletVerify(context.Context, address.Address, []byte, *crypto.Signature) bool + WalletVerify(context.Context, address.Address, []byte, *crypto.Signature) (bool, error) // WalletDefaultAddress returns the address marked as default in the wallet. WalletDefaultAddress(context.Context) (address.Address, error) // WalletSetDefault marks the given address as as the default one. diff --git a/api/api_test.go b/api/api_test.go index 1b438258a..065141426 100644 --- a/api/api_test.go +++ b/api/api_test.go @@ -1,12 +1,16 @@ package api import ( + "encoding/json" "os" "os/exec" "path/filepath" + "reflect" "runtime" "strings" "testing" + + "github.com/stretchr/testify/require" ) func goCmd() string { @@ -32,3 +36,68 @@ func TestDoesntDependOnFFI(t *testing.T) { } } } + +func TestReturnTypes(t *testing.T) { + errType := reflect.TypeOf(new(error)).Elem() + bareIface := reflect.TypeOf(new(interface{})).Elem() + jmarsh := reflect.TypeOf(new(json.Marshaler)).Elem() + + tst := func(api interface{}) func(t *testing.T) { + return func(t *testing.T) { + ra := reflect.TypeOf(api).Elem() + for i := 0; i < ra.NumMethod(); i++ { + m := ra.Method(i) + switch m.Type.NumOut() { + case 1: // if 1 return value, it must be an error + require.Equal(t, errType, m.Type.Out(0), m.Name) + + case 2: // if 2 return values, first cant be an interface/function, second must be an error + seen := map[reflect.Type]struct{}{} + todo := []reflect.Type{m.Type.Out(0)} + for len(todo) > 0 { + typ := todo[len(todo) - 1] + todo = todo[:len(todo)-1] + + if _, ok := seen[typ]; ok { + continue + } + seen[typ] = struct{}{} + + if typ.Kind() == reflect.Interface && typ != bareIface && !typ.Implements(jmarsh) { + t.Error("methods can't return interfaces", m.Name) + } + + switch typ.Kind() { + case reflect.Ptr: + fallthrough + case reflect.Array: + fallthrough + case reflect.Slice: + fallthrough + case reflect.Chan: + todo = append(todo, typ.Elem()) + case reflect.Map: + todo = append(todo, typ.Elem()) + todo = append(todo, typ.Key()) + case reflect.Struct: + for i := 0; i < typ.NumField(); i++ { + todo = append(todo, typ.Field(i).Type) + } + } + } + + require.NotEqual(t, reflect.Func.String(), m.Type.Out(0).Kind().String(), m.Name) + require.Equal(t, errType, m.Type.Out(1), m.Name) + + default: + t.Error("methods can only have 1 or 2 return values", m.Name) + } + } + } + } + + t.Run("common", tst(new(Common))) + t.Run("full", tst(new(FullNode))) + t.Run("miner", tst(new(StorageMiner))) + t.Run("worker", tst(new(WorkerAPI))) +} diff --git a/api/apistruct/struct.go b/api/apistruct/struct.go index ab86d7819..c8a2b91ac 100644 --- a/api/apistruct/struct.go +++ b/api/apistruct/struct.go @@ -134,7 +134,7 @@ type FullNodeStruct struct { WalletBalance func(context.Context, address.Address) (types.BigInt, error) `perm:"read"` WalletSign func(context.Context, address.Address, []byte) (*crypto.Signature, error) `perm:"sign"` WalletSignMessage func(context.Context, address.Address, *types.Message) (*types.SignedMessage, error) `perm:"sign"` - WalletVerify func(context.Context, address.Address, []byte, *crypto.Signature) bool `perm:"read"` + WalletVerify func(context.Context, address.Address, []byte, *crypto.Signature) (bool, error) `perm:"read"` WalletDefaultAddress func(context.Context) (address.Address, error) `perm:"write"` WalletSetDefault func(context.Context, address.Address) error `perm:"admin"` WalletExport func(context.Context, address.Address) (*types.KeyInfo, error) `perm:"admin"` @@ -604,7 +604,7 @@ func (c *FullNodeStruct) WalletSignMessage(ctx context.Context, k address.Addres return c.Internal.WalletSignMessage(ctx, k, msg) } -func (c *FullNodeStruct) WalletVerify(ctx context.Context, k address.Address, msg []byte, sig *crypto.Signature) bool { +func (c *FullNodeStruct) WalletVerify(ctx context.Context, k address.Address, msg []byte, sig *crypto.Signature) (bool, error) { return c.Internal.WalletVerify(ctx, k, msg, sig) } diff --git a/cli/wallet.go b/cli/wallet.go index 4339a1fb6..27993a1ba 100644 --- a/cli/wallet.go +++ b/cli/wallet.go @@ -382,7 +382,11 @@ var walletVerify = &cli.Command{ return err } - if api.WalletVerify(ctx, addr, msg, &sig) { + ok, err := api.WalletVerify(ctx, addr, msg, &sig) + if err != nil { + return err + } + if ok { fmt.Println("valid") return nil } diff --git a/extern/sector-storage/ffiwrapper/sealer_test.go b/extern/sector-storage/ffiwrapper/sealer_test.go index d59de2cab..bb26adb77 100644 --- a/extern/sector-storage/ffiwrapper/sealer_test.go +++ b/extern/sector-storage/ffiwrapper/sealer_test.go @@ -245,6 +245,10 @@ func TestDownloadParams(t *testing.T) { } func TestSealAndVerify(t *testing.T) { + if testing.Short() { + t.Skip("skipping test in short mode") + } + defer requireFDsClosed(t, openFDs(t)) if runtime.NumCPU() < 10 && os.Getenv("CI") == "" { // don't bother on slow hardware @@ -314,6 +318,10 @@ func TestSealAndVerify(t *testing.T) { } func TestSealPoStNoCommit(t *testing.T) { + if testing.Short() { + t.Skip("skipping test in short mode") + } + defer requireFDsClosed(t, openFDs(t)) if runtime.NumCPU() < 10 && os.Getenv("CI") == "" { // don't bother on slow hardware @@ -375,6 +383,10 @@ func TestSealPoStNoCommit(t *testing.T) { } func TestSealAndVerify3(t *testing.T) { + if testing.Short() { + t.Skip("skipping test in short mode") + } + defer requireFDsClosed(t, openFDs(t)) if runtime.NumCPU() < 10 && os.Getenv("CI") == "" { // don't bother on slow hardware diff --git a/node/impl/full/wallet.go b/node/impl/full/wallet.go index bda8824e7..64231b74e 100644 --- a/node/impl/full/wallet.go +++ b/node/impl/full/wallet.go @@ -67,8 +67,8 @@ func (a *WalletAPI) WalletSignMessage(ctx context.Context, k address.Address, ms }, nil } -func (a *WalletAPI) WalletVerify(ctx context.Context, k address.Address, msg []byte, sig *crypto.Signature) bool { - return sigs.Verify(sig, k, msg) == nil +func (a *WalletAPI) WalletVerify(ctx context.Context, k address.Address, msg []byte, sig *crypto.Signature) (bool, error) { + return sigs.Verify(sig, k, msg) == nil, nil } func (a *WalletAPI) WalletDefaultAddress(ctx context.Context) (address.Address, error) {