When converting between different denominations, errors happen easily and actually quite frequently (in particular the notorious off-by-factor-1000 error when converting between sat and msat). While in our current interface all arguments should have the appropriate suffix (_sat or _msat), they still may be easily misused in practice, e.g., as we require giving an amount_sat for channel operations but most other places we require amounts to be denominated in millisatoshi.
I therefore propose to mitigate this issue by introducing a LightningAmount type akin to/reusing bitcoin::Amount and use it consistently throughout our public API. Over time, we then may even want to transition to use this type internally wherever possible to ensure the correctness of conversions.
While being quite a refactoring of our API, I think this might be a worthwhile quality-of-life improvement that pays dividend over time. Looking for opinions, general thoughts, and concept ACKs here.
When converting between different denominations, errors happen easily and actually quite frequently (in particular the notorious off-by-factor-1000 error when converting between
satandmsat). While in our current interface all arguments should have the appropriate suffix (_sator_msat), they still may be easily misused in practice, e.g., as we require giving anamount_satfor channel operations but most other places we require amounts to be denominated in millisatoshi.I therefore propose to mitigate this issue by introducing a
LightningAmounttype akin to/reusingbitcoin::Amountand use it consistently throughout our public API. Over time, we then may even want to transition to use this type internally wherever possible to ensure the correctness of conversions.While being quite a refactoring of our API, I think this might be a worthwhile quality-of-life improvement that pays dividend over time. Looking for opinions, general thoughts, and concept ACKs here.