Skip to content

Commit

Permalink
Assume widely supported features (#2732)
Browse files Browse the repository at this point in the history
lightning/bolts#1092 makes some features compulsory
and lets us assume that other nodes have support for them.
  • Loading branch information
t-bast authored Sep 27, 2023
1 parent 3e1cc9d commit 37f3fbe
Show file tree
Hide file tree
Showing 8 changed files with 76 additions and 46 deletions.
6 changes: 3 additions & 3 deletions eclair-core/src/main/resources/reference.conf
Original file line number Diff line number Diff line change
Expand Up @@ -54,11 +54,11 @@ eclair {
// You will not be able to change this address, which can be dangerous, especially for very long lived channels.
// Make sure you understand what it implies before you activate this feature.
option_upfront_shutdown_script = disabled
option_data_loss_protect = optional
gossip_queries = optional
option_data_loss_protect = mandatory
gossip_queries = mandatory
gossip_queries_ex = optional
var_onion_optin = mandatory
option_static_remotekey = optional
option_static_remotekey = mandatory
payment_secret = mandatory
basic_mpp = optional
option_support_large_channel = optional
Expand Down
4 changes: 3 additions & 1 deletion eclair-core/src/main/scala/fr/acinq/eclair/NodeParams.scala
Original file line number Diff line number Diff line change
Expand Up @@ -326,9 +326,11 @@ object NodeParams extends Logging {
def validateFeatures(features: Features[Feature]): Unit = {
val featuresErr = Features.validateFeatureGraph(features)
require(featuresErr.isEmpty, featuresErr.map(_.message))
require(!features.hasFeature(Features.InitialRoutingSync), s"${Features.InitialRoutingSync.rfcName} is not supported anymore, use ${Features.ChannelRangeQueries.rfcName} instead")
require(features.hasFeature(Features.DataLossProtect), s"${Features.DataLossProtect.rfcName} must be enabled")
require(features.hasFeature(Features.VariableLengthOnion, Some(FeatureSupport.Mandatory)), s"${Features.VariableLengthOnion.rfcName} must be enabled and mandatory")
require(features.hasFeature(Features.PaymentSecret, Some(FeatureSupport.Mandatory)), s"${Features.PaymentSecret.rfcName} must be enabled and mandatory")
require(!features.hasFeature(Features.InitialRoutingSync), s"${Features.InitialRoutingSync.rfcName} is not supported anymore, use ${Features.ChannelRangeQueries.rfcName} instead")
require(features.hasFeature(Features.StaticRemoteKey), s"${Features.StaticRemoteKey.rfcName} must be enabled")
require(features.hasFeature(Features.ChannelType), s"${Features.ChannelType.rfcName} must be enabled")
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -393,13 +393,11 @@ class PeerConnection(keyPair: KeyPair, conf: PeerConnection.Conf, switchboard: A
stay() using d.copy(behavior = behavior1)

case Event(DoSync(replacePrevious), d: ConnectedData) =>
val canUseChannelRangeQueries = Features.canUseFeature(d.localInit.features, d.remoteInit.features, Features.ChannelRangeQueries)
// We assume support for standard range queries since https://github.com/lightning/bolts/pull/1092
val canUseChannelRangeQueriesEx = Features.canUseFeature(d.localInit.features, d.remoteInit.features, Features.ChannelRangeQueriesExtended)
if (canUseChannelRangeQueries || canUseChannelRangeQueriesEx) {
val flags_opt = if (canUseChannelRangeQueriesEx) Some(QueryChannelRangeTlv.QueryFlags(QueryChannelRangeTlv.QueryFlags.WANT_ALL)) else None
log.debug(s"sending sync channel range query with flags_opt=$flags_opt replacePrevious=$replacePrevious")
router ! SendChannelQuery(d.chainHash, d.remoteNodeId, self, replacePrevious, flags_opt)
}
val flags_opt = if (canUseChannelRangeQueriesEx) Some(QueryChannelRangeTlv.QueryFlags(QueryChannelRangeTlv.QueryFlags.WANT_ALL)) else None
log.debug(s"sending sync channel range query with flags_opt=$flags_opt replacePrevious=$replacePrevious")
router ! SendChannelQuery(d.chainHash, d.remoteNodeId, self, replacePrevious, flags_opt)
stay()

case Event(ResumeAnnouncements, d: ConnectedData) =>
Expand Down
32 changes: 27 additions & 5 deletions eclair-core/src/test/scala/fr/acinq/eclair/StartupSpec.scala
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,7 @@ class StartupSpec extends AnyFunSuite {
s"features.${VariableLengthOnion.rfcName}" -> "mandatory",
s"features.${PaymentSecret.rfcName}" -> "mandatory",
s"features.${BasicMultiPartPayment.rfcName}" -> "optional",
s"features.${StaticRemoteKey.rfcName}" -> "mandatory",
).asJava)

// var_onion_optin cannot be disabled
Expand All @@ -107,6 +108,7 @@ class StartupSpec extends AnyFunSuite {
s"features.${ChannelRangeQueries.rfcName}" -> "optional",
s"features.${ChannelRangeQueriesExtended.rfcName}" -> "optional",
s"features.${ChannelType.rfcName}" -> "optional",
s"features.${StaticRemoteKey.rfcName}" -> "optional",
).asJava)

// var_onion_optin cannot be optional
Expand All @@ -115,6 +117,7 @@ class StartupSpec extends AnyFunSuite {
s"features.${ChannelType.rfcName}" -> "optional",
s"features.${VariableLengthOnion.rfcName}" -> "optional",
s"features.${PaymentSecret.rfcName}" -> "mandatory",
s"features.${StaticRemoteKey.rfcName}" -> "optional",
).asJava)

// payment_secret cannot be optional
Expand All @@ -123,6 +126,7 @@ class StartupSpec extends AnyFunSuite {
s"features.${ChannelType.rfcName}" -> "optional",
s"features.${VariableLengthOnion.rfcName}" -> "mandatory",
s"features.${PaymentSecret.rfcName}" -> "optional",
s"features.${StaticRemoteKey.rfcName}" -> "optional",
).asJava)

// option_channel_type cannot be disabled
Expand All @@ -133,6 +137,18 @@ class StartupSpec extends AnyFunSuite {
s"features.${VariableLengthOnion.rfcName}" -> "mandatory",
s"features.${PaymentSecret.rfcName}" -> "mandatory",
s"features.${BasicMultiPartPayment.rfcName}" -> "optional",
s"features.${StaticRemoteKey.rfcName}" -> "optional",
).asJava)

