Skip to content

Conversation

@gijswijs
Copy link
Contributor

@gijswijs gijswijs commented Oct 30, 2024

This commit adds an integration test for the MinRelayFee check. The test ensures that transactions with fees below the minimum relay fee are rejected.

This PR depends on lightninglabs/taproot-assets#1163 which in turn depends on lightninglabs/lndclient#200

It uses pseudoversions in go.mod for both taproot-assets and lndclient that should be updated before merging.

@guggero guggero force-pushed the update-to-lnd-18-4 branch from 9c10445 to a4d76ed Compare October 31, 2024 16:00
@guggero guggero force-pushed the update-to-lnd-18-4 branch from 6b38cf0 to 3a8f1f6 Compare October 31, 2024 19:35
@gijswijs gijswijs force-pushed the validate-fee branch 3 times, most recently from 2506456 to de86073 Compare November 4, 2024 12:59
@gijswijs gijswijs marked this pull request as ready for review November 4, 2024 13:57
@gijswijs
Copy link
Contributor Author

gijswijs commented Nov 4, 2024

I've removed the default value of 1 for sat_per_vbyte that's being passed to litd rpc endpoint fundchannel. litd handles this now perfectly on its own, and adding logic here only adds to the confusion.

@gijswijs gijswijs requested review from guggero and jharveyb November 4, 2024 13:59
@dstadulis dstadulis added this to the tapd v0.4.2 milestone Nov 4, 2024
Copy link

@jharveyb jharveyb left a comment

Choose a reason for hiding this comment

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

LGTM 👍🏽

One related comment about how we handle this feerate arg, but it can be addressed in a separate PR.

This commit removes the default value of 1 for `sat_per_vbyte` that's
being passed to `litd` rpc endpoint `fundchannel`. `litd` handles this
now perfectly on its own, and adding logic here only adds to the
confusion.
This commit adds an integration test for the MinRelayFee check. The test
ensures that transactions with fees below the minimum relay fee are
rejected.
},
)

errFeeRateTooLow := fmt.Sprintf("fee rate %s too low, "+
Copy link
Contributor

Choose a reason for hiding this comment

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

This actually shows a problem with the sat/vByte fee rate... Because 1 sat/vByte is equal to 250 sat/kw but the absolute minimum relay fee there can ever be is defined as 253 sat/kw, this would mean that you could only fund channels with at least 2 sat/vBytes. Which is kind of unfortunate.
But I think we should fix this in tapd and unrelated to this PR. Just letting you know that this might change again in the future.

@guggero guggero merged commit f308629 into update-to-lnd-18-4 Nov 5, 2024
13 checks passed
@guggero guggero deleted the validate-fee branch November 5, 2024 11:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: ✅ Done

Development

Successfully merging this pull request may close these issues.

5 participants