diff --git a/CHANGELOG.md b/CHANGELOG.md index d1235fcf..735335db 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -17,6 +17,12 @@ ### 🐛 Fixes - [](https://github.com/vegaprotocol/vegawallet/pull/) - +## 0.15.1 + +### 🐛 Fixes +- [558](https://github.com/vegaprotocol/vegawallet/pull/558) - Use user-friendly RPC code as error message +- [569](https://github.com/vegaprotocol/vegawallet/pull/569) - Ensure the winfile scheme for logger is register only once + ## 0.15.0 ### 🛠 Improvements diff --git a/cmd/command_send.go b/cmd/command_send.go index 43cac478..6df5c18d 100644 --- a/cmd/command_send.go +++ b/cmd/command_send.go @@ -18,11 +18,10 @@ import ( "code.vegaprotocol.io/vegawallet/network" "code.vegaprotocol.io/vegawallet/node" "code.vegaprotocol.io/vegawallet/wallets" - "go.uber.org/zap" - "go.uber.org/zap/zapcore" - "github.com/golang/protobuf/jsonpb" "github.com/spf13/cobra" + "go.uber.org/zap" + "go.uber.org/zap/zapcore" ) var ( @@ -244,6 +243,9 @@ func SendCommand(w io.Writer, rf *RootFlags, req *SendCommandRequest) error { Hosts: hosts, Retries: req.Retries, }) + + log = log.Named("command") + if err != nil { return fmt.Errorf("couldn't initialise the node forwarder: %w", err) } @@ -290,7 +292,7 @@ func SendCommand(w io.Writer, rf *RootFlags, req *SendCommandRequest) error { Nonce: powNonce, } - log.Info("calculated proof of work for transaction with signature", zap.String("signature", tx.Signature.Value)) + log.Info("calculated proof of work for transaction", zap.String("signature", tx.Signature.Value)) txHash, err := forwarder.SendTx(ctx, tx, api.SubmitTransactionRequest_TYPE_ASYNC, cltIdx) if err != nil { diff --git a/cmd/logger.go b/cmd/logger.go index 2762e756..f3cf9caa 100644 --- a/cmd/logger.go +++ b/cmd/logger.go @@ -2,9 +2,7 @@ package cmd import ( "fmt" - "net/url" "os" - "runtime" "time" "code.vegaprotocol.io/shared/paths" @@ -78,15 +76,7 @@ func BuildJSONLogger(level string, vegaPaths paths.Paths, logsDir paths.StatePat return nil, "", fmt.Errorf("failed getting path for %s: %w", logFile, err) } - zapLogPath := appLogPath - if runtime.GOOS == "windows" { - err := zap.RegisterSink("winfile", newWinFileSink) - if err != nil { - return nil, "", fmt.Errorf("couldn't register the windows file sink: %w", err) - } - zapLogPath = "winfile:///" + appLogPath - } - + zapLogPath := toZapLogPath(appLogPath) cfg.OutputPaths = []string{zapLogPath} cfg.ErrorOutputPaths = []string{zapLogPath} @@ -159,12 +149,3 @@ func isSupportedLogLevel(level string) bool { } return false } - -// newWinFileSink creates a log sink on Windows machines as zap, by default, -// doesn't support Windows paths. A workaround is to create a fake winfile -// scheme and register it with zap instead. This workaround is taken from -// the GitHub issue at https://github.com/uber-go/zap/issues/621. -func newWinFileSink(u *url.URL) (zap.Sink, error) { - // Remove leading slash left by url.Parse(). - return os.OpenFile(u.Path[1:], os.O_WRONLY|os.O_APPEND|os.O_CREATE, 0o644) // nolint:gomnd -} diff --git a/cmd/logger_unix.go b/cmd/logger_unix.go new file mode 100644 index 00000000..7689c12f --- /dev/null +++ b/cmd/logger_unix.go @@ -0,0 +1,8 @@ +//go:build !windows +// +build !windows + +package cmd + +func toZapLogPath(p string) string { + return p +} diff --git a/cmd/logger_windows.go b/cmd/logger_windows.go new file mode 100644 index 00000000..64553d9b --- /dev/null +++ b/cmd/logger_windows.go @@ -0,0 +1,29 @@ +package cmd + +import ( + "fmt" + "net/url" + "os" + + "go.uber.org/zap" +) + +func init() { + err := zap.RegisterSink("winfile", newWinFileSink) + if err != nil { + panic(fmt.Errorf("couldn't register the windows file sink: %w", err)) + } +} + +func toZapLogPath(logPath string) string { + return "winfile:///" + logPath +} + +// newWinFileSink creates a log sink on Windows machines as zap, by default, +// doesn't support Windows paths. A workaround is to create a fake winfile +// scheme and register it with zap instead. This workaround is taken from +// the GitHub issue at https://github.com/uber-go/zap/issues/621. +func newWinFileSink(u *url.URL) (zap.Sink, error) { + // Remove leading slash left by url.Parse(). + return os.OpenFile(u.Path[1:], os.O_WRONLY|os.O_APPEND|os.O_CREATE, 0o644) // nolint:gomnd +} diff --git a/cmd/service_run.go b/cmd/service_run.go index 84ab1c6b..f4cae7db 100644 --- a/cmd/service_run.go +++ b/cmd/service_run.go @@ -8,7 +8,6 @@ import ( "net/http" "os" "os/signal" - "strings" "syscall" vgterm "code.vegaprotocol.io/shared/libs/term" @@ -24,12 +23,15 @@ import ( "code.vegaprotocol.io/vegawallet/service" svcstore "code.vegaprotocol.io/vegawallet/service/store/v1" "code.vegaprotocol.io/vegawallet/wallets" + "github.com/golang/protobuf/jsonpb" "github.com/skratchdot/open-golang/open" "github.com/spf13/cobra" "go.uber.org/zap" ) +const MaxConsentRequests = 100 + var ErrEnableAutomaticConsentFlagIsRequiredWithoutTTY = errors.New("--automatic-consent flag is required without TTY") var ( @@ -209,9 +211,6 @@ func RunService(w io.Writer, rf *RootFlags, f *RunServiceFlags) error { return fmt.Errorf("couldn't initialise the node forwarder: %w", err) } - pendingConsents := make(chan service.ConsentRequest, 1) - sentTxs := make(chan service.SentTransaction) - cliLog, cliLogPath, err := BuildJSONLogger(cfg.Level.String(), vegaPaths, paths.WalletCLILogsHome) if err != nil { return err @@ -220,6 +219,11 @@ func RunService(w io.Writer, rf *RootFlags, f *RunServiceFlags) error { cliLog = cliLog.Named("command") + consentRequests := make(chan service.ConsentRequest, MaxConsentRequests) + defer close(consentRequests) + sentTransactions := make(chan service.SentTransaction) + defer close(sentTransactions) + var policy service.Policy if vgterm.HasTTY() { cliLog.Info("TTY detected") @@ -228,7 +232,7 @@ func RunService(w io.Writer, rf *RootFlags, f *RunServiceFlags) error { policy = service.NewAutomaticConsentPolicy() } else { cliLog.Info("Explicit consent enabled") - policy = service.NewExplicitConsentPolicy(pendingConsents, sentTxs) + policy = service.NewExplicitConsentPolicy(ctx, consentRequests, sentTransactions) } } else { cliLog.Info("No TTY detected") @@ -296,7 +300,7 @@ func RunService(w io.Writer, rf *RootFlags, f *RunServiceFlags) error { p.NextLine() } - waitSig(ctx, cancel, cliLog, pendingConsents, sentTxs, p) + waitSig(ctx, cancel, cliLog, consentRequests, sentTransactions, p) return nil } @@ -366,54 +370,77 @@ func startTokenDApp(log *zap.Logger, f *RunServiceFlags, cfg *network.Network, c } // waitSig will wait for a sigterm or sigint interrupt. -func waitSig(ctx context.Context, cfunc func(), log *zap.Logger, pendingSigRequests chan service.ConsentRequest, sentTxs chan service.SentTransaction, p *printer.InteractivePrinter) { +func waitSig( + ctx context.Context, + cancelFunc context.CancelFunc, + log *zap.Logger, + consentRequests chan service.ConsentRequest, + sentTransactions chan service.SentTransaction, + p *printer.InteractivePrinter, +) { gracefulStop := make(chan os.Signal, 1) + defer close(gracefulStop) signal.Notify(gracefulStop, syscall.SIGTERM) signal.Notify(gracefulStop, syscall.SIGINT) signal.Notify(gracefulStop, syscall.SIGQUIT) + go func() { + if err := handleConsentRequests(ctx, log, consentRequests, sentTransactions, p); err != nil { + cancelFunc() + } + }() + for { select { case sig := <-gracefulStop: log.Info("caught signal", zap.String("signal", fmt.Sprintf("%+v", sig))) - cfunc() + cancelFunc() return case <-ctx.Done(): // nothing to do return - case signRequest := <-pendingSigRequests: - txStr, err := signRequest.String() + } + } +} + +func handleConsentRequests(ctx context.Context, log *zap.Logger, consentRequests chan service.ConsentRequest, sentTransactions chan service.SentTransaction, p *printer.InteractivePrinter) error { + for { + select { + case <-ctx.Done(): + // nothing to do + return nil + case consentRequest := <-consentRequests: + m := jsonpb.Marshaler{Indent: " "} + marshalledTx, err := m.MarshalToString(consentRequest.Tx) if err != nil { - log.Info("failed to marshall sign request content") - cfunc() - return + log.Error("couldn't marshal transaction from consent request", zap.Error(err)) + return err } + p.BlueArrow().Text("New transaction received: ").NextLine() - p.InfoText(txStr).NextLine() + p.InfoText(marshalledTx).NextLine() if flags.DoYouApproveTx() { - log.Info("user approved the signing of the transaction", zap.Any("transaction", txStr)) - signRequest.Confirmations <- service.ConsentConfirmation{Decision: true, TxStr: txStr} + log.Info("user approved the signing of the transaction", zap.Any("transaction", marshalledTx)) + consentRequest.Confirmation <- service.ConsentConfirmation{Decision: true} p.CheckMark().SuccessText("Transaction approved").NextLine() - sentTx := <-sentTxs + sentTx := <-sentTransactions log.Info("transaction sent", zap.Any("ID", sentTx.TxID), zap.Any("hash", sentTx.TxHash)) if sentTx.Error != nil { - log.Error("transaction failed", zap.Any("transaction", txStr)) - p.BangMark().DangerText("Transaction failed! ").NextLine() - p.BangMark().DangerText("Error: ").DangerText(sentTx.Error.Error()).NextLine() - p.BangMark().DangerText("Details: ").DangerText(strings.Join(sentTx.ErrorDetails, " ,")).NextSection() + log.Error("transaction failed", zap.Any("transaction", marshalledTx)) + p.BangMark().DangerText("Transaction failed").NextLine() + p.BangMark().DangerText("Error: ").DangerText(sentTx.Error.Error()).NextSection() } else { log.Info("transaction sent", zap.Any("hash", sentTx.TxHash)) p.CheckMark().Text("Transaction with hash ").SuccessText(sentTx.TxHash).Text(" sent!").NextSection() } } else { - log.Info("user rejected the signing of the transaction", zap.Any("transaction", txStr)) - signRequest.Confirmations <- service.ConsentConfirmation{Decision: false, TxStr: txStr} + log.Info("user rejected the signing of the transaction", zap.Any("transaction", marshalledTx)) + consentRequest.Confirmation <- service.ConsentConfirmation{Decision: false} p.BangMark().DangerText("Transaction rejected").NextSection() } - close(signRequest.Confirmations) } } } diff --git a/go.mod b/go.mod index badfb2e6..61f176db 100644 --- a/go.mod +++ b/go.mod @@ -3,7 +3,7 @@ module code.vegaprotocol.io/vegawallet go 1.17 require ( - code.vegaprotocol.io/protos v0.51.0 + code.vegaprotocol.io/protos v0.51.1-0.20220517145005-fc5e0192af7c code.vegaprotocol.io/shared v0.0.0-20220321185018-3b5684b00533 github.com/blang/semver/v4 v4.0.0 github.com/cenkalti/backoff/v4 v4.1.2 diff --git a/go.sum b/go.sum index ea68477b..13288056 100644 --- a/go.sum +++ b/go.sum @@ -30,10 +30,8 @@ cloud.google.com/go/storage v1.5.0/go.mod h1:tpKbwo567HUNpVclU5sGELwQWBDZ8gh0Zeo cloud.google.com/go/storage v1.6.0/go.mod h1:N7U0C8pVQ/+NIKOBQyamJIeKQKkZ+mxpohlUTyfDhBk= cloud.google.com/go/storage v1.8.0/go.mod h1:Wv1Oy7z6Yz3DshWRJFhqM/UCfaWIRTdp0RXyy7KQOVs= cloud.google.com/go/storage v1.10.0/go.mod h1:FLPqc6j+Ki4BU591ie1oL6qBQGu2Bl/tZ9ullr3+Kg0= -code.vegaprotocol.io/protos v0.50.4-0.20220503163151-7fc5916bf54e h1:8r9buW3Cl2aBkZj37Yuboq2Pj+pxcmXSRXI8lc3Z7Sk= -code.vegaprotocol.io/protos v0.50.4-0.20220503163151-7fc5916bf54e/go.mod h1:4BqwDw6jhc/mnwbXq8ZFUtYBFCnk8tBW6zuPsBt8OrQ= -code.vegaprotocol.io/protos v0.51.0 h1:HscwExZenysYze9N43JTqR1mStpUB82Hb48oY+lHX8M= -code.vegaprotocol.io/protos v0.51.0/go.mod h1:4BqwDw6jhc/mnwbXq8ZFUtYBFCnk8tBW6zuPsBt8OrQ= +code.vegaprotocol.io/protos v0.51.1-0.20220517145005-fc5e0192af7c h1:UtDLlSEiMHY8gNb4nHtJWhN59mu5GPXVymdAJ7UrQJ0= +code.vegaprotocol.io/protos v0.51.1-0.20220517145005-fc5e0192af7c/go.mod h1:4BqwDw6jhc/mnwbXq8ZFUtYBFCnk8tBW6zuPsBt8OrQ= code.vegaprotocol.io/shared v0.0.0-20220321185018-3b5684b00533 h1:IxTWvyF0i0AgUAS+FTgQKIpbsS6Ki/Y9Ug0GSlgChJw= code.vegaprotocol.io/shared v0.0.0-20220321185018-3b5684b00533/go.mod h1:pNHKwqRDkotUN0s6jErl7Eb2EpZa2HAf03lyPMTCUT0= dmitri.shuralyov.com/gpu/mtl v0.0.0-20190408044501-666a987793e9/go.mod h1:H6x//7gZCb22OMCxBHrMx7a5I7Hp++hsVxbQ4BYO7hU= diff --git a/node/errors.go b/node/errors.go index fb8a718b..2a19c980 100644 --- a/node/errors.go +++ b/node/errors.go @@ -1,5 +1,45 @@ package node -import "errors" +import ( + "errors" + "fmt" + "strings" + + typespb "code.vegaprotocol.io/protos/vega" + "google.golang.org/grpc/codes" + "google.golang.org/grpc/status" +) var ErrNoHostSpecified = errors.New("no host specified") + +type ErrorCode codes.Code + +type StatusError struct { + Code codes.Code + Details []string +} + +func (e *StatusError) Error() string { + return fmt.Sprintf("%s - %v", e.Code.String(), strings.Join(e.Details, ", ")) +} + +// intoStatusError extract useful information from a gRPC status error. +// Returns nil if the underlying error is not a gRPC status error. +func intoStatusError(err error) *StatusError { + st, ok := status.FromError(err) + if !ok { + return nil + } + statusErr := &StatusError{ + Code: st.Code(), + Details: []string{}, + } + for _, v := range st.Details() { + v, ok := v.(*typespb.ErrorDetail) + if !ok { + continue + } + statusErr.Details = append(statusErr.Details, v.GetMessage()) + } + return statusErr +} diff --git a/node/forwarder.go b/node/forwarder.go index 8ff86da6..22d37dd8 100644 --- a/node/forwarder.go +++ b/node/forwarder.go @@ -8,10 +8,11 @@ import ( api "code.vegaprotocol.io/protos/vega/api/v1" commandspb "code.vegaprotocol.io/protos/vega/commands/v1" "code.vegaprotocol.io/vegawallet/network" - "github.com/cenkalti/backoff/v4" "go.uber.org/zap" "google.golang.org/grpc" + "google.golang.org/grpc/codes" + "google.golang.org/grpc/credentials/insecure" ) type Forwarder struct { @@ -30,7 +31,7 @@ func NewForwarder(log *zap.Logger, nodeConfigs network.GRPCConfig) (*Forwarder, clts := make([]api.CoreServiceClient, 0, len(nodeConfigs.Hosts)) conns := make([]*grpc.ClientConn, 0, len(nodeConfigs.Hosts)) for _, v := range nodeConfigs.Hosts { - conn, err := grpc.Dial(v, grpc.WithInsecure()) + conn, err := grpc.Dial(v, grpc.WithTransportCredentials(insecure.NewCredentials())) if err != nil { log.Debug("Couldn't dial gRPC host", zap.String("address", v)) return nil, err @@ -145,15 +146,14 @@ func (n *Forwarder) SendTx(ctx context.Context, tx *commandspb.Transaction, ty a if cltIdx < 0 { cltIdx = n.nextClt() } - err := backoff.Retry( + if err := backoff.Retry( func() error { clt := n.clts[cltIdx] r, err := clt.SubmitTransaction(ctx, &req) if err != nil { - n.log.Error("Couldn't send transaction", zap.Error(err)) - return err + return n.handleSubmissionError(err) } - n.log.Debug("Response from SubmitTransaction", + n.log.Debug("Transaction successfully submitted", zap.Bool("success", r.Success), zap.String("hash", r.TxHash), ) @@ -161,13 +161,38 @@ func (n *Forwarder) SendTx(ctx context.Context, tx *commandspb.Transaction, ty a return nil }, backoff.WithMaxRetries(backoff.NewExponentialBackOff(), n.nodeCfgs.Retries), - ) - if err != nil { + ); err != nil { return "", err } + return resp.TxHash, nil } +func (n *Forwarder) handleSubmissionError(err error) error { + statusErr := intoStatusError(err) + + if statusErr == nil { + n.log.Error("couldn't submit transaction", + zap.Error(err), + ) + return err + } + + if statusErr.Code == codes.InvalidArgument { + n.log.Error( + "transaction has been rejected because of an invalid argument or state, skipping retry...", + zap.Error(statusErr), + ) + // Returning a permanent error kills the retry loop. + return backoff.Permanent(statusErr) + } + + n.log.Error("couldn't submit transaction", + zap.Error(statusErr), + ) + return statusErr +} + func (n *Forwarder) nextClt() int { i := atomic.AddUint64(&n.next, 1) n.log.Info("Sending transaction to Vega node", diff --git a/service/errors.go b/service/errors.go index 93a4d9c5..ebd00ba6 100644 --- a/service/errors.go +++ b/service/errors.go @@ -7,14 +7,15 @@ import ( ) var ( - ErrInvalidToken = errors.New("invalid token") - ErrInvalidClaims = errors.New("invalid claims") - ErrInvalidOrMissingToken = newErrorResponse("invalid or missing token") - ErrCouldNotReadRequest = errors.New("couldn't read request") - ErrCouldNotGetBlockHeight = errors.New("couldn't get last block height") - ErrShouldBeBase64Encoded = errors.New("should be base64 encoded") - ErrRSAKeysAlreadyExists = errors.New("RSA keys already exist") - ErrRejectedSignRequest = errors.New("user rejected sign request") + ErrInvalidToken = errors.New("invalid token") + ErrInvalidClaims = errors.New("invalid claims") + ErrInvalidOrMissingToken = newErrorResponse("invalid or missing token") + ErrCouldNotReadRequest = errors.New("couldn't read request") + ErrCouldNotGetBlockHeight = errors.New("couldn't get last block height") + ErrShouldBeBase64Encoded = errors.New("should be base64 encoded") + ErrRSAKeysAlreadyExists = errors.New("RSA keys already exist") + ErrRejectedSignRequest = errors.New("user rejected sign request") + ErrInterruptedConsentRequest = errors.New("process to request consent has been interrupted") ) type ErrorsResponse struct { @@ -35,10 +36,3 @@ func newErrorResponse(e string) ErrorResponse { ErrorStr: e, } } - -func newErrorWithDetails(e string, details []string) ErrorResponse { - return ErrorResponse{ - ErrorStr: e, - Details: details, - } -} diff --git a/service/mocks/policies_mock.go b/service/mocks/policies_mock.go index 6ed0daa6..395fe141 100644 --- a/service/mocks/policies_mock.go +++ b/service/mocks/policies_mock.go @@ -29,7 +29,3 @@ func (p *MockConsentPolicy) Ask(tx *v1.SubmitTransactionRequest, txID string, re } return true, nil } - -func (p *MockConsentPolicy) NeedsInteractiveOutput() bool { - return true -} diff --git a/service/policies.go b/service/policies.go index b9306dfc..2d7cade5 100644 --- a/service/policies.go +++ b/service/policies.go @@ -1,44 +1,36 @@ package service import ( + "context" "time" commandspb "code.vegaprotocol.io/protos/vega/commands/v1" v1 "code.vegaprotocol.io/protos/vega/wallet/v1" - "github.com/golang/protobuf/jsonpb" ) type ConsentConfirmation struct { - TxStr string + TxID string Decision bool } type ConsentRequest struct { - TxID string - Tx *v1.SubmitTransactionRequest - ReceivedAt time.Time - Confirmations chan ConsentConfirmation -} - -func (r *ConsentRequest) String() (string, error) { - m := jsonpb.Marshaler{Indent: " "} - marshalledRequest, err := m.MarshalToString(r.Tx) - return marshalledRequest, err + TxID string + Tx *v1.SubmitTransactionRequest + ReceivedAt time.Time + Confirmation chan ConsentConfirmation } type SentTransaction struct { - TxHash string - TxID string - ReceivedAt time.Time - Tx *commandspb.Transaction - Error error - ErrorDetails []string + TxHash string + TxID string + Tx *commandspb.Transaction + Error error + SentAt time.Time } type Policy interface { Ask(tx *v1.SubmitTransactionRequest, txID string, receivedAt time.Time) (bool, error) Report(tx SentTransaction) - NeedsInteractiveOutput() bool } type AutomaticConsentPolicy struct{} @@ -47,7 +39,7 @@ func NewAutomaticConsentPolicy() Policy { return &AutomaticConsentPolicy{} } -func (p *AutomaticConsentPolicy) Ask(_ *v1.SubmitTransactionRequest, txID string, receivedAt time.Time) (bool, error) { +func (p *AutomaticConsentPolicy) Ask(_ *v1.SubmitTransactionRequest, _ string, _ time.Time) (bool, error) { return true, nil } @@ -55,36 +47,62 @@ func (p *AutomaticConsentPolicy) Report(_ SentTransaction) { // Nothing to report as we expect this policy to be non-interactive. } -func (p *AutomaticConsentPolicy) NeedsInteractiveOutput() bool { - return false -} - type ExplicitConsentPolicy struct { - pendingEvents chan ConsentRequest - sentTxs chan SentTransaction + // ctx is used to interrupt the wait for consent confirmation + ctx context.Context + + consentRequestsChan chan ConsentRequest + sentTransactionsChan chan SentTransaction } -func NewExplicitConsentPolicy(pending chan ConsentRequest, sentTxs chan SentTransaction) Policy { +func NewExplicitConsentPolicy(ctx context.Context, consentRequests chan ConsentRequest, sentTransactions chan SentTransaction) Policy { return &ExplicitConsentPolicy{ - pendingEvents: pending, - sentTxs: sentTxs, + ctx: ctx, + consentRequestsChan: consentRequests, + sentTransactionsChan: sentTransactions, } } func (p *ExplicitConsentPolicy) Ask(tx *v1.SubmitTransactionRequest, txID string, receivedAt time.Time) (bool, error) { - confirmations := make(chan ConsentConfirmation) - consentReq := ConsentRequest{Tx: tx, Confirmations: confirmations, ReceivedAt: receivedAt} - consentReq.TxID = txID - p.pendingEvents <- consentReq + confirmationChan := make(chan ConsentConfirmation, 1) + defer close(confirmationChan) + + consentRequest := ConsentRequest{ + TxID: txID, + Tx: tx, + ReceivedAt: receivedAt, + Confirmation: confirmationChan, + } + + if err := p.sendConsentRequest(consentRequest); err != nil { + return false, err + } - c := <-confirmations - return c.Decision, nil + return p.receiveConsentConfirmation(consentRequest) } -func (p *ExplicitConsentPolicy) Report(tx SentTransaction) { - p.sentTxs <- tx +func (p *ExplicitConsentPolicy) receiveConsentConfirmation(consentRequest ConsentRequest) (bool, error) { + for { + select { + case <-p.ctx.Done(): + return false, ErrInterruptedConsentRequest + case decision := <-consentRequest.Confirmation: + return decision.Decision, nil + } + } } -func (p *ExplicitConsentPolicy) NeedsInteractiveOutput() bool { - return true +func (p *ExplicitConsentPolicy) sendConsentRequest(consentRequest ConsentRequest) error { + for { + select { + case <-p.ctx.Done(): + return ErrInterruptedConsentRequest + case p.consentRequestsChan <- consentRequest: + return nil + } + } +} + +func (p *ExplicitConsentPolicy) Report(tx SentTransaction) { + p.sentTransactionsChan <- tx } diff --git a/service/policies_test.go b/service/policies_test.go new file mode 100644 index 00000000..1fcfbb5c --- /dev/null +++ b/service/policies_test.go @@ -0,0 +1,97 @@ +package service_test + +import ( + "context" + "sync" + "testing" + "time" + + commandspb "code.vegaprotocol.io/protos/vega/commands/v1" + walletpb "code.vegaprotocol.io/protos/vega/wallet/v1" + vgrand "code.vegaprotocol.io/shared/libs/rand" + "code.vegaprotocol.io/vegawallet/service" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestExplicitConsentPolicy(t *testing.T) { + t.Run("Requesting explicit consent succeeds", testRequestingExplicitConsentSucceeds) + t.Run("Canceling consent requests succeeds", testCancelingConsentRequestSucceeds) + t.Run("Reporting sent transaction succeeds", testReportingSentTransactionSucceeds) +} + +func testRequestingExplicitConsentSucceeds(t *testing.T) { + // given + txn := &walletpb.SubmitTransactionRequest{} + txID := vgrand.RandomStr(5) + consentRequestsChan := make(chan service.ConsentRequest, 1) + sentTransactionsChan := make(chan service.SentTransaction, 1) + + // setup + p := service.NewExplicitConsentPolicy(context.Background(), consentRequestsChan, sentTransactionsChan) + + go func() { + req := <-consentRequestsChan + d := service.ConsentConfirmation{TxID: txID, Decision: false} + req.Confirmation <- d + }() + + // when + answer, err := p.Ask(txn, txID, time.Now()) + require.Nil(t, err) + require.False(t, answer) +} + +func testCancelingConsentRequestSucceeds(t *testing.T) { + // given + ctx, cancelFn := context.WithCancel(context.Background()) + txn := &walletpb.SubmitTransactionRequest{} + txID := vgrand.RandomStr(5) + // Channels have a smaller buffer than the number of requests, on purpose. + // We have to ensure channels are not blocking and preventing interruption + // when full. + consentRequestsChan := make(chan service.ConsentRequest, 1) + sentTransactionsChan := make(chan service.SentTransaction, 1) + + // setup + p := service.NewExplicitConsentPolicy(ctx, consentRequestsChan, sentTransactionsChan) + + var wg sync.WaitGroup + for i := 0; i < 5; i++ { + wg.Add(1) + go func() { + defer wg.Done() + answer, err := p.Ask(txn, txID, time.Now()) + require.ErrorIs(t, err, service.ErrInterruptedConsentRequest) + assert.False(t, answer) + }() + } + + // interrupting the consent requests + cancelFn() + + // waiting for all consent request to be interrupted + wg.Wait() +} + +func testReportingSentTransactionSucceeds(t *testing.T) { + txID := vgrand.RandomStr(5) + txHash := vgrand.RandomStr(5) + consentRequestsChan := make(chan service.ConsentRequest, 1) + sentTransactionsChan := make(chan service.SentTransaction, 1) + + // setup + p := service.NewExplicitConsentPolicy(context.Background(), consentRequestsChan, sentTransactionsChan) + + // when + p.Report(service.SentTransaction{ + TxHash: txHash, + TxID: txID, + Tx: &commandspb.Transaction{}, + }) + + // then + sentTransaction := <-sentTransactionsChan + require.Equal(t, txHash, sentTransaction.TxHash) + require.Equal(t, txID, sentTransaction.TxID) +} diff --git a/service/service.go b/service/service.go index 08ddb6f7..0c630f54 100644 --- a/service/service.go +++ b/service/service.go @@ -14,7 +14,6 @@ import ( vgrand "code.vegaprotocol.io/shared/libs/rand" "code.vegaprotocol.io/protos/commands" - typespb "code.vegaprotocol.io/protos/vega" api "code.vegaprotocol.io/protos/vega/api/v1" commandspb "code.vegaprotocol.io/protos/vega/commands/v1" walletpb "code.vegaprotocol.io/protos/vega/wallet/v1" @@ -28,7 +27,6 @@ import ( "github.com/julienschmidt/httprouter" "github.com/rs/cors" "go.uber.org/zap" - "google.golang.org/grpc/status" ) type Service struct { @@ -716,7 +714,7 @@ func (s *Service) VerifyAny(w http.ResponseWriter, r *http.Request, _ httprouter s.writeSuccess(w, VerifyAnyResponse{Valid: verified}) } -func (s *Service) CheckTx(token string, w http.ResponseWriter, r *http.Request, p httprouter.Params) { +func (s *Service) CheckTx(token string, w http.ResponseWriter, r *http.Request, _ httprouter.Params) { defer r.Body.Close() name, err := s.auth.VerifyToken(token) @@ -757,20 +755,7 @@ func (s *Service) CheckTx(token string, w http.ResponseWriter, r *http.Request, result, err := s.nodeForward.CheckTx(r.Context(), tx, cltIdx) if err != nil { - if st, ok := status.FromError(err); ok { - var details []string - for _, v := range st.Details() { - v, ok := v.(*typespb.ErrorDetail) - if !ok { - s.writeError(w, newErrorResponse(fmt.Sprintf("couldn't cast status details to error details: %v", v)), http.StatusInternalServerError) - } - details = append(details, v.Message) - } - - s.writeError(w, newErrorWithDetails(err.Error(), details), http.StatusInternalServerError) - } else { - s.writeInternalError(w, err) - } + s.writeInternalError(w, err) return } @@ -820,7 +805,9 @@ func (s *Service) signTx(token string, w http.ResponseWriter, r *http.Request, _ receivedAt := time.Now() approved, err := s.policy.Ask(req, txID, receivedAt) if err != nil { - s.log.Panic("failed getting transaction sign request answer", zap.Error(err)) + s.log.Error("couldn't get user consent", zap.Error(err)) + s.writeError(w, err, http.StatusServiceUnavailable) + return } if !approved { @@ -832,12 +819,20 @@ func (s *Service) signTx(token string, w http.ResponseWriter, r *http.Request, _ blockData, cltIdx, err := s.nodeForward.LastBlockHeightAndHash(r.Context()) if err != nil { + s.policy.Report(SentTransaction{ + TxID: txID, + Error: ErrCouldNotGetBlockHeight, + }) s.writeInternalError(w, ErrCouldNotGetBlockHeight) return } tx, err := s.handler.SignTx(name, req, blockData.Height) if err != nil { + s.policy.Report(SentTransaction{ + TxID: txID, + Error: err, + }) s.writeInternalError(w, err) return } @@ -846,6 +841,11 @@ func (s *Service) signTx(token string, w http.ResponseWriter, r *http.Request, _ tid := vgcrypto.RandomHash() powNonce, _, err := vgcrypto.PoW(blockData.Hash, tid, uint(blockData.SpamPowDifficulty), vgcrypto.Sha3) if err != nil { + s.policy.Report(SentTransaction{ + Tx: tx, + TxID: txID, + Error: err, + }) s.writeInternalError(w, err) return } @@ -854,52 +854,36 @@ func (s *Service) signTx(token string, w http.ResponseWriter, r *http.Request, _ Nonce: powNonce, } + sentAt := time.Now() txHash, err := s.nodeForward.SendTx(r.Context(), tx, ty, cltIdx) if err != nil { - if st, ok := status.FromError(err); ok { - var details []string - for _, v := range st.Details() { - v, ok := v.(*typespb.ErrorDetail) - if !ok { - s.writeError(w, newErrorResponse(fmt.Sprintf("couldn't cast status details to error details: %v", v)), http.StatusInternalServerError) - } - details = append(details, v.Message) - } - s.policy.Report(SentTransaction{ - Tx: tx, - ReceivedAt: receivedAt, - TxID: txID, - Error: err, - ErrorDetails: details, - }) - s.writeError(w, newErrorWithDetails(err.Error(), details), http.StatusInternalServerError) - } else { - s.policy.Report(SentTransaction{ - Tx: tx, - ReceivedAt: receivedAt, - TxID: txID, - Error: err, - }) - s.writeInternalError(w, err) - } + s.policy.Report(SentTransaction{ + Tx: tx, + TxID: txID, + Error: err, + SentAt: sentAt, + }) + s.writeInternalError(w, err) return } s.policy.Report(SentTransaction{ - TxHash: txHash, - ReceivedAt: receivedAt, - TxID: txID, - Tx: tx, + TxHash: txHash, + TxID: txID, + Tx: tx, + SentAt: sentAt, }) s.writeSuccess(w, struct { TxHash string `json:"txHash"` ReceivedAt time.Time `json:"receivedAt"` + SentAt time.Time `json:"sentAt"` TxID string `json:"txId"` Tx *commandspb.Transaction `json:"tx"` }{ TxHash: txHash, ReceivedAt: receivedAt, + SentAt: sentAt, TxID: txID, Tx: tx, }) @@ -1021,9 +1005,9 @@ func (s *Service) writeSuccess(w http.ResponseWriter, data interface{}) { func (s *Service) handle(method string, path string, handle httprouter.Handle) { loggedEndpoint := func(w http.ResponseWriter, r *http.Request, p httprouter.Params) { - s.log.Info(fmt.Sprintf("--> %s %s", method, path)) + s.log.Info(fmt.Sprintf("Entering %s %s", method, path)) handle(w, r, p) - s.log.Info(fmt.Sprintf("<-- %s %s", method, path)) + s.log.Info(fmt.Sprintf("Leaving %s %s", method, path)) } s.Handle(method, path, loggedEndpoint) } diff --git a/service/service_test.go b/service/service_test.go index fcf03b48..b694855e 100644 --- a/service/service_test.go +++ b/service/service_test.go @@ -11,10 +11,6 @@ import ( "testing" "time" - "github.com/stretchr/testify/require" - - v1 "code.vegaprotocol.io/protos/vega/wallet/v1" - api "code.vegaprotocol.io/protos/vega/api/v1" commandspb "code.vegaprotocol.io/protos/vega/commands/v1" vgrand "code.vegaprotocol.io/shared/libs/rand" @@ -100,7 +96,6 @@ func TestService(t *testing.T) { t.Run("update metadata", testServiceUpdateMetaOK) t.Run("update metadata invalid request", testServiceUpdateMetaFailInvalidRequest) t.Run("Signing transaction succeeds", testAcceptSigningTransactionSucceeds) - t.Run("using ask function in policy succeeds", testAskingConsentPolicySucceeds) t.Run("Checking transaction succeeds", testCheckTransactionSucceeds) t.Run("Checking transaction with rejected transaction succeeds", testCheckTransactionWithRejectedTransactionSucceeds) t.Run("Checking transaction with failed transaction fails", testCheckTransactionWithFailedTransactionFails) @@ -829,38 +824,6 @@ func testAcceptSigningTransactionSucceeds(t *testing.T) { assert.Equal(t, http.StatusOK, statusCode) } -func testAskingConsentPolicySucceeds(t *testing.T) { - txn := &v1.SubmitTransactionRequest{} - - pendingConsents := make(chan service.ConsentRequest, 1) - sentTxs := make(chan service.SentTransaction, 1) - p := service.NewExplicitConsentPolicy(pendingConsents, sentTxs) - - go func() { - req := <-pendingConsents - st, _ := req.String() - d := service.ConsentConfirmation{TxStr: st, Decision: false} - req.Confirmations <- d - }() - - nowTime := time.Now() - answer, err := p.Ask(txn, "testTx", nowTime) - require.Nil(t, err) - require.False(t, answer) - - p.Report(service.SentTransaction{ - TxHash: "txHash", - ReceivedAt: nowTime, - TxID: "testTx", - Tx: &commandspb.Transaction{}, - }) - - sent := <-sentTxs - require.Equal(t, "txHash", sent.TxHash) - require.Equal(t, "testTx", sent.TxID) - require.Equal(t, nowTime, sent.ReceivedAt) -} - func testDeclineSigningTransactionManuallySucceeds(t *testing.T) { s := getTestService(t, "manual") defer s.ctrl.Finish() diff --git a/version/version.go b/version/version.go index 660db4ad..a1819814 100644 --- a/version/version.go +++ b/version/version.go @@ -10,7 +10,7 @@ const ( ReleasesAPI = "https://api.github.com/repos/vegaprotocol/vegawallet/releases" ReleasesURL = "https://github.com/vegaprotocol/vegawallet/releases" defaultVersionHash = "unknown" - defaultVersion = "v0.15.0" + defaultVersion = "v0.15.1" ) var (