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

feat: Add min margin to channel trade constraints #1906

Merged
merged 5 commits into from
Jan 26, 2024

Conversation

holzeis
Copy link
Contributor

@holzeis holzeis commented Jan 26, 2024

  • Sets the min quantity to 1 dollar
  • Sets the min margin to 1 sat or 250k sat if there is no dlc channel yet.
Simulator.Screen.Recording.-.iPhone.15.Pro.-.2024-01-26.at.15.49.13.mp4

@holzeis holzeis requested review from bonomat and luckysori January 26, 2024 14:55
@holzeis holzeis self-assigned this Jan 26, 2024
Copy link
Contributor

@bonomat bonomat left a comment

Choose a reason for hiding this comment

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

I like how simple this has gotten :)

@holzeis holzeis enabled auto-merge January 26, 2024 15:09
@holzeis holzeis force-pushed the chore/set-minimum-amount-to-1-when-dlc-channel-is-open branch from 8cf5ed1 to 99c0529 Compare January 26, 2024 16:37
@holzeis holzeis added this pull request to the merge queue Jan 26, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Jan 26, 2024
@holzeis holzeis added this pull request to the merge queue Jan 26, 2024
Merged via the queue into main with commit 07e7ca7 Jan 26, 2024
9 checks passed
@holzeis holzeis deleted the chore/set-minimum-amount-to-1-when-dlc-channel-is-open branch January 26, 2024 17:33
@@ -7,6 +7,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

## [Unreleased]

- Chore: Set minimum quantity to 1 if dlc channel is open
Copy link
Contributor

Choose a reason for hiding this comment

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

Won't this result in tiny DLCs that cannot be claimed on-chain?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Won't this result in tiny DLCs that cannot be claimed on-chain?

Aren't we considering the full amount on the CET transaction including any potential off chain amount.

So I think we only have to care about the initial margin to be big enough for a channel but afterwards there shouldn't be a limit.

Either party may loose so much that their payout can't be claimed anymore. But I don't think that's a problem.

Copy link
Contributor

Choose a reason for hiding this comment

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

I think you are right!

I do think eventually we should track whether either side is getting too close to the dust limit, to at least inform about this.

Copy link
Contributor

Choose a reason for hiding this comment

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

I do think eventually we should track whether either side is getting too close to the dust limit, to at least inform about this.

afaik rust-dlc is taking care of this.

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.

3 participants