diff --git a/session/pingpong/hermes_caller.go b/session/pingpong/hermes_caller.go index d18820462..4cf67781c 100644 --- a/session/pingpong/hermes_caller.go +++ b/session/pingpong/hermes_caller.go @@ -44,11 +44,18 @@ type HermesErrorResponse struct { ErrorMessage string `json:"message"` ErrorData string `json:"data"` c error + statusCode int } // Error returns the associated error func (aer HermesErrorResponse) Error() string { - return aer.c.Error() + if aer.c != nil { + return aer.c.Error() + } + if aer.ErrorMessage != "" { + return aer.ErrorMessage + } + return "unknown hermes error" } // Cause returns the associated cause @@ -66,6 +73,46 @@ func (aer HermesErrorResponse) Data() string { return aer.ErrorData } +// HTTPStatusCode returns the HTTP status code returned by Hermes. +func (aer HermesErrorResponse) HTTPStatusCode() int { + return aer.statusCode +} + +type hermesHTTPError struct { + statusCode int + err error +} + +func (e hermesHTTPError) Error() string { + return e.err.Error() +} + +func (e hermesHTTPError) Unwrap() error { + return e.err +} + +func (e hermesHTTPError) HTTPStatusCode() int { + return e.statusCode +} + +type hermesHTTPStatusError interface { + HTTPStatusCode() int +} + +func isHermesServerError(err error) bool { + if err == nil { + return false + } + + var statusErr hermesHTTPStatusError + if !errors.As(err, &statusErr) { + return false + } + + status := statusErr.HTTPStatusCode() + return status >= http.StatusInternalServerError && status <= 599 +} + // UnmarshalJSON unmarshals given data to HermesErrorResponse func (aer *HermesErrorResponse) UnmarshalJSON(data []byte) error { var s struct { @@ -417,17 +464,25 @@ func (ac *HermesCaller) doRequest(req *http.Request, to any) error { return nil } - // parse error body - hermesError := HermesErrorResponse{} + // Parse the error body while preserving the HTTP status. Generic 5xx + // responses can be ambiguous: Hermes may have committed a promise even + // when the provider did not receive a successful response. + hermesError := HermesErrorResponse{statusCode: resp.StatusCode} if string(body) == "" { - hermesError.ErrorMessage = "Unknown error" - return hermesError + return hermesHTTPError{ + statusCode: resp.StatusCode, + err: errors.New("unknown hermes error"), + } } err = json.Unmarshal(body, &hermesError) if err != nil { - return fmt.Errorf("could not unmarshal error body: %w", err) + return hermesHTTPError{ + statusCode: resp.StatusCode, + err: fmt.Errorf("could not unmarshal error body: %w", err), + } } + hermesError.statusCode = resp.StatusCode return hermesError } @@ -494,34 +549,41 @@ type LatestPromise struct { Signature string `json:"Signature"` } -// isValid checks if the promise is really issued by the given identity -func (lp LatestPromise) isValid(id string) error { - // if we've not promised anything, that's fine for us. - // handles the case when we've just registered the identity. - if lp.Amount == nil || lp.Amount.Cmp(new(big.Int)) == 0 { - return nil - } - +func (lp LatestPromise) toPromise() (crypto.Promise, error) { decodedChannelID, err := hex.DecodeString(strings.TrimPrefix(lp.ChannelID, "0x")) if err != nil { - return fmt.Errorf("could not decode channel ID: %w", err) + return crypto.Promise{}, fmt.Errorf("could not decode channel ID: %w", err) } decodedHashlock, err := hex.DecodeString(strings.TrimPrefix(lp.Hashlock, "0x")) if err != nil { - return fmt.Errorf("could not decode hashlock: %w", err) + return crypto.Promise{}, fmt.Errorf("could not decode hashlock: %w", err) } decodedSignature, err := hex.DecodeString(strings.TrimPrefix(lp.Signature, "0x")) if err != nil { - return fmt.Errorf("could not decode signature: %w", err) + return crypto.Promise{}, fmt.Errorf("could not decode signature: %w", err) } - p := crypto.Promise{ + return crypto.Promise{ ChainID: lp.ChainID, ChannelID: decodedChannelID, Amount: lp.Amount, Fee: lp.Fee, Hashlock: decodedHashlock, Signature: decodedSignature, + }, nil +} + +// isValid checks if the promise is really issued by the given identity +func (lp LatestPromise) isValid(id string) error { + // if we've not promised anything, that's fine for us. + // handles the case when we've just registered the identity. + if lp.Amount == nil || lp.Amount.Cmp(new(big.Int)) == 0 { + return nil + } + + p, err := lp.toPromise() + if err != nil { + return err } if !p.IsPromiseValid(common.HexToAddress(id)) { diff --git a/session/pingpong/hermes_caller_test.go b/session/pingpong/hermes_caller_test.go index 3853bcda9..da09bb90b 100644 --- a/session/pingpong/hermes_caller_test.go +++ b/session/pingpong/hermes_caller_test.go @@ -70,6 +70,25 @@ func TestHermesCaller_RequestPromise_Error(t *testing.T) { assert.NotNil(t, err) } +func TestHermesCaller_RequestPromise_PreservesServerStatusForUnknownError(t *testing.T) { + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(http.StatusInternalServerError) + _, err := w.Write([]byte(`{"cause":"","data":"","message":""}`)) + assert.NoError(t, err) + })) + defer server.Close() + + c := requests.NewHTTPClient("0.0.0.0", time.Second) + caller := NewHermesCaller(c, server.URL) + _, err := caller.RequestPromise(RequestPromise{}) + assert.Error(t, err) + assert.True(t, isHermesServerError(err)) + + var statusErr hermesHTTPStatusError + assert.True(t, errors.As(err, &statusErr)) + assert.Equal(t, http.StatusInternalServerError, statusErr.HTTPStatusCode()) +} + func TestHermesCaller_RevealR_Error(t *testing.T) { server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { w.WriteHeader(http.StatusBadRequest) diff --git a/session/pingpong/hermes_promise_handler.go b/session/pingpong/hermes_promise_handler.go index a140b9e76..95634c1eb 100644 --- a/session/pingpong/hermes_promise_handler.go +++ b/session/pingpong/hermes_promise_handler.go @@ -23,6 +23,7 @@ import ( stdErr "errors" "fmt" "math/big" + "strings" "sync" "time" @@ -148,6 +149,17 @@ func (aph *HermesPromiseHandler) makeRequestPromiseFunc(providerID identity.Iden return p, nil } + // Do not blindly replay a request after an ambiguous Hermes 5xx. The + // request may already have committed. While this handler owns the + // serialized Hermes queue entry, check the provider's latest promise + // and only synthesize success when it exactly matches this request. + if isHermesServerError(err) { + if reconciled, ok := aph.reconcileAmbiguousRequestPromise(providerID, caller, rp); ok { + log.Warn().Msg("reconciled Hermes request_promise after ambiguous server error") + return reconciled, nil + } + } + if !stdErr.Is(err, ErrInvalidPreviuosLatestPromise) { // We can only really handle the previuos promise is invalid error. return crypto.Promise{}, err @@ -176,6 +188,45 @@ func (aph *HermesPromiseHandler) makeRequestPromiseFunc(providerID identity.Iden } } +func (aph *HermesPromiseHandler) reconcileAmbiguousRequestPromise(providerID identity.Identity, caller HermesHTTPRequester, rp RequestPromise) (crypto.Promise, bool) { + data, err := caller.GetProviderData(rp.ExchangeMessage.ChainID, providerID.Address) + if err != nil { + log.Warn().Err(err).Msg("could not reconcile ambiguous Hermes request_promise") + return crypto.Promise{}, false + } + + chid, err := crypto.GenerateProviderChannelID(providerID.Address, rp.ExchangeMessage.HermesID) + if err != nil { + log.Warn().Err(err).Msg("could not generate provider channel ID while reconciling Hermes request_promise") + return crypto.Promise{}, false + } + + latest := data.LatestPromise + expectedHashlock := "0x" + common.Bytes2Hex(rp.ExchangeMessage.Promise.Hashlock) + + if !strings.EqualFold(data.Identity, providerID.Address) || + !strings.EqualFold(data.ChannelID, chid) || + !strings.EqualFold(latest.ChannelID, chid) || + latest.ChainID != rp.ExchangeMessage.ChainID || + !strings.EqualFold(latest.Hashlock, expectedHashlock) || + latest.Amount == nil || + latest.Amount.Sign() <= 0 || + latest.Fee == nil || + rp.TransactorFee == nil || + latest.Fee.Cmp(rp.TransactorFee) != 0 || + latest.Signature == "" { + return crypto.Promise{}, false + } + + promise, err := latest.toPromise() + if err != nil { + log.Warn().Err(err).Msg("could not decode provider promise while reconciling Hermes request_promise") + return crypto.Promise{}, false + } + + return promise, true +} + // PayAndSettle adds the request to the queue. func (aph *HermesPromiseHandler) PayAndSettle(r []byte, em crypto.ExchangeMessage, providerID identity.Identity, sessionID string) <-chan error { er := enqueuedRequest{ diff --git a/session/pingpong/hermes_promise_handler_test.go b/session/pingpong/hermes_promise_handler_test.go index b8fdca1bd..f875165ac 100644 --- a/session/pingpong/hermes_promise_handler_test.go +++ b/session/pingpong/hermes_promise_handler_test.go @@ -19,6 +19,7 @@ package pingpong import ( "errors" + "math/big" "testing" "github.com/ethereum/go-ethereum/common" @@ -116,6 +117,137 @@ func TestHermesPromiseHandler_RequestPromise_BubblesErrors(t *testing.T) { assert.Nil(t, err) } +type reconcileHermesCaller struct { + mockHermesCaller + providerData HermesUserInfo + providerDataErr error + requestCalls int + providerDataCalls int +} + +func (c *reconcileHermesCaller) RequestPromise(rp RequestPromise) (crypto.Promise, error) { + c.requestCalls++ + return crypto.Promise{}, c.errToReturn +} + +func (c *reconcileHermesCaller) GetProviderData(chainID int64, id string) (HermesUserInfo, error) { + c.providerDataCalls++ + return c.providerData, c.providerDataErr +} + +func TestHermesPromiseHandler_makeRequestPromiseFunc_ReconcilesAmbiguousServerError(t *testing.T) { + providerID := identity.FromAddress("0x0000000000000000000000000000000000000001") + hermesID := common.HexToAddress("0x0000000000000000000000000000000000000002") + channelID, err := crypto.GenerateProviderChannelID(providerID.Address, hermesID.Hex()) + assert.NoError(t, err) + + fee := big.NewInt(42) + hashlock := []byte{0x01, 0x02, 0x03} + latest := LatestPromise{ + ChainID: 137, + ChannelID: channelID, + Amount: big.NewInt(100), + Fee: new(big.Int).Set(fee), + Hashlock: "0x" + common.Bytes2Hex(hashlock), + Signature: "0x01", + } + caller := &reconcileHermesCaller{ + mockHermesCaller: mockHermesCaller{ + errToReturn: hermesHTTPError{ + statusCode: 500, + err: errors.New("ambiguous server error"), + }, + }, + providerData: HermesUserInfo{ + Identity: providerID.Address, + ChannelID: channelID, + LatestPromise: latest, + }, + } + + rp := RequestPromise{ + ExchangeMessage: crypto.ExchangeMessage{ + ChainID: 137, + HermesID: hermesID.Hex(), + Promise: crypto.Promise{ + Hashlock: hashlock, + }, + }, + TransactorFee: fee, + } + + got, err := (&HermesPromiseHandler{}).makeRequestPromiseFunc(providerID, caller)(rp) + assert.NoError(t, err) + + expected, err := latest.toPromise() + assert.NoError(t, err) + assert.Equal(t, expected, got) + assert.Equal(t, 1, caller.requestCalls) + assert.Equal(t, 1, caller.providerDataCalls) +} + +func TestHermesPromiseHandler_makeRequestPromiseFunc_DoesNotReconcileMismatch(t *testing.T) { + providerID := identity.FromAddress("0x0000000000000000000000000000000000000001") + hermesID := common.HexToAddress("0x0000000000000000000000000000000000000002") + channelID, err := crypto.GenerateProviderChannelID(providerID.Address, hermesID.Hex()) + assert.NoError(t, err) + + fee := big.NewInt(42) + caller := &reconcileHermesCaller{ + mockHermesCaller: mockHermesCaller{ + errToReturn: hermesHTTPError{ + statusCode: 500, + err: errors.New("ambiguous server error"), + }, + }, + providerData: HermesUserInfo{ + Identity: providerID.Address, + ChannelID: channelID, + LatestPromise: LatestPromise{ + ChainID: 137, + ChannelID: channelID, + Amount: big.NewInt(100), + Fee: new(big.Int).Set(fee), + Hashlock: "0xdeadbeef", + Signature: "0x01", + }, + }, + } + + rp := RequestPromise{ + ExchangeMessage: crypto.ExchangeMessage{ + ChainID: 137, + HermesID: hermesID.Hex(), + Promise: crypto.Promise{ + Hashlock: []byte{0x01, 0x02, 0x03}, + }, + }, + TransactorFee: fee, + } + + _, err = (&HermesPromiseHandler{}).makeRequestPromiseFunc(providerID, caller)(rp) + assert.Error(t, err) + assert.Equal(t, 1, caller.requestCalls) + assert.Equal(t, 1, caller.providerDataCalls) +} + +func TestHermesPromiseHandler_makeRequestPromiseFunc_DoesNotReconcileNonServerError(t *testing.T) { + providerID := identity.FromAddress("0x0000000000000000000000000000000000000001") + caller := &reconcileHermesCaller{ + mockHermesCaller: mockHermesCaller{ + errToReturn: hermesHTTPError{ + statusCode: 400, + err: errors.New("bad request"), + }, + }, + } + + _, err := (&HermesPromiseHandler{}).makeRequestPromiseFunc(providerID, caller)(RequestPromise{}) + assert.Error(t, err) + assert.Equal(t, 1, caller.requestCalls) + assert.Zero(t, caller.providerDataCalls) +} + func TestHermesPromiseHandler_recoverR(t *testing.T) { type fields struct { deps HermesPromiseHandlerDeps