Skip to content

Commit 8142175

Browse files
philipr-zaphilipr-za
committed
Check affordability of new fee during update_fee call #204 
1 parent 74f59a2 commit 8142175

File tree

2 files changed

+105
-62
lines changed

2 files changed

+105
-62
lines changed

‎src/ln/channel.rs

Lines changed: 18 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -357,8 +357,8 @@ 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+
pub const COMMITMENT_TX_BASE_WEIGHT: u64 = 724;
361+
pub const COMMITMENT_TX_WEIGHT_PER_HTLC: u64 = 172;
362362
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
363363
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?)
364364
/// Maximmum `funding_satoshis` value, according to the BOLT #2 specification
@@ -1653,21 +1653,35 @@ impl Channel {
16531653

16541654
let local_keys = self.build_local_transaction_keys(self.cur_local_commitment_transaction_number)?;
16551655

1656+
let mut update_fee = false;
16561657
let feerate_per_kw = if !self.channel_outbound && self.pending_update_fee.is_some() {
1658+
update_fee = true;
16571659
self.pending_update_fee.unwrap()
16581660
} else {
16591661
self.feerate_per_kw
16601662
};
16611663

1664+
16621665
let mut local_commitment_tx = self.build_commitment_transaction(self.cur_local_commitment_transaction_number, &local_keys, true, false, feerate_per_kw);
16631666
let local_commitment_txid = local_commitment_tx.0.txid();
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.0.output.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+
if self.channel_value_satoshis - self.value_to_self_msat / 1000 < total_fee + self.their_channel_reserve_satoshis {
1675+
return Err(HandleError { err: "Funding remote cannot afford proposed new fee", action: Some(ErrorAction::DisconnectPeer { msg: None }) });
1676+
}
1677+
}
1678+
16671679
if msg.htlc_signatures.len() != local_commitment_tx.1.len() {
16681680
return Err(HandleError{err: "Got wrong number of HTLC signatures from remote", action: None});
16691681
}
16701682

1683+
println!("Len: {}, Num HTLC Sigs: {}", local_commitment_tx.0.output.len(), local_commitment_tx.1.len());
1684+
16711685
let mut new_local_commitment_txn = Vec::with_capacity(local_commitment_tx.1.len() + 1);
16721686
self.sign_commitment_transaction(&mut local_commitment_tx.0, &msg.signature);
16731687
new_local_commitment_txn.push(local_commitment_tx.0.clone());
@@ -1677,7 +1691,7 @@ impl Channel {
16771691
let mut htlc_tx = self.build_htlc_transaction(&local_commitment_txid, htlc, true, &local_keys, feerate_per_kw);
16781692
let htlc_redeemscript = chan_utils::get_htlc_redeemscript(&htlc, &local_keys);
16791693
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());
1694+
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());
16811695
let htlc_sig = if htlc.offered {
16821696
let htlc_sig = self.sign_htlc_transaction(&mut htlc_tx, &msg.htlc_signatures[idx], &None, htlc, &local_keys)?;
16831697
new_local_commitment_txn.push(htlc_tx);
@@ -1705,6 +1719,7 @@ impl Channel {
17051719
}
17061720
}
17071721
}
1722+
17081723
if self.channel_state & (ChannelState::MonitorUpdateFailed as u32) == 0 {
17091724
// This is a response to our post-monitor-failed unfreeze messages, so we can clear the
17101725
// monitor_pending_order requirement as we won't re-send the monitor_pending messages.
@@ -2188,7 +2203,6 @@ impl Channel {
21882203
return Err(ChannelError::Close("Peer sent update_fee when we needed a channel_reestablish"));
21892204
}
21902205
Channel::check_remote_fee(fee_estimator, msg.feerate_per_kw)?;
2191-
21922206
self.pending_update_fee = Some(msg.feerate_per_kw as u64);
21932207
self.channel_update_count += 1;
21942208
Ok(())

‎src/ln/channelmanager.rs

Lines changed: 87 additions & 58 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ use secp256k1;
2222

2323
use chain::chaininterface::{BroadcasterInterface,ChainListener,ChainWatchInterface,FeeEstimator};
2424
use chain::transaction::OutPoint;
25-
use ln::channel::{Channel, ChannelError};
25+
use ln::channel::{Channel, ChannelError, COMMITMENT_TX_BASE_WEIGHT, COMMITMENT_TX_WEIGHT_PER_HTLC};
2626
use ln::channelmonitor::{ChannelMonitor, ChannelMonitorUpdateErr, ManyChannelMonitor, CLTV_CLAIM_BUFFER, HTLC_FAIL_TIMEOUT_BLOCKS};
2727
use ln::router::{Route,RouteHop};
2828
use ln::msgs;
@@ -3501,6 +3501,17 @@ mod tests {
35013501
}
35023502
}
35033503

