Merge pull request #766 from ellemouton/tracing

accounts: address pendingPayments issues & add tracer

Author: ellemouton

accounts/checkers.go

@@ -3,6 +3,9 @@ package accounts
 import (
 	"context"
 	"fmt"
+
+	"github.com/btcsuite/btclog"
+	"github.com/lightningnetwork/lnd/build"
 )
 
 // ContextKey is the type that we use to identify account specific values in the
@@ -18,18 +21,23 @@ var (
 	// KeyAccount is the key under which we store the account in the request
 	// context.
 	KeyAccount = ContextKey{"account"}
+
+	// KeyRequestID is the key under which we store the middleware request
+	// ID.
+	KeyRequestID = ContextKey{"request_id"}
 )
 
 // FromContext tries to extract a value from the given context.
 func FromContext(ctx context.Context, key ContextKey) interface{} {
 	return ctx.Value(key)
 }
 
-// AddToContext adds the given value to the context for easy retrieval later on.
-func AddToContext(ctx context.Context, key ContextKey,
+// AddAccountToContext adds the given value to the context for easy retrieval
+// later on.
+func AddAccountToContext(ctx context.Context,
 	value *OffChainBalanceAccount) context.Context {
 
-	return context.WithValue(ctx, key, value)
+	return context.WithValue(ctx, KeyAccount, value)
 }
 
 // AccountFromContext attempts to extract an account from the given context.
@@ -46,3 +54,46 @@ func AccountFromContext(ctx context.Context) (*OffChainBalanceAccount, error) {
 
 	return acct, nil
 }
+
+// AddRequestIDToContext adds the given request ID to the context for easy
+// retrieval later on.
+func AddRequestIDToContext(ctx context.Context, value uint64) context.Context {
+	return context.WithValue(ctx, KeyRequestID, value)
+}
+
+// RequestIDFromContext attempts to extract a request ID from the given context.
+func RequestIDFromContext(ctx context.Context) (uint64, error) {
+	val := FromContext(ctx, KeyRequestID)
+	if val == nil {
+		return 0, fmt.Errorf("no request ID found in context")
+	}
+
+	reqID, ok := val.(uint64)
+	if !ok {
+		return 0, fmt.Errorf("invalid request ID value in context")
+	}
+
+	return reqID, nil
+}
+
+// requestScopedValuesFromCtx is a helper function that can be used to extract
+// an account and requestID from the given context. It also creates a new
+// prefixed logger that can be used by account request and response handlers.
+// Each log line will be prefixed by the account ID and the request ID.
+func requestScopedValuesFromCtx(ctx context.Context) (btclog.Logger,
+	*OffChainBalanceAccount, uint64, error) {
+
+	acc, err := AccountFromContext(ctx)
+	if err != nil {
+		return nil, nil, 0, err
+	}
+
+	reqID, err := RequestIDFromContext(ctx)
+	if err != nil {
+		return nil, nil, 0, err
+	}
+
+	prefix := fmt.Sprintf("[account: %s, request: %d]", acc.ID, reqID)
+
+	return build.NewPrefixLog(prefix, log), acc, reqID, nil
+}

accounts/checkers_test.go

@@ -101,9 +101,10 @@ func (s *InterceptorService) Intercept(ctx context.Context,
 		)
 	}
 
-	// We now add the account to the incoming context to give each checker
-	// access to it if required.
-	ctxAccount := AddToContext(ctx, KeyAccount, acct)
+	// We now add the account and request ID to the incoming context to give
+	// each checker access to them if required.
+	ctx = AddAccountToContext(ctx, acct)
+	ctx = AddRequestIDToContext(ctx, req.RequestId)
 
 	switch r := req.InterceptType.(type) {
 	// In the authentication phase we just check that the account hasn't
@@ -120,18 +121,42 @@ func (s *InterceptorService) Intercept(ctx context.Context,
 		}
 
 		return mid.RPCErr(req, s.checkers.checkIncomingRequest(
-			ctxAccount, r.Request.MethodFullUri, msg,
+			ctx, r.Request.MethodFullUri, msg,
 		))
 
 	// Parse and possibly manipulate outgoing responses.
 	case *lnrpc.RPCMiddlewareRequest_Response:
+		if r.Response.IsError {
+			parsedErr := mid.ParseResponseErr(r.Response.Serialized)
+
+			replacementErr, err := s.checkers.handleErrorResponse(
+				ctx, r.Response.MethodFullUri, parsedErr,
+			)
+			if err != nil {
+				return mid.RPCErr(req, err)
+			}
+
+			// No error occurred but the response error should be
+			// replaced with the given custom error. Wrap it in the
+			// correct RPC response of the interceptor now.
+			if replacementErr != nil {
+				return mid.RPCErrReplacement(
+					req, replacementErr,
+				)
+			}
+
+			// No error and no replacement, just return an empty
+			// response of the correct type.
+			return mid.RPCOk(req)
+		}
+
 		msg, err := parseRPCMessage(r.Response)
 		if err != nil {
 			return mid.RPCErr(req, err)
 		}
 
 		replacement, err := s.checkers.replaceOutgoingResponse(
-			ctxAccount, r.Response.MethodFullUri, msg,
+			ctx, r.Response.MethodFullUri, msg,
 		)
 		if err != nil {
 			return mid.RPCErr(req, err)

accounts/context.go

@@ -53,6 +53,11 @@ func ParseAccountID(idStr string) (*AccountID, error) {
 	return &id, nil
 }
 
+// String returns the string representation of the AccountID.
+func (a AccountID) String() string {
+	return hex.EncodeToString(a[:])
+}
+
 // PaymentEntry is the data we track per payment that is associated with an
 // account. This basically includes all information required to make sure
 // in-flight payments don't exceed the total available account balance.
@@ -252,4 +257,38 @@ type Service interface {
 	// restarted.
 	AssociatePayment(id AccountID, paymentHash lntypes.Hash,
 		fullAmt lnwire.MilliSatoshi) error
+
+	// PaymentErrored removes a pending payment from the accounts
+	// registered payment list. This should only ever be called if we are
+	// sure that the payment request errored out.
+	PaymentErrored(id AccountID, hash lntypes.Hash) error
+
+	RequestValuesStore
+}
+
+// RequestValues holds various values associated with a specific request that
+// we may want access to when handling the response. At the moment this only
+// stores payment related data.
+type RequestValues struct {
+	// PaymentHash is the hash of the payment that this request is
+	// associated with.
+	PaymentHash lntypes.Hash
+
+	// PaymentAmount is the value of the payment being made.
+	PaymentAmount lnwire.MilliSatoshi
+}
+
+// RequestValuesStore is a store that can be used to keep track of the mapping
+// between a request ID and various values associated with that request which
+// we may want access to when handling the request response.
+type RequestValuesStore interface {
+	// RegisterValues stores values for the given request ID.
+	RegisterValues(reqID uint64, values *RequestValues) error
+
+	// GetValues returns the corresponding request values for the given
+	// request ID if they exist.
+	GetValues(reqID uint64) (*RequestValues, bool)
+
+	// DeleteValues deletes any values stored for the given request ID.
+	DeleteValues(reqID uint64)
 }

accounts/interceptor.go

@@ -5,7 +5,7 @@ import (
 	"github.com/lightningnetwork/lnd/build"
 )
 
-const Subsystem = "ACCT"
+const Subsystem = "ACNT"
 
 // log is a logger that is initialized with no output filters.  This
 // means the package will not perform any logging by default until the caller