-
Notifications
You must be signed in to change notification settings - Fork 2.1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
MPP: Enable MultiPathPayments for payment lifecycle #3970
Merged
Roasbeef
merged 34 commits into
lightningnetwork:master
from
halseth:amp-router-mvp-2020
Apr 3, 2020
Merged
Changes from all commits
Commits
Show all changes
34 commits
Select commit
Hold shift + click to select a range
c2301c1
routing/payment_session: make NewPaymentSession take payment directly
halseth f9eeb6b
routing/router+lifecycle: remove LightningPayment from payment lifecycle
halseth 00903ef
routing/payment_session: make RequestRoute take max amt, fee limit and
halseth e61fcda
routing/payment_lifecycle: move requesting route out of createNewPaym…
halseth 3620721
routing/payment_lifecycle: move attempt DB checkpointing into payment
halseth 4485e82
routing/payment_lifecycle: move Fail call to payment loop
halseth 79227ba
routing/route: define route.ReceiverAmt() method
halseth 6d9f9c3
routing/router_test: remove preimage overwrite
halseth e1f4d89
routing: add FetchPayment method to ControlTower
halseth 9bcf41d
channeldb: make FailureReason Error()
halseth bcca1ab
routing/payment_lifeycle: remove payment level attempt and circuit
halseth 9712dd1
routing/payment_lifecycle: extract attempt sending logic
halseth 2c01e79
routing/payment_lifecycle: extract result collection into collectResult
halseth 6cc162e
router: make sendToRoute omit payment lifecycle
halseth a979b91
routing: remove errNoRoute and lastError
halseth 4509c4f
routing: move ErrMaxRouteHopsExceeded check
halseth 49efbef
routing/payment_session: remove prebuilt payment session
halseth 7b5c108
routing/payment_lifecycle+channeldb: collect existing outcome first
halseth 5adfc96
routing/payment_lifecycle: return recorded errors
halseth f6c97da
channeldb/payment_control_test: simplify HTLC status assertion
halseth 70202be
channeldb: make database logic MPP compatible
halseth 3610824
channeldb/payment_control_test: add TestPaymentControlMultiShard
halseth 4d343bb
routing tests: move TestRouterPaymentStateMachine to own file
halseth aa9c971
routing/payment_lifecycle_test: extract route creation into method
halseth 2e63b51
routing/payment_lifecycle_test+mock: set up listener for FailAttempt
halseth 0fd71cd
routing/payment_lifecycle_test: add step for terminal failure
halseth 7b318a4
routing/payment_lifecycle+channeldb: enable multi shard send
halseth 431372c
routing/payment_lifecycle_test: add MPP test cases
halseth 36a80b4
routing/router: enable MPP sends for SendToRoute
halseth 9a1ec95
channeldb/payments: extract common info fetch into fetchCreationInfo
halseth 864e64e
channeldb: validate MPP options when registering attempts
halseth 95c5a12
routing/router_test: add TestSendToRouteMultiShardSend
halseth fee5fd0
itest: add testSendToRouteMultiPath
halseth 5e72a4b
routing: exit on unexpected RequestRoute error
halseth File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -18,22 +18,59 @@ var ( | |
// already "in flight" on the network. | ||
ErrPaymentInFlight = errors.New("payment is in transition") | ||
|
||
// ErrPaymentNotInitiated is returned if payment wasn't initiated in | ||
// switch. | ||
// ErrPaymentNotInitiated is returned if the payment wasn't initiated. | ||
ErrPaymentNotInitiated = errors.New("payment isn't initiated") | ||
|
||
// ErrPaymentAlreadySucceeded is returned in the event we attempt to | ||
// change the status of a payment already succeeded. | ||
ErrPaymentAlreadySucceeded = errors.New("payment is already succeeded") | ||
|
||
// ErrPaymentAlreadyFailed is returned in the event we attempt to | ||
// re-fail a failed payment. | ||
// ErrPaymentAlreadyFailed is returned in the event we attempt to alter | ||
// a failed payment. | ||
ErrPaymentAlreadyFailed = errors.New("payment has already failed") | ||
|
||
// ErrUnknownPaymentStatus is returned when we do not recognize the | ||
// existing state of a payment. | ||
ErrUnknownPaymentStatus = errors.New("unknown payment status") | ||
|
||
// ErrPaymentTerminal is returned if we attempt to alter a payment that | ||
// already has reached a terminal condition. | ||
ErrPaymentTerminal = errors.New("payment has reached terminal condition") | ||
|
||
// ErrAttemptAlreadySettled is returned if we try to alter an already | ||
// settled HTLC attempt. | ||
ErrAttemptAlreadySettled = errors.New("attempt already settled") | ||
|
||
// ErrAttemptAlreadyFailed is returned if we try to alter an already | ||
// failed HTLC attempt. | ||
ErrAttemptAlreadyFailed = errors.New("attempt already failed") | ||
|
||
// ErrValueMismatch is returned if we try to register a non-MPP attempt | ||
// with an amount that doesn't match the payment amount. | ||
ErrValueMismatch = errors.New("attempted value doesn't match payment" + | ||
"amount") | ||
|
||
// ErrValueExceedsAmt is returned if we try to register an attempt that | ||
// would take the total sent amount above the payment amount. | ||
ErrValueExceedsAmt = errors.New("attempted value exceeds payment" + | ||
"amount") | ||
|
||
// ErrNonMPPayment is returned if we try to register an MPP attempt for | ||
// a payment that already has a non-MPP attempt regitered. | ||
ErrNonMPPayment = errors.New("payment has non-MPP attempts") | ||
|
||
// ErrMPPayment is returned if we try to register a non-MPP attempt for | ||
// a payment that already has an MPP attempt regitered. | ||
ErrMPPayment = errors.New("payment has MPP attempts") | ||
|
||
// ErrMPPPaymentAddrMismatch is returned if we try to register an MPP | ||
// shard where the payment address doesn't match existing shards. | ||
ErrMPPPaymentAddrMismatch = errors.New("payment address mismatch") | ||
|
||
// ErrMPPTotalAmountMismatch is returned if we try to register an MPP | ||
// shard where the total amount doesn't match existing shards. | ||
ErrMPPTotalAmountMismatch = errors.New("mp payment total amount mismatch") | ||
|
||
// errNoAttemptInfo is returned when no attempt info is stored yet. | ||
errNoAttemptInfo = errors.New("unable to find attempt info for " + | ||
"inflight payment") | ||
|
@@ -52,7 +89,7 @@ func NewPaymentControl(db *DB) *PaymentControl { | |
} | ||
|
||
// InitPayment checks or records the given PaymentCreationInfo with the DB, | ||
// making sure it does not already exist as an in-flight payment. Then this | ||
// making sure it does not already exist as an in-flight payment. When this | ||
// method returns successfully, the payment is guranteeed to be in the InFlight | ||
// state. | ||
func (p *PaymentControl) InitPayment(paymentHash lntypes.Hash, | ||
|
@@ -168,12 +205,69 @@ func (p *PaymentControl) RegisterAttempt(paymentHash lntypes.Hash, | |
return err | ||
} | ||
|
||
// We can only register attempts for payments that are | ||
// in-flight. | ||
if err := ensureInFlight(bucket); err != nil { | ||
payment, err := fetchPayment(bucket) | ||
if err != nil { | ||
return err | ||
} | ||
|
||
// Ensure the payment is in-flight. | ||
if err := ensureInFlight(payment); err != nil { | ||
return err | ||
} | ||
|
||
// We cannot register a new attempt if the payment already has | ||
// reached a terminal condition: | ||
settle, fail := payment.TerminalInfo() | ||
if settle != nil || fail != nil { | ||
return ErrPaymentTerminal | ||
} | ||
|
||
// Make sure any existing shards match the new one with regards | ||
// to MPP options. | ||
mpp := attempt.Route.FinalHop().MPP | ||
for _, h := range payment.InFlightHTLCs() { | ||
hMpp := h.Route.FinalHop().MPP | ||
|
||
switch { | ||
|
||
// We tried to register a non-MPP attempt for a MPP | ||
// payment. | ||
case mpp == nil && hMpp != nil: | ||
return ErrMPPayment | ||
|
||
// We tried to register a MPP shard for a non-MPP | ||
// payment. | ||
case mpp != nil && hMpp == nil: | ||
return ErrNonMPPayment | ||
|
||
// Non-MPP payment, nothing more to validate. | ||
case mpp == nil: | ||
continue | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There is a non-mpp inflight and we allow sending another non-mpp? Or is that caught below in the amt check? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Added exact amount check |
||
} | ||
|
||
// Check that MPP options match. | ||
if mpp.PaymentAddr() != hMpp.PaymentAddr() { | ||
return ErrMPPPaymentAddrMismatch | ||
} | ||
|
||
if mpp.TotalMsat() != hMpp.TotalMsat() { | ||
return ErrMPPTotalAmountMismatch | ||
} | ||
} | ||
|
||
// If this is a non-MPP attempt, it must match the total amount | ||
// exactly. | ||
amt := attempt.Route.ReceiverAmt() | ||
if mpp == nil && amt != payment.Info.Value { | ||
return ErrValueMismatch | ||
} | ||
|
||
// Ensure we aren't sending more than the total payment amount. | ||
sentAmt, _ := payment.SentAmt() | ||
if sentAmt+amt > payment.Info.Value { | ||
return ErrValueExceedsAmt | ||
} | ||
|
||
htlcsBucket, err := bucket.CreateBucketIfNotExists( | ||
paymentHtlcsBucket, | ||
) | ||
|
@@ -241,8 +335,15 @@ func (p *PaymentControl) updateHtlcKey(paymentHash lntypes.Hash, | |
return err | ||
} | ||
|
||
// We can only update keys of in-flight payments. | ||
if err := ensureInFlight(bucket); err != nil { | ||
p, err := fetchPayment(bucket) | ||
if err != nil { | ||
return err | ||
} | ||
|
||
// We can only update keys of in-flight payments. We allow | ||
// updating keys even if the payment has reached a terminal | ||
// condition, since the HTLC outcomes must still be updated. | ||
if err := ensureInFlight(p); err != nil { | ||
return err | ||
} | ||
halseth marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
|
@@ -257,6 +358,15 @@ func (p *PaymentControl) updateHtlcKey(paymentHash lntypes.Hash, | |
attemptID) | ||
} | ||
|
||
// Make sure the shard is not already failed or settled. | ||
if htlcBucket.Get(htlcFailInfoKey) != nil { | ||
return ErrAttemptAlreadyFailed | ||
} | ||
|
||
if htlcBucket.Get(htlcSettleInfoKey) != nil { | ||
return ErrAttemptAlreadySettled | ||
} | ||
|
||
// Add or update the key for this htlc. | ||
err = htlcBucket.Put(key, value) | ||
if err != nil { | ||
|
@@ -299,9 +409,17 @@ func (p *PaymentControl) Fail(paymentHash lntypes.Hash, | |
return err | ||
} | ||
|
||
// We can only mark in-flight payments as failed. | ||
if err := ensureInFlight(bucket); err != nil { | ||
updateErr = err | ||
// We mark the payent as failed as long as it is known. This | ||
// lets the last attempt to fail with a terminal write its | ||
// failure to the PaymentControl without synchronizing with | ||
// other attempts. | ||
paymentStatus, err := fetchPaymentStatus(bucket) | ||
if err != nil { | ||
return err | ||
} | ||
|
||
if paymentStatus == StatusUnknown { | ||
updateErr = ErrPaymentNotInitiated | ||
return nil | ||
} | ||
|
||
|
@@ -318,14 +436,6 @@ func (p *PaymentControl) Fail(paymentHash lntypes.Hash, | |
return err | ||
} | ||
|
||
// Final sanity check to see if there are no in-flight htlcs. | ||
for _, htlc := range payment.HTLCs { | ||
if htlc.Settle == nil && htlc.Failure == nil { | ||
return errors.New("payment failed with " + | ||
"in-flight htlc(s)") | ||
} | ||
} | ||
|
||
return nil | ||
}) | ||
if err != nil { | ||
|
@@ -428,45 +538,29 @@ func nextPaymentSequence(tx kvdb.RwTx) ([]byte, error) { | |
// fetchPaymentStatus fetches the payment status of the payment. If the payment | ||
// isn't found, it will default to "StatusUnknown". | ||
func fetchPaymentStatus(bucket kvdb.ReadBucket) (PaymentStatus, error) { | ||
htlcsBucket := bucket.NestedReadBucket(paymentHtlcsBucket) | ||
if htlcsBucket != nil { | ||
htlcs, err := fetchHtlcAttempts(htlcsBucket) | ||
if err != nil { | ||
return 0, err | ||
} | ||
|
||
// Go through all HTLCs, and return StatusSucceeded if any of | ||
// them did succeed. | ||
for _, h := range htlcs { | ||
if h.Settle != nil { | ||
return StatusSucceeded, nil | ||
} | ||
} | ||
// Creation info should be set for all payments, regardless of state. | ||
joostjager marked this conversation as resolved.
Show resolved
Hide resolved
|
||
// If not, it is unknown. | ||
if bucket.Get(paymentCreationInfoKey) == nil { | ||
return StatusUnknown, nil | ||
} | ||
|
||
if bucket.Get(paymentFailInfoKey) != nil { | ||
return StatusFailed, nil | ||
} | ||
|
||
if bucket.Get(paymentCreationInfoKey) != nil { | ||
return StatusInFlight, nil | ||
payment, err := fetchPayment(bucket) | ||
if err != nil { | ||
return 0, err | ||
} | ||
|
||
return StatusUnknown, nil | ||
return payment.Status, nil | ||
} | ||
|
||
// ensureInFlight checks whether the payment found in the given bucket has | ||
// status InFlight, and returns an error otherwise. This should be used to | ||
// ensure we only mark in-flight payments as succeeded or failed. | ||
func ensureInFlight(bucket kvdb.ReadBucket) error { | ||
paymentStatus, err := fetchPaymentStatus(bucket) | ||
if err != nil { | ||
return err | ||
} | ||
func ensureInFlight(payment *MPPayment) error { | ||
paymentStatus := payment.Status | ||
|
||
switch { | ||
|
||
// The payment was indeed InFlight, return. | ||
// The payment was indeed InFlight. | ||
case paymentStatus == StatusInFlight: | ||
return nil | ||
|
||
|
@@ -528,14 +622,7 @@ func (p *PaymentControl) FetchInFlightPayments() ([]*InFlightPayment, error) { | |
inFlight := &InFlightPayment{} | ||
|
||
// Get the CreationInfo. | ||
b := bucket.Get(paymentCreationInfoKey) | ||
if b == nil { | ||
return fmt.Errorf("unable to find creation " + | ||
"info for inflight payment") | ||
} | ||
|
||
r := bytes.NewReader(b) | ||
inFlight.Info, err = deserializePaymentCreationInfo(r) | ||
inFlight.Info, err = fetchCreationInfo(bucket) | ||
if err != nil { | ||
return err | ||
} | ||
|
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If we do validation here, we should probably also check that the mpp payment addresses and total amounts match with the in-flight routes? Or is that too strict for sendtoroute?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added validation + tests!