search

lightningd / plugins

Community curated plugins for core-lightning
BSD 3-Clause "New" or "Revised" License
269 stars 129 forks source link

feeadjuster: assert can trigger #199

Closed gallizoltan closed 4 months ago

gallizoltan commented 3 years ago

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.

m-schmoock commented 3 years ago

Good that we put that assert in ;)

m-schmoock commented 3 years ago

@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)

gallizoltan commented 3 years ago

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").

chrisguida commented 4 months ago

Fixed by #525