Skip to content

Test remote fee spike buffer violation #647

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

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 3 additions & 2 deletions lightning/src/ln/channel.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1755,8 +1755,8 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
self.commit_tx_fee_msat(their_acked_htlcs + addl_htlcs)
}

pub fn update_add_htlc<F>(&mut self, msg: &msgs::UpdateAddHTLC, mut pending_forward_status: PendingHTLCStatus, create_pending_htlc_status: F) -> Result<(), ChannelError>
where F: for<'a> Fn(&'a Self, PendingHTLCStatus, u16) -> PendingHTLCStatus {
pub fn update_add_htlc<F, L: Deref>(&mut self, msg: &msgs::UpdateAddHTLC, mut pending_forward_status: PendingHTLCStatus, create_pending_htlc_status: F, logger: &L) -> Result<(), ChannelError>
where F: for<'a> Fn(&'a Self, PendingHTLCStatus, u16) -> PendingHTLCStatus, L::Target: Logger {
// We can't accept HTLCs sent after we've sent a shutdown.
let local_sent_shutdown = (self.channel_state & (ChannelState::ChannelFunded as u32 | ChannelState::LocalShutdownSent as u32)) != (ChannelState::ChannelFunded as u32);
if local_sent_shutdown {
Expand Down Expand Up @@ -1845,6 +1845,7 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
if pending_remote_value_msat - msg.amount_msat - chan_reserve_msat < remote_fee_cost_incl_stuck_buffer_msat {
// Note that if the pending_forward_status is not updated here, then it's because we're already failing
// the HTLC, i.e. its status is already set to failing.
log_info!(logger, "Attempting to fail HTLC due to fee spike buffer violation");
pending_forward_status = create_pending_htlc_status(self, pending_forward_status, 0x1000|7);
}
} else {
Expand Down
2 changes: 1 addition & 1 deletion lightning/src/ln/channelmanager.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2519,7 +2519,7 @@ impl<ChanSigner: ChannelKeys, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref>
_ => pending_forward_info
}
};
try_chan_entry!(self, chan.get_mut().update_add_htlc(&msg, pending_forward_info, create_pending_htlc_status), channel_state, chan);
try_chan_entry!(self, chan.get_mut().update_add_htlc(&msg, pending_forward_info, create_pending_htlc_status, &self.logger), channel_state, chan);
},
hash_map::Entry::Vacant(_) => return Err(MsgHandleErrInternal::send_err_msg_no_close("Failed to find corresponding channel", msg.channel_id))
}
Expand Down
176 changes: 175 additions & 1 deletion lightning/src/ln/functional_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,8 @@ use util::config::UserConfig;

use bitcoin::util::hash::BitcoinHash;
use bitcoin::hashes::sha256d::Hash as Sha256dHash;
use bitcoin::hash_types::{Txid, BlockHash};
use bitcoin::hashes::HashEngine;
use bitcoin::hash_types::{Txid, BlockHash, WPubkeyHash};
use bitcoin::util::bip143;
use bitcoin::util::address::Address;
use bitcoin::util::bip32::{ChildNumber, ExtendedPubKey, ExtendedPrivKey};
Expand Down Expand Up @@ -1553,6 +1554,179 @@ fn test_basic_channel_reserve() {
send_payment(&nodes[0], &vec![&nodes[1]], max_can_send, max_can_send);
}

