Fix various spec bugs, can now open channels with real nodes!

 * commitment transaction number, as used in locktime/sequence
   fields is actually different from commitment transaction number,
   as used for revocation state. This is confusing and never stated
   in the spec, so we have to do the conversion.
 * max_htlc_value_in_flight is never constrained in the spec, but
   we were requiring it be <= channel size. Instead just clamp the
   values the peer sends us when storing.
 * channel_id calculation was incorrect, we now do some crazy
   conversion hops, which we shouldn't, but will need to change our
   types to fix.
 * Our channel_reserve_satoshis value was too low, just change the
   constant and leave the TODO to figure out what it really should
   be for now.

diff --git a/src/chain/transaction.rs b/src/chain/transaction.rs
index dad3c1271e4f0e5459691d44ddd092b101e6299c..42a4f952e4150550f240c9c59d5d89a9a885c7de 100644 (file)
--- a/src/chain/transaction.rs
+++ b/src/chain/transaction.rs
@@ -20,7 +20,9 @@ impl OutPoint {
 
        /// Convert an `OutPoint` to a lightning channel id.
        pub fn to_channel_id(&self) -> Uint256 {
-               // TODO: or le?
-               self.txid.into_be() ^ Uint256::from_u64(self.index as u64).unwrap()
+               let mut index = [0; 32];
+               index[30] = ((self.index >> 8) & 0xff) as u8;
+               index[31] = ((self.index >> 0) & 0xff) as u8;
+               self.txid.into_le() ^ Sha256dHash::from(&index[..]).into_le()
        }
 }

