Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

multi: bump to latest lnd/tapd, assert close chan data #1017

Merged
merged 3 commits into from
Apr 2, 2025

Conversation

guggero
Copy link
Member

@guggero guggero commented Mar 27, 2025

Depends on lightningnetwork/lnd#9504.
Depends on lightninglabs/taproot-assets#1441.

Serves as validation code for the changes in lightningnetwork/lnd#9504.

Also fixes #940.

Copy link
Contributor

@gijswijs gijswijs left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Only a few questions for my understanding.
The itests affected by this PR do currently fail.

return fmt.Errorf("expected asset ID %s, got %s",
assetIDStr, a.AssetInfo.AssetGenesis.AssetID)
assetID := mintedAsset.AssetGenesis.AssetId
if !haveFundingAsset(a, assetID) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For my understanding:
In the old situation there could only be one type of AssetID as the funding asset of a custom channel. But in the new situation we can have funding assets with different AssetID's and we check if one of them is the assetID we expect.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Correct. This is in preparation for when we can actually have multiple assets in a channel (#987). So there is a tiny bit of overlap between this PR and #987 (if this one gets in first, the other one will be rebased accordingly).

@guggero guggero force-pushed the closedchannel-data branch 2 times, most recently from c9ffb84 to 18fb369 Compare March 31, 2025 19:21
@guggero guggero force-pushed the closedchannel-data branch from 18fb369 to 7c8b3f4 Compare April 1, 2025 15:09
@guggero
Copy link
Member Author

guggero commented Apr 1, 2025

The test_custom_channels_htlc_force_close itest is still a bit flakey, but it passes some of the time... Will investigate at a later point, would really like to resolve some of the pending dependencies.
So ready for review, I'd say.

@gijswijs gijswijs self-requested a review April 1, 2025 19:30
Copy link
Contributor

@gijswijs gijswijs left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm 🎲

guggero added 2 commits April 1, 2025 21:51
This fixes an issue where the lnd sweeper sometimes doesn't sweep all
timeout HTLCs in a single sweep TX. If that happens, we just need to
wait for an additional one.
@guggero guggero force-pushed the closedchannel-data branch from 0491947 to 8c43bfe Compare April 2, 2025 08:54
@guggero
Copy link
Member Author

guggero commented Apr 2, 2025

Yesss, itests are stable now. cc @GeorgeTsagk

Copy link
Member

@GeorgeTsagk GeorgeTsagk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, tACK
no flakes 🦩

@guggero guggero merged commit 70143dc into master Apr 2, 2025
18 of 21 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[itest flake]: custom channels test_custom_channels_htlc_force_close flake
3 participants