// option_static_remotekey cannot be disabled
val noStaticRemoteKeyConf = ConfigFactory.parseMap(Map(
s"features.${DataLossProtect.rfcName}" -> "optional",
s"features.${ChannelRangeQueries.rfcName}" -> "optional",
s"features.${ChannelRangeQueriesExtended.rfcName}" -> "optional",
s"features.${ChannelType.rfcName}" -> "optional",
s"features.${VariableLengthOnion.rfcName}" -> "mandatory",
s"features.${PaymentSecret.rfcName}" -> "mandatory",
s"features.${BasicMultiPartPayment.rfcName}" -> "optional",
).asJava)

// initial_routing_sync cannot be enabled
Expand All @@ -144,6 +160,7 @@ class StartupSpec extends AnyFunSuite {
s"features.${ChannelType.rfcName}" -> "optional",
s"features.${VariableLengthOnion.rfcName}" -> "mandatory",
s"features.${PaymentSecret.rfcName}" -> "mandatory",
s"features.${StaticRemoteKey.rfcName}" -> "optional",
).asJava)

// extended channel queries without channel queries
Expand All @@ -153,13 +170,15 @@ class StartupSpec extends AnyFunSuite {
s"features.${ChannelType.rfcName}" -> "optional",
s"features.${VariableLengthOnion.rfcName}" -> "mandatory",
s"features.${PaymentSecret.rfcName}" -> "mandatory",
s"features.${StaticRemoteKey.rfcName}" -> "optional",
).asJava)

