plugins icon indicating copy to clipboard operation
plugins copied to clipboard

feeadjuster: assert can trigger

Open gallizoltan opened this issue 4 years ago • 3 comments

Description

I've seen a curious line in the lightningd log:

2020-12-06T20:32:06.150Z **BROKEN** plugin-feeadjuster.py: Adjusting fees:

I've investigated the situation and figured out what happened. It caused by the assert at line 122 in feeadjuster/feeadjuster.py:

assert 0 <= percentage and percentage <= 1

Steps to reproduce

  1. Let's assume I have a channel with a total size of 100,000 Sats. 20,000 Sats are mine in that channel.
  2. I call a feeadjust. It saves my balance in plugin.adj_balances[scid] at line 198.
  3. Then an incoming payment happens: 30,000 Sats arrive into the channel. (In my case it was actually a circular payment, rebalance, but it does not matter.) At this point I have 50,000 Sats in the channel, but plugin.adj_balances[scid] still stores the 20,000 Sats value. The feeadjuster is not aware of the incoming payments.
  4. After these a forward event occurs: 40,000 Sats leave the channel. The feeadjuster is subscribed to the forward event, so it reduces my stored balance by 40,000 Sats at line 173. The result will be negative: -20,000 Sats. This will trigger the assert.

A possible solution

The easy way: it will not happen again if I turn off the automatic fee updates in feeadjuster.

gallizoltan avatar Jan 15 '21 10:01 gallizoltan

Good that we put that assert in ;)

m-schmoock avatar Jan 15 '21 11:01 m-schmoock

@gallizoltan Do we need to fix this by i.e. by replacing the forward_event_subscription early return in forward_event to check for this in line 178 where maybe_adjust_fees is called? Can you make a test for this?

Partly addressed by https://github.com/lightningd/plugins/pull/198 (only rebalance)

m-schmoock avatar Jan 17 '21 13:01 m-schmoock

I'm not sure what would be the best solution. Maybe we should write a @plugin.subscribe("invoice_payment") and a @plugin.subscribe("sendpay_success").

gallizoltan avatar Jan 18 '21 20:01 gallizoltan