X-Git-Url: http://git.bitcoin.ninja/index.cgi?a=blobdiff_plain;f=lightning%2Fsrc%2Fln%2Fchannel.rs;h=879df8cdd79d062c4edab4af6ad2b99a757e04c7;hb=ba75b3ecd7f88a79ff6392a5229c4ab6c14a8591;hp=8b5d9120babb2e98eccdcccb3e472443e5b070d8;hpb=78c48f76d429b21dc34bcf636946b3d5a30d9a1d;p=rust-lightning diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs index 8b5d9120..879df8cd 100644 --- a/lightning/src/ln/channel.rs +++ b/lightning/src/ln/channel.rs @@ -433,10 +433,6 @@ impl Channel { cmp::max(at_open_background_feerate * B_OUTPUT_PLUS_SPENDING_INPUT_WEIGHT / 1000, 546) //TODO } - fn derive_our_htlc_minimum_msat(_at_open_channel_feerate_per_kw: u64) -> u64 { - 1000 // TODO - } - // Constructors: pub fn new_outbound(fee_estimator: &F, keys_provider: &K, their_node_id: PublicKey, channel_value_satoshis: u64, push_msat: u64, user_id: u64, logger: Arc, config: &UserConfig) -> Result, APIError> where K::Target: KeysInterface, @@ -519,7 +515,7 @@ impl Channel { their_max_htlc_value_in_flight_msat: 0, their_channel_reserve_satoshis: 0, their_htlc_minimum_msat: 0, - our_htlc_minimum_msat: Channel::::derive_our_htlc_minimum_msat(feerate), + our_htlc_minimum_msat: if config.own_channel_config.our_htlc_minimum_msat == 0 { 1 } else { config.own_channel_config.our_htlc_minimum_msat }, their_to_self_delay: 0, our_to_self_delay: config.own_channel_config.our_to_self_delay, their_max_accepted_htlcs: 0, @@ -744,7 +740,7 @@ impl Channel { their_max_htlc_value_in_flight_msat: cmp::min(msg.max_htlc_value_in_flight_msat, msg.funding_satoshis * 1000), their_channel_reserve_satoshis: msg.channel_reserve_satoshis, their_htlc_minimum_msat: msg.htlc_minimum_msat, - our_htlc_minimum_msat: Channel::::derive_our_htlc_minimum_msat(msg.feerate_per_kw as u64), + our_htlc_minimum_msat: if config.own_channel_config.our_htlc_minimum_msat == 0 { 1 } else { config.own_channel_config.our_htlc_minimum_msat }, their_to_self_delay: msg.to_self_delay, our_to_self_delay: config.own_channel_config.our_to_self_delay, their_max_accepted_htlcs: msg.max_accepted_htlcs, @@ -1142,8 +1138,11 @@ impl Channel { } /// Per HTLC, only one get_update_fail_htlc or get_update_fulfill_htlc call may be made. - /// In such cases we debug_assert!(false) and return an IgnoreError. Thus, will always return - /// Ok(_) if debug assertions are turned on and preconditions are met. + /// In such cases we debug_assert!(false) and return a ChannelError::Ignore. Thus, will always + /// return Ok(_) if debug assertions are turned on or preconditions are met. + /// + /// Note that it is still possible to hit these assertions in case we find a preimage on-chain + /// but then have a reorg which settles on an HTLC-failure on chain. fn get_update_fulfill_htlc(&mut self, htlc_id_arg: u64, payment_preimage_arg: PaymentPreimage) -> Result<(Option, Option), ChannelError> { // Either ChannelFunded got set (which means it won't be unset) or there is no way any // caller thought we could have something claimed (cause we wouldn't have accepted in an @@ -1171,6 +1170,7 @@ impl Channel { } else { log_warn!(self, "Have preimage and want to fulfill HTLC with payment hash {} we already failed against channel {}", log_bytes!(htlc.payment_hash.0), log_bytes!(self.channel_id())); } + debug_assert!(false, "Tried to fulfill an HTLC that was already fail/fulfilled"); return Ok((None, None)); }, _ => { @@ -1204,6 +1204,9 @@ impl Channel { match pending_update { &HTLCUpdateAwaitingACK::ClaimHTLC { htlc_id, .. } => { if htlc_id_arg == htlc_id { + // Make sure we don't leave latest_monitor_update_id incremented here: + self.latest_monitor_update_id -= 1; + debug_assert!(false, "Tried to fulfill an HTLC that was already fulfilled"); return Ok((None, None)); } }, @@ -1212,6 +1215,7 @@ impl Channel { log_warn!(self, "Have preimage and want to fulfill HTLC with pending failure against channel {}", log_bytes!(self.channel_id())); // TODO: We may actually be able to switch to a fulfill here, though its // rare enough it may not be worth the complexity burden. + debug_assert!(false, "Tried to fulfill an HTLC that was already failed"); return Ok((None, Some(monitor_update))); } }, @@ -1263,8 +1267,11 @@ impl Channel { } /// Per HTLC, only one get_update_fail_htlc or get_update_fulfill_htlc call may be made. - /// In such cases we debug_assert!(false) and return an IgnoreError. Thus, will always return - /// Ok(_) if debug assertions are turned on and preconditions are met. + /// In such cases we debug_assert!(false) and return a ChannelError::Ignore. Thus, will always + /// return Ok(_) if debug assertions are turned on or preconditions are met. + /// + /// Note that it is still possible to hit these assertions in case we find a preimage on-chain + /// but then have a reorg which settles on an HTLC-failure on chain. pub fn get_update_fail_htlc(&mut self, htlc_id_arg: u64, err_packet: msgs::OnionErrorPacket) -> Result, ChannelError> { if (self.channel_state & (ChannelState::ChannelFunded as u32)) != (ChannelState::ChannelFunded as u32) { panic!("Was asked to fail an HTLC when channel was not in an operational state"); @@ -1281,6 +1288,7 @@ impl Channel { match htlc.state { InboundHTLCState::Committed => {}, InboundHTLCState::LocalRemoved(_) => { + debug_assert!(false, "Tried to fail an HTLC that was already fail/fulfilled"); return Ok(None); }, _ => { @@ -1301,11 +1309,13 @@ impl Channel { match pending_update { &HTLCUpdateAwaitingACK::ClaimHTLC { htlc_id, .. } => { if htlc_id_arg == htlc_id { + debug_assert!(false, "Tried to fail an HTLC that was already fulfilled"); return Err(ChannelError::Ignore("Unable to find a pending HTLC which matched the given HTLC ID")); } }, &HTLCUpdateAwaitingACK::FailHTLC { htlc_id, .. } => { if htlc_id_arg == htlc_id { + debug_assert!(false, "Tried to fail an HTLC that was already failed"); return Err(ChannelError::Ignore("Unable to find a pending HTLC which matched the given HTLC ID")); } }, @@ -1656,6 +1666,9 @@ impl Channel { if msg.amount_msat > self.channel_value_satoshis * 1000 { return Err(ChannelError::Close("Remote side tried to send more than the total value of the channel")); } + if msg.amount_msat == 0 { + return Err(ChannelError::Close("Remote side tried to send a 0-msat HTLC")); + } if msg.amount_msat < self.our_htlc_minimum_msat { return Err(ChannelError::Close("Remote side tried to send less than our minimum HTLC value")); } @@ -3445,10 +3458,10 @@ impl Channel { my_current_per_commitment_point: PublicKey::from_secret_key(&self.secp_ctx, &self.build_local_commitment_secret(self.cur_local_commitment_transaction_number + 1)) }) } else { - log_debug!(self, "We don't seen yet any revoked secret, if this channnel has already been updated it means we are fallen-behind, you should wait for other peer closing"); + log_info!(self, "Sending a data_loss_protect with no previous remote per_commitment_secret"); OptionalField::Present(DataLossProtect { your_last_per_commitment_secret: [0;32], - my_current_per_commitment_point: PublicKey::from_secret_key(&self.secp_ctx, &self.build_local_commitment_secret(self.cur_local_commitment_transaction_number)) + my_current_per_commitment_point: PublicKey::from_secret_key(&self.secp_ctx, &self.build_local_commitment_secret(self.cur_local_commitment_transaction_number + 1)) }) }; msgs::ChannelReestablish { @@ -3493,6 +3506,11 @@ impl Channel { if amount_msat > self.channel_value_satoshis * 1000 { return Err(ChannelError::Ignore("Cannot send more than the total value of the channel")); } + + if amount_msat == 0 { + return Err(ChannelError::Ignore("Cannot send 0-msat HTLC")); + } + if amount_msat < self.their_htlc_minimum_msat { return Err(ChannelError::Ignore("Cannot send less than their minimum HTLC value")); } @@ -3756,7 +3774,7 @@ impl Channel { /// those explicitly stated to be allowed after shutdown completes, eg some simple getters). /// Also returns the list of payment_hashes for channels which we can safely fail backwards /// immediately (others we will have to allow to time out). - pub fn force_shutdown(&mut self) -> (Vec, Vec<(HTLCSource, PaymentHash)>) { + pub fn force_shutdown(&mut self, should_broadcast: bool) -> (Option, ChannelMonitorUpdate, Vec<(HTLCSource, PaymentHash)>) { assert!(self.channel_state != ChannelState::ShutdownComplete as u32); // We go ahead and "free" any holding cell HTLCs or HTLCs we haven't yet committed to and @@ -3779,12 +3797,11 @@ impl Channel { self.channel_state = ChannelState::ShutdownComplete as u32; self.update_time_counter += 1; - if self.channel_monitor.is_some() { - (self.channel_monitor.as_mut().unwrap().get_latest_local_commitment_txn(), dropped_outbound_htlcs) - } else { - // We aren't even signed funding yet, so can't broadcast anything - (Vec::new(), dropped_outbound_htlcs) - } + self.latest_monitor_update_id += 1; + (self.funding_txo.clone(), ChannelMonitorUpdate { + update_id: self.latest_monitor_update_id, + updates: vec![ChannelMonitorUpdateStep::ChannelForceClosed { should_broadcast }], + }, dropped_outbound_htlcs) } } @@ -4251,22 +4268,28 @@ impl ReadableArgs> for Channel = Arc::new(test_utils::TestLogger::new()); + let secp_ctx = Secp256k1::new(); + let mut seed = [0; 32]; + let mut rng = thread_rng(); + rng.fill_bytes(&mut seed); + let network = Network::Testnet; + let keys_provider = test_utils::TestKeysInterface::new(&seed, network, logger.clone() as Arc); + + // Go through the flow of opening a channel between two nodes. + + // Create Node A's channel + let node_a_node_id = PublicKey::from_secret_key(&secp_ctx, &SecretKey::from_slice(&[42; 32]).unwrap()); + let config = UserConfig::default(); + let mut node_a_chan = Channel::::new_outbound(&&feeest, &&keys_provider, node_a_node_id, 10000000, 100000, 42, Arc::clone(&logger), &config).unwrap(); + + // Create Node B's channel by receiving Node A's open_channel message + let open_channel_msg = node_a_chan.get_open_channel(genesis_block(network).header.bitcoin_hash(), &&feeest); + let node_b_node_id = PublicKey::from_secret_key(&secp_ctx, &SecretKey::from_slice(&[7; 32]).unwrap()); + let mut node_b_chan = Channel::::new_from_req(&&feeest, &&keys_provider, node_b_node_id, InitFeatures::supported(), &open_channel_msg, 7, logger, &config).unwrap(); + + // Node B --> Node A: accept channel + let accept_channel_msg = node_b_chan.get_accept_channel(); + node_a_chan.accept_channel(&accept_channel_msg, &config, InitFeatures::supported()).unwrap(); + + // Node A --> Node B: funding created + let output_script = node_a_chan.get_funding_redeemscript(); + let tx = Transaction { version: 1, lock_time: 0, input: Vec::new(), output: vec![TxOut { + value: 10000000, script_pubkey: output_script.clone(), + }]}; + let funding_outpoint = OutPoint::new(tx.txid(), 0); + let (funding_created_msg, _) = node_a_chan.get_outbound_funding_created(funding_outpoint).unwrap(); + let (funding_signed_msg, _) = node_b_chan.funding_created(&funding_created_msg).unwrap(); + + // Node B --> Node A: funding signed + let _ = node_a_chan.funding_signed(&funding_signed_msg); + + // Now disconnect the two nodes and check that the commitment point in + // Node B's channel_reestablish message is sane. + node_b_chan.remove_uncommitted_htlcs_and_mark_paused(); + let expected_commitment_point = PublicKey::from_secret_key(&secp_ctx, &node_b_chan.build_local_commitment_secret(node_b_chan.cur_local_commitment_transaction_number + 1)); + let msg = node_b_chan.get_channel_reestablish(); + match msg.data_loss_protect { + OptionalField::Present(DataLossProtect { my_current_per_commitment_point, .. }) => { + assert_eq!(expected_commitment_point, my_current_per_commitment_point); + }, + _ => panic!() + } + + // Check that the commitment point in Node A's channel_reestablish message + // is sane. + node_a_chan.remove_uncommitted_htlcs_and_mark_paused(); + let expected_commitment_point = PublicKey::from_secret_key(&secp_ctx, &node_a_chan.build_local_commitment_secret(node_a_chan.cur_local_commitment_transaction_number + 1)); + let msg = node_a_chan.get_channel_reestablish(); + match msg.data_loss_protect { + OptionalField::Present(DataLossProtect { my_current_per_commitment_point, .. }) => { + assert_eq!(expected_commitment_point, my_current_per_commitment_point); + }, + _ => panic!() + } + } + #[test] fn outbound_commitment_test() { // Test vectors from BOLT 3 Appendix C: @@ -4330,7 +4418,7 @@ mod tests { let logger : Arc = Arc::new(test_utils::TestLogger::new()); let secp_ctx = Secp256k1::new(); - let chan_keys = InMemoryChannelKeys::new( + let mut chan_keys = InMemoryChannelKeys::new( &secp_ctx, SecretKey::from_slice(&hex::decode("30ff4956bbdd3222d44cc5e8a1261dab1e07957bdac5ae88fe3261ef321f3749").unwrap()[..]).unwrap(), SecretKey::from_slice(&hex::decode("0fffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff").unwrap()[..]).unwrap(), @@ -4340,17 +4428,17 @@ mod tests { // These aren't set in the test vectors: [0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff], - 7000000000, + 10_000_000, ); assert_eq!(PublicKey::from_secret_key(&secp_ctx, chan_keys.funding_key()).serialize()[..], hex::decode("023da092f6980e58d2c037173180e9a465476026ee50f96695963e8efe436f54eb").unwrap()[..]); - let keys_provider = Keys { chan_keys }; + let keys_provider = Keys { chan_keys: chan_keys.clone() }; let their_node_id = PublicKey::from_secret_key(&secp_ctx, &SecretKey::from_slice(&[42; 32]).unwrap()); let mut config = UserConfig::default(); config.channel_options.announced_channel = false; - let mut chan = Channel::::new_outbound(&&feeest, &&keys_provider, their_node_id, 10000000, 100000, 42, Arc::clone(&logger), &config).unwrap(); // Nothing uses their network key in this test + let mut chan = Channel::::new_outbound(&&feeest, &&keys_provider, their_node_id, 10_000_000, 100000, 42, Arc::clone(&logger), &config).unwrap(); // Nothing uses their network key in this test chan.their_to_self_delay = 144; chan.our_dust_limit_satoshis = 546; @@ -4364,6 +4452,7 @@ mod tests { delayed_payment_basepoint: public_from_secret_hex(&secp_ctx, "1552dfba4f6cf29a62a0af13c8d6981d36d0ef8d61ba10fb0fe90da7634d7e13"), htlc_basepoint: public_from_secret_hex(&secp_ctx, "4444444444444444444444444444444444444444444444444444444444444444") }; + chan_keys.set_remote_channel_pubkeys(&their_pubkeys); assert_eq!(their_pubkeys.payment_basepoint.serialize()[..], hex::decode("032c0b7cf95324a07d05398b240174dc0c2be444d96b159aa6c7f7b1e668680991").unwrap()[..]); @@ -4387,6 +4476,7 @@ mod tests { let mut unsigned_tx: (Transaction, Vec); + let mut localtx; macro_rules! test_commitment { ( $their_sig_hex: expr, $our_sig_hex: expr, $tx_hex: expr) => { unsigned_tx = { @@ -4401,8 +4491,8 @@ mod tests { let sighash = Message::from_slice(&bip143::SighashComponents::new(&unsigned_tx.0).sighash_all(&unsigned_tx.0.input[0], &redeemscript, chan.channel_value_satoshis)[..]).unwrap(); secp_ctx.verify(&sighash, &their_signature, chan.their_funding_pubkey()).unwrap(); - let mut localtx = LocalCommitmentTransaction::new_missing_local_sig(unsigned_tx.0.clone(), &their_signature, &PublicKey::from_secret_key(&secp_ctx, chan.local_keys.funding_key()), chan.their_funding_pubkey()); - localtx.add_local_sig(chan.local_keys.funding_key(), &redeemscript, chan.channel_value_satoshis, &chan.secp_ctx); + localtx = LocalCommitmentTransaction::new_missing_local_sig(unsigned_tx.0.clone(), &their_signature, &PublicKey::from_secret_key(&secp_ctx, chan.local_keys.funding_key()), chan.their_funding_pubkey()); + chan_keys.sign_local_commitment(&mut localtx, &chan.secp_ctx); assert_eq!(serialize(localtx.with_valid_witness())[..], hex::decode($tx_hex).unwrap()[..]); @@ -4410,11 +4500,11 @@ mod tests { } macro_rules! test_htlc_output { - ( $htlc_idx: expr, $their_sig_hex: expr, $our_sig_hex: expr, $tx_hex: expr ) => { + ( $htlc_idx: expr, $their_sig_hex: expr, $our_sig_hex: expr, $tx_hex: expr) => { let remote_signature = Signature::from_der(&hex::decode($their_sig_hex).unwrap()[..]).unwrap(); let ref htlc = unsigned_tx.1[$htlc_idx]; - let mut htlc_tx = chan.build_htlc_transaction(&unsigned_tx.0.txid(), &htlc, true, &keys, chan.feerate_per_kw); + let htlc_tx = chan.build_htlc_transaction(&unsigned_tx.0.txid(), &htlc, true, &keys, chan.feerate_per_kw); let htlc_redeemscript = chan_utils::get_htlc_redeemscript(&htlc, &keys); let htlc_sighash = Message::from_slice(&bip143::SighashComponents::new(&htlc_tx).sighash_all(&htlc_tx.input[0], &htlc_redeemscript, htlc.amount_msat / 1000)[..]).unwrap(); secp_ctx.verify(&htlc_sighash, &remote_signature, &keys.b_htlc_key).unwrap(); @@ -4431,8 +4521,12 @@ mod tests { assert!(preimage.is_some()); } - chan_utils::sign_htlc_transaction(&mut htlc_tx, &remote_signature, &preimage, &htlc, &keys.a_htlc_key, &keys.b_htlc_key, &keys.revocation_key, &keys.per_commitment_point, chan.local_keys.htlc_base_key(), &chan.secp_ctx).unwrap(); - assert_eq!(serialize(&htlc_tx)[..], + let mut per_htlc = Vec::new(); + per_htlc.push((htlc.clone(), Some(remote_signature), None)); + localtx.set_htlc_cache(keys.clone(), chan.feerate_per_kw, per_htlc); + chan_keys.sign_htlc_transaction(&mut localtx, $htlc_idx, preimage, chan.their_to_self_delay, &chan.secp_ctx); + + assert_eq!(serialize(localtx.htlc_with_valid_witness($htlc_idx).as_ref().unwrap())[..], hex::decode($tx_hex).unwrap()[..]); }; }