Merge pull request #634 from valentinewallace/improve-inbound-htlc-docs
[rust-lightning] / lightning / src / ln / channel.rs
index 0c30a7e96cb53bfec845174a2417e002aedce211..d754b393c389bc441089310c49856005d78066be 100644 (file)
@@ -53,19 +53,43 @@ enum InboundHTLCRemovalReason {
 }
 
 enum InboundHTLCState {
-       /// Added by remote, to be included in next local commitment tx.
+       /// Offered by remote, to be included in next local commitment tx. I.e., the remote sent an
+       /// update_add_htlc message for this HTLC.
        RemoteAnnounced(PendingHTLCStatus),
-       /// Included in a received commitment_signed message (implying we've revoke_and_ack'ed it), but
-       /// the remote side hasn't yet revoked their previous state, which we need them to do before we
-       /// accept this HTLC. Implies AwaitingRemoteRevoke.
-       /// We also have not yet included this HTLC in a commitment_signed message, and are waiting on
-       /// a remote revoke_and_ack on a previous state before we can do so.
+       /// Included in a received commitment_signed message (implying we've
+       /// revoke_and_ack'd it), but the remote hasn't yet revoked their previous
+       /// state (see the example below). We have not yet included this HTLC in a
+       /// commitment_signed message because we are waiting on the remote's
+       /// aforementioned state revocation. One reason this missing remote RAA
+       /// (revoke_and_ack) blocks us from constructing a commitment_signed message
+       /// is because every time we create a new "state", i.e. every time we sign a
+       /// new commitment tx (see [BOLT #2]), we need a new per_commitment_point,
+       /// which are provided one-at-a-time in each RAA. E.g., the last RAA they
+       /// sent provided the per_commitment_point for our current commitment tx.
+       /// The other reason we should not send a commitment_signed without their RAA
+       /// is because their RAA serves to ACK our previous commitment_signed.
+       ///
+       /// Here's an example of how an HTLC could come to be in this state:
+       /// remote --> update_add_htlc(prev_htlc)   --> local
+       /// remote --> commitment_signed(prev_htlc) --> local
+       /// remote <-- revoke_and_ack               <-- local
+       /// remote <-- commitment_signed(prev_htlc) <-- local
+       /// [note that here, the remote does not respond with a RAA]
+       /// remote --> update_add_htlc(this_htlc)   --> local
+       /// remote --> commitment_signed(prev_htlc, this_htlc) --> local
+       /// Now `this_htlc` will be assigned this state. It's unable to be officially
+       /// accepted, i.e. included in a commitment_signed, because we're missing the
+       /// RAA that provides our next per_commitment_point. The per_commitment_point
+       /// is used to derive commitment keys, which are used to construct the
+       /// signatures in a commitment_signed message.
+       /// Implies AwaitingRemoteRevoke.
+       /// [BOLT #2]: https://github.com/lightningnetwork/lightning-rfc/blob/master/02-peer-protocol.md
        AwaitingRemoteRevokeToAnnounce(PendingHTLCStatus),
-       /// Included in a received commitment_signed message (implying we've revoke_and_ack'ed it), but
-       /// the remote side hasn't yet revoked their previous state, which we need them to do before we
-       /// accept this HTLC. Implies AwaitingRemoteRevoke.
-       /// We have included this HTLC in our latest commitment_signed and are now just waiting on a
-       /// revoke_and_ack.
+       /// Included in a received commitment_signed message (implying we've revoke_and_ack'd it).
+       /// We have also included this HTLC in our latest commitment_signed and are now just waiting
+       /// on the remote's revoke_and_ack to make this HTLC an irrevocable part of the state of the
+       /// channel (before it can then get forwarded and/or removed).
+       /// Implies AwaitingRemoteRevoke.
        AwaitingAnnouncedRemoteRevoke(PendingHTLCStatus),
        Committed,
        /// Removed by us and a new commitment_signed was sent (if we were AwaitingRemoteRevoke when we
@@ -1095,10 +1119,10 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
        fn build_local_transaction_keys(&self, commitment_number: u64) -> Result<TxCreationKeys, ChannelError> {
                let per_commitment_point = PublicKey::from_secret_key(&self.secp_ctx, &self.build_local_commitment_secret(commitment_number));
                let delayed_payment_base = &self.local_keys.pubkeys().delayed_payment_basepoint;
-               let htlc_basepoint = PublicKey::from_secret_key(&self.secp_ctx, self.local_keys.htlc_base_key());
+               let htlc_basepoint = &self.local_keys.pubkeys().htlc_basepoint;
                let their_pubkeys = self.their_pubkeys.as_ref().unwrap();
 
-               Ok(secp_check!(TxCreationKeys::new(&self.secp_ctx, &per_commitment_point, delayed_payment_base, &htlc_basepoint, &their_pubkeys.revocation_basepoint, &their_pubkeys.htlc_basepoint), "Local tx keys generation got bogus keys"))
+               Ok(secp_check!(TxCreationKeys::new(&self.secp_ctx, &per_commitment_point, delayed_payment_base, htlc_basepoint, &their_pubkeys.revocation_basepoint, &their_pubkeys.htlc_basepoint), "Local tx keys generation got bogus keys"))
        }
 
        #[inline]
@@ -1109,10 +1133,10 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
                //TODO: Ensure that the payment_key derived here ends up in the library users' wallet as we
                //may see payments to it!
                let revocation_basepoint = &self.local_keys.pubkeys().revocation_basepoint;
-               let htlc_basepoint = PublicKey::from_secret_key(&self.secp_ctx, self.local_keys.htlc_base_key());
+               let htlc_basepoint = &self.local_keys.pubkeys().htlc_basepoint;
                let their_pubkeys = self.their_pubkeys.as_ref().unwrap();
 
-               Ok(secp_check!(TxCreationKeys::new(&self.secp_ctx, &self.their_cur_commitment_point.unwrap(), &their_pubkeys.delayed_payment_basepoint, &their_pubkeys.htlc_basepoint, revocation_basepoint, &htlc_basepoint), "Remote tx keys generation got bogus keys"))
+               Ok(secp_check!(TxCreationKeys::new(&self.secp_ctx, &self.their_cur_commitment_point.unwrap(), &their_pubkeys.delayed_payment_basepoint, &their_pubkeys.htlc_basepoint, revocation_basepoint, htlc_basepoint), "Remote tx keys generation got bogus keys"))
        }
 
        /// Gets the redeemscript for the funding transaction output (ie the funding transaction output
