-
Notifications
You must be signed in to change notification settings - Fork 23
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
Let the coordinator open single-funded DLC channels #2541
Conversation
d3cf819
to
b5cd2ae
Compare
Nicely done 💪 I think there is not much left to get this ready 🙂 |
b5cd2ae
to
fd49abe
Compare
fd49abe
to
c9addaa
Compare
/// We can extend this enum with a `ForTradeCost` variant to denote that the trader has to pay for | ||
/// everything except for transaction fees. | ||
enum TraderRequiredLiquidity { | ||
/// Pay for margin, collateral reserve, order-matching fees and transaction fees. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it may be worthwhile to split this enum up into the individual components like you have mentioned in your comment.
margin, collateral reserve, order-matching fees and transaction fees.
that way we can compose more easily who will pay what.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
obviously not necessary now.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, I had an earlier version with other variants, but they were unused and I did not want to add any more dead code.
mobile/native/src/dlc/node.rs
Outdated
@@ -540,7 +540,7 @@ impl Node { | |||
match self | |||
.inner | |||
.dlc_manager | |||
.accept_channel(&channel_id) | |||
.accept_channel(&channel_id, dlc::FeeConfig::AllOffer) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🤔 Don't we need to separate between the two flows here? or don't we support the EvenSplit
flow any more from the app?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think you will have to look into the offered channel and use the corresponding FeeConfig
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice catch! This must be why the tests are failing. I forgot to update this after I added the FeeConfig
to the message.
3be0980
to
9877932
Compare
With the `TradeAction::OpenSingleFundedChannel`, the coordinator can choose to open a DLC channel with a trader, without using any trader UTXOs. This is useful if the coordinator wants to onboard trading through alternate means, such as Lightning. `rust-dlc` now allows to configure the TX fee split between the offer party and the accept party. The feature has been tested locally, but is currently disabled as we need a different flow to trigger the creation of a single-funded DLC channel. Also, the DLC channel is now 0-conf: the trader can keep trading as soon as the fund transaction is found in mempool.
9877932
to
0bdea10
Compare
With the
TradeAction::OpenSingleFundedDlcChannel
, the coordinator can choose to open a DLC channel with a trader, without using any trader UTXOs. This is useful if the coordinator wants to onboard trading through alternate means, such as Lightning.rust-dlc
now allows to configure the TX fee split between the offer party and the accept party.The feature has been tested locally, but is currently disabled as we need a different flow to trigger the creation of a single-funded DLC channel.
Also, the DLC channel is now 0-conf: the trader can keep trading as soon as the fund transaction is found in mempool.
one-sided-channel-v1.mp4
In the video you can see that we still have to fund the on-chain wallet, because the app still thinks that on-chain funds are needed to open a channel. But once the channel is created, you can see that the on-chain funds were not used!
See get10101/rust-dlc#18 for the matching PR in
rust-dlc
.