Add extra tests for added wallet inputs

Author: sstone

eclair-core/src/main/scala/fr/acinq/eclair/channel/publish/ReplaceableTxFunder.scala

@@ -274,7 +274,7 @@ private class ReplaceableTxFunder(nodeParams: NodeParams,
         Behaviors.stopped
       case AdjustPreviousTxOutputResult.TxOutputAdjusted(updatedTx) =>
         log.debug("bumping {} fees without adding new inputs: txid={}", cmd.desc, updatedTx.txInfo.tx.txid)
-        sign(updatedTx, targetFeerate, previousTx.totalAmountIn, Map.empty)
+        sign(updatedTx, targetFeerate, previousTx.totalAmountIn, previousTx.walletInputs)
       case AdjustPreviousTxOutputResult.AddWalletInputs(tx) =>
         log.debug("bumping {} fees requires adding new inputs (feerate={})", cmd.desc, targetFeerate)
         // We restore the original transaction (remove previous attempt's wallet inputs).

eclair-core/src/test/scala/fr/acinq/eclair/channel/publish/ReplaceableTxPublisherSpec.scala

@@ -407,35 +407,18 @@ class ReplaceableTxPublisherSpec extends TestKitBaseClass with AnyFunSuiteLike w
     }
   }
 
+  def getTransaction(probe: TestProbe, txid: TxId): Transaction = {
+    bitcoinrpcclient.invoke("getrawtransaction", txid).pipeTo(probe.ref)
+    Transaction.read(probe.expectMsgType[JString].values)
+  }
+
+  def getExtraInputsMap(probe: TestProbe, tx: Transaction): Map[OutPoint, TxOut] = {
+    tx.txIn.tail.map(input => input.outPoint -> getTransaction(probe, input.outPoint.txid).txOut(input.outPoint.index.toInt)).toMap
+  }
+
   test("commit tx feerate too low, spending anchor output (local commit)") {
     //we create a new channel key manager that delegates all call to Alice's key manager but track extra utxos passed to the sign() methods
-    val walletInputs = new AtomicReference[Map[OutPoint, TxOut]](Map.empty)
-    val channelKeyManagerA = TestConstants.Alice.nodeParams.channelKeyManager
-    // @formatter:off
-    val channelKeyManager = new ChannelKeyManager {
-      override def fundingPublicKey(fundingKeyPath: DeterministicWallet.KeyPath, fundingTxIndex: Long): DeterministicWallet.ExtendedPublicKey = channelKeyManagerA.fundingPublicKey(fundingKeyPath, fundingTxIndex)
-      override def revocationPoint(channelKeyPath: DeterministicWallet.KeyPath): DeterministicWallet.ExtendedPublicKey = channelKeyManagerA.revocationPoint(channelKeyPath)
-      override def paymentPoint(channelKeyPath: DeterministicWallet.KeyPath): DeterministicWallet.ExtendedPublicKey = channelKeyManagerA.paymentPoint(channelKeyPath)
-      override def delayedPaymentPoint(channelKeyPath: DeterministicWallet.KeyPath): DeterministicWallet.ExtendedPublicKey = channelKeyManagerA.delayedPaymentPoint(channelKeyPath)
-      override def htlcPoint(channelKeyPath: DeterministicWallet.KeyPath): DeterministicWallet.ExtendedPublicKey = channelKeyManagerA.htlcPoint(channelKeyPath)
-      override def commitmentSecret(channelKeyPath: DeterministicWallet.KeyPath, index: Long): Crypto.PrivateKey = channelKeyManagerA.commitmentSecret(channelKeyPath, index)
-      override def commitmentPoint(channelKeyPath: DeterministicWallet.KeyPath, index: Long): PublicKey = channelKeyManagerA.commitmentPoint(channelKeyPath, index)
-      override def newFundingKeyPath(isChannelOpener: Boolean): DeterministicWallet.KeyPath = channelKeyManagerA.newFundingKeyPath(isChannelOpener)
-      override def sign(tx: TransactionWithInputInfo, publicKey: DeterministicWallet.ExtendedPublicKey, txOwner: TxOwner, commitmentFormat: CommitmentFormat, extraUtxos: Map[OutPoint, TxOut]): ByteVector64 = {
-        walletInputs.set(extraUtxos)
-        channelKeyManagerA.sign(tx, publicKey, txOwner, commitmentFormat, extraUtxos)
-      }
-      override def sign(tx: TransactionWithInputInfo, publicKey: DeterministicWallet.ExtendedPublicKey, remotePoint: PublicKey, txOwner: TxOwner, commitmentFormat: CommitmentFormat, extraUtxos: Map[OutPoint, TxOut]): ByteVector64 = {
-        walletInputs.set(extraUtxos)
-        channelKeyManagerA.sign(tx, publicKey, remotePoint, txOwner, commitmentFormat, extraUtxos)
-      }
-      override def sign(tx: TransactionWithInputInfo, publicKey: DeterministicWallet.ExtendedPublicKey, remoteSecret: Crypto.PrivateKey, txOwner: TxOwner, commitmentFormat: CommitmentFormat, extraUtxos: Map[OutPoint, TxOut]): ByteVector64 = {
-        walletInputs.set(extraUtxos)
-        channelKeyManagerA.sign(tx, publicKey, remoteSecret, txOwner, commitmentFormat, extraUtxos)
-      }
-      override def signChannelAnnouncement(witness: ByteVector, fundingKeyPath: DeterministicWallet.KeyPath): ByteVector64 = channelKeyManagerA.signChannelAnnouncement(witness, fundingKeyPath)
-    }
-    // @formatter:on
+    val channelKeyManager = new DummyChannelKeyManager(TestConstants.Alice.nodeParams.channelKeyManager)
 
     withFixture(Seq(500 millibtc), ChannelTypes.AnchorOutputsZeroFeeHtlcTx(), Some(channelKeyManager)) { f =>
       import f._
@@ -455,16 +438,10 @@ class ReplaceableTxPublisherSpec extends TestKitBaseClass with AnyFunSuiteLike w
       // check that the wallet input added by the tx funder was passed to the key manager
       assert(mempoolTxs.map(_.txid).contains(commitTx.tx.txid))
 
-      def getTransaction(txid: TxId): Transaction = {
-        bitcoinrpcclient.invoke("getrawtransaction", txid).pipeTo(probe.ref)
-        Transaction.read(probe.expectMsgType[JString].values)
-      }
-
       // there are 2 transactions in the mempool, the one that is not the commit tx has to be the anchor tx
-      val publishedAnchorTx = getTransaction(mempoolTxs.filterNot(_.txid == commitTx.tx.txid).head.txid)
-      val extraInputs = publishedAnchorTx.txIn.tail.map(input => input.outPoint -> getTransaction(input.outPoint.txid).txOut(input.outPoint.index.toInt)).toMap
+      val publishedAnchorTx = getTransaction(probe, mempoolTxs.filterNot(_.txid == commitTx.tx.txid).head.txid)
       // check that inputs added to the anchor tx match what was passed to our key manager's sign() method
-      assert(walletInputs.get() == extraInputs)
+      assert(channelKeyManager.walletInputs.get() == getExtraInputsMap(probe, publishedAnchorTx))
 
       val targetFee = Transactions.weight2fee(targetFeerate, mempoolTxs.map(_.weight).sum.toInt)
       val actualFee = mempoolTxs.map(_.fees).sum
@@ -1293,7 +1270,8 @@ class ReplaceableTxPublisherSpec extends TestKitBaseClass with AnyFunSuiteLike w
   }
 
   test("htlc success tx not confirming, lowering output amount") {
-    withFixture(Seq(500 millibtc), ChannelTypes.AnchorOutputsZeroFeeHtlcTx()) { f =>
+    val channelKeyManager = new DummyChannelKeyManager(TestConstants.Alice.nodeParams.channelKeyManager)
+    withFixture(Seq(500 millibtc), ChannelTypes.AnchorOutputsZeroFeeHtlcTx(), channelKeyManager_opt = Some(channelKeyManager)) { f =>
       import f._
 
       val initialFeerate = FeeratePerKw(15_000 sat)
@@ -1321,13 +1299,19 @@ class ReplaceableTxPublisherSpec extends TestKitBaseClass with AnyFunSuiteLike w
       assert(htlcSuccessTx2.txid == htlcSuccessTxId2)
       assert(htlcSuccessTx1.fees < htlcSuccessTx2.fees)
       assert(htlcSuccessInputs1 == htlcSuccessInputs2)
+
+      // check that wallet inputs were passed to the sign() methods
+      val publishedHtlcSuccessTx = getTransaction(probe, htlcSuccessTxId2)
+      assert(channelKeyManager.walletInputs.get() == getExtraInputsMap(probe, publishedHtlcSuccessTx))
+
       val htlcSuccessTargetFee = Transactions.weight2fee(targetFeerate, htlcSuccessTx2.weight.toInt)
       assert(htlcSuccessTargetFee * 0.9 <= htlcSuccessTx2.fees && htlcSuccessTx2.fees <= htlcSuccessTargetFee * 1.1, s"actualFee=${htlcSuccessTx2.fees} targetFee=$htlcSuccessTargetFee")
     }
   }
 
   test("htlc success tx not confirming, adding other wallet inputs") {
-    withFixture(Seq(1_010_000 sat, 10_000 sat), ChannelTypes.AnchorOutputsZeroFeeHtlcTx()) { f =>
+    val channelKeyManager = new DummyChannelKeyManager(TestConstants.Alice.nodeParams.channelKeyManager)
+    withFixture(Seq(1_010_000 sat, 10_000 sat), ChannelTypes.AnchorOutputsZeroFeeHtlcTx(), channelKeyManager_opt = Some(channelKeyManager)) { f =>
       import f._
 
       val initialFeerate = FeeratePerKw(3_000 sat)
@@ -1355,6 +1339,11 @@ class ReplaceableTxPublisherSpec extends TestKitBaseClass with AnyFunSuiteLike w
       assert(htlcSuccessTx2.txid == htlcSuccessTxId2)
       assert(htlcSuccessTx1.fees < htlcSuccessTx2.fees)
       assert(htlcSuccessInputs1 != htlcSuccessInputs2)
+
+      // check that wallet inputs were passed to the sign() methods
+      val publishedHtlcSuccessTx = getTransaction(probe, htlcSuccessTxId2)
+      assert(channelKeyManager.walletInputs.get() == getExtraInputsMap(probe, publishedHtlcSuccessTx))
+
       val htlcSuccessTargetFee = Transactions.weight2fee(targetFeerate, htlcSuccessTx2.weight.toInt)
       assert(htlcSuccessTargetFee * 0.9 <= htlcSuccessTx2.fees && htlcSuccessTx2.fees <= htlcSuccessTargetFee * 1.1, s"actualFee=${htlcSuccessTx2.fees} targetFee=$htlcSuccessTargetFee")
     }
@@ -1422,7 +1411,9 @@ class ReplaceableTxPublisherSpec extends TestKitBaseClass with AnyFunSuiteLike w
   }
 
   test("htlc timeout tx not confirming, increasing fees") {
-    withFixture(Seq(500 millibtc), ChannelTypes.AnchorOutputsZeroFeeHtlcTx()) { f =>
+    val channelKeyManager = new DummyChannelKeyManager(TestConstants.Alice.channelKeyManager)
+
+    withFixture(Seq(500 millibtc), ChannelTypes.AnchorOutputsZeroFeeHtlcTx(), channelKeyManager_opt = Some(channelKeyManager)) { f =>
       import f._
 
       val feerate = FeeratePerKw(15_000 sat)
@@ -1452,6 +1443,10 @@ class ReplaceableTxPublisherSpec extends TestKitBaseClass with AnyFunSuiteLike w
       assert(htlcTimeoutTx2.txid == htlcTimeoutTxId2)
       assert(htlcTimeoutTx1.fees < htlcTimeoutTx2.fees)
       assert(htlcTimeoutInputs1 == htlcTimeoutInputs2)
+
+      val publishedHtlcTimeoutTx = getTransaction(probe, htlcTimeoutTxId2)
+      assert(channelKeyManager.walletInputs.get() == getExtraInputsMap(probe, publishedHtlcTimeoutTx))
+
       // Once the confirmation target is reach, we should raise the feerate by at least 20% at every block.
       val htlcTimeoutTargetFee = Transactions.weight2fee(feerate * 1.2, htlcTimeoutTx2.weight.toInt)
       assert(htlcTimeoutTargetFee * 0.9 <= htlcTimeoutTx2.fees && htlcTimeoutTx2.fees <= htlcTimeoutTargetFee * 1.1, s"actualFee=${htlcTimeoutTx2.fees} targetFee=$htlcTimeoutTargetFee")
@@ -1922,4 +1917,33 @@ class ReplaceableTxPublisherWithEclairSignerSpec extends ReplaceableTxPublisherS
 
     (walletRpcClient, walletClient)
   }
-}
+}
+
+// wrapper that delegates operations to an actual ChannelKeyManager and extra inputs passed to the sign() methods
+class DummyChannelKeyManager(channelKeyManager: ChannelKeyManager) extends ChannelKeyManager {
+  val walletInputs = new AtomicReference[Map[OutPoint, TxOut]](Map.empty)
+
+  // @formatter:off
+  override def fundingPublicKey(fundingKeyPath: DeterministicWallet.KeyPath, fundingTxIndex: Long): DeterministicWallet.ExtendedPublicKey = channelKeyManager.fundingPublicKey(fundingKeyPath, fundingTxIndex)
+  override def revocationPoint(channelKeyPath: DeterministicWallet.KeyPath): DeterministicWallet.ExtendedPublicKey = channelKeyManager.revocationPoint(channelKeyPath)
+  override def paymentPoint(channelKeyPath: DeterministicWallet.KeyPath): DeterministicWallet.ExtendedPublicKey = channelKeyManager.paymentPoint(channelKeyPath)
+  override def delayedPaymentPoint(channelKeyPath: DeterministicWallet.KeyPath): DeterministicWallet.ExtendedPublicKey = channelKeyManager.delayedPaymentPoint(channelKeyPath)
+  override def htlcPoint(channelKeyPath: DeterministicWallet.KeyPath): DeterministicWallet.ExtendedPublicKey = channelKeyManager.htlcPoint(channelKeyPath)
+  override def commitmentSecret(channelKeyPath: DeterministicWallet.KeyPath, index: Long): Crypto.PrivateKey = channelKeyManager.commitmentSecret(channelKeyPath, index)
+  override def commitmentPoint(channelKeyPath: DeterministicWallet.KeyPath, index: Long): PublicKey = channelKeyManager.commitmentPoint(channelKeyPath, index)
+  override def newFundingKeyPath(isChannelOpener: Boolean): DeterministicWallet.KeyPath = channelKeyManager.newFundingKeyPath(isChannelOpener)
+  override def sign(tx: TransactionWithInputInfo, publicKey: DeterministicWallet.ExtendedPublicKey, txOwner: TxOwner, commitmentFormat: CommitmentFormat, extraUtxos: Map[OutPoint, TxOut]): ByteVector64 = {
+    walletInputs.set(extraUtxos)
+    channelKeyManager.sign(tx, publicKey, txOwner, commitmentFormat, extraUtxos)
+  }
+  override def sign(tx: TransactionWithInputInfo, publicKey: DeterministicWallet.ExtendedPublicKey, remotePoint: PublicKey, txOwner: TxOwner, commitmentFormat: CommitmentFormat, extraUtxos: Map[OutPoint, TxOut]): ByteVector64 = {
+    walletInputs.set(extraUtxos)
+    channelKeyManager.sign(tx, publicKey, remotePoint, txOwner, commitmentFormat, extraUtxos)
+  }
+  override def sign(tx: TransactionWithInputInfo, publicKey: DeterministicWallet.ExtendedPublicKey, remoteSecret: Crypto.PrivateKey, txOwner: TxOwner, commitmentFormat: CommitmentFormat, extraUtxos: Map[OutPoint, TxOut]): ByteVector64 = {
+    walletInputs.set(extraUtxos)
+    channelKeyManager.sign(tx, publicKey, remoteSecret, txOwner, commitmentFormat, extraUtxos)
+  }
+  override def signChannelAnnouncement(witness: ByteVector, fundingKeyPath: DeterministicWallet.KeyPath): ByteVector64 = channelKeyManager.signChannelAnnouncement(witness, fundingKeyPath)
+  // @formatter:on
+}