@@ -3318,7 +3342,7 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
                        revocation_basepoint: local_keys.revocation_basepoint,
                        payment_point: local_keys.payment_point,
                        delayed_payment_basepoint: local_keys.delayed_payment_basepoint,
-                       htlc_basepoint: PublicKey::from_secret_key(&self.secp_ctx, self.local_keys.htlc_base_key()),
+                       htlc_basepoint: local_keys.htlc_basepoint,
                        first_per_commitment_point: PublicKey::from_secret_key(&self.secp_ctx, &local_commitment_secret),
                        channel_flags: if self.config.announced_channel {1} else {0},
                        shutdown_scriptpubkey: OptionalField::Present(if self.config.commit_upfront_shutdown_pubkey { self.get_closing_scriptpubkey() } else { Builder::new().into_script() })
@@ -3352,7 +3376,7 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
                        revocation_basepoint: local_keys.revocation_basepoint,
                        payment_point: local_keys.payment_point,
                        delayed_payment_basepoint: local_keys.delayed_payment_basepoint,
-                       htlc_basepoint: PublicKey::from_secret_key(&self.secp_ctx, self.local_keys.htlc_base_key()),
+                       htlc_basepoint: local_keys.htlc_basepoint,
                        first_per_commitment_point: PublicKey::from_secret_key(&self.secp_ctx, &local_commitment_secret),
                        shutdown_scriptpubkey: OptionalField::Present(if self.config.commit_upfront_shutdown_pubkey { self.get_closing_scriptpubkey() } else { Builder::new().into_script() })
                }
@@ -4479,8 +4503,8 @@ mod tests {
                let delayed_payment_base = &chan.local_keys.pubkeys().delayed_payment_basepoint;
                let per_commitment_secret = SecretKey::from_slice(&hex::decode("1f1e1d1c1b1a191817161514131211100f0e0d0c0b0a09080706050403020100").unwrap()[..]).unwrap();
                let per_commitment_point = PublicKey::from_secret_key(&secp_ctx, &per_commitment_secret);
-               let htlc_basepoint = PublicKey::from_secret_key(&secp_ctx, chan.local_keys.htlc_base_key());
-               let keys = TxCreationKeys::new(&secp_ctx, &per_commitment_point, delayed_payment_base, &htlc_basepoint, &their_pubkeys.revocation_basepoint, &their_pubkeys.htlc_basepoint).unwrap();
+               let htlc_basepoint = &chan.local_keys.pubkeys().htlc_basepoint;
+               let keys = TxCreationKeys::new(&secp_ctx, &per_commitment_point, delayed_payment_base, htlc_basepoint, &their_pubkeys.revocation_basepoint, &their_pubkeys.htlc_basepoint).unwrap();
 
                chan.their_pubkeys = Some(their_pubkeys);