Bring Upstream v0.13 changes #193
Merged
Conversation
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
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.
Also fixes error-handling in the Verify test when expectedErr == ""
This ensures the cleanupForceClose assertion attempts to cleanup any pending HTLCs in the target channel when cleaning up a force-closed channel. This ensures any HTLCs not yet asserted to have been swept are given an attempt to do so. This will be necessary to ensure future tests won't leave the channel in a pending state.
This commit updates our multi-hop force close test to use a hodl invoice so that we can reproduce some bugs which will require the preimage for the invoice that is timed out on chain.
Reproduce the case where we allow settling of invoices that have htlcs that have actually timed out on chain. This bug can rarely occur if a hodl invoice goes to chain and is manually settled after it has timed out. Funds are SAFU, but this could be a headache because the invoice says it's settled when no funds were claimed.
This commit adds a test for a hold invoice which is accepted off-chain, and held by the recipient until it expired and the payer force-closes the channel. With this test we demonstrate two bugs in our handling of hold invoice state in the invoice registry when we expire on chain: - Htlcs not updated: even when we've timed out, we don't update the htlc state accordingly. - Invoice can be settled: the invoice can be settled even though it's expired on chain.
We're going to add block based expiry, so we clarify now.
In preparation for having more than one expiry type, we alias the queue.PrioirtyQueueItem interface for readability.
Useful for tests which need to mock cancelInvoiceImpl
Our aggregate htlc test depends on our previous behavior where recipients would allow channels with pending hold invoice htlcs to force close. Now that we have an expiry watcher to prevent these force closes, we can't rely on this for tests because the recipient will cancel the htlcs back before they expire.
To distinguish the attempt's unique ID from the overall payment identifier, we name it attemptID everywhere, and note that the paymentHash argument won't be the actual payment hash for AMP payments.
We'll use this to keep track of the outstanding shards and which preimages we are using for each. For now this is a simple map from attempt ID to hash, but later we'll hide the AMP child derivation behind this interface.
We'll let the payment's lifecycle register each shard it's sending with the ShardTracker, canceling failed shards. This will be the foundation for correct AMP derivation for each shard we'll send.
For AMP payments the hash used for each HTLC will differ, and we will need to retrive it after a restart. We therefore persist it with each attempt.
We'll use this AMP-specific ShardTracker for AMP payments. It will be used to derive hashes for each HTLC attempt using the underlying AMP derivation scheme.
We'll use the AMP-specific ShardTracker for payments having non-nil AMPOptions.
For now this is how you indicate you want to perform an AMP payment to the destination.
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.
…hance error In this commit, we modify the existing `TestSendToRouteStructuredError` test to return an error that doesn't trigger the second chance logic. Otherwise, we'll get a nil failure result from the mission control interpretation, meaning we won't exercise the full code path. Instead, we use a terminal error to ensure that the expected code path is hit. As is, this test will fail as a recent refactoring causes us to return a `channeldb.FailureReason` error, since the newly added `handleSendError` code path in the `SendToRoute` method will return the raw error, rather than the `shardError`, which is of the expected type.
In this commit, we fix a regression introduced by a recent bug fix in this area. Before this change, we'd inspect the error returned by `processSendError`, and then fail the payment from the PoV of mission control using the returned error. A recent refactoring removed `processSendError` and combined the logic with `tryApplyChannelUpdate` in order to introduce a new `handleSendError` method that consolidates the logic within the `shardHandler`. Along the way, the behavior of the prior check was replicated in the form of a new internal `failPayment` closure. However, the new function closure ends up returning a `channeldb.FailureReason` instance, which is actually an `error`. In the wild, when `SendToRoute` fails due to an error at the destination, then this new logic caused the `handleSendErorr` method to fail with an error, returning an unstructured error back to the caller, instead of the usual payment failure details. We fix this by no longer checking the `handleSendErorr` for an error as normal. The `handleSendErorr` function as is will always return an error of type `*channeldb.FailureReason`, therefore we don't need to treat it as a normal error. Instead, we check for the type of error returned, and update the control tower state accordingly. With this commit, the test added in the prior commit now passes. Fixes lightningnetwork#5477.
In this commit, we fix a bug that would cause attempts to re-use an AMP invoice to fail. Without this commit, we would only attempt to parse the payment addr if no invoice was specified, so a user manually specifying the pubkey of the detonation. The fix is straight forward: always parse the `pay_addr` field as the user may be attempting to re-use an AMP invoice w/o open coding each of the sections.
In this commit, we add a new option (`--amp-reuse`) that allows for easy AMP invoice re-use when using either the `sendpayment` command.
This allows building the package in CI.
This commit updates call-sites to use the proper dust limits for various script types. This also updates the default dust limit used in the funding flow to be 354 satoshis instead of 573 satoshis.
This is necessary and is implied by BOLT#02. Both ChannelReserve parameters should be above both DustLimit parameters. Otherwise, it is possible for one side to have nothing at stake.
matheusd
force-pushed
the
bring-upstream-v013
branch
from
4d7ce91 to
bdbf8dc
Compare
It over-estimates the local or remote commitment's dust sum by counting all updates in both updateLogs that are dust using the trimmed-to-dust mechanism if applicable. The over-estimation is done because ensuring an accurate counting is a trade-off between code simplicity and accuracy.
This commit extends the Mailbox interface with the SetDustClosure, SetFeeRate, and DustPackets methods. This enables the mailbox to report the dust exposure to the Switch when the Switch decides whether to forward a dust packet. The dust is counted from the time an Add is introduced via AddPacket until it is removed via AckPacket. This can lead to some packets being counted twice before they are signed for, but this is a trade-off between accuracy and simplicity.
This allows the Switch to determine the dust exposure of a certain channel and allows the link to set the feerate of the mailbox given a fee update.
This commit makes SendHTLC (we are the source) evaluate the dust threshold of the outgoing channel against the default threshold of 500K satoshis. If the threshold is exceeded by adding this HTLC, we fail backwards. It also makes handlePacketForward (we are forwarding) evaluate the dust threshold of the incoming channel and the outgoing channel and fails backwards if either channel's dust sum exceeds the default threshold.
Sometimes the node shuts down before the gRPC response is sent, so this avoids a flaky failure in that situation.
This logs the time the Fatalf() call was made, to make it easier to locate log lines around the failure time.
mockChainBackend simulates a chain backend and tracks the number of calls via a private property called chainCallCount. This commit uses a write mutex instead of read mutex in the call to GetBlock, adds a getter method to access chainCallCount with a read mutex and uses a write mutex in resetChainCallCount.
This ensures the results of a previous test on the global Alice and Bob won't interfere with the next test.
This changes some logging to attempt to make it easier to track down some itest flakes.
This is a port of the upstream commit b78164. It ensures that clients see not just the channel itself in their list of channels, but also that the routing subsystem registers the topology change, such that the channels are suitable for routing. It also adds an additional check to the testUpdateChannelPolicyForPrivateChannel to attempt to prevent it from flaking.
This adds the list of the upstream PRs that were ported in the effort to bring in the changes up to the upstream version 0.13.4.
This should help debug some issues.
User doesn't have to do anything about this, so logging at debug level is fine.
matheusd
force-pushed
the
bring-upstream-v013
branch
from
bdbf8dc to
7e40ab0
Compare
|
This has been running in production on a few mainnet hubs for a while, therefore I'm merging it. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
This brings the changes from the upstream version of lnd up to version 0.13.4.