Skip to content

Commit a51dbb4

Browse files
philipr-zaphilipr-za
committed
Check affordability of new fee during update_fee call #204 
1 parent 3c44d6b commit a51dbb4

File tree

2 files changed

+91
-61
lines changed

2 files changed

+91
-61
lines changed

‎src/ln/channel.rs

Lines changed: 17 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -357,8 +357,9 @@ const UNCONF_THRESHOLD: u32 = 6;
357357
const BREAKDOWN_TIMEOUT: u16 = 6 * 24 * 7; //TODO?
358358
/// The amount of time we're willing to wait to claim money back to us
359359
const MAX_LOCAL_BREAKDOWN_TIMEOUT: u16 = 6 * 24 * 14;
360-
const COMMITMENT_TX_BASE_WEIGHT: u64 = 724;
361-
const COMMITMENT_TX_WEIGHT_PER_HTLC: u64 = 172;
360+
/// Exposing these two constants for use in test in ChannelMonitor
361+
pub const COMMITMENT_TX_BASE_WEIGHT: u64 = 724;
362+
pub const COMMITMENT_TX_WEIGHT_PER_HTLC: u64 = 172;
362363
const SPENDING_INPUT_FOR_A_OUTPUT_WEIGHT: u64 = 79; // prevout: 36, nSequence: 4, script len: 1, witness lengths: (3+1)/4, sig: 73/4, if-selector: 1, redeemScript: (6 ops + 2*33 pubkeys + 1*2 delay)/4
363364
const B_OUTPUT_PLUS_SPENDING_INPUT_WEIGHT: u64 = 104; // prevout: 40, nSequence: 4, script len: 1, witness lengths: 3/4, sig: 73/4, pubkey: 33/4, output: 31 (TODO: Wrong? Useless?)
364365
/// Maximmum `funding_satoshis` value, according to the BOLT #2 specification
@@ -1653,7 +1654,9 @@ impl Channel {
16531654

16541655
let local_keys = self.build_local_transaction_keys(self.cur_local_commitment_transaction_number)?;
16551656

1657+
let mut update_fee = false;
16561658
let feerate_per_kw = if !self.channel_outbound && self.pending_update_fee.is_some() {
1659+
update_fee = true;
16571660
self.pending_update_fee.unwrap()
16581661
} else {
16591662
self.feerate_per_kw
@@ -1664,6 +1667,16 @@ impl Channel {
16641667
let local_sighash = Message::from_slice(&bip143::SighashComponents::new(&local_commitment_tx.0).sighash_all(&local_commitment_tx.0.input[0], &funding_script, self.channel_value_satoshis)[..]).unwrap();
16651668
secp_call!(self.secp_ctx.verify(&local_sighash, &msg.signature, &self.their_funding_pubkey.unwrap()), "Invalid commitment tx signature from peer", self.channel_id());
16661669

1670+
//If channel fee was updated by funder confirm funder can afford the new fee rate when applied to the current local commitment transaction
1671+
if update_fee {
1672+
let num_htlcs = local_commitment_tx.1.len();
1673+
let total_fee: u64 = feerate_per_kw as u64 * (COMMITMENT_TX_BASE_WEIGHT + (num_htlcs as u64) * COMMITMENT_TX_WEIGHT_PER_HTLC) / 1000;
1674+
1675+
if self.channel_value_satoshis - self.value_to_self_msat / 1000 < total_fee + self.their_channel_reserve_satoshis {
1676+
return Err(HandleError { err: "Funding remote cannot afford proposed new fee", action: Some(ErrorAction::DisconnectPeer { msg: None }) });
1677+
}
1678+
}
1679+
16671680
if msg.htlc_signatures.len() != local_commitment_tx.1.len() {
16681681
return Err(HandleError{err: "Got wrong number of HTLC signatures from remote", action: None});
16691682
}
@@ -1677,7 +1690,7 @@ impl Channel {
16771690
let mut htlc_tx = self.build_htlc_transaction(&local_commitment_txid, htlc, true, &local_keys, feerate_per_kw);
16781691
let htlc_redeemscript = chan_utils::get_htlc_redeemscript(&htlc, &local_keys);
16791692
let htlc_sighash = Message::from_slice(&bip143::SighashComponents::new(&htlc_tx).sighash_all(&htlc_tx.input[0], &htlc_redeemscript, htlc.amount_msat / 1000)[..]).unwrap();
1680-
secp_call!(self.secp_ctx.verify(&htlc_sighash, &msg.htlc_signatures[idx], &local_keys.b_htlc_key), "Invalid HTLC tx siganture from peer", self.channel_id());
1693+
secp_call!(self.secp_ctx.verify(&htlc_sighash, &msg.htlc_signatures[idx], &local_keys.b_htlc_key), "Invalid HTLC tx signature from peer", self.channel_id());
16811694
let htlc_sig = if htlc.offered {
16821695
let htlc_sig = self.sign_htlc_transaction(&mut htlc_tx, &msg.htlc_signatures[idx], &None, htlc, &local_keys)?;
16831696
new_local_commitment_txn.push(htlc_tx);
@@ -1705,6 +1718,7 @@ impl Channel {
17051718
}
17061719
}
17071720
}
1721+
17081722
if self.channel_state & (ChannelState::MonitorUpdateFailed as u32) == 0 {
17091723
// This is a response to our post-monitor-failed unfreeze messages, so we can clear the
17101724
// monitor_pending_order requirement as we won't re-send the monitor_pending messages.
@@ -2188,7 +2202,6 @@ impl Channel {
21882202
return Err(ChannelError::Close("Peer sent update_fee when we needed a channel_reestablish"));
21892203
}
21902204
Channel::check_remote_fee(fee_estimator, msg.feerate_per_kw)?;
2191-
21922205
self.pending_update_fee = Some(msg.feerate_per_kw as u64);
21932206
self.channel_update_count += 1;
21942207
Ok(())

‎src/ln/channelmanager.rs

Lines changed: 74 additions & 57 deletions
Original file line numberDiff line numberDiff line change
@@ -3212,6 +3212,7 @@ mod tests {
32123212
use chain::chaininterface::{ChainListener, ChainWatchInterface};
32133213
use chain::keysinterface::KeysInterface;
32143214
use chain::keysinterface;
3215+
use ln::channel::{COMMITMENT_TX_BASE_WEIGHT, COMMITMENT_TX_WEIGHT_PER_HTLC};
32153216
use ln::channelmanager::{ChannelManager,ChannelManagerReadArgs,OnionKeys,PaymentFailReason,RAACommitmentOrder};
32163217
use ln::channelmonitor::{ChannelMonitor, ChannelMonitorUpdateErr, CLTV_CLAIM_BUFFER, HTLC_FAIL_TIMEOUT_BLOCKS, ManyChannelMonitor};
32173218
use ln::router::{Route, RouteHop, Router};
@@ -3501,6 +3502,17 @@ mod tests {
35013502
}
35023503
}
35033504

3505+
macro_rules! get_feerate {
3506+
($node: expr, $channel_id: expr) => {
3507+
{
3508+
let chan_lock = $node.node.channel_state.lock().unwrap();
3509+
let chan = chan_lock.by_id.get(&$channel_id).unwrap();
3510+
chan.get_feerate()
3511+
}
3512+
}
3513+
}
3514+
3515+
35043516
fn create_chan_between_nodes_with_value_init(node_a: &Node, node_b: &Node, channel_value: u64, push_msat: u64) -> Transaction {
35053517
node_a.node.create_channel(node_b.node.get_our_node_id(), channel_value, push_msat, 42).unwrap();
35063518
node_b.node.handle_open_channel(&node_a.node.get_our_node_id(), &get_event_msg!(node_a, MessageSendEvent::SendOpenChannel, node_b.node.get_our_node_id())).unwrap();
@@ -4134,14 +4146,6 @@ mod tests {
41344146
let chan = create_announced_chan_between_nodes(&nodes, 0, 1);
41354147
let channel_id = chan.2;
41364148

4137-
macro_rules! get_feerate {
4138-
($node: expr) => {{
4139-
let chan_lock = $node.node.channel_state.lock().unwrap();
4140-
let chan = chan_lock.by_id.get(&channel_id).unwrap();
4141-
chan.get_feerate()
4142-
}}
4143-
}
4144-
41454149
// balancing
41464150
send_payment(&nodes[0], &vec!(&nodes[1])[..], 8000000);
41474151

@@ -4163,7 +4167,7 @@ mod tests {
41634167
// (6) RAA is delivered ->
41644168

41654169
// First nodes[0] generates an update_fee
4166-
nodes[0].node.update_fee(channel_id, get_feerate!(nodes[0]) + 20).unwrap();
4170+
nodes[0].node.update_fee(channel_id, get_feerate!(nodes[0], channel_id) + 20).unwrap();
41674171
check_added_monitors!(nodes[0], 1);
41684172

41694173
let events_0 = nodes[0].node.get_and_clear_pending_msg_events();
@@ -4252,19 +4256,11 @@ mod tests {
42524256
let chan = create_announced_chan_between_nodes(&nodes, 0, 1);
42534257
let channel_id = chan.2;
42544258

4255-
macro_rules! get_feerate {
4256-
($node: expr) => {{
4257-
let chan_lock = $node.node.channel_state.lock().unwrap();
4258-
let chan = chan_lock.by_id.get(&channel_id).unwrap();
4259-
chan.get_feerate()
4260-
}}
4261-
}
4262-
42634259
// balancing
42644260
send_payment(&nodes[0], &vec!(&nodes[1])[..], 8000000);
42654261

42664262
// First nodes[0] generates an update_fee
4267-
nodes[0].node.update_fee(channel_id, get_feerate!(nodes[0]) + 20).unwrap();
4263+
nodes[0].node.update_fee(channel_id, get_feerate!(nodes[0], channel_id) + 20).unwrap();
42684264
check_added_monitors!(nodes[0], 1);
42694265

42704266
let events_0 = nodes[0].node.get_and_clear_pending_msg_events();
@@ -4310,14 +4306,6 @@ mod tests {
43104306
let chan = create_announced_chan_between_nodes(&nodes, 0, 1);
43114307
let channel_id = chan.2;
43124308

4313-
macro_rules! get_feerate {
4314-
($node: expr) => {{
4315-
let chan_lock = $node.node.channel_state.lock().unwrap();
4316-
let chan = chan_lock.by_id.get(&channel_id).unwrap();
4317-
chan.get_feerate()
4318-
}}
4319-
}
4320-
43214309
// A B
43224310
// update_fee/commitment_signed ->
43234311
// .- send (1) RAA and (2) commitment_signed
@@ -4338,7 +4326,7 @@ mod tests {
43384326
// revoke_and_ack ->
43394327

43404328
// First nodes[0] generates an update_fee
4341-
let initial_feerate = get_feerate!(nodes[0]);
4329+
let initial_feerate = get_feerate!(nodes[0], channel_id);
43424330
nodes[0].node.update_fee(channel_id, initial_feerate + 20).unwrap();
43434331
check_added_monitors!(nodes[0], 1);
43444332

@@ -4422,16 +4410,8 @@ mod tests {
44224410
let chan = create_announced_chan_between_nodes(&nodes, 0, 1);
44234411
let channel_id = chan.2;
44244412

4425-
macro_rules! get_feerate {
4426-
($node: expr) => {{
4427-
let chan_lock = $node.node.channel_state.lock().unwrap();
4428-
let chan = chan_lock.by_id.get(&channel_id).unwrap();
4429-
chan.get_feerate()
4430-
}}
4431-
}
4432-
4433-
let feerate = get_feerate!(nodes[0]);
4434-
nodes[0].node.update_fee(channel_id, feerate+20).unwrap();
4413+
let feerate = get_feerate!(nodes[0], channel_id);
4414+
nodes[0].node.update_fee(channel_id, feerate+25).unwrap();
44354415
check_added_monitors!(nodes[0], 1);
44364416

44374417
let events_0 = nodes[0].node.get_and_clear_pending_msg_events();
@@ -4462,24 +4442,69 @@ mod tests {
44624442
check_added_monitors!(nodes[1], 1);
44634443
}
44644444

4445+
#[test]
4446+
fn test_update_fee_that_funder_cannot_afford() {
4447+
let mut nodes = create_network(2);
4448+
let channel_value = 1888;
4449+
let chan = create_announced_chan_between_nodes_with_value(&nodes, 0, 1, channel_value, 700000);
4450+
let channel_id = chan.2;
4451+
4452+
let feerate = 260;
4453+
nodes[0].node.update_fee(channel_id, feerate).unwrap();
4454+
check_added_monitors!(nodes[0], 1);
4455+
let update_msg = get_htlc_update_msgs!(nodes[0], nodes[1].node.get_our_node_id());
4456+
4457+
nodes[1].node.handle_update_fee(&nodes[0].node.get_our_node_id(), &update_msg.update_fee.unwrap()).unwrap();
4458+
4459+
commitment_signed_dance!(nodes[1], nodes[0], update_msg.commitment_signed, false);
4460+
4461+
//Confirm that the new fee based on the last local commitment txn is what we expected based on the feerate of 260 set above.
4462+
//This value results in a fee that is exactly what the funder can afford (277 sat + 1000 sat channel reserve)
4463+
{
4464+
let chan_lock = nodes[1].node.channel_state.lock().unwrap();
4465+
let chan = chan_lock.by_id.get(&channel_id).unwrap();
4466+
4467+
//We made sure neither party's funds are below the dust limit so -2 non-HTLC txns from number of outputs
4468+
let num_htlcs = chan.last_local_commitment_txn[0].output.len() - 2;
4469+
let total_fee: u64 = feerate * (COMMITMENT_TX_BASE_WEIGHT + (num_htlcs as u64) * COMMITMENT_TX_WEIGHT_PER_HTLC) / 1000;
4470+
let mut actual_fee = chan.last_local_commitment_txn[0].output.iter().fold(0, |acc, output| acc + output.value);
4471+
actual_fee = channel_value - actual_fee;
4472+
assert_eq!(total_fee, actual_fee);
4473+
} //drop the mutex
4474+
4475+
//Add 2 to the previous fee rate to the final fee increases by 1 (with no HTLCs the fee is essentially
4476+
//fee_rate*(724/1000) so the increment of 1*0.724 is rounded back down)
4477+
nodes[0].node.update_fee(channel_id, feerate+2).unwrap();
4478+
check_added_monitors!(nodes[0], 1);
4479+
4480+
let update2_msg = get_htlc_update_msgs!(nodes[0], nodes[1].node.get_our_node_id());
4481+
4482+
nodes[1].node.handle_update_fee(&nodes[0].node.get_our_node_id(), &update2_msg.update_fee.unwrap());
4483+
4484+
//While producing the commitment_signed response after handling a received update_fee request the
4485+
//check to see if the funder, who sent the update_fee request, can afford the new fee (funder_balance >= fee+channel_reserve)
4486+
//Should produce and error.
4487+
let err = nodes[1].node.handle_commitment_signed(&nodes[0].node.get_our_node_id(), &update2_msg.commitment_signed).unwrap_err();
4488+
4489+
assert!(match err.err {
4490+
"Funding remote cannot afford proposed new fee" => true,
4491+
_ => false,
4492+
});
4493+
4494+
//clear the message we could not handle
4495+
nodes[1].node.get_and_clear_pending_msg_events();
4496+
}
4497+
44654498
#[test]
44664499
fn test_update_fee_with_fundee_update_add_htlc() {
44674500
let mut nodes = create_network(2);
44684501
let chan = create_announced_chan_between_nodes(&nodes, 0, 1);
44694502
let channel_id = chan.2;
44704503

4471-
macro_rules! get_feerate {
4472-
($node: expr) => {{
4473-
let chan_lock = $node.node.channel_state.lock().unwrap();
4474-
let chan = chan_lock.by_id.get(&channel_id).unwrap();
4475-
chan.get_feerate()
4476-
}}
4477-
}
4478-
44794504
// balancing
44804505
send_payment(&nodes[0], &vec!(&nodes[1])[..], 8000000);
44814506

4482-
let feerate = get_feerate!(nodes[0]);
4507+
let feerate = get_feerate!(nodes[0], channel_id);
44834508
nodes[0].node.update_fee(channel_id, feerate+20).unwrap();
44844509
check_added_monitors!(nodes[0], 1);
44854510

@@ -4577,14 +4602,6 @@ mod tests {
45774602
let chan = create_announced_chan_between_nodes(&nodes, 0, 1);
45784603
let channel_id = chan.2;
45794604

4580-
macro_rules! get_feerate {
4581-
($node: expr) => {{
4582-
let chan_lock = $node.node.channel_state.lock().unwrap();
4583-
let chan = chan_lock.by_id.get(&channel_id).unwrap();
4584-
chan.get_feerate()
4585-
}}
4586-
}
4587-
45884605
// A B
45894606
// (1) update_fee/commitment_signed ->
45904607
// <- (2) revoke_and_ack
@@ -4600,7 +4617,7 @@ mod tests {
46004617
// revoke_and_ack ->
46014618

46024619
// Create and deliver (1)...
4603-
let feerate = get_feerate!(nodes[0]);
4620+
let feerate = get_feerate!(nodes[0], channel_id);
46044621
nodes[0].node.update_fee(channel_id, feerate+20).unwrap();
46054622
check_added_monitors!(nodes[0], 1);
46064623

@@ -4674,8 +4691,8 @@ mod tests {
46744691
check_added_monitors!(nodes[1], 1);
46754692
assert!(nodes[1].node.get_and_clear_pending_msg_events().is_empty());
46764693

4677-
assert_eq!(get_feerate!(nodes[0]), feerate + 30);
4678-
assert_eq!(get_feerate!(nodes[1]), feerate + 30);
4694+
assert_eq!(get_feerate!(nodes[0], channel_id), feerate + 30);
4695+
assert_eq!(get_feerate!(nodes[1], channel_id), feerate + 30);
46794696
close_channel(&nodes[0], &nodes[1], &chan.2, chan.3, true);
46804697
}
46814698

0 commit comments

Comments
 (0)