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

[custom channels]: test forward traffic shaper #912

Merged
merged 3 commits into from
Dec 10, 2024

Conversation

guggero
Copy link
Member

@guggero guggero commented Dec 4, 2024

Depends on lightningnetwork/lnd#9333.
Depends on lightninglabs/taproot-assets#1236.

Makes sure channels aren't force closed in certain edge cases.

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.

looks good, pending base PRs

itest/litd_custom_channels_test.go Show resolved Hide resolved
itest/litd_custom_channels_test.go Show resolved Hide resolved
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
pending base PR

@dstadulis
Copy link

Will need to highly prioritize for rc2

@guggero guggero force-pushed the forward-traffic-shaper branch from 03db2c7 to 59a6553 Compare December 10, 2024 08:30
itest/litd_custom_channels_test.go Outdated Show resolved Hide resolved
Comment on lines +3620 to +3623
t.Logf("Got quote for %v asset units at %3f msat/unit from peer %s "+
"with SCID %d", numUnits, msatPerUnit, erin.PubKeyStr,
quote.AcceptedQuote.Scid)
Copy link
Contributor

Choose a reason for hiding this comment

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

In general, I think it would be better if log messages were written with args at the end:

t.Logf("Got quote from peer (asset_units=%v, msat_to_asset_units=%3f, peer=%s, "+
		"scid=%d)", numUnits, msatPerUnit, erin.PubKeyStr,
		quote.AcceptedQuote.Scid)

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah, I generally agree. And with the new slog based logging library we'll be able to do that in a more generalized manner (at least in lnd).
For now I'm trying to keep this aligned with other such messages in other tests.

@guggero guggero force-pushed the forward-traffic-shaper branch from 59a6553 to a08e051 Compare December 10, 2024 17:34
@guggero guggero merged commit 127e016 into update-to-lnd-18-4 Dec 10, 2024
12 of 13 checks passed
@guggero guggero deleted the forward-traffic-shaper branch December 10, 2024 18:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: ✅ Done
Development

Successfully merging this pull request may close these issues.

4 participants