-
Notifications
You must be signed in to change notification settings - Fork 130
Add tests and improve documentation for zero reserve channels #873
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
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -142,9 +142,9 @@ pub struct LSPS2ServiceConfig { | |
| /// | ||
| /// [`bLIP-52`]: https://github.com/lightning/blips/blob/master/blip-0052.md#trust-models | ||
| pub client_trusts_lsp: bool, | ||
| /// When set, clients that we open channels to will be allowed to spend their entire channel | ||
| /// balance. This allows clients to try to steal your funds with no financial penalty, so | ||
| /// this should only be set if you trust your clients. | ||
| /// When set, we will allow clients to spend their entire channel balance in the channels | ||
| /// we open to them. This allows clients to try to steal your funds in those channels with | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Maybe just say 'your channel balance' rather than funds, which should allow to drop the 'in those channels' part? (also note it should be 'which', not 'with' in the current version, but probably better to drop that altogether). |
||
| /// no financial penalty, so this should only be set if you trust your clients. | ||
| pub allow_client_0reserve: bool, | ||
| } | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -790,7 +790,7 @@ pub async fn splice_in_with_all( | |
|
|
||
| pub(crate) async fn do_channel_full_cycle<E: ElectrumApi>( | ||
| node_a: TestNode, node_b: TestNode, bitcoind: &BitcoindClient, electrsd: &E, allow_0conf: bool, | ||
| expect_anchor_channel: bool, force_close: bool, | ||
| allow_0reserve: bool, expect_anchor_channel: bool, force_close: bool, | ||
| ) { | ||
| let addr_a = node_a.onchain_payment().new_address().unwrap(); | ||
| let addr_b = node_b.onchain_payment().new_address().unwrap(); | ||
|
|
@@ -846,15 +846,27 @@ pub(crate) async fn do_channel_full_cycle<E: ElectrumApi>( | |
| println!("\nA -- open_channel -> B"); | ||
| let funding_amount_sat = 2_080_000; | ||
| let push_msat = (funding_amount_sat / 2) * 1000; // balance the channel | ||
| node_a | ||
| .open_announced_channel( | ||
| node_b.node_id(), | ||
| node_b.listening_addresses().unwrap().first().unwrap().clone(), | ||
| funding_amount_sat, | ||
| Some(push_msat), | ||
| None, | ||
| ) | ||
| .unwrap(); | ||
| if allow_0reserve { | ||
| node_a | ||
| .open_0reserve_channel( | ||
tnull marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| node_b.node_id(), | ||
| node_b.listening_addresses().unwrap().first().unwrap().clone(), | ||
| funding_amount_sat, | ||
| Some(push_msat), | ||
| None, | ||
| ) | ||
| .unwrap(); | ||
| } else { | ||
| node_a | ||
| .open_announced_channel( | ||
| node_b.node_id(), | ||
| node_b.listening_addresses().unwrap().first().unwrap().clone(), | ||
| funding_amount_sat, | ||
| Some(push_msat), | ||
| None, | ||
| ) | ||
| .unwrap(); | ||
| } | ||
|
|
||
| assert_eq!(node_a.list_peers().first().unwrap().node_id, node_b.node_id()); | ||
| assert!(node_a.list_peers().first().unwrap().is_persisted); | ||
|
|
@@ -913,6 +925,22 @@ pub(crate) async fn do_channel_full_cycle<E: ElectrumApi>( | |
| node_b_anchor_reserve_sat | ||
| ); | ||
|
|
||
| // Note that only node B has 0-reserve, we don't yet have an API to allow the opener of the | ||
| // channel to have 0-reserve. | ||
| if allow_0reserve { | ||
| assert_eq!(node_b.list_channels()[0].unspendable_punishment_reserve, Some(0)); | ||
| assert_eq!(node_b.list_channels()[0].outbound_capacity_msat, push_msat); | ||
| assert_eq!(node_b.list_channels()[0].next_outbound_htlc_limit_msat, push_msat); | ||
|
|
||
| assert_eq!(node_b.list_balances().total_lightning_balance_sats * 1000, push_msat); | ||
| let LightningBalance::ClaimableOnChannelClose { amount_satoshis, .. } = | ||
| node_b.list_balances().lightning_balances[0] | ||
| else { | ||
| panic!("Unexpected `LightningBalance` variant"); | ||
| }; | ||
| assert_eq!(amount_satoshis * 1000, push_msat); | ||
| } | ||
|
|
||
| let user_channel_id_a = expect_channel_ready_event!(node_a, node_b.node_id()); | ||
| let user_channel_id_b = expect_channel_ready_event!(node_b, node_a.node_id()); | ||
|
|
||
|
|
@@ -1267,6 +1295,37 @@ pub(crate) async fn do_channel_full_cycle<E: ElectrumApi>( | |
| 2 | ||
| ); | ||
|
|
||
| if allow_0reserve { | ||
| let node_a_outbound_capacity_msat = node_a.list_channels()[0].outbound_capacity_msat; | ||
| let node_a_reserve_msat = | ||
| node_a.list_channels()[0].unspendable_punishment_reserve.unwrap() * 1000; | ||
| // TODO: Update this for zero-fee commitment channels | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Mind opening an issue for this TODO, so we don't forget to actually address it. Could also be a bit more verbose, i.e., what are we waiting for exactly/what do we intend to update? |
||
| let node_a_anchors_msat = if expect_anchor_channel { 2 * 330 * 1000 } else { 0 }; | ||
| let funding_amount_msat = node_a.list_channels()[0].channel_value_sats * 1000; | ||
| // Node B does not have any reserve, so we only subtract a few items on node A's | ||
| // side to arrive at node B's capacity | ||
| let node_b_capacity_msat = funding_amount_msat | ||
| - node_a_outbound_capacity_msat | ||
| - node_a_reserve_msat | ||
| - node_a_anchors_msat; | ||
| let got_capacity_msat = node_b.list_channels()[0].outbound_capacity_msat; | ||
| assert_eq!(got_capacity_msat, node_b_capacity_msat); | ||
| assert_ne!(got_capacity_msat, 0); | ||
| // Sanity check to make sure this is a non-trivial amount | ||
| assert!(got_capacity_msat > 15_000_000); | ||
|
|
||
| // This is a private channel, so node B can send 100% of the value over | ||
| assert_eq!(node_b.list_channels()[0].next_outbound_htlc_limit_msat, node_b_capacity_msat); | ||
|
|
||
| node_b.spontaneous_payment().send(node_b_capacity_msat, node_a.node_id(), None).unwrap(); | ||
| expect_event!(node_b, PaymentSuccessful); | ||
| expect_event!(node_a, PaymentReceived); | ||
|
|
||
| node_a.spontaneous_payment().send(node_b_capacity_msat, node_b.node_id(), None).unwrap(); | ||
| expect_event!(node_a, PaymentSuccessful); | ||
| expect_event!(node_b, PaymentReceived); | ||
| } | ||
|
|
||
| println!("\nB close_channel (force: {})", force_close); | ||
| tokio::time::sleep(Duration::from_secs(1)).await; | ||
| if force_close { | ||
|
|
||
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.
nit: Please avoid the 'this channel (..) that channel' duplication (here and below).