Skip to content

Commit

Permalink
Simplify and improve typing in NodeParams (#603)
Browse files Browse the repository at this point in the history
Remove all unused parameters, and use proper type for durations.

Also, increase `checkHtlcTimeoutAfterStartupDelay` to 30s as connecting can be quite long with Tor.
  • Loading branch information
pm47 authored Feb 15, 2024
1 parent 35dc4eb commit 9bd3489
Show file tree
Hide file tree
Showing 9 changed files with 46 additions and 70 deletions.
79 changes: 28 additions & 51 deletions src/commonMain/kotlin/fr/acinq/lightning/NodeParams.kt
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,9 @@ import kotlinx.coroutines.flow.MutableSharedFlow
import kotlinx.coroutines.flow.MutableStateFlow
import kotlinx.coroutines.flow.SharedFlow
import kotlinx.coroutines.flow.asSharedFlow
import kotlin.time.Duration
import kotlin.time.Duration.Companion.minutes
import kotlin.time.Duration.Companion.seconds

data class NodeUri(val id: PublicKey, val host: String, val port: Int)

Expand Down Expand Up @@ -48,11 +51,13 @@ data class SwapInParams(val minConfirmations: Int, val maxConfirmations: Int, va
object DefaultSwapInParams {
/** When doing a swap-in, the funds must be confirmed before we can use them in a 0-conf splice. */
const val MinConfirmations = 3

/**
* When doing a swap-in, the corresponding splice must be triggered before we get too close to the refund delay.
* Users would otherwise be able to steal funds if the splice transaction doesn't confirm before the refund delay.
*/
const val MaxConfirmations = 144 * 30 * 4 // ~4 months

/** When doing a swap-in, the user's funds are locked in a 2-of-2: they can claim them unilaterally after that delay. */
const val RefundDelay = 144 * 30 * 6 // ~6 months
}
Expand Down Expand Up @@ -88,8 +93,8 @@ data class RecipientCltvExpiryParams(val min: CltvExpiryDelta, val max: CltvExpi

/**
* @param loggerFactory factory for creating [Logger] objects sharing the same configuration.
* @param chain bitcoin chain we're interested in.
* @param keyManager derive private keys and secrets from your seed.
* @param alias name of the lightning node.
* @param features features supported by the lightning node.
* @param dustLimit threshold below which outputs will not be generated in commitment or HTLC transactions (i.e. HTLCs below this amount plus HTLC transaction fees are not enforceable on-chain).
* @param maxRemoteDustLimit maximum dust limit we let our peer use for his commitment (in theory it should always be 546 sats).
Expand All @@ -98,36 +103,26 @@ data class RecipientCltvExpiryParams(val min: CltvExpiryDelta, val max: CltvExpi
* @param maxAcceptedHtlcs cap on the number of pending HTLCs in a channel: this lets us limit our exposure to HTLCs risk.
* @param expiryDeltaBlocks cltv-expiry-delta used in our channel_update: since our channels are private and we don't relay payments, this will be basically ignored.
* @param fulfillSafetyBeforeTimeoutBlocks number of blocks necessary to react to a malicious peer that doesn't acknowledge and sign our HTLC preimages.
* @param checkHtlcTimeoutAfterStartupDelaySeconds delay in seconds before we check for timed out HTLCs in our channels after a wallet restart.
* @param checkHtlcTimeoutAfterStartupDelay delay before we check for timed out HTLCs in our channels after a wallet restart.
* @param htlcMinimum minimum accepted htlc value.
* @param toRemoteDelayBlocks number of blocks our peer will have to wait before they get their main output back in case they force-close a channel.
* @param maxToLocalDelayBlocks maximum number of blocks we will have to wait before we get our main output back in case we force-close a channel.
* @param minDepthBlocks minimum depth of a transaction before we consider it safely confirmed.
* @param feeBase base fee used in our channel_update: since our channels are private and we don't relay payments, this will be basically ignored.
* @param feeProportionalMillionth proportional fee used in our channel_update: since our channels are private and we don't relay payments, this will be basically ignored.
* @param revocationTimeoutSeconds delay after which we disconnect from our peer if they don't send us a revocation after a new commitment is signed.
* @param authTimeoutSeconds timeout for the connection authentication phase.
* @param initTimeoutSeconds timeout for the connection initialization phase.
* @param pingIntervalSeconds delay between ping messages.
* @param pingTimeoutSeconds timeout when waiting for a response to our ping.
* @param pingDisconnect disconnect when a peer doesn't respond to our ping.
* @param autoReconnect automatically reconnect to our peers.
* @param initialRandomReconnectDelaySeconds delay before which we reconnect to our peers (will be randomized based on this value).
* @param maxReconnectIntervalSeconds maximum delay between reconnection attempts.
* @param chain bitcoin chain we're interested in.
* @param channelFlags channel flags used to temporarily enable or disable channels.
* @param paymentRequestExpirySeconds our Bolt 11 invoices will only be valid for this duration.
* @param multiPartPaymentExpirySeconds number of seconds we will wait to receive all parts of a multi-part payment.
* @param feeProportionalMillionths proportional fee used in our channel_update: since our channels are private and we don't relay payments, this will be basically ignored.
* @param pingInterval delay between ping messages.
* @param initialRandomReconnectDelay delay before which we reconnect to our peers (will be randomized based on this value).
* @param maxReconnectInterval maximum delay between reconnection attempts.
* @param mppAggregationWindow amount of time we will wait to receive all parts of a multi-part payment.
* @param maxPaymentAttempts maximum number of retries when attempting an outgoing payment.
* @param paymentRecipientExpiryParams configure the expiry delta used for the final node when sending payments.
* @param zeroConfPeers list of peers with whom we use zero-conf (note that this is a strong trust assumption).
* @param enableTrampolinePayment enable trampoline payments.
* @param liquidityPolicy fee policy for liquidity events, can be modified at any time.
*/
data class NodeParams(
val loggerFactory: LoggerFactory,
val chain: Bitcoin.Chain,
val keyManager: KeyManager,
val alias: String,
val features: Features,
val dustLimit: Satoshi,
val maxRemoteDustLimit: Satoshi,
Expand All @@ -136,30 +131,21 @@ data class NodeParams(
val maxAcceptedHtlcs: Int,
val expiryDeltaBlocks: CltvExpiryDelta,
val fulfillSafetyBeforeTimeoutBlocks: CltvExpiryDelta,
val checkHtlcTimeoutAfterStartupDelaySeconds: Int,
val checkHtlcTimeoutAfterStartupDelay: Duration,
val checkHtlcTimeoutInterval: Duration,
val htlcMinimum: MilliSatoshi,
val toRemoteDelayBlocks: CltvExpiryDelta,
val maxToLocalDelayBlocks: CltvExpiryDelta,
val minDepthBlocks: Int,
val feeBase: MilliSatoshi,
val feeProportionalMillionth: Int,
val revocationTimeoutSeconds: Long,
val authTimeoutSeconds: Long,
val initTimeoutSeconds: Long,
val pingIntervalSeconds: Long,
val pingTimeoutSeconds: Long,
val pingDisconnect: Boolean,
val autoReconnect: Boolean,
val initialRandomReconnectDelaySeconds: Long,
val maxReconnectIntervalSeconds: Long,
val chain: Bitcoin.Chain,
val channelFlags: Byte,
val paymentRequestExpirySeconds: Long,
val multiPartPaymentExpirySeconds: Long,
val feeProportionalMillionths: Int,
val pingInterval: Duration,
val initialRandomReconnectDelay: Duration,
val maxReconnectInterval: Duration,
val mppAggregationWindow: Duration,
val maxPaymentAttempts: Int,
val paymentRecipientExpiryParams: RecipientCltvExpiryParams,
val zeroConfPeers: Set<PublicKey>,
val enableTrampolinePayment: Boolean,
val liquidityPolicy: MutableStateFlow<LiquidityPolicy>
) {
val nodePrivateKey get() = keyManager.nodeKeys.nodeKey.privateKey
Expand All @@ -185,8 +171,8 @@ data class NodeParams(
*/
constructor(chain: Bitcoin.Chain, loggerFactory: LoggerFactory, keyManager: KeyManager) : this(
loggerFactory = loggerFactory,
chain = chain,
keyManager = keyManager,
alias = "lightning-kmp",
features = Features(
Feature.OptionDataLossProtect to FeatureSupport.Optional,
Feature.VariableLengthOnion to FeatureSupport.Mandatory,
Expand Down Expand Up @@ -218,28 +204,19 @@ data class NodeParams(
maxAcceptedHtlcs = 6,
expiryDeltaBlocks = CltvExpiryDelta(144),
fulfillSafetyBeforeTimeoutBlocks = CltvExpiryDelta(6),
checkHtlcTimeoutAfterStartupDelaySeconds = 15,
checkHtlcTimeoutAfterStartupDelay = 30.seconds,
checkHtlcTimeoutInterval = 10.seconds,
htlcMinimum = 1000.msat,
minDepthBlocks = 3,
toRemoteDelayBlocks = CltvExpiryDelta(2016),
maxToLocalDelayBlocks = CltvExpiryDelta(1008),
feeBase = 1000.msat,
feeProportionalMillionth = 100,
revocationTimeoutSeconds = 20,
authTimeoutSeconds = 10,
initTimeoutSeconds = 10,
pingIntervalSeconds = 30,
pingTimeoutSeconds = 10,
pingDisconnect = true,
autoReconnect = false,
initialRandomReconnectDelaySeconds = 5,
maxReconnectIntervalSeconds = 3600,
chain = chain,
channelFlags = 1,
paymentRequestExpirySeconds = 3600,
multiPartPaymentExpirySeconds = 60,
feeProportionalMillionths = 100,
pingInterval = 30.seconds,
initialRandomReconnectDelay = 5.seconds,
maxReconnectInterval = 60.minutes,
mppAggregationWindow = 60.seconds,
maxPaymentAttempts = 5,
enableTrampolinePayment = true,
zeroConfPeers = emptySet(),
paymentRecipientExpiryParams = RecipientCltvExpiryParams(CltvExpiryDelta(75), CltvExpiryDelta(200)),
liquidityPolicy = MutableStateFlow<LiquidityPolicy>(LiquidityPolicy.Auto(maxAbsoluteFee = 2_000.sat, maxRelativeFeeBasisPoints = 3_000 /* 3000 = 30 % */, skipAbsoluteFeeCheck = false))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ data class LegacyWaitForFundingLocked(
staticParams.nodeParams.expiryDeltaBlocks,
commitments.params.remoteParams.htlcMinimum,
staticParams.nodeParams.feeBase,
staticParams.nodeParams.feeProportionalMillionth.toLong(),
staticParams.nodeParams.feeProportionalMillionths.toLong(),
commitments.latest.fundingAmount.toMilliSatoshi(),
enable = Helpers.aboveReserve(commitments)
)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ data class WaitForChannelReady(
staticParams.nodeParams.expiryDeltaBlocks,
commitments.params.remoteParams.htlcMinimum,
staticParams.nodeParams.feeBase,
staticParams.nodeParams.feeProportionalMillionth.toLong(),
staticParams.nodeParams.feeProportionalMillionths.toLong(),
commitments.latest.fundingAmount.toMilliSatoshi(),
enable = Helpers.aboveReserve(commitments)
)
Expand Down
6 changes: 3 additions & 3 deletions src/commonMain/kotlin/fr/acinq/lightning/io/Peer.kt
Original file line number Diff line number Diff line change
Expand Up @@ -250,7 +250,7 @@ class Peer(
// If we have some htlcs that have timed out, we may need to close channels to ensure we don't lose funds.
// But maybe we were offline for too long and it is why our peer couldn't settle these htlcs in time.
// We give them a bit of time after we reconnect to send us their latest htlc updates.
delay(timeMillis = nodeParams.checkHtlcTimeoutAfterStartupDelaySeconds.toLong() * 1000)
delay(nodeParams.checkHtlcTimeoutAfterStartupDelay)
logger.info { "checking for timed out htlcs for channels: ${channelIds.joinToString(", ")}" }
channelIds.forEach { input.send(WrappedChannelCommand(it, ChannelCommand.Commitment.CheckHtlcTimeout)) }
}
Expand Down Expand Up @@ -379,14 +379,14 @@ class Peer(
suspend fun doPing() {
val ping = Ping(10, ByteVector("deadbeef"))
while (isActive) {
delay(30.seconds)
delay(nodeParams.pingInterval)
peerConnection.send(ping)
}
}

suspend fun checkPaymentsTimeout() {
while (isActive) {
delay(10.seconds) // we schedule a check every 10 seconds
delay(nodeParams.checkHtlcTimeoutInterval)
input.send(CheckPaymentsTimeout)
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -375,7 +375,7 @@ class IncomingPaymentHandler(val nodeParams: NodeParams, val db: IncomingPayment
// - SHOULD wait for at least 60 seconds after the initial HTLC.
// - SHOULD use mpp_timeout for the failure message.
pending.forEach { (paymentHash, payment) ->
val isExpired = currentTimestampSeconds > (payment.startedAtSeconds + nodeParams.multiPartPaymentExpirySeconds)
val isExpired = currentTimestampSeconds > (payment.startedAtSeconds + nodeParams.mppAggregationWindow.inWholeSeconds)
if (isExpired) {
keysToRemove += paymentHash
payment.parts.forEach { part ->
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@ data class LNChannel<out S : ChannelState>(

fun process(cmd: ChannelCommand): Pair<LNChannel<ChannelState>, List<ChannelAction>> =
state
.run { ctx.copy(logger = ctx.logger.copy(staticMdc = mapOf("alias" to ctx.staticParams.nodeParams.alias) + state.mdc())).process(cmd) }
.run { ctx.copy(logger = ctx.logger.copy(staticMdc = state.mdc())).process(cmd) }
.let { (newState, actions) ->
checkSerialization(actions)
JsonSerializers.json.encodeToString(newState)
Expand Down
7 changes: 4 additions & 3 deletions src/commonTest/kotlin/fr/acinq/lightning/io/peer/PeerTest.kt
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ import fr.acinq.lightning.wire.*
import kotlinx.coroutines.flow.first
import kotlinx.coroutines.flow.map
import kotlin.test.*
import kotlin.time.Duration.Companion.seconds

class PeerTest : LightningTestSuite() {

Expand Down Expand Up @@ -331,7 +332,7 @@ class PeerTest : LightningTestSuite() {

val peer = buildPeer(
this,
alice0.staticParams.nodeParams.copy(checkHtlcTimeoutAfterStartupDelaySeconds = 5),
alice0.staticParams.nodeParams.copy(checkHtlcTimeoutAfterStartupDelay = 5.seconds),
TestConstants.Alice.walletParams,
databases = InMemoryDatabases().also { it.channels.addOrUpdateChannel(alice1.state) },
currentTip = htlc.cltvExpiry.toLong().toInt() to Block.RegtestGenesisBlock.header
Expand Down Expand Up @@ -401,7 +402,7 @@ class PeerTest : LightningTestSuite() {

val peer = buildPeer(
this,
bob0.staticParams.nodeParams.copy(checkHtlcTimeoutAfterStartupDelaySeconds = 5),
bob0.staticParams.nodeParams.copy(checkHtlcTimeoutAfterStartupDelay = 5.seconds),
TestConstants.Bob.walletParams,
databases = InMemoryDatabases(), // NB: empty database (Bob has lost its channel state)
currentTip = htlc.cltvExpiry.toLong().toInt() to Block.RegtestGenesisBlock.header
Expand Down Expand Up @@ -431,7 +432,7 @@ class PeerTest : LightningTestSuite() {

val peer = buildPeer(
this,
bob0.staticParams.nodeParams.copy(checkHtlcTimeoutAfterStartupDelaySeconds = 5),
bob0.staticParams.nodeParams.copy(checkHtlcTimeoutAfterStartupDelay = 5.seconds),
TestConstants.Bob.walletParams,
databases = InMemoryDatabases().also { it.channels.addOrUpdateChannel(bob0.state) }, // NB: outdated channel data
currentTip = htlc.cltvExpiry.toLong().toInt() to Block.RegtestGenesisBlock.header
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -757,7 +757,7 @@ class IncomingPaymentHandlerTestsCommon : LightningTestSuite() {
assertTrue(result1.actions.isEmpty())

// It expires after a while.
val actions1 = paymentHandler.checkPaymentsTimeout(currentTimestampSeconds() + paymentHandler.nodeParams.multiPartPaymentExpirySeconds + 2)
val actions1 = paymentHandler.checkPaymentsTimeout(currentTimestampSeconds() + paymentHandler.nodeParams.mppAggregationWindow.inWholeSeconds + 2)
val addTimeout = ChannelCommand.Htlc.Settlement.Fail(add.id, ChannelCommand.Htlc.Settlement.Fail.Reason.Failure(PaymentTimeout), commit = true)
assertEquals(listOf(WrappedChannelCommand(add.channelId, addTimeout)), actions1)

Expand All @@ -767,7 +767,7 @@ class IncomingPaymentHandlerTestsCommon : LightningTestSuite() {
assertTrue(result2.actions.isEmpty())

// It expires again.
val actions2 = paymentHandler.checkPaymentsTimeout(currentTimestampSeconds() + paymentHandler.nodeParams.multiPartPaymentExpirySeconds + 2)
val actions2 = paymentHandler.checkPaymentsTimeout(currentTimestampSeconds() + paymentHandler.nodeParams.mppAggregationWindow.inWholeSeconds + 2)
assertEquals(listOf(WrappedChannelCommand(add.channelId, addTimeout)), actions2)

// The channel was offline again, didn't process the failure and retransmits the htlc, but it is now close to its expiry.
Expand Down Expand Up @@ -943,15 +943,15 @@ class IncomingPaymentHandlerTestsCommon : LightningTestSuite() {
// Step 2 of 3:
// - don't expire the multipart htlcs too soon.
run {
val currentTimestampSeconds = startTime + paymentHandler.nodeParams.multiPartPaymentExpirySeconds - 2
val currentTimestampSeconds = startTime + paymentHandler.nodeParams.mppAggregationWindow.inWholeSeconds - 2
val actions = paymentHandler.checkPaymentsTimeout(currentTimestampSeconds)
assertTrue(actions.isEmpty())
}

// Step 3 of 3:
// - expire the htlc-set after configured expiration.
run {
val currentTimestampSeconds = startTime + paymentHandler.nodeParams.multiPartPaymentExpirySeconds + 2
val currentTimestampSeconds = startTime + paymentHandler.nodeParams.mppAggregationWindow.inWholeSeconds + 2
val actions = paymentHandler.checkPaymentsTimeout(currentTimestampSeconds)
val expected = setOf(
WrappedChannelCommand(channelId, ChannelCommand.Htlc.Settlement.Fail(1, ChannelCommand.Htlc.Settlement.Fail.Reason.Failure(PaymentTimeout), commit = true)),
Expand Down Expand Up @@ -981,7 +981,7 @@ class IncomingPaymentHandlerTestsCommon : LightningTestSuite() {
// Step 2 of 4:
// - the MPP set times out
run {
val currentTimestampSeconds = startTime + paymentHandler.nodeParams.multiPartPaymentExpirySeconds + 2
val currentTimestampSeconds = startTime + paymentHandler.nodeParams.mppAggregationWindow.inWholeSeconds + 2
val actions = paymentHandler.checkPaymentsTimeout(currentTimestampSeconds)
val expected = WrappedChannelCommand(channelId, ChannelCommand.Htlc.Settlement.Fail(1, ChannelCommand.Htlc.Settlement.Fail.Reason.Failure(PaymentTimeout), commit = true))
assertEquals(setOf(expected), actions.toSet())
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,6 @@ object TestConstants {
loggerFactory = testLoggerFactory,
keyManager = keyManager,
).copy(
alias = "alice",
features = Features(
Feature.OptionDataLossProtect to FeatureSupport.Optional,
Feature.VariableLengthOnion to FeatureSupport.Mandatory,
Expand Down Expand Up @@ -85,7 +84,7 @@ object TestConstants {
toRemoteDelayBlocks = CltvExpiryDelta(144),
maxToLocalDelayBlocks = CltvExpiryDelta(2048),
feeBase = 100.msat,
feeProportionalMillionth = 10,
feeProportionalMillionths = 10,
paymentRecipientExpiryParams = RecipientCltvExpiryParams(CltvExpiryDelta(0), CltvExpiryDelta(0)),
)

Expand All @@ -104,7 +103,6 @@ object TestConstants {
loggerFactory = testLoggerFactory,
keyManager = keyManager,
).copy(
alias = "bob",
dustLimit = 1_000.sat,
maxRemoteDustLimit = 1_500.sat,
onChainFeeConf = OnChainFeeConf(
Expand All @@ -117,7 +115,7 @@ object TestConstants {
toRemoteDelayBlocks = CltvExpiryDelta(144),
maxToLocalDelayBlocks = CltvExpiryDelta(1024),
feeBase = 10.msat,
feeProportionalMillionth = 10,
feeProportionalMillionths = 10,
paymentRecipientExpiryParams = RecipientCltvExpiryParams(CltvExpiryDelta(0), CltvExpiryDelta(0)),
)

Expand Down

0 comments on commit 9bd3489

Please sign in to comment.