renepickhardt / pickhardtpayments

python package to showcase, test and build your own version of Pickhardt Payments
Apache License 2.0
42 stars 14 forks source link

Add option to `settle` payments in SyncSimulatedPaymentSession #16

Open renepickhardt opened 2 years ago

renepickhardt commented 2 years ago

As realized through discussions with @sebulino in #15 the current lib does only decide if a payment can be delivered or not. In many cases it would be preferable that the liquidity is actually being moved in the OracleChannel and our belief about the uncertainty is updated in both arcs of the UncertaintyChannel where we would also have to set the inflight back to 0.

sebulino commented 2 years ago

This is my understanding of what is happening when pickhardt_pay is called on SyncSimulatedPaymentSession:

So far no update on OracleChannels in OracleLightningNetwork. This is what would make sense to be added.

renepickhardt commented 2 years ago

This is a very accurate summary of what is happening! Note that _attempt_payments is having this line:

https://github.com/renepickhardt/pickhardtpayments/blob/8e4449097c305c5d0d11ed9869c20b136cf03d16/pickhardtpayments/SyncSimulatedPaymentSession.py#L250

This is where we update our belief on the uncertainty network. IF you were to also update the Oracle here all future statistics would be skewed.

Therefor I guess it would be best from the point of view of the simulation to update the OracleChannels after all rounds have been completed. There may be two paths to success:

  1. after the payment loop is completed go through all channels that have inflight and move it to the other side while removing it
  2. if #17 has landed and all the payment paths and statistics of a payment session are in a single data structure one could just go throught that (though it would have to include staistics of all rounds - right now we have stats just for a single round)

It's quite remarkable how involved such a little change would be and how much refactorig would probably be necessary. From the point of view that a siulation may wish to do concurrent tests it is probably better and cleaner to persue the 2nd option. That being said if you have better / other ideas I will be open