Include an `outbound_payment` flag in `MaybeTimeoutClaimableHTLC`

When the user is fetching their current balances after forwarding a
payment (before it clears), they'll see a
`MaybePreimageClaimableHTLC` and a `MaybeTimeoutClaimableHTLC` but
if they sum up their balance using
`Balance::claimable_amount_satoshis` neither will be included.

Obviously, exactly one of the two balances should be included - one
of the two resolutions should happen in our favor. This causes our
visible balance to fluctuate up and down by the full value of any
HTLCs we're in the middle of forwarding, which is incredibly
confusing to see. If we want to stop the fluctuations, we need to
pick one of the two balances to include. The obvious candidate is
`MaybeTimeoutClaimableHTLC` as it is the lower of the two, and
represents our balance without the fee we'd receive from the
forward.

Sadly, if we always include it, we'll end up also including any
HTLCs which we've sent but which haven't yet been claimed by their
recipient, which is the wrong behavior.

Luckily, we have access to the `Option<HTLCSource>` while walking
HTLCs, which allows us to add an `outbound_payment` flag to
`MaybeTimeoutClaimableHTLC`. This allows us to only include
forwarded payments in `claimable_amount_satoshis`.

Sadly, even with this in place our balance still fluctuates by the
changes in the commitment transaction fees we have to pay during
forwarding, but addressing that is left for later.

diff --git a/lightning/src/chain/channelmonitor.rs b/lightning/src/chain/channelmonitor.rs
index af0f56d882ad591531163df08f9aa05516af92a3..3845328cda43f7a1c29e0cf3529b6d13b41bec7d 100644 (file)
--- a/lightning/src/chain/channelmonitor.rs
+++ b/lightning/src/chain/channelmonitor.rs
@@ -620,6 +620,10 @@ pub enum Balance {
                claimable_height: u32,
                /// The payment hash whose preimage our counterparty needs to claim this HTLC.
                payment_hash: PaymentHash,
+               /// Whether this HTLC represents a payment which was sent outbound from us. Otherwise it
+               /// represents an HTLC which was forwarded (and should, thus, have a corresponding inbound
+               /// edge on another channel).
+               outbound_payment: bool,
        },
        /// HTLCs which we received from our counterparty which are claimable with a preimage which we
        /// do not currently have. This will only be claimable if we receive the preimage from the node
@@ -662,9 +666,9 @@ impl Balance {
                        Balance::ContentiousClaimable { amount_satoshis, .. }|
                        Balance::CounterpartyRevokedOutputClaimable { amount_satoshis, .. }
                                => *amount_satoshis,
-                       Balance::MaybeTimeoutClaimableHTLC { .. }|
-                       Balance::MaybePreimageClaimableHTLC { .. }
-                               => 0,
+                       Balance::MaybeTimeoutClaimableHTLC { amount_satoshis, outbound_payment, .. }
+                               => if *outbound_payment { 0 } else { *amount_satoshis },
+                       Balance::MaybePreimageClaimableHTLC { .. } => 0,
                }
        }
 }
