lightningequipment / circuitbreaker

MIT License
118 stars 18 forks source link

[feature request] Setting Limits on channels also considering their outgoing HTLCs #68

Open ziggie1984 opened 1 year ago

ziggie1984 commented 1 year ago

This tool is awesome to prevent some severe attacks like floot and loot. Therefore I understand why you are only focusing on the incoming HTLCs on channels.

I would also like to have the control of outgoing HTLCs I put onto a channel. So what I mean by that is, that this tool could maybe provide the possibility for users to also rate-limit channels regarding the Offered HTLCs I put onto the Channel. This does not correspond to the floot and loot attack but could be interesting in preventing costly FCs. Current setup would allow an attacker in the worst case scenario when he has 100 Channels with the standard setting to fill up one channel (with the default setting of 5) to 483 HTLCs (100*5).

So I propose introducing separate limits for outgoing HTLCs for channels, which are separate from the incoming limits. I thought of mimicking some kind of nftables design here.

An HTLC arrives at your node first the Incoming Channel Limits are expected. Further the whole amount of HTLCs (including Incoming and Outgoing HTLCs) are inspected, so we do not fill up the incoming channel when the limit is already reached. Then we could take also the Expiry Time into account which could provide the user with a more fine granular setting when he wants to allow no HTLC at all on this channel in case one of the HTLCs times out in maybe 5 blocks or even less . After passing the incoming Channel, the Outgoing Channel is evaluated. So first check if the outgoing HTLC limit would be exceeded in case we add this HTLC, and further also check for the whole number of HTLCs (outgoing+incoming) whether they reach our limit. The outgoing HTLC could also take the SENDS into account not only the FORWARDs. One implementation hurdle is that non-strict-routing means we need to evaluate every channel of the same peer for the specific limits, and if one of them fails the limits the HTLC fails. Moreover because the interceptor can only influence FORWARDS not SENDS, it would mean that rebalance software could potentially always block all Outgoing HTLCs slot on channels preventing routing. But thats exactly the usecase for this feature, preventing the filling-up of HTLCs, without the user knowing or being aware, costing him a lot of sats when he needs to resolve everything on chain.

I would like to work on this, if anybody is interested in this feature?

feelancer21 commented 1 year ago

I like the idea ot separate limits for incoming and outgoings too and that FORWARDS are blocked if the channels is filled with SENDS is also a very good idea.

M1ch43lV commented 1 year ago

Very good proposal. Not only controling the incoming but also the outgoing htlcs. Sometimes I had the weird situation of accumulating up to 10 outgoing htlcs for one channel, although circuitbreaker was limited to allow a maximum of 2 incoming htlcs per channel. The accumulation occurs because I got the incoming htlcs on different channels but all with the same target of using one specific outgoing channel. In case of FC it gets quite expensive. @ziggie1984 Do you think you can implement the modifications also for a non-docker linux environment? The recent new modifications is only suitable for docker environments.

joostjager commented 1 year ago

I've considered this when I first started working on CB, but decided to keep it simple initially. Can definitely see the value of an outgoing limit to manage the cost of potential force closes.

Another idea that takes it even further is to apply pair-wise limits. So specific limits for a combination of in and outgoing peer. Not sure if there is any real value in that fine-grained level of control though.

ziggie1984 commented 1 year ago

Ok cool, I see your idea definitely something one can think about. I will give the idea with considering the Outgoing HTLCs a shot. Lets see where it brings us. Thanks all for your comments so far :)

@M1ch43lV the problem seems to be locally on your system. The changes are available for all systems. So mine will be too. Just follow jager's advice in just using the build commands from the docker file. For the frontend just cd in the web dir and execute:

yarn install --frozen-lockfile --network-timeout 1000000 && yarn build-export then you should find the build in the webui-build and cb should start the frontend without problems.

jjager-nydig commented 1 year ago

Interested to see how it goes!

KnockOnWoodNode commented 1 year ago

I just had a chat with @ziggie1984 about this, and I find out now this issue basically covers what I was wishing to see implemented in CB.

My scenario: I'd like to have tops N outgoing htlcs on a channel, be them forwards or rebalances. My own rebalance logic is already checking if and how many pending htlcs are on a channel, and will tailor its activity based on that. So, the rebalance side is taken care of, and rebalancer would fill the remaining slots of the allowed N. Where CB comes into play, is to allow or reject an incoming htlc depending on the same metric, so if max is 3 in CB, and I have 2 forwards and 1 rebalance pending, CB will not allow any more incoming htlcs.

But I also find very useful the "do not allow any more htlc that will be outgoing to a channel where another htlc is about to expire".

Possibilities are endless, and as far as I understand this shouldn't impact significantly the code complexity.