Skip to content
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
107 changes: 106 additions & 1 deletion payments/db/sql_store.go
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ type SQLQueries interface {
Payment DB write operations.
*/
InsertPaymentIntent(ctx context.Context, arg sqlc.InsertPaymentIntentParams) (int64, error)
InsertPayment(ctx context.Context, arg sqlc.InsertPaymentParams) error
InsertPayment(ctx context.Context, arg sqlc.InsertPaymentParams) (int64, error)
InsertPaymentFirstHopCustomRecord(ctx context.Context, arg sqlc.InsertPaymentFirstHopCustomRecordParams) error

InsertHtlcAttempt(ctx context.Context, arg sqlc.InsertHtlcAttemptParams) (int64, error)
Expand Down Expand Up @@ -629,3 +629,108 @@ func (s *SQLStore) DeleteFailedAttempts(paymentHash lntypes.Hash) error {

return nil
}

// InitPayment initializes a payment.
//
// This is part of the DB interface.
func (s *SQLStore) InitPayment(paymentHash lntypes.Hash,
paymentCreationInfo *PaymentCreationInfo) error {

ctx := context.TODO()

// Create the payment in the database.
err := s.db.ExecTx(ctx, sqldb.WriteTxOpt(), func(db SQLQueries) error {
existingPayment, err := db.FetchPayment(ctx, paymentHash[:])
if err == nil {
completePayment, err := s.fetchPaymentWithCompleteData(
ctx, db, existingPayment,
)
Comment on lines +647 to +649
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

again i dont think you need to fetch the whole thing & all its normalised data

if err != nil {
return fmt.Errorf("failed to fetch payment "+
"with complete data: %w", err)
}

// Check if the payment is initializable otherwise
// we'll return early.
err = completePayment.Status.initializable()
if err != nil {
return err
}
} else if !errors.Is(err, sql.ErrNoRows) {
// Some other error occurred
return fmt.Errorf("failed to check existing "+
"payment: %w", err)
}

// If payment exists and is failed, delete it first.
if existingPayment.Payment.ID != 0 {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i find this part quite hard to read. I suggest using a switch with 3 different cases.
Cause here, you are referencing the returned existingPayment in the case that the error is not nil which is an antipattern.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this will only be !=0 in the case where err == nil - so just put it in that block

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

so like:

existingPayment, err := db.FetchPayment(ctx, paymentHash[:])
		switch {
		case err == nil:
			completePayment, err := s.fetchPaymentWithCompleteData(
				ctx, db, existingPayment,
			)
			if err != nil {
				return fmt.Errorf("failed to fetch payment "+
					"with complete data: %w", err)
			}

			// Check if the payment is initializable otherwise
			// we'll return early.
			err = completePayment.Status.initializable()
			if err != nil {
				return err
			}

			// If the initializable check above passes, then the
			// existing payment has failed. So we delete it and
			// all of its previous artifacts. We rely on
			// cascading deletes to clean up the rest.
			err = db.DeletePayment(ctx, existingPayment.Payment.ID)
			if err != nil {
				return fmt.Errorf("failed to delete "+
					"payment: %w", err)
			}

		case !errors.Is(err, sql.ErrNoRows):
			// Some other error occurred
			return fmt.Errorf("failed to check existing "+
				"payment: %w", err)

		// The payment does not yet exist.
		default:
		}

err := db.DeletePayment(ctx, existingPayment.Payment.ID)
if err != nil {
return fmt.Errorf("failed to delete "+
"payment: %w", err)
}
}

var intentID *int64
if len(paymentCreationInfo.PaymentRequest) > 0 {
intentIDValue, err := db.InsertPaymentIntent(ctx,
sqlc.InsertPaymentIntentParams{
IntentType: int16(
PaymentIntentTypeBolt11,
),
IntentPayload: paymentCreationInfo.
PaymentRequest,
})
if err != nil {
return fmt.Errorf("failed to initialize "+
"payment intent: %w", err)
}
intentID = &intentIDValue
}

// Only set the intent ID if it's not nil.
var intentIDParam sql.NullInt64
if intentID != nil {
intentIDParam = sqldb.SQLInt64(*intentID)
}
Comment on lines +693 to +697
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i think rather then dont use the sqldb.SQLInt64 helper

	var intentID int64
		if len(paymentCreationInfo.PaymentRequest) > 0 {
			intentID, err = db.InsertPaymentIntent(ctx,
				sqlc.InsertPaymentIntentParams{
					IntentType: int16(
						PaymentIntentTypeBolt11,
					),
					IntentPayload: paymentCreationInfo.
						PaymentRequest,
				})
			if err != nil {
				return fmt.Errorf("failed to initialize "+
					"payment intent: %w", err)
			}
		}

		paymentID, err := db.InsertPayment(ctx,
			sqlc.InsertPaymentParams{
				IntentID: sql.NullInt64{
					Int64: intentID,
					Valid: intentID != 0,
				},


paymentID, err := db.InsertPayment(ctx,
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ctx on next line

sqlc.InsertPaymentParams{
IntentID: intentIDParam,
AmountMsat: int64(
paymentCreationInfo.Value,
),
CreatedAt: paymentCreationInfo.
CreationTime.UTC(),
PaymentIdentifier: paymentHash[:],
})
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

incorrect wrapping

if err != nil {
return fmt.Errorf("failed to insert payment: %w", err)
}

firstHopCustomRecords := paymentCreationInfo.
FirstHopCustomRecords

for key, value := range firstHopCustomRecords {
err = db.InsertPaymentFirstHopCustomRecord(ctx,
sqlc.InsertPaymentFirstHopCustomRecordParams{
PaymentID: paymentID,
Key: int64(key),
Value: value,
})
if err != nil {
return fmt.Errorf("failed to insert "+
"payment first hop custom "+
"record: %w", err)
}
}

return nil
}, func() {
})
Comment on lines +731 to +732
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sqldb.NoOpReset

if err != nil {
return fmt.Errorf("failed to initialize payment: %w", err)
}

return nil
}
11 changes: 7 additions & 4 deletions sqldb/sqlc/payments.sql.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion sqldb/sqlc/querier.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

5 changes: 3 additions & 2 deletions sqldb/sqlc/queries/payments.sql
Original file line number Diff line number Diff line change
Expand Up @@ -195,7 +195,7 @@ ON CONFLICT (intent_type, intent_payload) DO UPDATE SET
intent_payload = EXCLUDED.intent_payload
RETURNING id;

-- name: InsertPayment :exec
-- name: InsertPayment :one
-- Insert a new payment with the given intent ID and return its ID.
INSERT INTO payments (
intent_id,
Expand All @@ -209,7 +209,8 @@ VALUES (
@created_at,
@payment_identifier,
NULL
);
)
RETURNING id;

-- name: InsertPaymentFirstHopCustomRecord :exec
INSERT INTO payment_first_hop_custom_records (
Expand Down