@@ -1719,9 +1723,10 @@ impl<Signer: WriteableEcdsaChannelSigner> ChannelMonitor<Signer> {
 impl<Signer: WriteableEcdsaChannelSigner> ChannelMonitorImpl<Signer> {
        /// Helper for get_claimable_balances which does the work for an individual HTLC, generating up
        /// to one `Balance` for the HTLC.
-       fn get_htlc_balance(&self, htlc: &HTLCOutputInCommitment, holder_commitment: bool,
-               counterparty_revoked_commitment: bool, confirmed_txid: Option<Txid>)
-       -> Option<Balance> {
+       fn get_htlc_balance(&self, htlc: &HTLCOutputInCommitment, source: Option<&HTLCSource>,
+               holder_commitment: bool, counterparty_revoked_commitment: bool,
+               confirmed_txid: Option<Txid>
+       ) -> Option<Balance> {
                let htlc_commitment_tx_output_idx =
                        if let Some(v) = htlc.transaction_output_index { v } else { return None; };
 
@@ -1858,10 +1863,19 @@ impl<Signer: WriteableEcdsaChannelSigner> ChannelMonitorImpl<Signer> {
                                        confirmation_height: conf_thresh,
                                });
                        } else {
+                               let outbound_payment = match source {
+                                       None => {
+                                               debug_assert!(false, "Outbound HTLCs should have a source");
+                                               true
+                                       },
+                                       Some(&HTLCSource::PreviousHopData(_)) => false,
+                                       Some(&HTLCSource::OutboundRoute { .. }) => true,
+                               };
                                return Some(Balance::MaybeTimeoutClaimableHTLC {
                                        amount_satoshis: htlc.amount_msat / 1000,
                                        claimable_height: htlc.cltv_expiry,
                                        payment_hash: htlc.payment_hash,
+                                       outbound_payment,
                                });
                        }
                } else if let Some(payment_preimage) = self.payment_preimages.get(&htlc.payment_hash) {
@@ -1934,10 +1948,12 @@ impl<Signer: WriteableEcdsaChannelSigner> ChannelMonitor<Signer> {
 
                macro_rules! walk_htlcs {
                        ($holder_commitment: expr, $counterparty_revoked_commitment: expr, $htlc_iter: expr) => {
-                               for htlc in $htlc_iter {
+                               for (htlc, source) in $htlc_iter {
                                        if htlc.transaction_output_index.is_some() {
 
-                                               if let Some(bal) = us.get_htlc_balance(htlc, $holder_commitment, $counterparty_revoked_commitment, confirmed_txid) {
+                                               if let Some(bal) = us.get_htlc_balance(
+                                                       htlc, source, $holder_commitment, $counterparty_revoked_commitment, confirmed_txid
+                                               ) {
                                                        res.push(bal);
                                                }
                                        }
@@ -1968,9 +1984,9 @@ impl<Signer: WriteableEcdsaChannelSigner> ChannelMonitor<Signer> {
                                        }
                                }
                                if Some(txid) == us.current_counterparty_commitment_txid || Some(txid) == us.prev_counterparty_commitment_txid {
-                                       walk_htlcs!(false, false, counterparty_tx_htlcs.iter().map(|(a, _)| a));
+                                       walk_htlcs!(false, false, counterparty_tx_htlcs.iter().map(|(a, b)| (a, b.as_ref().map(|b| &**b))));
                                } else {
-                                       walk_htlcs!(false, true, counterparty_tx_htlcs.iter().map(|(a, _)| a));
+                                       walk_htlcs!(false, true, counterparty_tx_htlcs.iter().map(|(a, b)| (a, b.as_ref().map(|b| &**b))));
                                        // The counterparty broadcasted a revoked state!
                                        // Look for any StaticOutputs first, generating claimable balances for those.
                                        // If any match the confirmed counterparty revoked to_self output, skip
@@ -2010,7 +2026,7 @@ impl<Signer: WriteableEcdsaChannelSigner> ChannelMonitor<Signer> {
                                }
                                found_commitment_tx = true;
                        } else if txid == us.current_holder_commitment_tx.txid {
-                               walk_htlcs!(true, false, us.current_holder_commitment_tx.htlc_outputs.iter().map(|(a, _, _)| a));
+                               walk_htlcs!(true, false, us.current_holder_commitment_tx.htlc_outputs.iter().map(|(a, _, c)| (a, c.as_ref())));
                                if let Some(conf_thresh) = pending_commitment_tx_conf_thresh {
                                        res.push(Balance::ClaimableAwaitingConfirmations {
                                                amount_satoshis: us.current_holder_commitment_tx.to_self_value_sat,
@@ -2020,7 +2036,7 @@ impl<Signer: WriteableEcdsaChannelSigner> ChannelMonitor<Signer> {
                                found_commitment_tx = true;
                        } else if let Some(prev_commitment) = &us.prev_holder_signed_commitment_tx {
                                if txid == prev_commitment.txid {
-                                       walk_htlcs!(true, false, prev_commitment.htlc_outputs.iter().map(|(a, _, _)| a));
+                                       walk_htlcs!(true, false, prev_commitment.htlc_outputs.iter().map(|(a, _, c)| (a, c.as_ref())));
                                        if let Some(conf_thresh) = pending_commitment_tx_conf_thresh {
                                                res.push(Balance::ClaimableAwaitingConfirmations {
                                                        amount_satoshis: prev_commitment.to_self_value_sat,
@@ -2043,13 +2059,22 @@ impl<Signer: WriteableEcdsaChannelSigner> ChannelMonitor<Signer> {
                        }
                } else {
                        let mut claimable_inbound_htlc_value_sat = 0;
-                       for (htlc, _, _) in us.current_holder_commitment_tx.htlc_outputs.iter() {
+                       for (htlc, _, source) in us.current_holder_commitment_tx.htlc_outputs.iter() {
                                if htlc.transaction_output_index.is_none() { continue; }
                                if htlc.offered {
+                                       let outbound_payment = match source {
+                                               None => {
+                                                       debug_assert!(false, "Outbound HTLCs should have a source");
+                                                       true
+                                               },
+                                               Some(HTLCSource::PreviousHopData(_)) => false,
+                                               Some(HTLCSource::OutboundRoute { .. }) => true,
+                                       };
                                        res.push(Balance::MaybeTimeoutClaimableHTLC {
                                                amount_satoshis: htlc.amount_msat / 1000,
                                                claimable_height: htlc.cltv_expiry,
                                                payment_hash: htlc.payment_hash,
+                                               outbound_payment,
                                        });
                                } else if us.payment_preimages.get(&htlc.payment_hash).is_some() {
                                        claimable_inbound_htlc_value_sat += htlc.amount_msat / 1000;

diff --git a/lightning/src/ln/monitor_tests.rs b/lightning/src/ln/monitor_tests.rs
index 60caed4dbdc8c0fe4974137a233bc733e8886603..685ec75c7e9a4d16fcceeea09d7ad3eccad77bf6 100644 (file)
--- a/lightning/src/ln/monitor_tests.rs
+++ b/lightning/src/ln/monitor_tests.rs
@@ -346,11 +346,13 @@ fn do_test_claim_value_force_close(anchors: bool, prev_commitment_tx: bool) {
                amount_satoshis: 3_000,
                claimable_height: htlc_cltv_timeout,
                payment_hash,
+               outbound_payment: true,
        };
        let sent_htlc_timeout_balance = Balance::MaybeTimeoutClaimableHTLC {
                amount_satoshis: 4_000,
                claimable_height: htlc_cltv_timeout,
                payment_hash: timeout_payment_hash,
+               outbound_payment: true,
        };
        let received_htlc_balance = Balance::MaybePreimageClaimableHTLC {
                amount_satoshis: 3_000,
@@ -746,11 +748,13 @@ fn do_test_balances_on_local_commitment_htlcs(anchors: bool) {
                amount_satoshis: 10_000,
                claimable_height: htlc_cltv_timeout,
                payment_hash,
+               outbound_payment: true,
        };
        let htlc_balance_unknown_preimage = Balance::MaybeTimeoutClaimableHTLC {
                amount_satoshis: 20_000,
                claimable_height: htlc_cltv_timeout,
                payment_hash: payment_hash_2,
+               outbound_payment: true,
        };
 
        let commitment_tx_fee = chan_feerate *
@@ -893,6 +897,7 @@ fn test_no_preimage_inbound_htlc_balances() {
                amount_satoshis: 10_000,
                claimable_height: htlc_cltv_timeout,
                payment_hash: to_b_failed_payment_hash,
+               outbound_payment: true,
        };
        let a_received_htlc_balance = Balance::MaybePreimageClaimableHTLC {
                amount_satoshis: 20_000,
@@ -908,6 +913,7 @@ fn test_no_preimage_inbound_htlc_balances() {
                amount_satoshis: 20_000,
                claimable_height: htlc_cltv_timeout,
                payment_hash: to_a_failed_payment_hash,
+               outbound_payment: true,
        };
 
        // Both A and B will have an HTLC that's claimable on timeout and one that's claimable if they
@@ -1204,14 +1210,17 @@ fn do_test_revoked_counterparty_commitment_balances(anchors: bool, confirm_htlc_
                        amount_satoshis: 2_000,
                        claimable_height: missing_htlc_cltv_timeout,
                        payment_hash: missing_htlc_payment_hash,
+                       outbound_payment: true,
                }, Balance::MaybeTimeoutClaimableHTLC {
                        amount_satoshis: 4_000,
                        claimable_height: htlc_cltv_timeout,
                        payment_hash: timeout_payment_hash,
+                       outbound_payment: true,
                }, Balance::MaybeTimeoutClaimableHTLC {
                        amount_satoshis: 5_000,
                        claimable_height: live_htlc_cltv_timeout,
                        payment_hash: live_payment_hash,
+                       outbound_payment: true,
                }]),
                sorted_vec(nodes[1].chain_monitor.chain_monitor.get_monitor(funding_outpoint).unwrap().get_claimable_balances()));
 
@@ -1754,10 +1763,12 @@ fn do_test_revoked_counterparty_aggregated_claims(anchors: bool) {
                        amount_satoshis: 4_000,
                        claimable_height: htlc_cltv_timeout,
                        payment_hash: revoked_payment_hash,
+                       outbound_payment: true,
                }, Balance::MaybeTimeoutClaimableHTLC {
                        amount_satoshis: 3_000,
                        claimable_height: htlc_cltv_timeout,
                        payment_hash: claimed_payment_hash,
+                       outbound_payment: true,
                }]),
                sorted_vec(nodes[1].chain_monitor.chain_monitor.get_monitor(funding_outpoint).unwrap().get_claimable_balances()));