Update lnd compile-time dependency to v0.19 #1422
Open
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
Pull Request Test Coverage Report for Build 13822880107Details
💛 - Coveralls |
guggero
force-pushed
the
lnd-19
branch
5 times, most recently
from
be66843 to
12575eb
Compare
ellemouton
approved these changes
Mar 6, 2025
| @@ -569,10 +572,16 @@ func (a *AuxSweeper) createAndSignSweepVpackets( | |||
| return lfn.Ok(vPkts) | |||
| } | |||
|
|
|||
| return lfn.AndThen2( | |||
| return lfn.FlatMapResult( | |||
There was a problem hiding this comment.
nice 🙏 glad we didnt need to do AndThen(AndThen())
There was a problem hiding this comment.
We kind of had to... Just that AndThen is now called FlatMapResult 😅
GeorgeTsagk
approved these changes
Mar 6, 2025
There was a problem hiding this comment.
LGTM 🍰
seems like we need to do some bump in LiT too?
LiT itest failure log
@GeorgeTsagk - see the PR description |
guggero
force-pushed
the
lnd-19
branch
2 times, most recently
from
77fe4c2 to
7f57edc
Compare
|
Since this will break the |
|
opened this pr against this branch: #1427 |
guggero
force-pushed
the
lnd-19
branch
3 times, most recently
from
321a9e1 to
8d4b4d0
Compare
We no longer need our custom action for adding Go module caching, since that functionality is now included in version 5 of the setup-go action. We also bump the checkout action to the latest one.
To reduce the size of the diff in the next commit, we update all usages of the lnd fn package where they are independent from lnd (meaning we don't use the results to interact with lnd). Anywhere we interact with lnd, we need to use the same version as lnd, so we'll need to do it in the next commit where we bump the lnd dependency.
Currently (*AuxTrafficShaper).ProduceHtlcExtraData exits early if the htlcCustomRecords are empty. However, we need to cater for the case when htlc custom records exist from other contexts (such as endorsement signaling) and so we change this check to be more strict and to check that the records dont contain any asset records.
Because GetInfo now (lnd 0.19) waits for all subsystems to be synced to the latest block, we might get into a deadlock situation if we call GetInfo downstream of an aux component call (which might happen in case of a force closure): Observed deadlock case: force closure of custom channel -> lnd detects force closure during block beat -> calls into aux component to resolve -> resolution involves registering a transfer in the chain porter -> chain porter gets CurrentHeight from chain bridge -> chain bridge calls into lnd's GetInfo -> GetInfo is blocked because the block beat sync is still waiting for the resolution of the aux channel. By diverting the call to the chain backend directly, we resolve this circular dependency.
Until we've merged the litd version bump PR for lnd (which in term requires this PR to be merged), we resolve the circular dependency by just pointing to that branch to prove the changes in this PR work.
Because spew.Sdump will iterate through the whole struct, it will also access maps, such as the underlying sync.Map. Which can then lead to concurrent map access, because that access isn't protected by the mutex.
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 updates the compile-time dependency of
lndto the currentmasterbranch, which will be 0.19 RC1 very soon.The diff is quite large because of the following major changes in
lnd:lnd/fnwas updated tolnd/fn/v2which included some breaking API changesgithub.com/btcsuite/btclog/v2lnditest framework removed the standby nodesNOTE: The
LiTintegration test is expected to fail for this PR, since this PR is required forlitdto update. So we have a circular dependency here.