]> git.bitcoin.ninja Git - rust-lightning/commitdiff
Drop HTLCState::LocalRemovedAwaitingCommitment
authorMatt Corallo <git@bluematt.me>
Wed, 5 Sep 2018 18:47:43 +0000 (14:47 -0400)
committerMatt Corallo <git@bluematt.me>
Wed, 5 Sep 2018 23:47:01 +0000 (19:47 -0400)
This was redundant and was included because the HTLC still needed
to be monitored, but that happens in ChannelMonitor, so there is no
need for it in Channel itself.

src/ln/channel.rs

index e5538a0599d2bdab566c37fe97f649e6672b48eb..b2d6a70e13ae3b5f79910df694a667781fdc1273 100644 (file)
@@ -138,18 +138,16 @@ enum HTLCState {
        AwaitingRemovedRemoteRevoke,
        /// Removed by us and a new commitment_signed was sent (if we were AwaitingRemoteRevoke when we
        /// created it we would have put it in the holding cell instead). When they next revoke_and_ack
-       /// we'll promote to LocalRemovedAwaitingCommitment if we fulfilled, otherwise we'll drop at
-       /// that point.
+       /// we'll drop it.
        /// Note that we have to keep an eye on the HTLC until we've received a broadcastable
        /// commitment transaction without it as otherwise we'll have to force-close the channel to
        /// claim it before the timeout (obviously doesn't apply to revoked HTLCs that we can't claim
-       /// anyway).
+       /// anyway). That said, ChannelMonitor does this for us (see
+       /// ChannelMonitor::would_broadcast_at_height) so we actually remove the HTLC from our own
+       /// local state before then, once we're sure that the next commitment_signed and
+       /// ChannelMonitor::provide_latest_local_commitment_tx_info will not include this HTLC.
        /// Implies HTLCOutput::outbound: false
        LocalRemoved,
-       /// Removed by us, sent a new commitment_signed and got a revoke_and_ack. Just waiting on an
-       /// updated local commitment transaction. Implies local_removed_fulfilled.
-       /// Implies HTLCOutput::outbound: false
-       LocalRemovedAwaitingCommitment,
 }
 
 struct HTLCOutput { //TODO: Refactor into Outbound/InboundHTLCOutput (will save memory and fewer panics)
@@ -706,7 +704,6 @@ impl Channel {
                                HTLCState::AwaitingRemoteRevokeToRemove => generated_by_local,
                                HTLCState::AwaitingRemovedRemoteRevoke => false,
                                HTLCState::LocalRemoved => !generated_by_local,
-                               HTLCState::LocalRemovedAwaitingCommitment => false,
                        };
 
                        if include {
@@ -749,10 +746,6 @@ impl Channel {
                                                        value_to_self_msat_offset += htlc.amount_msat as i64;
                                                }
                                        },
-                                       HTLCState::LocalRemovedAwaitingCommitment => {
-                                               assert!(htlc.local_removed_fulfilled);
-                                               value_to_self_msat_offset += htlc.amount_msat as i64;
-                                       },
                                        _ => {},
                                }
                        }
@@ -1018,7 +1011,7 @@ impl Channel {
                let mut pending_idx = std::usize::MAX;
                for (idx, htlc) in self.pending_htlcs.iter().enumerate() {
                        if !htlc.outbound && htlc.payment_hash == payment_hash_calc &&
-                                       htlc.state != HTLCState::LocalRemoved && htlc.state != HTLCState::LocalRemovedAwaitingCommitment {
+                                       htlc.state != HTLCState::LocalRemoved {
                                if let Some(PendingHTLCStatus::Fail(_)) = htlc.pending_forward_state {
                                } else {
                                        if pending_idx != std::usize::MAX {
@@ -1143,7 +1136,7 @@ impl Channel {
                                                // we'll fail this one as soon as remote commits to it.
                                                continue;
                                        }
-                               } else if htlc.state == HTLCState::LocalRemoved || htlc.state == HTLCState::LocalRemovedAwaitingCommitment {
+                               } else if htlc.state == HTLCState::LocalRemoved {
                                        return Err(HandleError{err: "Unable to find a pending HTLC which matched the given payment preimage", action: None});
                                } else {
                                        panic!("Have an inbound HTLC when not awaiting remote revoke that had a garbage state");
@@ -1379,7 +1372,6 @@ impl Channel {
                                HTLCState::AwaitingRemoteRevokeToRemove => { if for_remote_update_check { continue; } },
                                HTLCState::AwaitingRemovedRemoteRevoke => { if for_remote_update_check { continue; } },
                                HTLCState::LocalRemoved => {},
-                               HTLCState::LocalRemovedAwaitingCommitment => { if for_remote_update_check { continue; } },
                        }
                        if !htlc.outbound {
                                inbound_htlc_count += 1;
@@ -1556,16 +1548,6 @@ impl Channel {
                                need_our_commitment = true;
                        }
                }
-               // Finally delete all the LocalRemovedAwaitingCommitment HTLCs
-               // We really shouldnt have two passes here, but retain gives a non-mutable ref (Rust bug)
-               let mut claimed_value_msat = 0;
-               self.pending_htlcs.retain(|htlc| {
-                       if htlc.state == HTLCState::LocalRemovedAwaitingCommitment {
-                               claimed_value_msat += htlc.amount_msat;
-                               false
-                       } else { true }
-               });
-               self.value_to_self_msat += claimed_value_msat;
 
                self.cur_local_commitment_transaction_number -= 1;
                self.last_local_commitment_txn = new_local_commitment_txn;
@@ -1689,7 +1671,10 @@ impl Channel {
                // We really shouldnt have two passes here, but retain gives a non-mutable ref (Rust bug)
                self.pending_htlcs.retain(|htlc| {
                        if htlc.state == HTLCState::LocalRemoved {
-                               if htlc.local_removed_fulfilled { true } else { false }
+                               if htlc.local_removed_fulfilled {
+                                       value_to_self_msat_diff += htlc.amount_msat as i64;
+                               }
+                               false
                        } else if htlc.state == HTLCState::AwaitingRemovedRemoteRevoke {
                                if let Some(reason) = htlc.fail_reason.clone() { // We really want take() here, but, again, non-mut ref :(
                                        revoked_htlcs.push((htlc.payment_hash, reason));
@@ -1724,9 +1709,6 @@ impl Channel {
                        } else if htlc.state == HTLCState::AwaitingRemoteRevokeToRemove {
                                htlc.state = HTLCState::AwaitingRemovedRemoteRevoke;
                                require_commitment = true;
-                       } else if htlc.state == HTLCState::LocalRemoved {
-                               assert!(htlc.local_removed_fulfilled);
-                               htlc.state = HTLCState::LocalRemovedAwaitingCommitment;
                        }
                }
                self.value_to_self_msat = (self.value_to_self_msat as i64 + value_to_self_msat_diff) as u64;