Finer grained liquidity rejections (#498)

This allows for a more informative message to the user.
ACINQ · Jul 3, 2023 · b650de2 · b650de2
1 parent 0ca3d2e
commit b650de2
Show file tree

Hide file tree

Showing 3 changed files with 57 additions and 15 deletions.
diff --git a/src/commonMain/kotlin/fr/acinq/lightning/NodeEvents.kt b/src/commonMain/kotlin/fr/acinq/lightning/NodeEvents.kt
@@ -30,8 +30,10 @@ sealed interface LiquidityEvents : NodeEvents {
     data class Rejected(override val amount: MilliSatoshi, override val fee: MilliSatoshi, override val source: Source, val reason: Reason) : LiquidityEvents {
         sealed class Reason {
             object PolicySetToDisabled : Reason()
-            object RejectedByUser : Reason()
-            data class TooExpensive(val maxAllowed: MilliSatoshi, val actual: MilliSatoshi) : Reason()
+            sealed class TooExpensive : Reason() {
+                data class OverAbsoluteFee(val maxAbsoluteFee: Satoshi) : TooExpensive()
+                data class OverRelativeFee(val maxRelativeFeeBasisPoints: Int) : TooExpensive()
+            }
             object ChannelInitializing : Reason()
         }
     }

diff --git a/src/commonMain/kotlin/fr/acinq/lightning/payment/LiquidityPolicy.kt b/src/commonMain/kotlin/fr/acinq/lightning/payment/LiquidityPolicy.kt
@@ -4,6 +4,7 @@ import fr.acinq.bitcoin.Satoshi
 import fr.acinq.lightning.LiquidityEvents
 import fr.acinq.lightning.MilliSatoshi
 import fr.acinq.lightning.utils.MDCLogger
+import fr.acinq.lightning.utils.msat
 import fr.acinq.lightning.utils.toMilliSatoshi
 
 
@@ -17,25 +18,20 @@ sealed class LiquidityPolicy {
      * @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
      */
-    data class Auto(val maxAbsoluteFee: Satoshi, val maxRelativeFeeBasisPoints: Int, val skipAbsoluteFeeCheck: Boolean) : LiquidityPolicy() {
-        /** Maximum fee that we are willing to pay for a particular amount */
-        fun maxFee(amount: MilliSatoshi) =
-            if (skipAbsoluteFeeCheck) {
-                amount * maxRelativeFeeBasisPoints / 10_000
-            } else {
-                maxAbsoluteFee.toMilliSatoshi().min(amount * maxRelativeFeeBasisPoints / 10_000)
-            }
-    }
+    data class Auto(val maxAbsoluteFee: Satoshi, val maxRelativeFeeBasisPoints: Int, val skipAbsoluteFeeCheck: Boolean) : LiquidityPolicy()
 
     /** Make decision for a particular liquidity event */
     fun maybeReject(amount: MilliSatoshi, fee: MilliSatoshi, source: LiquidityEvents.Source, logger: MDCLogger): LiquidityEvents.Rejected? {
         return when (this) {
             is Disable -> LiquidityEvents.Rejected.Reason.PolicySetToDisabled
             is Auto -> {
-                val maxAllowedFee = maxFee(amount)
-                logger.info { "liquidity policy check: fee=$fee maxAllowedFee=$maxAllowedFee policy=$this" }
-                if (fee > maxAllowedFee) {
-                    LiquidityEvents.Rejected.Reason.TooExpensive(maxAllowed = maxAllowedFee, actual = fee)
+                val maxAbsoluteFee = if (skipAbsoluteFeeCheck) 0.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) {
+                    LiquidityEvents.Rejected.Reason.TooExpensive.OverRelativeFee(maxRelativeFeeBasisPoints)
+                } else if (fee > maxAbsoluteFee) {
+                    LiquidityEvents.Rejected.Reason.TooExpensive.OverAbsoluteFee(this.maxAbsoluteFee)
                 } else null
             }
         }?.let { reason -> LiquidityEvents.Rejected(amount, fee, source, reason) }

diff --git a/src/commonTest/kotlin/fr/acinq/lightning/payment/LiquidityPolicyTestsCommon.kt b/src/commonTest/kotlin/fr/acinq/lightning/payment/LiquidityPolicyTestsCommon.kt
@@ -0,0 +1,44 @@
+package fr.acinq.lightning.payment
+
+import fr.acinq.lightning.LiquidityEvents
+import fr.acinq.lightning.tests.utils.LightningTestSuite
+import fr.acinq.lightning.utils.MDCLogger
+import fr.acinq.lightning.utils.msat
+import fr.acinq.lightning.utils.sat
+import org.kodein.log.LoggerFactory
+import org.kodein.log.newLogger
+import kotlin.test.Test
+import kotlin.test.assertEquals
+import kotlin.test.assertNull
+
+class LiquidityPolicyTestsCommon : LightningTestSuite() {
+
+    private val logger = MDCLogger(LoggerFactory.default.newLogger(this::class))
+
+    @Test
+    fun `policy rejection`() {
+
+        val policy = LiquidityPolicy.Auto(maxAbsoluteFee = 2_000.sat, maxRelativeFeeBasisPoints = 3_000 /* 3000 = 30 % */, skipAbsoluteFeeCheck = false)
+
+        // fee over both absolute and relative
+        assertEquals(
+            expected = LiquidityEvents.Rejected.Reason.TooExpensive.OverRelativeFee(policy.maxRelativeFeeBasisPoints),
+            actual = policy.maybeReject(amount = 4_000_000.msat, fee = 3_000_000.msat, source = LiquidityEvents.Source.OffChainPayment, logger)?.reason
+        )
+
+        // fee over absolute
+        assertEquals(
+            expected = LiquidityEvents.Rejected.Reason.TooExpensive.OverAbsoluteFee(policy.maxAbsoluteFee),
+            actual = policy.maybeReject(amount = 15_000_000.msat, fee = 3_000_000.msat, source = LiquidityEvents.Source.OffChainPayment, logger)?.reason
+        )
+
+        // fee over relative
+        assertEquals(
+            expected = LiquidityEvents.Rejected.Reason.TooExpensive.OverRelativeFee(policy.maxRelativeFeeBasisPoints),
+            actual = policy.maybeReject(amount = 4_000_000.msat, fee = 2_000_000.msat, source = LiquidityEvents.Source.OffChainPayment, logger)?.reason
+        )
+
+        assertNull(policy.maybeReject(amount = 10_000_000.msat, fee = 2_000_000.msat, source = LiquidityEvents.Source.OffChainPayment, logger))
+
+    }
+}