[feature]: Add htlc_id to `lnrpc.HTLCAttempt` and `lnrpc.ForwardingEvent`

[feelancer21]

There is already a field htlc_index in lnrpc.InvoiceHTLC. The indices should also be available in lnrpc.HTLCAttempt for payments or lnrpc.ForwardingEvent.
The availability of the indices would be particularly useful in order to be able to store and reconcile the data better externally.

Example:
There is currently no stream for ForwardingEvents. You can use SubscribeHtlcEvents, but after a downtime you might have to search for gaps with ForwardingHistory. Without a unique index, it will be difficult to find these gaps, as the timestamps are also different.

[guggero]

Unfortunately, the forwarding events data doesn't actually store the HtlcIndex: https://github.com/lightningnetwork/lnd/blob/master/channeldb/forwarding_log.go#L96

But, I agree that having the HtlcIndex would be useful for matching such an event to an actual HTLC in the revocation log or other places.

@yyforyongyu do you know of a good and reliable way to match forwarding events to the entries in the revocation log? I also kind of need something like this for lightninglabs/taproot-assets#1005.

[yyforyongyu]

Yeah we need to make following changes,

1. Return the HtlcIndex in PaymentResult when sending it from the switch
   

   lnd/htlcswitch/payment_result.go

   Lines 31 to 42 in 856dc2d

   	// PaymentResult wraps a decoded result received from the network after a
   	// payment attempt was made. This is what is eventually handed to the router
   	// for processing.
   	type PaymentResult struct {
   	// Preimage is set by the switch in case a sent HTLC was settled.
   	Preimage [32]byte
   	// Error is non-nil in case a HTLC send failed, and the HTLC is now
   	// irrevocably canceled. If the payment failed during forwarding, this
   	// error will be a *ForwardingError.
   	Error error
   	}

2. modify this method to take an HtlcIndex,
   

   lnd/routing/control_tower.go

   Lines 234 to 253 in 856dc2d

   	// SettleAttempt marks the given attempt settled with the preimage. If
   	// this is a multi shard payment, this might implicitly mean the the
   	// full payment succeeded.
   	func (p *controlTower) SettleAttempt(paymentHash lntypes.Hash,
   	attemptID uint64, settleInfo *channeldb.HTLCSettleInfo) (
   	*channeldb.HTLCAttempt, error) {
   	p.paymentsMtx.Lock(paymentHash)
   	defer p.paymentsMtx.Unlock(paymentHash)
   	payment, err := p.db.SettleAttempt(paymentHash, attemptID, settleInfo)
   	if err != nil {
   	return nil, err
   	}
   	// Notify subscribers of success event.
   	p.notifySubscribers(paymentHash, payment)
   	return payment.GetAttempt(attemptID)
   	}

3. pass the HtlcIndex here,
   

   lnd/routing/payment_lifecycle.go

   Lines 1127 to 1135 in 856dc2d

   	// In case of success we atomically store settle result to the DB and
   	// move the shard to the settled state.
   	htlcAttempt, err := p.router.cfg.Control.SettleAttempt(
   	p.identifier, attempt.AttemptID,
   	&channeldb.HTLCSettleInfo{
   	Preimage: result.Preimage,
   	SettleTime: p.router.cfg.Clock.Now(),
   	},
   	)

Target for 0.20?

[Abdulkbk]

I would like to work on this @saubyk

[saubyk]

I would like to work on this @saubyk

Assigned