Check input out of bounds early in tx_add_input (#2393)

We need to validate early that a `tx_add_input` message can be converted into an `OutPoint` without raising an out of bounds exception. We also fix a flaky test on slow machines.
ACINQ · Aug 23, 2022 · c1daaf3 · c1daaf3
1 parent fb6eb48
commit c1daaf3
Show file tree

Hide file tree

Showing 2 changed files with 6 additions and 4 deletions.
diff --git a/eclair-core/src/main/scala/fr/acinq/eclair/channel/InteractiveTxBuilder.scala b/eclair-core/src/main/scala/fr/acinq/eclair/channel/InteractiveTxBuilder.scala
@@ -479,12 +479,12 @@ private class InteractiveTxBuilder(replyTo: ActorRef[InteractiveTxBuilder.Respon
           } else if (session.remoteInputs.exists(_.serialId == addInput.serialId)) {
             replyTo ! RemoteFailure(DuplicateSerialId(fundingParams.channelId, addInput.serialId))
             unlockAndStop(session)
-          } else if (session.localInputs.exists(i => toOutPoint(i) == toOutPoint(addInput)) || session.remoteInputs.exists(i => toOutPoint(i) == toOutPoint(addInput))) {
-            replyTo ! RemoteFailure(DuplicateInput(fundingParams.channelId, addInput.serialId, addInput.previousTx.txid, addInput.previousTxOutput))
-            unlockAndStop(session)
           } else if (addInput.previousTx.txOut.length <= addInput.previousTxOutput) {
             replyTo ! RemoteFailure(InputOutOfBounds(fundingParams.channelId, addInput.serialId, addInput.previousTx.txid, addInput.previousTxOutput))
             unlockAndStop(session)
+          } else if (session.localInputs.exists(i => toOutPoint(i) == toOutPoint(addInput)) || session.remoteInputs.exists(i => toOutPoint(i) == toOutPoint(addInput))) {
+            replyTo ! RemoteFailure(DuplicateInput(fundingParams.channelId, addInput.serialId, addInput.previousTx.txid, addInput.previousTxOutput))
+            unlockAndStop(session)
           } else if (!Script.isNativeWitnessScript(addInput.previousTx.txOut(addInput.previousTxOutput.toInt).publicKeyScript)) {
             replyTo ! RemoteFailure(NonSegwitInput(fundingParams.channelId, addInput.serialId, addInput.previousTx.txid, addInput.previousTxOutput))
             unlockAndStop(session)
@@ -612,7 +612,7 @@ private class InteractiveTxBuilder(replyTo: ActorRef[InteractiveTxBuilder.Respon
       return Left(InvalidCompleteInteractiveTx(fundingParams.channelId))
     }
 
-    // The transaction must double-spent every previous attempt, otherwise there is a risk that two funding transactions
+    // The transaction must double-spend every previous attempt, otherwise there is a risk that two funding transactions
     // confirm for the same channel.
     val currentInputs = tx.txIn.map(_.outPoint).toSet
     val doubleSpendsPreviousTransactions = previousTransactions.forall(previousTx => previousTx.tx.buildUnsignedTx().txIn.map(_.outPoint).exists(o => currentInputs.contains(o)))

diff --git a/...re/src/test/scala/fr/acinq/eclair/channel/states/c/WaitForDualFundingReadyStateSpec.scala b/...re/src/test/scala/fr/acinq/eclair/channel/states/c/WaitForDualFundingReadyStateSpec.scala
@@ -82,6 +82,8 @@ class WaitForDualFundingReadyStateSpec extends TestKitBaseClass with FixtureAnyF
       alice2bob.expectMsgType[TxSignatures]
       alice2bob.forward(bob)
       if (!test.tags.contains(ChannelStateTestsTags.ZeroConf)) {
+        awaitCond(alice.stateName == WAIT_FOR_DUAL_FUNDING_CONFIRMED)
+        awaitCond(bob.stateName == WAIT_FOR_DUAL_FUNDING_CONFIRMED)
         val fundingTx = alice.stateData.asInstanceOf[DATA_WAIT_FOR_DUAL_FUNDING_CONFIRMED].fundingTx.asInstanceOf[FullySignedSharedTransaction].signedTx
         assert(alice2blockchain.expectMsgType[WatchFundingConfirmed].txId == fundingTx.txid)
         assert(bob2blockchain.expectMsgType[WatchFundingConfirmed].txId == fundingTx.txid)