Update to upstream v0.14 branch #205
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
dajohi
approved these changes
Feb 19, 2024
We'll refactor the wallet creation and unlock process in a following commit and want to make it possible to not need a direct reference to the macaroon service in our main function. Since we store it in the interceptor chain anyway (if we're using macaroons in the first place), we might as well use the instance there directly.
As a preparation for separating the pure config related chain control setup from the wallet related chain control, we store more information in the chain control instance that is required for the wallet initialization.
To remove one more direct dependency to a variable in our main function, we pass in the required parameter to the autopilot only instead of the whole chain configuration.
As a preparation for adding the wallet unlock params to the chain control, we first need to move them out of the main package.
This reduces the default recovery window (i.e. gap limit) used when creating or restoring wallets via cli to the same used by default by other decred software. The new value is 20. Previously this value was not actually used because it was overridden in the node software to the default of 20, but the next commit will enable it to be correctly used. Thus we adjust this limit to the correct one.
This brings the upstream refactor made to the init procedures in the upstream PR 5708. This is done as a single commit instead of multiple commits in the original because of particular dependencies on dcrlnd init. In particular, the wallet is needed in certain sync modes at an earlier stage, therefore the init procedure is different between lnd and dcrlnd.
Use the addrMap for connReq creation to prevent creating duplicate connection requests if persistentPeerAddrs contains duplicate addresses.
This commit renames dial to be dialProxy to make the connections clearer. It also cleans the column width and provides more verbose error messages.
This commit adds a new method to call DEL_ONION when lnd is shutting down. Tor controller will now be aware of the active serviceID and removes the service upon shutdown.
This commit adds a new response reader which replaces the old textproto.Reader.ReadResponse. The older reader cannot handle the case when the reply from Tor server contains a data reply line, which uses the symbol "+" to signal such a case.
A new method, Reconnect, is added to tor controller which can be used to reset the current connection. This will be later used in healthcheck to help us reset the connection to Tor Daemon.
This commit adds a new health check, tor connection, to our liveness monitor. A monitor refactor is applied to the server creation such that the scope of health check creation is managed within one function.
Using the default, global ServeMux prevents the same process from calling `lntest.NewNetworkHarness` multiple times, because we get a panic when registering HTTP routes. Instead, we use the ServeMux beloning to the fee service struct.
Decred's chainhash.Hash method does not do double hashing by default (as opposed to Bitcoin's). This means it's an error to attempt to double hash a message before signing it in Decred. This commit improves reliability of the software by preventing every implementation of SignMessage to perform double hash, even if the flag is true. This should flag any call sites that are incorrectly specifying this flag in dcrlnd. If a particular message actually needs to be double hashed, then the first hash should be done prior to calling SignMessage. The flag on SignMessage is kept in order to ease future porting efforts from lnd.
To make it possible to supply our own implementation of a secret key ring, we extract that part from the chain control and split the whole chain control creation into two parts. Note(decred): We also split the chain control init into a separate implementation for remote wallets.
This is a partial port of the upstream commit 19db382, written manually because only a few changes were needed from it.
Note(decred): We only pick the rpc changes from the upstream commit, but otherwise disable creating watching-only wallets in dcrlnd.
In this commit, we add a new field to the WalletBalance call that permits users to account for the set of outputs that may be locked due to a pending transaction. Without this field any time users locked outputs for things like PSBT signing, then they disappear from the WalletBalance call, which may cause a panic.
…nflicts Before this commit, we we were trying to sweep an anchor output, and that output was spent by someone else (not the sweeper), then we would report this back to the original resolver (allowing it to be cleaned up), and also remove the set of inputs spent by that transaction from the set we need to sweep. However, it's possible that if a user is spending unconfirmed outputs, then the wallet is holding onto an invalid transaction, as the outputs that were used as inputs have been double spent elsewhere. In this commit, we fix this issue by recursively removing all descendant transactions of our past sweeps that have an intersecting input set as the spending transaction. In cases where a user spent an unconfirmed output to funding a channel, and that output was a descendant of the now swept anchor output, the funds will now properly be marked as available. Fixes lightningnetwork#6241
In this commit, we add a new integration tests to exercise the fix introduced in the prior commit. In this test, we reconstruct a scenario for a 3rd party to sweep an anchor spend after force closing, causing a prior spend we had to be invalidated. Without the prior commit, this test fails as the original anchor sweep is still found in the wallet.
It turns out that when a REST call to an endpoint (in this specific example /v1/payments, which for GET returns all payments but for DELETE removes all payments) is made with POST instead of the correct registered method, the grpc-gateway tried to find a fallback method. That resulted in randomly choosing between any of the calls with the same URI pattern. This is of course catasrophic if the user attempts to query the list of payments (but using POST instead of GET by accident) and then ending up calling the DELETE endpoint instead.
To help debug remote signing issues, it's helpful to get the raw PSBT that failed to be parsed. This is necessary since serializing an invalid PSBT is allowed and the checks only fail when trying to de-serialize such an invalid packet.
This changes the HDKeyRing implementation to test private keys for every family when deriving a key known only by its pubkey. This allows deriving keys when the keyfamily is not known, which will be required to run a test on the next commit.
We need to be able to query the watch-only wallet about a public key when trying to sign with a key that we don't know the family or index of. The easiest way to do that is to leverage the wallet's address index to query the derivation path for a public key. To give the RPC wallet access to that functionality, we need to expose the method on the WalletController interface.
[skip-ci] payments-expiration-grace-period needs time unit h/m/s or else the lnd does not start. ; payments-expiration-grace-period=30s Corrected minor typo.
In this commit, we start to ignore the option to allow the caller to use the legacy onion payload. The new payload is much more flexible and efficient, so there's really no reason to still use it, other than for backwards compatibility tests. Our existing tests that exercise the legacy feature uses a build tag, which forces nodes to not advertise the new payload format, which then forces path finding to include the legacy payload, so we can be confident that route is still being tested. The existence of this option (which actually makes the TLV payload opt-in for `SendToRoute` users) makes it harder to remove it from the protocol all together. With this PR, we take a step forward to allowing such a change which is being tracked on the spec level at: lightning/bolts#962. In a future release, we'll move to remove the field all together. Ignoring the field today doesn't seem to have any clear downsides, as most payments always include the MPP payload (due to payment secrets), so this shouldn't impact users in a significant way.
In case of a multi shard payment with more than one in-flight shards, one shard quitting with a terminal failure will stop the payment lifecycle and close the `shardHandler`'s `quit` channel. In the `collectResult` function we're waiting for the `Switch` to asynchronously return a result for each shard. This may have been interrupted by the aformentioned `quit` channel's closing skipping attempt failure (or success) notification towards the control tower and therefore skipping proper settle/fail info fill in the channel db. Since payments have a composite state of a global failure reason and settle/fail info for all attempts, any attempt with an unfilled settle/fail info keeps a payment in-flight even if the payment itself isn't in-flight anymore.
The breach arbiter waits for spend events of inputs of a recently created justice transaction and, after processing them, creates a new justice tx for any missing inputs. When the tx notifier subsystem is slow in notifying about the spends, the function that watches for the spend events may have only partially processed the actually spent inputs, therefore erroneously triggering the creation of another justice tx. This can be manifested on SPV wallets by the presence of a new justice tx that double spends some of the breach inputs, and as a flake during the revoked_uncooperative_close_retribution_remote_hodl itest (in the SPV backend). To fix this issue, an arbitrary delay of 10 milliseconds is added to ensure all spend notifications have been received by the processing goroutines before a new justice transaction is attempted. In the aforementioned test, this ensures that all inputs have been swept already and that the final balance in the wallet is correct with no extraneous unconfirmed transactions.
This happened because the tx to sweep the HTLC was sent on the block prior to the limit expiring. When fast mining, the node was sometimes fast enough process the blocks and sweep only after when the blocks finished generation, but on CI this would sometimes flake. This is fixed by ensuring only the correct number of blocks are mined prior to asserting the state of the sweep to close the contract.
TxNotifier's ConnectTip() and NotifyHeight() are not atomic. Therefore, the ending height used for a historical search may still be notified for as a match for a tip confirmation when NotifyHeight() is called. When using the chainscan driver, the timings are such that registering for a conf notification just as a block with the target is coming in may race, and produce double write to the Updates chan, causing a deadlock. This is evidenced by the historical_conf_dispatch_with_script_dispatch test of the chainntfns interface test when running with the chainscan-based driver. This error is not triggered in upstream lnd because the mining, notification and historical search intervals are faster. This is fixed by this commit by double checking whether the full conf event has already been dispatched or if there are any pending updates to be read by the client of the conf notification before sending one that will deadlock.
This fixes a flake where a NO_ROUTE error is generated when attempting to pay for the invoice.
These are the PRs ported on the latest porting effort.
This follows the practice of bumping the minor version whenever a new upstream version is ported.
matheusd
force-pushed
the
upstream-v014
branch
from
a471e53 to
e958fc3
Compare
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 in a port of the upstream v0.14 line, with changes up to the v0.14.5 version.
577 commits representing changes from 229 upstream PRs have been merged.