#[test]
fn test_fee_spike_violation_fails_htlc() {
let chanmon_cfgs = create_chanmon_cfgs(2);
let node_cfgs = create_node_cfgs(2, &chanmon_cfgs);
let node_chanmgrs = create_node_chanmgrs(2, &node_cfgs, &[None, None]);
let mut nodes = create_network(2, &node_cfgs, &node_chanmgrs);
let chan = create_announced_chan_between_nodes_with_value(&nodes, 0, 1, 100000, 95000000, InitFeatures::known(), InitFeatures::known());
let logger = test_utils::TestLogger::new();

macro_rules! get_route_and_payment_hash {
($recv_value: expr) => {{
let (payment_preimage, payment_hash) = get_payment_preimage_hash!(nodes[1]);
let net_graph_msg_handler = &nodes[0].net_graph_msg_handler;
let route = get_route(&nodes[0].node.get_our_node_id(), net_graph_msg_handler, &nodes.last().unwrap().node.get_our_node_id(), None, &Vec::new(), $recv_value, TEST_FINAL_CLTV, &logger).unwrap();
(route, payment_hash, payment_preimage)
}}
};

let (route, payment_hash, _) = get_route_and_payment_hash!(3460001);
// Need to manually create the update_add_htlc message to go around the channel reserve check in send_htlc()
let secp_ctx = Secp256k1::new();
let session_priv = SecretKey::from_slice(&[42; 32]).expect("RNG is bad!");

let cur_height = nodes[1].node.latest_block_height.load(Ordering::Acquire) as u32 + 1;

let onion_keys = onion_utils::construct_onion_keys(&secp_ctx, &route.paths[0], &session_priv).unwrap();
let (onion_payloads, htlc_msat, htlc_cltv) = onion_utils::build_onion_payloads(&route.paths[0], 3460001, &None, cur_height).unwrap();
let onion_packet = onion_utils::construct_onion_packet(onion_payloads, onion_keys, [0; 32], &payment_hash);
let msg = msgs::UpdateAddHTLC {
channel_id: chan.2,
htlc_id: 0,
amount_msat: htlc_msat,
payment_hash: payment_hash,
cltv_expiry: htlc_cltv,
onion_routing_packet: onion_packet,
};

nodes[1].node.handle_update_add_htlc(&nodes[0].node.get_our_node_id(), &msg);

// Now manually create the commitment_signed message corresponding to the update_add
// nodes[0] just sent. In the code for construction of this message, "local" refers
// to the sender of the message, and "remote" refers to the receiver.

let feerate_per_kw = get_feerate!(nodes[0], chan.2);

// Get the EnforcingChannelKeys for each channel, which will be used to (1) get the keys
// needed to sign the new commitment tx and (2) sign the new commitment tx.
let (local_revocation_basepoint, local_htlc_basepoint, local_payment_point, local_chan_commitment_seed) = {
let chan_lock = nodes[0].node.channel_state.lock().unwrap();
let local_chan = chan_lock.by_id.get(&chan.2).unwrap();
let chan_keys = local_chan.get_local_keys();
let pubkeys = chan_keys.pubkeys();
(pubkeys.revocation_basepoint, pubkeys.htlc_basepoint, pubkeys.payment_point, *chan_keys.commitment_seed())
};
let (remote_delayed_payment_basepoint, remote_htlc_basepoint, remote_payment_point, remote_chan_commitment_seed) = {
let chan_lock = nodes[1].node.channel_state.lock().unwrap();
let remote_chan = chan_lock.by_id.get(&chan.2).unwrap();
let chan_keys = remote_chan.get_local_keys();
let pubkeys = chan_keys.pubkeys();
(pubkeys.delayed_payment_basepoint, pubkeys.htlc_basepoint, pubkeys.payment_point, *chan_keys.commitment_seed())
};

// Assemble the set of keys we can use for signatures for our commitment_signed message.
const INITIAL_COMMITMENT_NUMBER: u64 = (1 << 48) - 1;
let commitment_secret = {
let res = chan_utils::build_commitment_secret(&remote_chan_commitment_seed, INITIAL_COMMITMENT_NUMBER - 1);
SecretKey::from_slice(&res).unwrap()
};
let per_commitment_point = PublicKey::from_secret_key(&secp_ctx, &commitment_secret);
let commit_tx_keys = chan_utils::TxCreationKeys::new(&secp_ctx, &per_commitment_point, &remote_delayed_payment_basepoint,
&remote_htlc_basepoint, &local_revocation_basepoint, &local_htlc_basepoint).unwrap();

// Build the remote commitment transaction so we can sign it, and then later use the
// signature for the commitment_signed message.
let local_chan_balance = 1313;
let static_payment_pk = local_payment_point.serialize();
let remote_commit_tx_output = TxOut {
script_pubkey: Builder::new().push_opcode(opcodes::all::OP_PUSHBYTES_0)
.push_slice(&WPubkeyHash::hash(&static_payment_pk)[..])
.into_script(),
value: local_chan_balance as u64
};

let local_commit_tx_output = TxOut {
script_pubkey: chan_utils::get_revokeable_redeemscript(&commit_tx_keys.revocation_key,
BREAKDOWN_TIMEOUT,
&commit_tx_keys.a_delayed_payment_key).to_v0_p2wsh(),
value: 95000,
};

let accepted_htlc_info = chan_utils::HTLCOutputInCommitment {
offered: false,
amount_msat: 3460001,
cltv_expiry: htlc_cltv,
payment_hash: payment_hash,
transaction_output_index: Some(1),
};

let htlc_output = TxOut {
script_pubkey: chan_utils::get_htlc_redeemscript(&accepted_htlc_info, &commit_tx_keys).to_v0_p2wsh(),
value: 3460001 / 1000
};

let commit_tx_obscure_factor = {
let mut sha = Sha256::engine();
let remote_payment_point = &remote_payment_point.serialize();
sha.input(&local_payment_point.serialize());
sha.input(remote_payment_point);
let res = Sha256::from_engine(sha).into_inner();

((res[26] as u64) << 5*8) |
((res[27] as u64) << 4*8) |
((res[28] as u64) << 3*8) |
((res[29] as u64) << 2*8) |
((res[30] as u64) << 1*8) |
((res[31] as u64) << 0*8)
};
let commitment_number = 1;
let obscured_commitment_transaction_number = commit_tx_obscure_factor ^ commitment_number;
let lock_time = ((0x20 as u32) << 8*3) | ((obscured_commitment_transaction_number & 0xffffffu64) as u32);
let input = TxIn {
previous_output: BitcoinOutPoint { txid: chan.3.txid(), vout: 0 },
script_sig: Script::new(),
sequence: ((0x80 as u32) << 8*3) | ((obscured_commitment_transaction_number >> 3*8) as u32),
witness: Vec::new(),
};

let commit_tx = Transaction {
version: 2,
lock_time,
input: vec![input],
output: vec![remote_commit_tx_output, htlc_output, local_commit_tx_output],
};
let res = {
let local_chan_lock = nodes[0].node.channel_state.lock().unwrap();
let local_chan = local_chan_lock.by_id.get(&chan.2).unwrap();
let local_chan_keys = local_chan.get_local_keys();
local_chan_keys.sign_remote_commitment(feerate_per_kw, &commit_tx, &commit_tx_keys, &[&accepted_htlc_info],
BREAKDOWN_TIMEOUT, &secp_ctx).unwrap()
};

let commit_signed_msg = msgs::CommitmentSigned {
channel_id: chan.2,
signature: res.0,
htlc_signatures: res.1
};

// Send the commitment_signed message to the nodes[1].
nodes[1].node.handle_commitment_signed(&nodes[0].node.get_our_node_id(), &commit_signed_msg);
let _ = nodes[1].node.get_and_clear_pending_msg_events();

// Send the RAA to nodes[1].
let per_commitment_secret = chan_utils::build_commitment_secret(&local_chan_commitment_seed, INITIAL_COMMITMENT_NUMBER);
let next_secret = SecretKey::from_slice(&chan_utils::build_commitment_secret(&local_chan_commitment_seed, INITIAL_COMMITMENT_NUMBER - 2)).unwrap();
let next_per_commitment_point = PublicKey::from_secret_key(&secp_ctx, &next_secret);
let raa_msg = msgs::RevokeAndACK{ channel_id: chan.2, per_commitment_secret, next_per_commitment_point};
nodes[1].node.handle_revoke_and_ack(&nodes[0].node.get_our_node_id(), &raa_msg);

let events = nodes[1].node.get_and_clear_pending_msg_events();
assert_eq!(events.len(), 1);
// Make sure the HTLC failed in the way we expect.
match events[0] {
MessageSendEvent::UpdateHTLCs { updates: msgs::CommitmentUpdate { ref update_fail_htlcs, .. }, .. } => {
assert_eq!(update_fail_htlcs.len(), 1);
update_fail_htlcs[0].clone()
},
_ => panic!("Unexpected event"),
};
nodes[1].logger.assert_log("lightning::ln::channel".to_string(), "Attempting to fail HTLC due to fee spike buffer violation".to_string(), 1);

check_added_monitors!(nodes[1], 2);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Not that its really critical, but in general I prefer to check for added monitors right after they get added (ie after the handle_commitment_signed and handle_revoke_and_ack) since it enforces better that those calls succeeded all the way through to generating a monitor update.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Makes sense 👍

}

#[test]
fn test_chan_reserve_violation_outbound_htlc_inbound_chan() {
let mut chanmon_cfgs = create_chanmon_cfgs(2);
Expand Down