3504+
macro_rules! get_feerate {
3505+
($node: expr, $channel_id: expr) => {
3506+
{
3507+
let chan_lock = $node.node.channel_state.lock().unwrap();
3508+
let chan = chan_lock.by_id.get(&$channel_id).unwrap();
3509+
chan.get_feerate()
3510+
}
3511+
}
3512+
}
3513+
3514+
35043515
fn create_chan_between_nodes_with_value_init(node_a: &Node, node_b: &Node, channel_value: u64, push_msat: u64) -> Transaction {
35053516
node_a.node.create_channel(node_b.node.get_our_node_id(), channel_value, push_msat, 42).unwrap();
35063517
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 +4145,6 @@ mod tests {
41344145
let chan = create_announced_chan_between_nodes(&nodes, 0, 1);
41354146
let channel_id = chan.2;
41364147

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-
41454148
// balancing
41464149
send_payment(&nodes[0], &vec!(&nodes[1])[..], 8000000);
41474150

@@ -4163,7 +4166,7 @@ mod tests {
41634166
// (6) RAA is delivered ->
41644167

41654168
// First nodes[0] generates an update_fee
4166-
nodes[0].node.update_fee(channel_id, get_feerate!(nodes[0]) + 20).unwrap();
4169+
nodes[0].node.update_fee(channel_id, get_feerate!(nodes[0], channel_id) + 20).unwrap();
41674170
check_added_monitors!(nodes[0], 1);
41684171

41694172
let events_0 = nodes[0].node.get_and_clear_pending_msg_events();
@@ -4252,19 +4255,11 @@ mod tests {
42524255
let chan = create_announced_chan_between_nodes(&nodes, 0, 1);
42534256
let channel_id = chan.2;
42544257

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-
42634258
// balancing
42644259
send_payment(&nodes[0], &vec!(&nodes[1])[..], 8000000);
42654260

42664261
// First nodes[0] generates an update_fee
4267-
nodes[0].node.update_fee(channel_id, get_feerate!(nodes[0]) + 20).unwrap();
4262+
nodes[0].node.update_fee(channel_id, get_feerate!(nodes[0], channel_id) + 20).unwrap();
42684263
check_added_monitors!(nodes[0], 1);
42694264

42704265
let events_0 = nodes[0].node.get_and_clear_pending_msg_events();
@@ -4310,14 +4305,6 @@ mod tests {
43104305
let chan = create_announced_chan_between_nodes(&nodes, 0, 1);
43114306
let channel_id = chan.2;
43124307

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-
43214308
// A B
43224309
// update_fee/commitment_signed ->
43234310
// .- send (1) RAA and (2) commitment_signed
@@ -4338,7 +4325,7 @@ mod tests {
43384325
// revoke_and_ack ->
43394326

43404327
// First nodes[0] generates an update_fee
4341-
let initial_feerate = get_feerate!(nodes[0]);
4328+
let initial_feerate = get_feerate!(nodes[0], channel_id);
43424329
nodes[0].node.update_fee(channel_id, initial_feerate + 20).unwrap();
43434330
check_added_monitors!(nodes[0], 1);
43444331

@@ -4422,16 +4409,8 @@ mod tests {
44224409
let chan = create_announced_chan_between_nodes(&nodes, 0, 1);
44234410
let channel_id = chan.2;
44244411

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();
4412+
let feerate = get_feerate!(nodes[0], channel_id);
4413+
nodes[0].node.update_fee(channel_id, feerate+25).unwrap();
44354414
check_added_monitors!(nodes[0], 1);
44364415

44374416
let events_0 = nodes[0].node.get_and_clear_pending_msg_events();
@@ -4462,24 +4441,82 @@ mod tests {
44624441
check_added_monitors!(nodes[1], 1);
44634442
}
44644443

4444+
#[test]
4445+
fn test_update_fee_that_funder_cannot_afford() {
4446+
let mut nodes = create_network(2);
4447+
let chan = create_announced_chan_between_nodes_with_value(&nodes, 0, 1, 1432, 200000);
4448+
let channel_id = chan.2;
4449+
4450+
let feerate = 260;
4451+
nodes[0].node.update_fee(channel_id, feerate).unwrap();
4452+
check_added_monitors!(nodes[0], 1);
4453+
4454+
let events_0 = nodes[0].node.get_and_clear_pending_msg_events();
4455+
assert_eq!(events_0.len(), 1);
4456+
let (update_msg, commitment_signed) = match events_0[0] {
4457+
MessageSendEvent::UpdateHTLCs {
4458+
node_id: _, updates: msgs::CommitmentUpdate {
4459+
update_add_htlcs: _, update_fulfill_htlcs: _, update_fail_htlcs: _, update_fail_malformed_htlcs: _, ref update_fee, ref commitment_signed
4460+
}
4461+
} => {
4462+
(update_fee.as_ref(), commitment_signed)
4463+
},
4464+
_ => panic!("Unexpected event"),
4465+
};
4466+
nodes[1].node.handle_update_fee(&nodes[0].node.get_our_node_id(), update_msg.unwrap()).unwrap();
4467+
4468+
commitment_signed_dance!(nodes[1], nodes[0], commitment_signed, false);
4469+
4470+
//Confirm that the new fee based on the last local commitment txn is what we expected based on the feerate of 260 set above.
4471+
//This value results in a fee that is exactly what the funder can afford (277 sat + 1000 sat channel reserve)
4472+
{
4473+
let chan_lock = nodes[1].node.channel_state.lock().unwrap();
4474+
let chan = chan_lock.by_id.get(&channel_id).unwrap();
4475+
4476+
let num_htlcs = chan.last_local_commitment_txn[0].output.len();
4477+
let total_fee: u64 = feerate * (super::COMMITMENT_TX_BASE_WEIGHT + (num_htlcs as u64) * super::COMMITMENT_TX_WEIGHT_PER_HTLC) / 1000;
4478+
let fee_for_single_update_to_commitment = feerate * (super::COMMITMENT_TX_BASE_WEIGHT + 1 * super::COMMITMENT_TX_WEIGHT_PER_HTLC) / 1000;
4479+
assert_eq!(total_fee, fee_for_single_update_to_commitment);
4480+
} //drop the mutex
4481+
4482+
nodes[0].node.update_fee(channel_id, feerate+1).unwrap();
4483+
check_added_monitors!(nodes[0], 1);
4484+
4485+
let events_0 = nodes[0].node.get_and_clear_pending_msg_events();
4486+
assert_eq!(events_0.len(), 1);
4487+
let (update_msg, commitment_signed) = match events_0[0] {
4488+
MessageSendEvent::UpdateHTLCs { node_id:_, updates: msgs::CommitmentUpdate { update_add_htlcs:_, update_fulfill_htlcs:_, update_fail_htlcs:_, update_fail_malformed_htlcs:_, ref update_fee, ref commitment_signed } } => {
4489+
(update_fee.as_ref(), commitment_signed)
4490+
},
4491+
_ => panic!("Unexpected event"),
4492+
};
4493+
4494+
nodes[1].node.handle_update_fee(&nodes[0].node.get_our_node_id(), update_msg.unwrap());
4495+
4496+
//While producing the commitment_signed response after handling a received update_fee request the
4497+
//check to see if the funder, who sent the update_fee request, can afford the new fee (funder_balance >= fee+channel_reserve)
4498+
//Should produce and error.
4499+
let err = nodes[1].node.handle_commitment_signed(&nodes[0].node.get_our_node_id(), &commitment_signed).unwrap_err();
4500+
4501+
assert!(match err.action.unwrap() {
4502+
msgs::ErrorAction::DisconnectPeer{..} => true,
4503+
_ => false,
4504+
});
4505+
4506+
//clear the message we could not handle
4507+
nodes[1].node.get_and_clear_pending_msg_events();
4508+
}
4509+
44654510
#[test]
44664511
fn test_update_fee_with_fundee_update_add_htlc() {
44674512
let mut nodes = create_network(2);
44684513
let chan = create_announced_chan_between_nodes(&nodes, 0, 1);
44694514
let channel_id = chan.2;
44704515

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-
44794516
// balancing
44804517
send_payment(&nodes[0], &vec!(&nodes[1])[..], 8000000);
44814518

4482-
let feerate = get_feerate!(nodes[0]);
4519+
let feerate = get_feerate!(nodes[0], channel_id);
44834520
nodes[0].node.update_fee(channel_id, feerate+20).unwrap();
44844521
check_added_monitors!(nodes[0], 1);
44854522

@@ -4577,14 +4614,6 @@ mod tests {
45774614
let chan = create_announced_chan_between_nodes(&nodes, 0, 1);
45784615
let channel_id = chan.2;
45794616

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-
45884617
// A B
45894618
// (1) update_fee/commitment_signed ->
45904619
// <- (2) revoke_and_ack
@@ -4600,7 +4629,7 @@ mod tests {
46004629
// revoke_and_ack ->
46014630

46024631
// Create and deliver (1)...
4603-
let feerate = get_feerate!(nodes[0]);
4632+
let feerate = get_feerate!(nodes[0], channel_id);
46044633
nodes[0].node.update_fee(channel_id, feerate+20).unwrap();
46054634
check_added_monitors!(nodes[0], 1);
46064635

@@ -4674,8 +4703,8 @@ mod tests {
46744703
check_added_monitors!(nodes[1], 1);
46754704
assert!(nodes[1].node.get_and_clear_pending_msg_events().is_empty());
46764705

4677-
assert_eq!(get_feerate!(nodes[0]), feerate + 30);
4678-
assert_eq!(get_feerate!(nodes[1]), feerate + 30);
4706+
assert_eq!(get_feerate!(nodes[0], channel_id), feerate + 30);
4707+
assert_eq!(get_feerate!(nodes[1], channel_id), feerate + 30);
46794708
close_channel(&nodes[0], &nodes[1], &chan.2, chan.3, true);
46804709
}
46814710

0 commit comments

Comments
 (0)