With this commit we move the traffic shaper definition from the routing
package to the HTLC switch package as a preparation for being able to
use it there as well.
At the same time we rename it to AuxTrafficShaper to be more in line
with the other auxiliary components.
In this commit we set up the payment loop context
according to user-provided parameters. The
`cancelable` parameter indicates whether the user
is able to interrupt the payment loop by cancelling
the server stream context. We'll additionally wrap
the context in a deadline if the user provided a
payment timeout.
We remove the timeout channel of the payment_lifecycle.go
and in favor of the deadline context.
This commit adds unit tests for `resumePayment`. In addition, the
`resumePayment` has been split into two parts so it's easier to be
tested, 1) sending the htlc, and 2) collecting results. As seen in the
new tests, this split largely reduces the complexity involved and makes
the unit test flow sequential.
This commit also makes full use of `mock.Mock` in the unit tests to
provide a more clear testing flow.
The old payment lifecycle is removed due to it's not "unit" -
maintaining these tests probably takes as much work as the actual
methods being tested, if not more so. Moreover, the usage of the old
mockers in current payment lifecycle test is removed as it re-implements
other interfaces and sometimes implements it uniquely just for the
tests. This is bad as, not only we need to work on the actual interface
implementations and test them , but also re-implement them again in the
test without testing them!
This commit adds a new struct, `stateStep`, to decide the workflow
inside `resumePayment`.
It also refactors `collectResultAsync` introducing a new channel
`resultCollected`. This channel is used to signal the payment
lifecycle that an HTLC attempt result is ready to be processed.
This commit refactors the `resumePayment` method by adding the methods
`checkTimeout` and `requestRoute` so it's easier to understand the flow
and reason about the error handling.
This commit removes the method `launchShard` and splits its original
functionality into two steps - first create the attempt, second send the
attempt. This enables us to have finer control over "which error is
returned from which system and how to handle it".
This commit turns `MPPayment` into an interface inside `routing`. Having
this interface gives us the benefit to write more granular unit tests
inside payment lifecycle. As seen from the modified unit tests, several
hacky ways of testing the `SendPayment` method is now replaced by a mock
over `MPPayment`.
This commit moves the struct `paymentState` used in `routing` into
`channeldb` and replaces it with `MPPaymentState`. In the following
commit we'd see the benefit, that we don't need to pass variables back
and forth between the two packages. More importantly, this state is put
closer to its origin, and is strictly updated whenever a payment is read
from disk. This approach is less error-prone comparing to the previous
one, which both the `payment` and `paymentState` need to be updated at
the same time to make sure the data stay consistant in a parallel
environment.
The test cases in `TestUpdatePaymentState` run in parallel. One of the
parameters is a pointer and the value to the struct it points to gets
modified during the test.
The race condition was introduced in 8d49dfb07e
To test the fix run from the main folder `go test ./routing/. -race`
before this fix and after.
This commit renames the method `GetPaymentResult` to be
`GetAttemptResult` to avoid potential confusion and to address the
one-to-many relationship between a payment and its attempts.
This allows the router to determine what is and isn't an alias from
lnd's definition of an alias. Any ChannelAnnouncement that has an
alias ShortChannelID field is not verified on-chain. To prevent a
DoS vector from existing, the gossiper ensures that only the local
lnd node can send its ChannelAnnouncements to the router with an
alias ShortChannelID.
This commit was previously split into the following parts to ease
review:
- 2d746f68: replace imports
- 4008f0fd: use ecdsa.Signature
- 849e33d1: remove btcec.S256()
- b8f6ebbd: use v2 library correctly
- fa80bca9: bump go modules
A followup commit for PR#5332. In this commit we add more docs, rename
function updatePaymentState to fetchePaymentState, and add back the
check for channeldb.ErrPaymentTerminal after we launch shard.
This commit refactors the resumePayment to extract some logics back to
paymentState so that the code is more testable. It also adds unit tests
for paymentState, and breaks the original MPPayment tests into independent tests
so that it's easier to maintain and debug. All the new tests are built
using mock so that the control flow is eaiser to setup and change.
This commit renames the mock structs by appending Old in their names. In
doing so the old tests stay unchanged and new mock structs can be added
in the following commit.
Since we want to support AMP payment using a different unique payment
identifier (AMP payments don't go to one specific hash), we change the
nomenclature to be Identifier instead of PaymentHash.
If we have processed a terminal state while we're pathfinding
for another shard, the payment loop should not error out on
ErrPaymentTerminal. Instead, it would wait for our shards to
complete then cleanly exit.
Move our more generic terminal check forward so that we only
need to handle a single class of expected errors. This change
is mirrored in our mock, and our reproducing tests are updated
to assert that this move catches both classes of errors we get.
Add an additional stuck-payment case, where our payment gets
a terminal error while it has other htlcs in-flight, and a
shard fails with ErrTerminalPayment. This payment also falls in
our class of expected errors, but is not currently handled. The
mock is updated accordingly, using the same ordering as in our
real RegisterAttempt implementation.
This commit adds a test which demonstrates that payments can
get stuck if we receive a payment failure while we're pathfinding
for another shard, then try to dispatch a shard after we've
recorded a permanent failure. It also updates our mock to
only consider payments with no in-flight htlcs as in-flight,
to more closely represent our actual RegisterAttempt.
This commit adds a step to our payment lifecycle test to add
control over when we find a path for our payment, This is
required for testing race conditions around pathfinding
completing and payment failures being reported.
Update our single shard success case to use a route which
splits the payment amount in half. This change still tests
the case where reveal of the preimage counts as a success,
even if we don't have the full amount. This change is made
to cut down on potential races in this test case. While we
are waiting for collectResultAsync to report a success, the
payment lifecycle will continue trying to dispatch shards.
In the case where we send 1/4 of the payment amount, we
send 1 or 2 more shards, depending on how long collectAsync
takes. Reducing this test to send 1/2 of the payment amount
means that we will always only try one more shard before
waiting for our shard.
This commit updates our mock to more closely follow the behavior of the
switch for mocked calls to GetPaymentResult. As it stands, our tests
send a test-created error from the switch when we want to mock shutdown.
In reality, the switch will close its result channel, so we update this
test to follow that behavior. This matters for the commit that follows,
because we start checking the error our payments return. If we have an
error from the switch, our tests will fail with an error that we do
not encounter in practice.
Now that we run each test individually, we don't need to buffer
our mock's channels anymore. This helps to tighten our test loop,
which currently can move on from a step before it's actually
been processed by the mock. This removal ensures that our payment
loop processes each of the test's steps before moving on to the
next once.
Update our payment lifecycle test to run each test case with
a fresh router. This prevents test cases from interacting with
each other. Names are also added for easy debugging.
As is, we don't check that our SendPayment call in
TestRouterPaymentStateMachine completes. This makes it easier
to create malformed tests that just run through steps but leave
the SendPayment call hanging. This commit adds a check that we
have completed our payment to help catch tests like this. We
also remove an unused quit channel.
