Port upstream changes until v0.8.0-beta #42
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
Alice and Dave don't need to be connected in order to receive the node announcement as we assume that she can receive it from Bob because they are connected at the beginning of every test.
The test assumed that transactions would be broadcast and confirmed at incorrect heights. Due to timing issues, it was possible for the test to still succeed, resulting in a flake. The test assumes that Bob will sweep a pending outgoing HTLC and commit output back to their wallet. This commit ensures that these operations are done when expected, i.e.: 1. Bob force closes the channel due to the HTLC timing out. 2. Once the channel is confirmed, Bob broadcasts their HTLC timeout transaction. 3. Bob broadcasts their commit output sweep transaction once its CSV expires. 4. Bob broadcasts their second layer sweep transaction for the timed out HTLC once its CSV expires.
Previously a check was made for accepted and settled invoices against the paid amount. This opens up a probe vector where an attacker can pay to an invoice with an amt that is higher than the invoice amount and find out if the invoice is already paid or not.
Add logic to specifically exercise the replay behavior of invoice registry for hodl invoices.
Currently the invoice registry cannot tell apart the htlcs that pay to an invoice. Because htlcs may also be replayed on startup, it isn't possible to determine the total amount paid to an invoice. This commit is a first step towards fixing that. It reports the circuit keys of htlcs to the invoice registry, which forms the basis for accurate invoice accounting.
This commit adds a set of htlcs to the Invoice struct and serializes/deserializes this set to/from disk. It is a preparation for accurate invoice accounting across restarts of lnd. A migration is added for the invoice htlcs. In addition to these changes, separate final cltv delta and expiry invoice fields are created and populated. Previously it was required to decode this from the stored payment request. The reason to create a combined commit is to prevent multiple migrations.
Now that the Invoice struct contains the decoded final cltv delta value, the decoding of payment requests can be removed from the invoice registry.
As the logic around invoice mutations gets more complex, the friction caused by having this logic split between invoice registry and channeldb becomes more apparent. This commit brings a clearer separation of concerns by centralizing the accept/settle logic in the invoice registry. The original AcceptOrSettle method is renamed to UpdateInvoice because the update to perform is controlled by the callback.
This commit is a continuation of the centralization of invoice state transition logic in the invoice registry.
This commit is a continuation of the centralization of invoice state transition logic in the invoice registry.
This commit refactors the invoice registry accept/settle logic so that it doesn't rely anymore on a set of error values to indirectly communicate from the update callback to the main function what action is required on the htlc.
Needed for time control in unit tests.
Previously the invoice registry wasn't aware of replayed htlcs. This was dealt with by keeping the invoice accept/settle logic idempotent, so that a replay wouldn't have an effect. This mechanism has two limitations: 1. No accurate tracking of the total amount paid to an invoice. The total amount couldn't just be increased with every htlc received, because it could be a replay which would lead to counting the htlc amount multiple times. Therefore the total amount was set to the amount of the first htlc that was received, even though there may have been multiple htlcs paying to the invoice. 2. Impossible to check htlc expiry consistently for hodl invoices. When an htlc is new, its expiry needs to be checked against the invoice cltv delta. But for a replay, that check must be skipped. The htlc was accepted in time, the invoice was moved to the accepted state and a replay some blocks later shouldn't lead to that htlc being cancelled. Because the invoice registry couldn't recognize replays, it stopped checking htlc expiry heights when the invoice reached the accepted state. This prevents hold htlcs from being cancelled after a restart. But unfortunately this also caused additional htlcs to be accepted on an already accepted invoice without their expiry being checked. In this commit, the invoice registry starts to persistently track htlcs so that replays can be recognized. For replays, an htlc resolution action is returned early. This fixes both limitations mentioned above.
BOLT04 says to omit this field for final hops, but must be present on all other hops.
Currently the underlying array backing the hop's TLVRecords is modified when combining custom records with the primitive forwarding info. This commit uses a fresh slice to prevent modifications from mutating the hop itself.
Without this, it was possible for a combination of our balance and max fee allocation to result in a fee rate below the fee floor causing the remote party to reject the update and close the channel.
In this commit, we move to make a full deep copy of the commitment transaction in `getSignedCommitTx` to ensure that we don't mutate the commitment on disk, possibly resulting in a "hot commitment".
In this commit, we create a new `chanvalidate` package which it to house all logic required for 1st and 3rd party channel verification. 1st party verification occurs when we find a channel in the chain that is allegedly ours, while 3rd party verification will occur when a peer sends us a channel proof of a new channel. In the scope of the recent CVE, we actually fully verified 3rd party channels, but failed to also include those checks in our 1st party verification code. In order to unify this logic, and prevent future issues, in this PR we move to concentrate all validation logic into a single function. Both 1st and 3rd party validation will then use this function. Additionally, having all the logic in a single place makes it easier to audit, and also write tests against.
… package In the process of moving to use the new package, we no longer need to fetch the outpoint directly, and instead only need to pass the funding transaction into the new verification logic.
…idation In this commit, we use the recently added `chanvalidate` package to verify channels once they have been confirmed in the funding manager. We expose a new method on the `LightningWallet` struct: `ValidateChannels` which calls the new shared 1st party verification code. After the channel is fully confirmed in the funding manager, we'll now use this newly exposed method to handle all validation. As a result, we can remove the existing validation code in the funding manager, and rely on the new code in isolation.
In order to prevent future unforeseen issues, we are temporarily disabling the ability to send custom tlv records to the receiver of a payment. Currently the receiver does not process or expose these additional fields via rpc or internally, so they are being disabled until the end-to-end flow is finished and fully validated.
In this commit, we fix a bug that would prevent users that had unresolved contracts at the time of update from starting their nodes. Before we added the conf commit set, the information needed to supplement the resolvers was found in the chain action map. As a result, if the conf commit set is nil, then we also need to check this legacy data to ensure that we can supplement the resolvers to the best of our ability based on the available data. Fixes lightningnetwork#3549.
This adds support to specifying the sync freelist option to the wallet unlocker. This is a port of the upstream commit 194a9de. Since dcrwallet does not support specifying this parameter (but this will be used in future commits in the channeldb) only the walletunlocker service was modified.
Notably, solaris/amd64 and illumos/amd64 were added in go1.13. The others were have been supported previously but were not build targets. After go1.13, nacl/* will no longer be supported and plan9/* is still considered experiemental. As a result they are omitted.
These result in an error with the current release script as is. Those that want to run `lnd` on their iPhone/iPad can instead use the `gomobile` bindings.
This fixes the fee rates and fees used in a few unit tests of the lnwallet and htlcswitch packages to be in line with the realities of the Decred network.
We might hit a connection refused error in cases where the peer connects to us exactly as we try to connect to it. We retry the connection within a wait predicat, as it should be the case that the other peer establishes the connection, and the two peers actually connects.
Fixes lightningnetwork#3357. When start_time isn't specified, its default value is 0. This meant when users explicitly specified a start_time of 0, we would incorrectly set start_time to 24 hours in the past. Now, n0 means n0.
Here we set start_time to 24 hours prior if it's not provided on the CLI. The effect of this is when you don't provide a start_time: CLI: -24h RPC: Unix Epoch
This reverts commit f1942a4.
With the introduction of the max CLTV limit parameter, nodes are able to reject HTLCs that exceed it. This should also be applied to path finding, otherwise HTLCs crafted by the same node that exceed it never left the switch. This wasn't a big deal since the previous max CLTV limit was ~5000 blocks. Once it was lowered to 1008, the issue became more apparent. Therefore, all of our path finding attempts now have a restriction of said limit in in order to properly carry out HTLCs to the network.
The previous limit of 1008 proved to be low, given that almost 50% of the network still advertises CLTV deltas of 144 blocks, possibly resulting in routes with many hops failing.
It was missing a space "addingto waiting batch" -> "adding to waiting batch"
matheusd
force-pushed
the
upstream-updt
branch
from
91a58c2
to
da7ae86
Compare
This changes travis to use the suggested install method for golangci-lint. It also displays the version before linting so errors are easier to track.
This fixes a race condition in the mock chainWatcher.
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 upstream changes from our latest merge point done in PR #36 up to the recently tagged v0.8.0-beta lnd version.
A total of 379 commits and 90 PRs were merged.