assert(Try(makeNodeParamsWithDefaults(finalizeConf(legalFeaturesConf))).isSuccess)
assert(Try(makeNodeParamsWithDefaults(finalizeConf(noVariableLengthOnionConf))).isFailure)
assert(Try(makeNodeParamsWithDefaults(finalizeConf(optionalVarOnionOptinConf))).isFailure)
assert(Try(makeNodeParamsWithDefaults(finalizeConf(optionalPaymentSecretConf))).isFailure)
assert(Try(makeNodeParamsWithDefaults(finalizeConf(noChannelTypeConf))).isFailure)
assert(Try(makeNodeParamsWithDefaults(finalizeConf(noStaticRemoteKeyConf))).isFailure)
assert(Try(makeNodeParamsWithDefaults(finalizeConf(initialRoutingSyncConf))).isFailure)
assert(Try(makeNodeParamsWithDefaults(finalizeConf(illegalFeaturesConf))).isFailure)
}
Expand All @@ -168,8 +187,10 @@ class StartupSpec extends AnyFunSuite {
val perNodeConf = ConfigFactory.parseString(
"""
| features {
| option_data_loss_protect = optional
| var_onion_optin = mandatory
| payment_secret = mandatory
| option_static_remotekey = optional
| option_channel_type = optional
| }
| override-init-features = [ // optional per-node features
Expand All @@ -186,13 +207,14 @@ class StartupSpec extends AnyFunSuite {

val nodeParams = makeNodeParamsWithDefaults(perNodeConf.withFallback(defaultConf.withoutPath("features")))
val perNodeFeatures = nodeParams.initFeaturesFor(PublicKey(ByteVector.fromValidHex("031b84c5567b126440995d3ed5aaba0565d71e1834604819ff9c17f5e9d5dd078f")))
assert(perNodeFeatures == Features(VariableLengthOnion -> Mandatory, PaymentSecret -> Mandatory, BasicMultiPartPayment -> Mandatory, ChannelRangeQueries -> Optional, ChannelType -> Optional))
assert(perNodeFeatures == Features(DataLossProtect -> Optional, VariableLengthOnion -> Mandatory, PaymentSecret -> Mandatory, BasicMultiPartPayment -> Mandatory, ChannelRangeQueries -> Optional, StaticRemoteKey -> Optional, ChannelType -> Optional))
}

test("combine node override features with default features") {
val perNodeConf = ConfigFactory.parseString(
"""
| features {
| option_data_loss_protect = optional
| var_onion_optin = mandatory
| payment_secret = mandatory
| basic_mpp = mandatory
Expand All @@ -211,7 +233,7 @@ class StartupSpec extends AnyFunSuite {
| nodeid = "024d4b6cd1361032ca9bd2aeb9d900aa4d45d9ead80ac9423374c451a7254d0766"
| features {
| basic_mpp = optional
| option_static_remotekey = disabled
| option_static_remotekey = optional
| }
| }
| ]
Expand All @@ -220,11 +242,11 @@ class StartupSpec extends AnyFunSuite {

val nodeParams = makeNodeParamsWithDefaults(perNodeConf.withFallback(defaultConf.withoutPath("features")))
val defaultFeatures = nodeParams.features
assert(defaultFeatures == Features(VariableLengthOnion -> Mandatory, PaymentSecret -> Mandatory, BasicMultiPartPayment -> Mandatory, StaticRemoteKey -> Optional, ChannelType -> Optional))
assert(defaultFeatures == Features(DataLossProtect -> Optional, VariableLengthOnion -> Mandatory, PaymentSecret -> Mandatory, BasicMultiPartPayment -> Mandatory, StaticRemoteKey -> Optional, ChannelType -> Optional))
val perNodeFeatures1 = nodeParams.initFeaturesFor(PublicKey(ByteVector.fromValidHex("031b84c5567b126440995d3ed5aaba0565d71e1834604819ff9c17f5e9d5dd078f")))
assert(perNodeFeatures1 == Features(VariableLengthOnion -> Mandatory, PaymentSecret -> Mandatory, BasicMultiPartPayment -> Mandatory, StaticRemoteKey -> Mandatory, AnchorOutputsZeroFeeHtlcTx -> Optional, ChannelType -> Optional))
assert(perNodeFeatures1 == Features(DataLossProtect -> Optional, VariableLengthOnion -> Mandatory, PaymentSecret -> Mandatory, BasicMultiPartPayment -> Mandatory, StaticRemoteKey -> Mandatory, AnchorOutputsZeroFeeHtlcTx -> Optional, ChannelType -> Optional))
val perNodeFeatures2 = nodeParams.initFeaturesFor(PublicKey(ByteVector.fromValidHex("024d4b6cd1361032ca9bd2aeb9d900aa4d45d9ead80ac9423374c451a7254d0766")))
assert(perNodeFeatures2 == Features(VariableLengthOnion -> Mandatory, PaymentSecret -> Mandatory, BasicMultiPartPayment -> Optional, ChannelType -> Optional))
assert(perNodeFeatures2 == Features(DataLossProtect -> Optional, VariableLengthOnion -> Mandatory, PaymentSecret -> Mandatory, BasicMultiPartPayment -> Optional, StaticRemoteKey -> Optional, ChannelType -> Optional))
}

test("reject non-init features in node override") {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -183,12 +183,15 @@ abstract class ChannelIntegrationSpec extends IntegrationSpec {
paymentSender.expectMsgType[PaymentSent](max = 60 seconds)
// we then generate enough blocks so that nodes get their main delayed output
generateBlocks(25, Some(minerAddress))
// F should have 2 recv transactions: the redeemed htlc and its main output
// C should have 1 recv transaction: its main output
val expectedTxCountC = 1 // C should have 1 recv transaction: its main output
val expectedTxCountF = commitmentFormat match {
case _: AnchorOutputsCommitmentFormat => 2 // F should have 2 recv transactions: the redeemed htlc and its main output
case Transactions.DefaultCommitmentFormat => 1 // F's main output uses static_remotekey
}
awaitCond({
val receivedByC = listReceivedByAddress(finalAddressC, sender)
val receivedByF = listReceivedByAddress(finalAddressF)
(receivedByF diff previouslyReceivedByF).size == 2 && (receivedByC diff previouslyReceivedByC).size == 1
(receivedByF diff previouslyReceivedByF).size == expectedTxCountF && (receivedByC diff previouslyReceivedByC).size == expectedTxCountC
}, max = 30 seconds, interval = 1 second)
// we generate blocks to make txs confirm
generateBlocks(2, Some(minerAddress))
Expand Down Expand Up @@ -221,12 +224,15 @@ abstract class ChannelIntegrationSpec extends IntegrationSpec {
paymentSender.expectMsgType[PaymentSent](max = 60 seconds)
// we then generate enough blocks so that F gets its htlc-success delayed output
generateBlocks(25, Some(minerAddress))
// F should have 2 recv transactions: the redeemed htlc and its main output
// C should have 1 recv transaction: its main output
val expectedTxCountC = commitmentFormat match {
case _: AnchorOutputsCommitmentFormat => 1 // C should have 1 recv transaction: its main output
case Transactions.DefaultCommitmentFormat => 0 // C's main output uses static_remotekey
}
val expectedTxCountF = 2 // F should have 2 recv transactions: the redeemed htlc and its main output
awaitCond({
val receivedByC = listReceivedByAddress(finalAddressC, sender)
val receivedByF = listReceivedByAddress(finalAddressF, sender)
(receivedByF diff previouslyReceivedByF).size == 2 && (receivedByC diff previouslyReceivedByC).size == 1
(receivedByF diff previouslyReceivedByF).size == expectedTxCountF && (receivedByC diff previouslyReceivedByC).size == expectedTxCountC
}, max = 30 seconds, interval = 1 second)
// we generate blocks to make txs confirm
generateBlocks(2, Some(minerAddress))
Expand Down Expand Up @@ -271,12 +277,15 @@ abstract class ChannelIntegrationSpec extends IntegrationSpec {
assert(failed.failures.head.asInstanceOf[RemoteFailure].e == DecryptedFailurePacket(nodes("C").nodeParams.nodeId, PermanentChannelFailure()))
// we then generate enough blocks to confirm all delayed transactions
generateBlocks(25, Some(minerAddress))
// C should have 2 recv transactions: its main output and the htlc timeout
// F should have 1 recv transaction: its main output
val expectedTxCountC = 2 // C should have 2 recv transactions: its main output and the htlc timeout
val expectedTxCountF = commitmentFormat match {
case _: AnchorOutputsCommitmentFormat => 1 // F should have 1 recv transaction: its main output
case Transactions.DefaultCommitmentFormat => 0 // F's main output uses static_remotekey
}
awaitCond({
val receivedByC = listReceivedByAddress(finalAddressC, sender)
val receivedByF = listReceivedByAddress(finalAddressF, sender)
(receivedByF diff previouslyReceivedByF).size == 1 && (receivedByC diff previouslyReceivedByC).size == 2
(receivedByF diff previouslyReceivedByF).size == expectedTxCountF && (receivedByC diff previouslyReceivedByC).size == expectedTxCountC
}, max = 30 seconds, interval = 1 second)
// we generate blocks to make txs confirm
generateBlocks(2, Some(minerAddress))
Expand Down Expand Up @@ -324,12 +333,15 @@ abstract class ChannelIntegrationSpec extends IntegrationSpec {
assert(failed.failures.head.asInstanceOf[RemoteFailure].e == DecryptedFailurePacket(nodes("C").nodeParams.nodeId, PermanentChannelFailure()))
// we then generate enough blocks to confirm all delayed transactions
generateBlocks(25, Some(minerAddress))
// C should have 2 recv transactions: its main output and the htlc timeout
// F should have 1 recv transaction: its main output
val expectedTxCountC = commitmentFormat match {
case _: AnchorOutputsCommitmentFormat => 2 // C should have 2 recv transactions: its main output and the htlc timeout
case Transactions.DefaultCommitmentFormat => 1 // C's main output uses static_remotekey
}
val expectedTxCountF = 1 // F should have 1 recv transaction: its main output
awaitCond({
val receivedByC = listReceivedByAddress(finalAddressC, sender)
val receivedByF = listReceivedByAddress(finalAddressF, sender)
(receivedByF diff previouslyReceivedByF).size == 1 && (receivedByC diff previouslyReceivedByC).size == 2
(receivedByF diff previouslyReceivedByF).size == expectedTxCountF && (receivedByC diff previouslyReceivedByC).size == expectedTxCountC
}, max = 30 seconds, interval = 1 second)
// we generate blocks to make tx confirm
generateBlocks(2, Some(minerAddress))
Expand Down Expand Up @@ -455,9 +467,9 @@ abstract class ChannelIntegrationSpec extends IntegrationSpec {
class StandardChannelIntegrationSpec extends ChannelIntegrationSpec {

test("start eclair nodes") {
instantiateEclairNode("A", ConfigFactory.parseMap(Map("eclair.node-alias" -> "A", "eclair.channel.expiry-delta-blocks" -> 40, "eclair.channel.fulfill-safety-before-timeout-blocks" -> 12, "eclair.server.port" -> (if (useEclairSigner) 29840 else 29740), "eclair.api.port" -> (if (useEclairSigner) 28190 else 28090)).asJava).withFallback(withDefaultCommitment).withFallback(commonConfig))
instantiateEclairNode("A", ConfigFactory.parseMap(Map("eclair.node-alias" -> "A", "eclair.channel.expiry-delta-blocks" -> 40, "eclair.channel.fulfill-safety-before-timeout-blocks" -> 12, "eclair.server.port" -> (if (useEclairSigner) 29840 else 29740), "eclair.api.port" -> (if (useEclairSigner) 28190 else 28090)).asJava).withFallback(withStaticRemoteKey).withFallback(commonConfig))
instantiateEclairNode("C", ConfigFactory.parseMap(Map("eclair.node-alias" -> "C", "eclair.channel.expiry-delta-blocks" -> 40, "eclair.channel.fulfill-safety-before-timeout-blocks" -> 12, "eclair.server.port" -> (if (useEclairSigner) 29841 else 29741), "eclair.api.port" -> (if (useEclairSigner) 28191 else 28091)).asJava).withFallback(withAnchorOutputs).withFallback(commonConfig))
instantiateEclairNode("F", ConfigFactory.parseMap(Map("eclair.node-alias" -> "F", "eclair.channel.expiry-delta-blocks" -> 40, "eclair.channel.fulfill-safety-before-timeout-blocks" -> 12, "eclair.server.port" -> (if (useEclairSigner) 29842 else 29742), "eclair.api.port" -> (if (useEclairSigner) 28192 else 28092)).asJava).withFallback(withDefaultCommitment).withFallback(commonConfig))
instantiateEclairNode("F", ConfigFactory.parseMap(Map("eclair.node-alias" -> "F", "eclair.channel.expiry-delta-blocks" -> 40, "eclair.channel.fulfill-safety-before-timeout-blocks" -> 12, "eclair.server.port" -> (if (useEclairSigner) 29842 else 29742), "eclair.api.port" -> (if (useEclairSigner) 28192 else 28092)).asJava).withFallback(withStaticRemoteKey).withFallback(commonConfig))
}

test("connect nodes") {
Expand Down Expand Up @@ -614,10 +626,11 @@ class StandardChannelIntegrationSpec extends ChannelIntegrationSpec {
sender.expectMsg(htlcSuccess.head.txid)
bitcoinClient.publishTransaction(htlcTimeout.head).pipeTo(sender.ref)
sender.expectMsg(htlcTimeout.head.txid)
// at this point C should have 6 recv transactions: its previous main output, F's main output and all htlc outputs (taken as punishment)
// at this point C should have 5 recv transactions: F's main output and all htlc outputs (taken as punishment)
// C's main output uses static_remotekey, so C doesn't need to claim it
awaitCond({
val receivedByC = listReceivedByAddress(finalAddressC, sender)
(receivedByC diff previouslyReceivedByC).size == 6
(receivedByC diff previouslyReceivedByC).size == 5
}, max = 30 seconds, interval = 1 second)
// we generate blocks to make txs confirm
generateBlocks(2)
Expand Down
Loading

0 comments on commit 37f3fbe

Please sign in to comment.