search

lightningdevkit / lightning-liquidity

Other
27 stars 17 forks source link

allow overriding min/max payment sizes #97

Closed johncantrell97 closed 8 months ago

johncantrell97 commented 9 months ago

C= will need to provide a dynamic max payment size based on the current total capacity a user has open with us at the time of the request. In order to do that we need to be able to provide a value at the time of fee request.

I wasn't sure if we should just remove these from the config and make these required when calling opening_fee_params_generated.

johncantrell97 commented 9 months ago

I think generally an override pattern is fine here, however, you'll also need to find a solution for the variable amount case that was fixed in #91. This might mean we need to introduce a per-channel/request ServiceConfig that is stored (and will be persisted) in the OutboundJITChannel that allows to override the defaults set via LSPS2ServiceConfig.

Hm yeah. It's tricky because currently we don't even create the OutboundJITChannel until the buy request is handled.

There's really no way to link the GetInfo request/response with the BuyRequest that comes in. So we don't really know what min/max payment we gave them in the GetInfo when they make the BuyRequest. I guess we could just match on counterparty_node_id and just always store the "last one we gave out to this pubkey". I guess another way is to include min/max payment size (or timestamp?) into the computation of the promise we give out and store map from promise to min/max payment and then we can use the promise as a key to fetching the config/min_max payment sizes

Thoughts?

tnull commented 9 months ago

Hm yeah. It's tricky because currently we don't even create the OutboundJITChannel until the buy request is handled.

There's really no way to link the GetInfo request/response with the BuyRequest that comes in. So we don't really know what min/max payment we gave them in the GetInfo when they make the BuyRequest. I guess we could just match on counterparty_node_id and just always store the "last one we gave out to this pubkey". I guess another way is to include min/max payment size (or timestamp?) into the computation of the promise we give out and store map from promise to min/max payment and then we can use the promise as a key to fetching the config/min_max payment sizes

Thoughts?

Ah, yeah, that's kind of what I meant with "you'll also need to find a solution" above. :grin:

I think the issue really is that {min,max}_payment_size_msat are (potentially dynamic, per-request) parameters chosen by the LSP and therefore should be clearly communicated and commited to via the OpeningFeeParameters. If we did that, there would also be no requirement to keep state between a client just querying our rates and then actually issuing a buy request. I think this should be fixed in the spec: https://github.com/BitcoinAndLightningLayerSpecs/lsp/pull/82

johncantrell97 commented 9 months ago

@tnull Just updated this PR assuming the spec change you proposed gets merged. It looks like it will since it has two "LGTM" comments. I moved the min/max into the promised params and then copy them into outbound jit channel state where they can be used to enforce limits on the payment size that were previously using the config.

This makes it pretty pointless to have these fields in the config so I removed them.