diff --git a/src/ln/channel.rs b/src/ln/channel.rs
index 93e4a2cf3be5d12f0d903725b0ddcd32e6a2f1a7..f36ce1c506f767e4f1b0821e96bc981813abdd4d 100644 (file)
--- a/src/ln/channel.rs
+++ b/src/ln/channel.rs
@@ -245,6 +245,10 @@ pub struct Channel {
 
        local_keys: ChannelKeys,
 
+       // Our commitment numbers start at 2^48-1 and count down, whereas the ones used in transaction
+       // generation start at 0 and count up...this simplifies some parts of implementation at the
+       // cost of others, but should really just be changed.
+
        cur_local_commitment_transaction_number: u64,
        cur_remote_commitment_transaction_number: u64,
        value_to_self_msat: u64, // Excluding all pending_htlcs, excluding fees
@@ -343,7 +347,7 @@ impl Channel {
 
        /// Guaranteed to return a value no larger than channel_value_satoshis
        fn get_our_channel_reserve_satoshis(channel_value_satoshis: u64) -> u64 {
-               cmp::min(channel_value_satoshis, 10) //TODO
+               cmp::min(channel_value_satoshis, 1000) //TODO
        }
 
        fn derive_our_dust_limit_satoshis(at_open_background_feerate: u64) -> u64 {
@@ -456,9 +460,6 @@ impl Channel {
                if msg.dust_limit_satoshis > msg.funding_satoshis {
                        return Err(HandleError{err: "Peer never wants payout outputs?", msg: Some(msgs::ErrorAction::DisconnectPeer{})});
                }
-               if msg.max_htlc_value_in_flight_msat > msg.funding_satoshis * 1000 {
-                       return Err(HandleError{err: "Bogus max_htlc_value_in_flight_satoshis", msg: Some(msgs::ErrorAction::DisconnectPeer{})});
-               }
                if msg.htlc_minimum_msat >= (msg.funding_satoshis - msg.channel_reserve_satoshis) * 1000 {
                        return Err(HandleError{err: "Minimum htlc value is full channel value", msg: Some(msgs::ErrorAction::DisconnectPeer{})});
                }
@@ -521,7 +522,7 @@ impl Channel {
                        channel_value_satoshis: msg.funding_satoshis,
                        their_dust_limit_satoshis: msg.dust_limit_satoshis,
                        our_dust_limit_satoshis: Channel::derive_our_dust_limit_satoshis(background_feerate),
-                       their_max_htlc_value_in_flight_msat: msg.max_htlc_value_in_flight_msat,
+                       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),
@@ -595,7 +596,7 @@ impl Channel {
        /// which peer generated this transaction and "to whom" this transaction flows.
        #[inline]
        fn build_commitment_transaction(&self, commitment_number: u64, keys: &TxCreationKeys, local: bool, generated_by_local: bool) -> (Transaction, Vec<HTLCOutputInCommitment>) {
-               let obscured_commitment_transaction_number = self.get_commitment_transaction_number_obscure_factor() ^ commitment_number;
+               let obscured_commitment_transaction_number = self.get_commitment_transaction_number_obscure_factor() ^ (0xffffffffffff - commitment_number);
 
                let txins = {
                        let mut ins: Vec<TxIn> = Vec::new();
@@ -1081,9 +1082,6 @@ impl Channel {
                if msg.dust_limit_satoshis > 21000000 * 100000000 {
                        return Err(HandleError{err: "Peer never wants payout outputs?", msg: None});
                }
-               if msg.max_htlc_value_in_flight_msat > self.channel_value_satoshis * 1000 {
-                       return Err(HandleError{err: "Bogus max_htlc_value_in_flight_satoshis", msg: None});
-               }
                if msg.channel_reserve_satoshis > self.channel_value_satoshis {
                        return Err(HandleError{err: "Bogus channel_reserve_satoshis", msg: None});
                }
@@ -1101,7 +1099,7 @@ impl Channel {
                self.channel_monitor.set_their_htlc_base_key(&msg.htlc_basepoint);
 
                self.their_dust_limit_satoshis = msg.dust_limit_satoshis;
-               self.their_max_htlc_value_in_flight_msat = msg.max_htlc_value_in_flight_msat;
+               self.their_max_htlc_value_in_flight_msat = cmp::min(msg.max_htlc_value_in_flight_msat, self.channel_value_satoshis * 1000);
                self.their_channel_reserve_satoshis = msg.channel_reserve_satoshis;
                self.their_htlc_minimum_msat = msg.htlc_minimum_msat;
                self.their_to_self_delay = msg.to_self_delay;
@@ -2407,7 +2405,7 @@ mod tests {
 
                macro_rules! test_commitment {
                        ( $their_sig_hex: expr, $our_sig_hex: expr, $tx_hex: expr) => {
-                               unsigned_tx = chan.build_commitment_transaction(42, &keys, true, false);
+                               unsigned_tx = chan.build_commitment_transaction(0xffffffffffff - 42, &keys, true, false);
                                let their_signature = Signature::from_der(&secp_ctx, &hex_bytes($their_sig_hex).unwrap()[..]).unwrap();
                                let sighash = Message::from_slice(&bip143::SighashComponents::new(&unsigned_tx.0).sighash_all(&unsigned_tx.0.input[0], &chan.get_funding_redeemscript(), chan.channel_value_satoshis)[..]).unwrap();
                                secp_ctx.verify(&sighash, &their_signature, &chan.their_funding_pubkey).unwrap();

diff --git a/src/ln/channelmonitor.rs b/src/ln/channelmonitor.rs
index 8c3fcc3f0a02ad21dbb838759241e57eec4e70b7..00b348002dd3faf9a63f4e6c458b088c161eda01 100644 (file)
--- a/src/ln/channelmonitor.rs
+++ b/src/ln/channelmonitor.rs
@@ -910,7 +910,7 @@ impl ChannelMonitor {
                let commitment_txid = tx.txid(); //TODO: This is gonna be a performance bottleneck for watchtowers!
                let per_commitment_option = self.remote_claimable_outpoints.get(&commitment_txid);
 
-               let commitment_number = (((tx.input[0].sequence as u64 & 0xffffff) << 3*8) | (tx.lock_time as u64 & 0xffffff)) ^ self.commitment_transaction_number_obscure_factor;
+               let commitment_number = 0xffffffffffff - ((((tx.input[0].sequence as u64 & 0xffffff) << 3*8) | (tx.lock_time as u64 & 0xffffff)) ^ self.commitment_transaction_number_obscure_factor);
                if commitment_number >= self.get_min_seen_secret() {
                        let secret = self.get_secret(commitment_number).unwrap();
                        let per_commitment_key = ignore_error!(SecretKey::from_slice(&self.secp_ctx, &secret));