lnd icon indicating copy to clipboard operation
lnd copied to clipboard

Published 20 hours ago • verified publisher icon lightningnetwork

channeldb+routing: refactor payment lifecycle

Open yyforyongyu opened this issue 2 years ago • 1 comments

This PR refactors the payment lifecycle to provide cleaner layers of abstractions so it’s easier to understand and reason about them. Changes included,

  • Expand PaymentStatus to reflect every possible state of a given payment. Our current PaymentStatus doesn’t reflect the “real” complexity of the payment’s state which makes operations on the payment difficult as different states need different treatment.
  • Remove duplicate logic re payment state in the payment lifecycle and treat the PaymentStatus as the single source of truth.
  • Remove shardHandler as its responsibility is too vague.
  • Decomposed launchShard so requesting route, registering, and sending HTLC are separated since they deal with different subsystems like PaymentSession, ControlTower and htlcswitch. Previously the errors returned from different systems were mixed which made it hard to distinguish a terminal error vs a temp error. By separating them we can now easily decide when to fail the lifecycle, fail or retry the payment.
  • Fail an HTLC attempt before failing a payment, if applicable. This makes sure we won’t attempt another HTLC while the payment is being marked as failed.

Soon we’ll be deleting data from payments, such as #6438 and coming onion blob removal, and this PR should put those changes on safe ground.

TODOs

  • [ ] refactor SendToRoute
  • [ ] fix unit tests in routing
  • [ ] add mo unit tests for the new methods
  • [ ] update itest to reflect the new statuses

yyforyongyu avatar Jun 28 '22 13:06 yyforyongyu

@bhandras: review reminder @roasbeef: review reminder @yyforyongyu, remember to re-request review from reviewers when ready

lightninglabs-deploy avatar Sep 19 '22 10:09 lightninglabs-deploy

This PR refactors the payment lifecycle to provide cleaner layers of abstractions so it’s easier to understand and reason about them.

For context, is this a pure refactor PR or are there actual functional changes for users? If that is the case, it might be useful to describe the visible problems that are fixed in the PR description to emphasise the relevance.

joostjager avatar Feb 07 '23 19:02 joostjager

For context, is this a pure refactor PR or are there actual functional changes for users? If that is the case, it might be useful to describe the visible problems that are fixed in the PR description to emphasise the relevance.

Def. Will update it once it's out of draft mode.

yyforyongyu avatar Feb 07 '23 19:02 yyforyongyu

Closed and replaced it with a new one as the comments here are very outdated.

yyforyongyu avatar Mar 10 '23 09:03 yyforyongyu