Skip to content

Conversation

@guggero
Copy link
Contributor

@guggero guggero commented Dec 13, 2024

Depends on lightninglabs/taproot-assets#1252.

Adds test cases for the new minimum amount enforcement introduced in above PR.

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! 🥼

commits need reorder to compile individually

@guggero
Copy link
Contributor Author

guggero commented Dec 13, 2024

commits need reorder to compile individually

Yeah, I'll squash them into the existing commits of the update-to-lnd-18-4 branch. So keeping them separately (go.mod changes and itest changes) will make that easier.

cmd/litcli/ln.go Outdated
"asset ID present",
}

allowUnEconomicalFlag = cli.BoolFlag{
Copy link
Contributor

Choose a reason for hiding this comment

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

How about allowOverpay or allowAssetOverpay or allowTotalValueOverpay

I think allowTotalValueOverpay is most clear.

Copy link
Contributor

Choose a reason for hiding this comment

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

Couldn't see this change in the latest commits. Are they incoming?

@dstadulis dstadulis added this to the tapd v0.5 milestone Dec 13, 2024
@guggero guggero requested a review from ffranr December 16, 2024 12:50
@ffranr
Copy link
Contributor

ffranr commented Dec 17, 2024

@guggero does this need a rebase before I review?

Copy link
Contributor

@ffranr ffranr left a comment

Choose a reason for hiding this comment

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

There's a "check commit" lint error but otherwise ok.

@guggero guggero merged commit 47f213d into update-to-lnd-18-4 Dec 17, 2024
11 of 13 checks passed
@guggero guggero deleted the enforce-min-amount branch December 17, 2024 18:51
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.

5 participants