From 04f8f24beaed2336d679a9632814f2677df132d8 Mon Sep 17 00:00:00 2001 From: pm47 Date: Thu, 9 May 2019 11:04:07 +0200 Subject: [PATCH 1/2] accept `commit_sig` without changes LND sometimes sends a new signature without any changes, which is a (harmless) spec violation. --- .../src/main/scala/fr/acinq/eclair/channel/Commitments.scala | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/eclair-core/src/main/scala/fr/acinq/eclair/channel/Commitments.scala b/eclair-core/src/main/scala/fr/acinq/eclair/channel/Commitments.scala index 0da1e760d4..a908f0def1 100644 --- a/eclair-core/src/main/scala/fr/acinq/eclair/channel/Commitments.scala +++ b/eclair-core/src/main/scala/fr/acinq/eclair/channel/Commitments.scala @@ -395,8 +395,9 @@ object Commitments { // we will reply to this sig with our old revocation hash preimage (at index) and our next revocation hash (at index + 1) // and will increment our index - if (!remoteHasChanges(commitments)) - throw CannotSignWithoutChanges(commitments.channelId) + // LND sometimes sends a new signature without any changes, which is a (harmless) spec violation + //if (!remoteHasChanges(commitments)) + // throw CannotSignWithoutChanges(commitments.channelId) // check that their signature is valid // signatures are now optional in the commit message, and will be sent only if the other party is actually From 94cdf4353cae89e71b624895b37229ca2ad4ff1b Mon Sep 17 00:00:00 2001 From: pm47 Date: Thu, 9 May 2019 12:13:47 +0200 Subject: [PATCH 2/2] added log and ignored test Note that the test was previously not failing because it wasn't specific enough. The test now fails and has been ignored. --- .../main/scala/fr/acinq/eclair/channel/Commitments.scala | 8 +++++--- .../acinq/eclair/channel/states/e/NormalStateSpec.scala | 6 ++++-- 2 files changed, 9 insertions(+), 5 deletions(-) diff --git a/eclair-core/src/main/scala/fr/acinq/eclair/channel/Commitments.scala b/eclair-core/src/main/scala/fr/acinq/eclair/channel/Commitments.scala index a908f0def1..8f620e4545 100644 --- a/eclair-core/src/main/scala/fr/acinq/eclair/channel/Commitments.scala +++ b/eclair-core/src/main/scala/fr/acinq/eclair/channel/Commitments.scala @@ -395,9 +395,11 @@ object Commitments { // we will reply to this sig with our old revocation hash preimage (at index) and our next revocation hash (at index + 1) // and will increment our index - // LND sometimes sends a new signature without any changes, which is a (harmless) spec violation - //if (!remoteHasChanges(commitments)) - // throw CannotSignWithoutChanges(commitments.channelId) + // lnd sometimes sends a new signature without any changes, which is a (harmless) spec violation + if (!remoteHasChanges(commitments)) { + // throw CannotSignWithoutChanges(commitments.channelId) + log.warning("received a commit sig with no changes (probably coming from lnd)") + } // check that their signature is valid // signatures are now optional in the commit message, and will be sent only if the other party is actually diff --git a/eclair-core/src/test/scala/fr/acinq/eclair/channel/states/e/NormalStateSpec.scala b/eclair-core/src/test/scala/fr/acinq/eclair/channel/states/e/NormalStateSpec.scala index 5da32b81ef..0852aa3959 100644 --- a/eclair-core/src/test/scala/fr/acinq/eclair/channel/states/e/NormalStateSpec.scala +++ b/eclair-core/src/test/scala/fr/acinq/eclair/channel/states/e/NormalStateSpec.scala @@ -714,13 +714,14 @@ class NormalStateSpec extends TestkitBaseClass with StateTestsHelperMethods { assert(bob.stateData.asInstanceOf[DATA_NORMAL].commitments.localCommit.publishableTxs.commitTx.tx.txOut.count(_.amount == Satoshi(50000)) == 2) } - test("recv CommitSig (no changes)") { f => + ignore("recv CommitSig (no changes)") { f => import f._ val tx = bob.stateData.asInstanceOf[DATA_NORMAL].commitments.localCommit.publishableTxs.commitTx.tx val sender = TestProbe() // signature is invalid but it doesn't matter sender.send(bob, CommitSig(ByteVector32.Zeroes, ByteVector.fill(64)(0), Nil)) - bob2alice.expectMsgType[Error] + val error = bob2alice.expectMsgType[Error] + assert(new String(error.data.toArray).startsWith("cannot sign when there are no changes")) awaitCond(bob.stateName == CLOSING) // channel should be advertised as down assert(channelUpdateListener.expectMsgType[LocalChannelDown].channelId === bob.stateData.asInstanceOf[DATA_CLOSING].channelId) @@ -739,6 +740,7 @@ class NormalStateSpec extends TestkitBaseClass with StateTestsHelperMethods { sender.send(bob, CommitSig(ByteVector32.Zeroes, ByteVector.fill(64)(0), Nil)) val error = bob2alice.expectMsgType[Error] assert(new String(error.data.toArray).startsWith("invalid commitment signature")) + awaitCond(bob.stateName == CLOSING) bob2blockchain.expectMsg(PublishAsap(tx)) bob2blockchain.expectMsgType[PublishAsap] bob2blockchain.expectMsgType[WatchConfirmed]