diff --git a/signer/core/api.go b/signer/core/api.go index b282170f64..3b02cc09d7 100644 --- a/signer/core/api.go +++ b/signer/core/api.go @@ -36,7 +36,7 @@ import ( "github.com/ethereum/go-ethereum/rlp" ) -const( +const ( // numberOfAccountsToDerive For hardware wallets, the number of accounts to derive numberOfAccountsToDerive = 10 // ExternalAPIVersion -- see extapi_changelog.md diff --git a/signer/core/timeout.go b/signer/core/timeout.go index a416e78748..378351a8c7 100644 --- a/signer/core/timeout.go +++ b/signer/core/timeout.go @@ -20,12 +20,13 @@ import ( "context" "encoding/json" "fmt" + "time" + "github.com/ethereum/go-ethereum/accounts" "github.com/ethereum/go-ethereum/common" "github.com/ethereum/go-ethereum/common/hexutil" "github.com/ethereum/go-ethereum/internal/ethapi" "github.com/ethereum/go-ethereum/log" - "time" ) // The TimedExternalAPI implements ExternalAPI, but can be configured to time out after a specified interval. @@ -49,18 +50,16 @@ func (t *TimedExternalAPI) List(ctx context.Context) ([]common.Address, error) { addr []common.Address err error } - ch := make(chan response) + ch := make(chan response, 1) go func() { addr, err := t.next.List(ctx) ch <- response{addr, err} }() - select { case r := <-ch: return r.addr, r.err case <-time.After(t.timeout): log.Info("timeout", "op", "list") - go func() { <-ch }() return []common.Address{}, ErrTimeout } } @@ -70,18 +69,16 @@ func (t *TimedExternalAPI) New(ctx context.Context) (accounts.Account, error) { acc accounts.Account err error } - ch := make(chan response) + ch := make(chan response, 1) go func() { acc, err := t.next.New(ctx) ch <- response{acc, err} }() - select { case r := <-ch: return r.acc, r.err case <-time.After(t.timeout): log.Info("timeout", "op", "list") - go func() { <-ch }() return accounts.Account{}, ErrTimeout } } @@ -91,7 +88,7 @@ func (t *TimedExternalAPI) SignTransaction(ctx context.Context, args SendTxArgs, res *ethapi.SignTransactionResult err error } - ch := make(chan response) + ch := make(chan response, 1) go func() { res, err := t.next.SignTransaction(ctx, args, methodSelector) ch <- response{res, err} @@ -101,7 +98,6 @@ func (t *TimedExternalAPI) SignTransaction(ctx context.Context, args SendTxArgs, return r.res, r.err case <-time.After(t.timeout): log.Info("timeout", "op", "signTransaction") - go func() { <-ch }() return nil, ErrTimeout } } @@ -111,7 +107,7 @@ func (t *TimedExternalAPI) Sign(ctx context.Context, addr common.MixedcaseAddres res hexutil.Bytes err error } - ch := make(chan response) + ch := make(chan response, 1) go func() { res, err := t.next.Sign(ctx, addr, data) ch <- response{res, err} @@ -121,7 +117,6 @@ func (t *TimedExternalAPI) Sign(ctx context.Context, addr common.MixedcaseAddres return r.res, r.err case <-time.After(t.timeout): log.Info("timeout", "op", "sign") - go func() { <-ch }() return nil, ErrTimeout } } @@ -131,7 +126,7 @@ func (t *TimedExternalAPI) Export(ctx context.Context, addr common.Address) (jso res json.RawMessage err error } - ch := make(chan response) + ch := make(chan response, 1) go func() { res, err := t.next.Export(ctx, addr) ch <- response{res, err} @@ -141,7 +136,6 @@ func (t *TimedExternalAPI) Export(ctx context.Context, addr common.Address) (jso return r.res, r.err case <-time.After(t.timeout): log.Info("timeout", "op", "sign") - go func() { <-ch }() return json.RawMessage{}, ErrTimeout } } diff --git a/signer/core/timeout_test.go b/signer/core/timeout_test.go new file mode 100644 index 0000000000..b307720290 --- /dev/null +++ b/signer/core/timeout_test.go @@ -0,0 +1,65 @@ +package core + +import ( + "context" + "encoding/json" + "testing" + "time" + + "github.com/ethereum/go-ethereum/accounts" + "github.com/ethereum/go-ethereum/common" + "github.com/ethereum/go-ethereum/common/hexutil" + "github.com/ethereum/go-ethereum/internal/ethapi" +) + +type nonResponsiveHander struct { + sleepTime time.Duration +} + +func (n nonResponsiveHander) List(ctx context.Context) ([]common.Address, error) { + time.Sleep(n.sleepTime) + return []common.Address{common.HexToAddress("0xdeadbeef")}, nil +} + +func (nonResponsiveHander) New(ctx context.Context) (accounts.Account, error) { + panic("implement me") +} + +func (nonResponsiveHander) SignTransaction(ctx context.Context, args SendTxArgs, methodSelector *string) (*ethapi.SignTransactionResult, error) { + panic("implement me") +} + +func (nonResponsiveHander) Sign(ctx context.Context, addr common.MixedcaseAddress, data hexutil.Bytes) (hexutil.Bytes, error) { + panic("implement me") +} + +func (nonResponsiveHander) Export(ctx context.Context, addr common.Address) (json.RawMessage, error) { + panic("implement me") +} + +// TestTimeout checks that it does timeout +func TestTimeout(t *testing.T) { + a := &nonResponsiveHander{1 * time.Second} + api := NewTimedExternalAPI(a, 1*time.Millisecond) + addr, err := api.List(nil) + if len(addr) > 0 { + t.Errorf("expected no accounts, got %d", len(addr)) + } + if err == nil { + t.Errorf("expected err") + + } +} + +// TestTimeout checks that it does timeout +func TestNotTimeout(t *testing.T) { + a := &nonResponsiveHander{1 * time.Millisecond} + api := NewTimedExternalAPI(a, 1*time.Second) + addr, err := api.List(nil) + if len(addr) != 1 { + t.Errorf("expected 1 accounts, got %d", len(addr)) + } + if err != nil { + t.Errorf("expected no err, got %v", err) + } +}