Skip to content

Commit

Permalink
replace maxAbsoluteFee by maxMiningFee
Browse files Browse the repository at this point in the history
It makes much more sense to consider only the mining fee for absolute
fee check, as it is volatile and amount-independent. The service fee is
in % and predictable.
  • Loading branch information
pm47 committed Jul 9, 2024
1 parent 0f3d645 commit 84a1eb1
Show file tree
Hide file tree
Showing 8 changed files with 38 additions and 26 deletions.
6 changes: 5 additions & 1 deletion src/commonMain/kotlin/fr/acinq/lightning/NodeEvents.kt
Original file line number Diff line number Diff line change
Expand Up @@ -41,14 +41,16 @@ sealed interface LiquidityEvents : NodeEvents {
sealed class Reason {
data object PolicySetToDisabled : Reason()
sealed class TooExpensive : Reason() {
data class OverAbsoluteFee(val maxAbsoluteFee: Satoshi) : TooExpensive()
data class OverMaxMiningFee(val maxMiningFee: Satoshi) : TooExpensive()
data class OverRelativeFee(val maxRelativeFeeBasisPoints: Int) : TooExpensive()
}

data class OverMaxCredit(val maxAllowedCredit: Satoshi) : TooExpensive()

data object ChannelInitializing : Reason()
}
}

data class AddedToFeeCredit(override val amount: MilliSatoshi, override val fee: MilliSatoshi, override val source: Source) : Decision
data class Accepted(override val amount: MilliSatoshi, override val fee: MilliSatoshi, override val source: Source) : Decision
}
Expand All @@ -62,8 +64,10 @@ sealed interface SensitiveTaskEvents : NodeEvents {
data class InteractiveTx(val channelId: ByteVector32, val fundingTxIndex: Long) : TaskIdentifier() {
constructor(fundingParams: InteractiveTxParams) : this(fundingParams.channelId, (fundingParams.sharedInput as? SharedFundingInput.Multisig2of2)?.fundingTxIndex?.let { it + 1 } ?: 0)
}

data class IncomingMultiPartPayment(val paymentHash: ByteVector32) : TaskIdentifier()
}

data class TaskStarted(val id: TaskIdentifier) : SensitiveTaskEvents
data class TaskEnded(val id: TaskIdentifier) : SensitiveTaskEvents

