From b5769bdb28237b093cc5dc804d301a4a190f8135 Mon Sep 17 00:00:00 2001 From: Pierre-Marie Padiou Date: Mon, 10 Jul 2023 10:40:25 +0200 Subject: [PATCH] Fix `skipAbsoluteFeeCheck` in liquidity policy (#509) There were two separate issues: - the bypass wasn't working at all because we were using a max of 0.msat - the bypass should only apply to off-chain payments --- .../acinq/lightning/payment/LiquidityPolicy.kt | 4 ++-- .../payment/LiquidityPolicyTestsCommon.kt | 16 ++++++++++++++++ 2 files changed, 18 insertions(+), 2 deletions(-) diff --git a/src/commonMain/kotlin/fr/acinq/lightning/payment/LiquidityPolicy.kt b/src/commonMain/kotlin/fr/acinq/lightning/payment/LiquidityPolicy.kt index 3b26565e0..b774daf8b 100644 --- a/src/commonMain/kotlin/fr/acinq/lightning/payment/LiquidityPolicy.kt +++ b/src/commonMain/kotlin/fr/acinq/lightning/payment/LiquidityPolicy.kt @@ -16,7 +16,7 @@ sealed class LiquidityPolicy { * Allow automated liquidity managements, within relative and absolute fee limits. Both conditions must be met. * @param maxAbsoluteFee max absolute fee * @param maxRelativeFeeBasisPoints max relative fee (all included: service fee and mining fee) (1_000 bips = 10 %) - * @param skipAbsoluteFeeCheck useful for pay-to-open, being more lax may make sense when the sender doesn't retry payments + * @param skipAbsoluteFeeCheck only applies for off-chain payments, being more lax may make sense when the sender doesn't retry payments */ data class Auto(val maxAbsoluteFee: Satoshi, val maxRelativeFeeBasisPoints: Int, val skipAbsoluteFeeCheck: Boolean) : LiquidityPolicy() @@ -25,7 +25,7 @@ sealed class LiquidityPolicy { return when (this) { is Disable -> LiquidityEvents.Rejected.Reason.PolicySetToDisabled is Auto -> { - val maxAbsoluteFee = if (skipAbsoluteFeeCheck) 0.msat else this.maxAbsoluteFee.toMilliSatoshi() + val maxAbsoluteFee = if (skipAbsoluteFeeCheck && source == LiquidityEvents.Source.OffChainPayment) Long.MAX_VALUE.msat else this.maxAbsoluteFee.toMilliSatoshi() val maxRelativeFee = amount * maxRelativeFeeBasisPoints / 10_000 logger.info { "liquidity policy check: fee=$fee maxAbsoluteFee=$maxAbsoluteFee maxRelativeFee=$maxRelativeFee policy=$this" } if (fee > maxRelativeFee) { diff --git a/src/commonTest/kotlin/fr/acinq/lightning/payment/LiquidityPolicyTestsCommon.kt b/src/commonTest/kotlin/fr/acinq/lightning/payment/LiquidityPolicyTestsCommon.kt index 387a21e7e..5be38375c 100644 --- a/src/commonTest/kotlin/fr/acinq/lightning/payment/LiquidityPolicyTestsCommon.kt +++ b/src/commonTest/kotlin/fr/acinq/lightning/payment/LiquidityPolicyTestsCommon.kt @@ -41,4 +41,20 @@ class LiquidityPolicyTestsCommon : LightningTestSuite() { assertNull(policy.maybeReject(amount = 10_000_000.msat, fee = 2_000_000.msat, source = LiquidityEvents.Source.OffChainPayment, logger)) } + + @Test + fun `policy rejection skip absolute check`() { + + val policy = LiquidityPolicy.Auto(maxAbsoluteFee = 1_000.sat, maxRelativeFeeBasisPoints = 5_000 /* 3000 = 30 % */, skipAbsoluteFeeCheck = true) + + // fee is over absolute, and it's an offchain payment so the check passes + assertNull(policy.maybeReject(amount = 4_000_000.msat, fee = 2_000_000.msat, source = LiquidityEvents.Source.OffChainPayment, logger)) + + // fee is over absolute, but it's an on-chain payment so the check fails + assertEquals( + expected = LiquidityEvents.Rejected.Reason.TooExpensive.OverAbsoluteFee(policy.maxAbsoluteFee), + actual = policy.maybeReject(amount = 4_000_000.msat, fee = 2_000_000.msat, source = LiquidityEvents.Source.OnChainWallet, logger)?.reason + ) + + } } \ No newline at end of file