diff --git a/eclair-core/src/main/scala/fr/acinq/eclair/Features.scala b/eclair-core/src/main/scala/fr/acinq/eclair/Features.scala index 20db6a6c87..204dd04238 100644 --- a/eclair-core/src/main/scala/fr/acinq/eclair/Features.scala +++ b/eclair-core/src/main/scala/fr/acinq/eclair/Features.scala @@ -77,6 +77,20 @@ trait ChannelTypeFeature extends PermanentChannelFeature case class UnknownFeature(bitIndex: Int) +// @formatter:off +sealed trait FeatureCompatibilityResult { + def areCompatible: Boolean = this == FeatureCompatibilityResult.Compatible + def errorHints: Set[String] = this match { + case FeatureCompatibilityResult.Compatible => Set.empty + case r: FeatureCompatibilityResult.NotCompatible => r.hints + } +} +object FeatureCompatibilityResult { + case object Compatible extends FeatureCompatibilityResult + case class NotCompatible(hints: Set[String]) extends FeatureCompatibilityResult +} +// @formatter:on + case class Features[T <: Feature](activated: Map[T, FeatureSupport], unknown: Set[UnknownFeature] = Set.empty) { def isEmpty: Boolean = activated.isEmpty && unknown.isEmpty @@ -87,17 +101,20 @@ case class Features[T <: Feature](activated: Map[T, FeatureSupport], unknown: Se } /** NB: this method is not reflexive, see [[Features.areCompatible]] if you want symmetric validation. */ - def areSupported(remoteFeatures: Features[T]): Boolean = { + def testSupported(remoteFeatures: Features[T]): FeatureCompatibilityResult = { // we allow unknown odd features (it's ok to be odd) - val unknownFeaturesOk = remoteFeatures.unknown.forall(_.bitIndex % 2 == 1) + val incompatibleUnknownFeatures = remoteFeatures.unknown.filter(_.bitIndex % 2 == 0) // we verify that we activated every mandatory feature they require - val knownFeaturesOk = remoteFeatures.activated.forall { - case (_, Optional) => true - case (feature, Mandatory) => hasFeature(feature) - } - unknownFeaturesOk && knownFeaturesOk + val incompatibleKnownFeatures = remoteFeatures.activated.filter { + case (_, Optional) => false + case (feature, Mandatory) => !hasFeature(feature) + }.keySet + val incompatibleFeatures = incompatibleUnknownFeatures.map(u => s"unknown_${u.bitIndex}") ++ incompatibleKnownFeatures.map(_.rfcName) + if (incompatibleFeatures.isEmpty) FeatureCompatibilityResult.Compatible else FeatureCompatibilityResult.NotCompatible(incompatibleFeatures) } + def areSupported(remoteFeatures: Features[T]): Boolean = testSupported(remoteFeatures).areCompatible + def initFeatures(): Features[InitFeature] = Features(activated.collect { case (f: InitFeature, s) => (f, s) }, unknown) def nodeAnnouncementFeatures(): Features[NodeFeature] = Features(activated.collect { case (f: NodeFeature, s) => (f, s) }, unknown) @@ -354,8 +371,13 @@ object Features { FeatureException(s"$feature is set but is missing a dependency (${dependencies.filter(d => !features.unscoped().hasFeature(d)).mkString(" and ")})") } + def testCompatible[T <: Feature](ours: Features[T], theirs: Features[T]): FeatureCompatibilityResult = (ours.testSupported(theirs), theirs.testSupported(ours)) match { + case (FeatureCompatibilityResult.Compatible, FeatureCompatibilityResult.Compatible) => FeatureCompatibilityResult.Compatible + case (r1, r2) => FeatureCompatibilityResult.NotCompatible(r1.errorHints ++ r2.errorHints) + } + /** Returns true if both feature sets are compatible. */ - def areCompatible[T <: Feature](ours: Features[T], theirs: Features[T]): Boolean = ours.areSupported(theirs) && theirs.areSupported(ours) + def areCompatible[T <: Feature](ours: Features[T], theirs: Features[T]): Boolean = testCompatible(ours, theirs).areCompatible /** returns true if both have at least optional support */ def canUseFeature[T <: Feature](localFeatures: Features[T], remoteFeatures: Features[T], feature: T): Boolean = { diff --git a/eclair-core/src/main/scala/fr/acinq/eclair/io/PeerConnection.scala b/eclair-core/src/main/scala/fr/acinq/eclair/io/PeerConnection.scala index 228db37857..e52417c008 100644 --- a/eclair-core/src/main/scala/fr/acinq/eclair/io/PeerConnection.scala +++ b/eclair-core/src/main/scala/fr/acinq/eclair/io/PeerConnection.scala @@ -28,7 +28,7 @@ import fr.acinq.eclair.remote.EclairInternalsSerializer.RemoteTypes import fr.acinq.eclair.router.Router._ import fr.acinq.eclair.wire.protocol import fr.acinq.eclair.wire.protocol._ -import fr.acinq.eclair.{FSMDiagnosticActorLogging, Features, InitFeature, Logs, TimestampMilli, TimestampSecond} +import fr.acinq.eclair.{FSMDiagnosticActorLogging, FeatureCompatibilityResult, Features, InitFeature, Logs, TimestampMilli, TimestampSecond} import scodec.Attempt import scodec.bits.ByteVector @@ -132,6 +132,7 @@ class PeerConnection(keyPair: KeyPair, conf: PeerConnection.Conf, switchboard: A remoteInit.remoteAddress_opt.foreach(address => log.info("peer reports that our IP address is {} (public={})", address.toString, NodeAddress.isPublicIPAddress(address))) val featureGraphErr_opt = Features.validateFeatureGraph(remoteInit.features) + val featuresCompatibilityResult = Features.testCompatible(d.localInit.features, remoteInit.features) if (remoteInit.networks.nonEmpty && remoteInit.networks.intersect(d.localInit.networks).isEmpty) { log.warning(s"incompatible networks (${remoteInit.networks}), disconnecting") d.pendingAuth.origin_opt.foreach(_ ! ConnectionResult.InitializationFailed("incompatible networks")) @@ -143,9 +144,9 @@ class PeerConnection(keyPair: KeyPair, conf: PeerConnection.Conf, switchboard: A d.pendingAuth.origin_opt.foreach(_ ! ConnectionResult.InitializationFailed(featureGraphErr.message)) d.transport ! PoisonPill stay() - } else if (!Features.areCompatible(d.localInit.features, remoteInit.features)) { - log.warning("incompatible features, disconnecting") - d.pendingAuth.origin_opt.foreach(_ ! ConnectionResult.InitializationFailed("incompatible features")) + } else if (!featuresCompatibilityResult.areCompatible) { + log.warning(s"incompatible features (${featuresCompatibilityResult.errorHints.mkString(",")}), disconnecting") + d.pendingAuth.origin_opt.foreach(_ ! ConnectionResult.InitializationFailed(s"incompatible features (${featuresCompatibilityResult.errorHints.mkString(",")})")) d.transport ! PoisonPill stay() } else { diff --git a/eclair-core/src/test/scala/fr/acinq/eclair/io/PeerConnectionSpec.scala b/eclair-core/src/test/scala/fr/acinq/eclair/io/PeerConnectionSpec.scala index 78616b5d36..5f4eac9775 100644 --- a/eclair-core/src/test/scala/fr/acinq/eclair/io/PeerConnectionSpec.scala +++ b/eclair-core/src/test/scala/fr/acinq/eclair/io/PeerConnectionSpec.scala @@ -153,7 +153,7 @@ class PeerConnectionSpec extends TestKitBaseClass with FixtureAnyFunSuiteLike wi transport.send(peerConnection, LightningMessageCodecs.initCodec.decode(hex"0000 00050100000000".bits).require.value) transport.expectMsgType[TransportHandler.ReadAck] probe.expectTerminated(transport.ref) - origin.expectMsg(PeerConnection.ConnectionResult.InitializationFailed("incompatible features")) + origin.expectMsg(PeerConnection.ConnectionResult.InitializationFailed("incompatible features (unknown_32,payment_secret,var_onion_optin)")) peer.expectMsg(ConnectionDown(peerConnection)) } @@ -170,7 +170,7 @@ class PeerConnectionSpec extends TestKitBaseClass with FixtureAnyFunSuiteLike wi transport.send(peerConnection, LightningMessageCodecs.initCodec.decode(hex"00050100000000 0000".bits).require.value) transport.expectMsgType[TransportHandler.ReadAck] probe.expectTerminated(transport.ref) - origin.expectMsg(PeerConnection.ConnectionResult.InitializationFailed("incompatible features")) + origin.expectMsg(PeerConnection.ConnectionResult.InitializationFailed("incompatible features (unknown_32,payment_secret,var_onion_optin)")) peer.expectMsg(ConnectionDown(peerConnection)) }