Expand Down
4 changes: 2 additions & 2 deletions src/commonMain/kotlin/fr/acinq/lightning/NodeParams.kt
Original file line number Diff line number Diff line change
Expand Up @@ -231,9 +231,9 @@ data class NodeParams(
paymentRecipientExpiryParams = RecipientCltvExpiryParams(CltvExpiryDelta(75), CltvExpiryDelta(200)),
liquidityPolicy = MutableStateFlow<LiquidityPolicy>(
LiquidityPolicy.Auto(
maxAbsoluteFee = 2_000.sat,
maxMiningFee = 2_000.sat,
maxRelativeFeeBasisPoints = 3_000 /* 3000 = 30 % */,
skipAbsoluteFeeCheck = false,
skipMiningFeeCheck = false,
maxAllowedCredit = 0.sat
)
),
Expand Down
4 changes: 2 additions & 2 deletions src/commonMain/kotlin/fr/acinq/lightning/io/Peer.kt
Original file line number Diff line number Diff line change
Expand Up @@ -1037,7 +1037,7 @@ class Peer(
is Origin.PleaseOpenChannelOrigin -> when (val request = channelRequests[origin.requestId]) {
is RequestChannelOpen -> {
val totalFee = origin.serviceFee + origin.miningFee.toMilliSatoshi() - msg.pushAmount
val decision = nodeParams.liquidityPolicy.value.maybeReject(request.walletInputs.balance.toMilliSatoshi(), totalFee, LiquidityEvents.Source.OnChainWallet, logger, nodeParams.feeCredit.value)
val decision = nodeParams.liquidityPolicy.value.maybeReject(request.walletInputs.balance.toMilliSatoshi(), totalFee, LiquidityEvents.Source.OnChainWallet, logger, currentFeeCredit = nodeParams.feeCredit.value)
when (decision) {
is LiquidityEvents.Decision.Rejected -> {
logger.info { "rejecting open_channel2: reason=${decision.reason}" }
Expand Down Expand Up @@ -1249,7 +1249,7 @@ class Peer(
val (feerate, fee) = client.computeSpliceCpfpFeerate(available.channel.commitments, targetFeerate, spliceWeight = weight, logger)

logger.info { "requesting splice-in using balance=${cmd.walletInputs.balance} feerate=$feerate fee=$fee" }
val decision = nodeParams.liquidityPolicy.value.maybeReject(cmd.walletInputs.balance.toMilliSatoshi(), fee.toMilliSatoshi(), LiquidityEvents.Source.OnChainWallet, logger, nodeParams.feeCredit.value)
val decision = nodeParams.liquidityPolicy.value.maybeReject(cmd.walletInputs.balance.toMilliSatoshi(), fee.toMilliSatoshi(), LiquidityEvents.Source.OnChainWallet, logger, currentFeeCredit = nodeParams.feeCredit.value)
nodeParams._nodeEvents.emit(decision)
when (decision) {
is LiquidityEvents.Decision.Rejected -> {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -258,7 +258,8 @@ class IncomingPaymentHandler(val nodeParams: NodeParams, val db: IncomingPayment
val liquidityDecision = if (payment.parts.filterIsInstance<PayToOpenPart>().isNotEmpty()) {
// We consider the total amount received (not only the pay-to-open parts) to evaluate whether or not to accept the payment
val payToOpenFee = payment.parts.filterIsInstance<PayToOpenPart>().map { it.payToOpenRequest.payToOpenFeeSatoshis }.sum()
val decision = nodeParams.liquidityPolicy.value.maybeReject(payment.amountReceived, payToOpenFee.toMilliSatoshi(), LiquidityEvents.Source.OffChainPayment, logger, nodeParams.feeCredit.value)
val liquidity = payment.parts.filterIsInstance<PayToOpenPart>().map { it.payToOpenRequest.liquidity }.sum()
val decision = nodeParams.liquidityPolicy.value.maybeReject(payment.amountReceived, payToOpenFee.toMilliSatoshi(), LiquidityEvents.Source.OffChainPayment, logger, currentFeeCredit = nodeParams.feeCredit.value, liquidity = liquidity)
logger.info { "pay-to-open decision: $decision" }
nodeParams._nodeEvents.emit(decision)
when (decision) {
Expand Down
27 changes: 16 additions & 11 deletions src/commonMain/kotlin/fr/acinq/lightning/payment/LiquidityPolicy.kt
Original file line number Diff line number Diff line change
Expand Up @@ -15,32 +15,37 @@ sealed class LiquidityPolicy {

/**
* Allow automated liquidity managements, within relative and absolute fee limits. Both conditions must be met.
* @param maxAbsoluteFee max absolute fee
* @param maxMiningFee max mining fee
* @param maxRelativeFeeBasisPoints max relative fee (all included: service fee and mining fee) (1_000 bips = 10 %)
* @param skipAbsoluteFeeCheck only applies for off-chain payments, being more lax may make sense when the sender doesn't retry payments
* @param skipMiningFeeCheck only applies for off-chain payments, being more lax may make sense when the sender doesn't retry payments
* @param maxAllowedCredit if other checks fail, accept the payment and add the corresponding amount to fee credit up to this max value (only applies to offline payments, 0 sat to disable)
*/
data class Auto(val maxAbsoluteFee: Satoshi, val maxRelativeFeeBasisPoints: Int, val skipAbsoluteFeeCheck: Boolean, val maxAllowedCredit: Satoshi) : LiquidityPolicy()
data class Auto(val maxMiningFee: Satoshi, val maxRelativeFeeBasisPoints: Int, val skipMiningFeeCheck: Boolean, val maxAllowedCredit: Satoshi) : LiquidityPolicy()

/** Make decision for a particular liquidity event */
fun maybeReject(amount: MilliSatoshi, fee: MilliSatoshi, source: LiquidityEvents.Source, logger: MDCLogger, currentFeeCredit: Satoshi): LiquidityEvents.Decision {
fun maybeReject(amount: MilliSatoshi, fee: MilliSatoshi, source: LiquidityEvents.Source, logger: MDCLogger, currentFeeCredit: Satoshi, liquidity: Satoshi = 0.sat): LiquidityEvents.Decision {
val serviceFee = when (source) {
LiquidityEvents.Source.OnChainWallet -> 0.msat // no service fee for swap-ins
LiquidityEvents.Source.OffChainPayment -> (amount + liquidity.toMilliSatoshi()) * 0.01 // 1% service fee
}
val miningFee = fee - serviceFee
return when (this) {
is Disable -> LiquidityEvents.Decision.Rejected(amount, fee, source, LiquidityEvents.Decision.Rejected.Reason.PolicySetToDisabled)
is Auto -> {
val maxAbsoluteFee = if (skipAbsoluteFeeCheck && source == LiquidityEvents.Source.OffChainPayment) Long.MAX_VALUE.msat else this.maxAbsoluteFee.toMilliSatoshi()
val maxMiningFee = if (skipMiningFeeCheck && source == LiquidityEvents.Source.OffChainPayment) Long.MAX_VALUE.msat else this.maxMiningFee.toMilliSatoshi()
if (maxAllowedCredit == 0.sat || source == LiquidityEvents.Source.OnChainWallet) {
val maxRelativeFee = amount * maxRelativeFeeBasisPoints / 10_000
logger.info { "auto liquidity policy check: amount=$amount fee=$fee maxAbsoluteFee=$maxAbsoluteFee maxRelativeFee=$maxRelativeFee policy=$this" }
logger.info { "auto liquidity policy check: amount=$amount fee=$fee (miningFee=$miningFee serviceFee=$serviceFee) maxMiningFee=$maxMiningFee maxRelativeFee=$maxRelativeFee policy=$this" }
if (fee > maxRelativeFee) {
LiquidityEvents.Decision.Rejected(amount, fee, source, LiquidityEvents.Decision.Rejected.Reason.TooExpensive.OverRelativeFee(maxRelativeFeeBasisPoints))
} else if (fee > maxAbsoluteFee) {
LiquidityEvents.Decision.Rejected(amount, fee, source, LiquidityEvents.Decision.Rejected.Reason.TooExpensive.OverAbsoluteFee(this.maxAbsoluteFee))
} else if (miningFee > maxMiningFee) {
LiquidityEvents.Decision.Rejected(amount, fee, source, LiquidityEvents.Decision.Rejected.Reason.TooExpensive.OverMaxMiningFee(this.maxMiningFee))
} else LiquidityEvents.Decision.Accepted(amount, fee, source)
} else {
logger.info { "fee-credit liquidity policy check: amount=$amount fee=$fee maxAbsoluteFee=$maxAbsoluteFee currentFeeCredit=$currentFeeCredit maxAllowedCredit=$maxAllowedCredit policy=$this" }
// NB: we do check the max absolute fee, but will never raise an explicit error for it, because the payment will either be added to fee credit or rejected due to exceeding the
logger.info { "fee-credit liquidity policy check: amount=$amount fee=$fee (miningFee=$miningFee serviceFee=$serviceFee) maxMiningFee=$maxMiningFee currentFeeCredit=$currentFeeCredit maxAllowedCredit=$maxAllowedCredit policy=$this" }
// NB: we do check the max mining fee, but will never raise an explicit error for it, because the payment will either be added to fee credit or rejected due to exceeding the
// max allowed credit
if (fee <= maxAbsoluteFee && fee < (amount + currentFeeCredit.toMilliSatoshi())) {
if (miningFee <= maxMiningFee && fee < (amount + currentFeeCredit.toMilliSatoshi())) {
LiquidityEvents.Decision.Accepted(amount, fee, source)
} else if ((amount + currentFeeCredit.toMilliSatoshi()) > maxAllowedCredit.toMilliSatoshi()) {
LiquidityEvents.Decision.Rejected(amount, fee, source, LiquidityEvents.Decision.Rejected.Reason.OverMaxCredit(maxAllowedCredit))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1619,7 +1619,7 @@ data class PayToOpenRequest(
val paymentHash: ByteVector32,
val expireAt: Long,
val finalPacket: OnionRoutingPacket,
val liquidity: Satoshi = 0.sat,
val liquidity: Satoshi,
val tlvStream: TlvStream<PayToOpenRequestTlv> = TlvStream.empty(),
) : LightningMessage, HasChainHash {
override val type: Long get() = PayToOpenRequest.type
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -248,7 +248,8 @@ class IncomingPaymentHandlerTestsCommon : LightningTestSuite() {
hops = channelHops(paymentHandler.nodeParams.nodeId),
finalPayload = makeMppPayload(defaultAmount, defaultAmount, randomBytes32()),
payloadLength = OnionRoutingPacket.PaymentPacketLength
).third.packet
).third.packet,
liquidity = 0.sat
)
val result = paymentHandler.process(payToOpenRequest, TestConstants.defaultBlockHeight)

Expand Down Expand Up @@ -338,7 +339,8 @@ class IncomingPaymentHandlerTestsCommon : LightningTestSuite() {
hops = trampolineHops,
finalPayload = makeMppPayload(defaultAmount, defaultAmount, paymentSecret.reversed()), // <-- wrong secret
payloadLength = 400
).third.packet
).third.packet,
liquidity = 0.sat
)
val result = paymentHandler.process(payToOpenRequest, TestConstants.defaultBlockHeight)

Expand Down Expand Up @@ -1405,7 +1407,8 @@ class IncomingPaymentHandlerTestsCommon : LightningTestSuite() {
hops = channelHops(TestConstants.Bob.nodeParams.nodeId),
finalPayload = finalPayload,
payloadLength = OnionRoutingPacket.PaymentPacketLength
).third.packet
).third.packet,
liquidity = 0.sat
)
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ import fr.acinq.lightning.utils.msat
import fr.acinq.lightning.utils.sat
import kotlin.test.Test
import kotlin.test.assertEquals
import kotlin.test.assertNull

class LiquidityPolicyTestsCommon : LightningTestSuite() {

Expand All @@ -16,7 +15,7 @@ class LiquidityPolicyTestsCommon : LightningTestSuite() {
@Test
fun `policy rejection`() {

val policy = LiquidityPolicy.Auto(maxAbsoluteFee = 2_000.sat, maxRelativeFeeBasisPoints = 3_000 /* 3000 = 30 % */, skipAbsoluteFeeCheck = false, maxAllowedCredit = 0.sat)
val policy = LiquidityPolicy.Auto(maxMiningFee = 2_000.sat, maxRelativeFeeBasisPoints = 3_000 /* 3000 = 30 % */, skipMiningFeeCheck = false, maxAllowedCredit = 0.sat)

// fee over both absolute and relative
assertEquals(
Expand All @@ -26,7 +25,7 @@ class LiquidityPolicyTestsCommon : LightningTestSuite() {

// fee over absolute
assertEquals(
expected = LiquidityEvents.Decision.Rejected.Reason.TooExpensive.OverAbsoluteFee(policy.maxAbsoluteFee),
expected = LiquidityEvents.Decision.Rejected.Reason.TooExpensive.OverMaxMiningFee(policy.maxMiningFee),
actual = (policy.maybeReject(amount = 15_000_000.msat, fee = 3_000_000.msat, source = LiquidityEvents.Source.OffChainPayment, logger, currentFeeCredit = 0.sat) as? LiquidityEvents.Decision.Rejected)?.reason
)

Expand All @@ -45,7 +44,7 @@ class LiquidityPolicyTestsCommon : LightningTestSuite() {
@Test
fun `policy rejection skip absolute check`() {

val policy = LiquidityPolicy.Auto(maxAbsoluteFee = 1_000.sat, maxRelativeFeeBasisPoints = 5_000 /* 3000 = 30 % */, skipAbsoluteFeeCheck = true, maxAllowedCredit = 0.sat)
val policy = LiquidityPolicy.Auto(maxMiningFee = 1_000.sat, maxRelativeFeeBasisPoints = 5_000 /* 3000 = 30 % */, skipMiningFeeCheck = true, maxAllowedCredit = 0.sat)

// fee is over absolute, and it's an offchain payment so the check passes
assertEquals(
Expand All @@ -54,7 +53,7 @@ class LiquidityPolicyTestsCommon : LightningTestSuite() {

// fee is over absolute, but it's an on-chain payment so the check fails
assertEquals(
expected = LiquidityEvents.Decision.Rejected.Reason.TooExpensive.OverAbsoluteFee(policy.maxAbsoluteFee),
expected = LiquidityEvents.Decision.Rejected.Reason.TooExpensive.OverMaxMiningFee(policy.maxMiningFee),
actual = (policy.maybeReject(amount = 4_000_000.msat, fee = 2_000_000.msat, source = LiquidityEvents.Source.OnChainWallet, logger, currentFeeCredit = 0.sat) as? LiquidityEvents.Decision.Rejected)?.reason
)

Expand Down

0 comments on commit 84a1eb1

Please sign in to comment.