From aecae9aded205cb0bbacca99a3a23b520821fb4e Mon Sep 17 00:00:00 2001 From: sstone Date: Mon, 14 Aug 2023 11:46:03 +0200 Subject: [PATCH] Do not track our wallet inputs/outputs It is currently easy to identify the wallet inputs/outputs that we added to fund/bump transactions, we don't need to explicitly track them. --- .../channel/publish/ReplaceableTxFunder.scala | 14 +++++++--- .../publish/ReplaceableTxPrePublisher.scala | 27 ++++++------------- .../publish/ReplaceableTxFunderSpec.scala | 6 ++--- 3 files changed, 21 insertions(+), 26 deletions(-) diff --git a/eclair-core/src/main/scala/fr/acinq/eclair/channel/publish/ReplaceableTxFunder.scala b/eclair-core/src/main/scala/fr/acinq/eclair/channel/publish/ReplaceableTxFunder.scala index 509c9ca859..160fdb2d1b 100644 --- a/eclair-core/src/main/scala/fr/acinq/eclair/channel/publish/ReplaceableTxFunder.scala +++ b/eclair-core/src/main/scala/fr/acinq/eclair/channel/publish/ReplaceableTxFunder.scala @@ -364,8 +364,14 @@ private class ReplaceableTxFunder(nodeParams: NodeParams, log.error(s"cannot sign ${cmd.desc}: ", failure) unlockAndStop(locallySignedTx.txInfo.input.outPoint, locallySignedTx.txInfo.tx, TxPublisher.TxRejectedReason.UnknownTxFailure) case Right(psbt1) => - val ourWalletInputs = locallySignedTx.walletInputs - val ourWalletOutputs = locallySignedTx.walletOutputs + // the transaction that we want to fund/replace has one input, the first one. Additional inputs are provided by our onchain wallet. + val ourWalletInputs = locallySignedTx.txInfo.tx.txIn.indices.drop(1) + // for "claim anchor txs" there is a single change output that sends to our onchain wallet + // for htlc txs the first output is the one we want to fund/bump, additional outputs send to our onchain wallet + val ourWalletOutputs = locallySignedTx match { + case _: ClaimLocalAnchorWithWitnessData => Seq(0) + case _: HtlcWithWitnessData => locallySignedTx.txInfo.tx.txOut.indices.drop(1) + } context.pipeToSelf(bitcoinClient.signPsbt(psbt1, ourWalletInputs, ourWalletOutputs)) { case Success(processPsbtResponse) => val signedTx = processPsbtResponse.finalTx @@ -469,7 +475,7 @@ private class ReplaceableTxFunder(nodeParams: NodeParams, changeAmount = dustLimit.max(fundTxResponse.amountIn - anchorTxFee) fundedTx = fundTxResponse.tx.copy(txOut = Seq(changeOutput.copy(amount = changeAmount))) } yield { - (anchorTx.updateTx(fundedTx).updateWalletInputsAndOutputs(ourWalletInputs, ourWalletOutputs), fundTxResponse.amountIn) + (anchorTx.updateTx(fundedTx), fundTxResponse.amountIn) } } @@ -483,7 +489,7 @@ private class ReplaceableTxFunder(nodeParams: NodeParams, bitcoinClient.fundTransaction(htlcTx.txInfo.tx, FundTransactionOptions(targetFeerate, changePosition = Some(1), inputWeights = Seq(htlcInputWeight))).flatMap(fundTxResponse => { val ourWalletInputs = fundTxResponse.tx.txIn.indices.filterNot(i => fundTxResponse.tx.txIn(i).outPoint == htlcTx.txInfo.input.outPoint) val ourWalletOutputs = if (fundTxResponse.tx.txOut.size > 1) Seq(1) else Nil // there may not be a change output - val unsignedTx = htlcTx.updateTx(fundTxResponse.tx).updateWalletInputsAndOutputs(ourWalletInputs, ourWalletOutputs) + val unsignedTx = htlcTx.updateTx(fundTxResponse.tx) val psbt = new Psbt(fundTxResponse.tx) bitcoinClient.signPsbt(psbt, ourWalletInputs, ourWalletOutputs).map(processPsbtResponse => { val actualFees: Satoshi = processPsbtResponse.psbt.computeFees() diff --git a/eclair-core/src/main/scala/fr/acinq/eclair/channel/publish/ReplaceableTxPrePublisher.scala b/eclair-core/src/main/scala/fr/acinq/eclair/channel/publish/ReplaceableTxPrePublisher.scala index 7ff2166b7c..8990627cf2 100644 --- a/eclair-core/src/main/scala/fr/acinq/eclair/channel/publish/ReplaceableTxPrePublisher.scala +++ b/eclair-core/src/main/scala/fr/acinq/eclair/channel/publish/ReplaceableTxPrePublisher.scala @@ -64,33 +64,22 @@ object ReplaceableTxPrePublisher { def txInfo: ReplaceableTransactionWithInputInfo def updateTx(tx: Transaction): ReplaceableTxWithWitnessData } - /** Replaceable transaction for which we may need to add wallet inputs and outputs. */ + /** Replaceable transaction for which we may need to add wallet inputs. */ sealed trait ReplaceableTxWithWalletInputs extends ReplaceableTxWithWitnessData { override def updateTx(tx: Transaction): ReplaceableTxWithWalletInputs - - def updateWalletInputsAndOutputs(walletIn: Seq[Int], walletOut: Seq[Int]): ReplaceableTxWithWalletInputs - def walletInputs: Seq[Int] - def walletOutputs: Seq[Int] } - case class ClaimLocalAnchorWithWitnessData(txInfo: ClaimLocalAnchorOutputTx, override val walletInputs: Seq[Int], override val walletOutputs: Seq[Int]) extends ReplaceableTxWithWalletInputs { + case class ClaimLocalAnchorWithWitnessData(txInfo: ClaimLocalAnchorOutputTx) extends ReplaceableTxWithWalletInputs { override def updateTx(tx: Transaction): ClaimLocalAnchorWithWitnessData = this.copy(txInfo = this.txInfo.copy(tx = tx)) - - override def updateWalletInputsAndOutputs(walletIn: Seq[Int], walletOut: Seq[Int]): ClaimLocalAnchorWithWitnessData = this.copy(walletInputs = walletIn, walletOutputs = walletOut) - } sealed trait HtlcWithWitnessData extends ReplaceableTxWithWalletInputs { override def txInfo: HtlcTx override def updateTx(tx: Transaction): HtlcWithWitnessData - - override def updateWalletInputsAndOutputs(walletIn: Seq[Int], walletOut: Seq[Int]): HtlcWithWitnessData } - case class HtlcSuccessWithWitnessData(txInfo: HtlcSuccessTx, remoteSig: ByteVector64, preimage: ByteVector32, override val walletInputs: Seq[Int], override val walletOutputs: Seq[Int]) extends HtlcWithWitnessData { + case class HtlcSuccessWithWitnessData(txInfo: HtlcSuccessTx, remoteSig: ByteVector64, preimage: ByteVector32) extends HtlcWithWitnessData { override def updateTx(tx: Transaction): HtlcSuccessWithWitnessData = this.copy(txInfo = this.txInfo.copy(tx = tx)) - override def updateWalletInputsAndOutputs(walletIn: Seq[Int], walletOut: Seq[Int]): HtlcSuccessWithWitnessData = this.copy(walletInputs = walletIn, walletOutputs = walletOut) } - case class HtlcTimeoutWithWitnessData(txInfo: HtlcTimeoutTx, remoteSig: ByteVector64, override val walletInputs: Seq[Int], override val walletOutputs: Seq[Int]) extends HtlcWithWitnessData { + case class HtlcTimeoutWithWitnessData(txInfo: HtlcTimeoutTx, remoteSig: ByteVector64) extends HtlcWithWitnessData { override def updateTx(tx: Transaction): HtlcTimeoutWithWitnessData = this.copy(txInfo = this.txInfo.copy(tx = tx)) - override def updateWalletInputsAndOutputs(walletIn: Seq[Int], walletOut: Seq[Int]): HtlcTimeoutWithWitnessData = this.copy(walletInputs = walletIn, walletOutputs = walletOut) } sealed trait ClaimHtlcWithWitnessData extends ReplaceableTxWithWitnessData { override def txInfo: ClaimHtlcTx @@ -164,7 +153,7 @@ private class ReplaceableTxPrePublisher(nodeParams: NodeParams, } Behaviors.receiveMessagePartial { case ParentTxOk => - replyTo ! PreconditionsOk(ClaimLocalAnchorWithWitnessData(localAnchorTx, Nil, Nil)) + replyTo ! PreconditionsOk(ClaimLocalAnchorWithWitnessData(localAnchorTx)) Behaviors.stopped case FundingTxNotFound => log.debug("funding tx could not be found, we don't know yet if we need to claim our anchor") @@ -182,7 +171,7 @@ private class ReplaceableTxPrePublisher(nodeParams: NodeParams, case UnknownFailure(reason) => log.error(s"could not check ${cmd.desc} preconditions, proceeding anyway: ", reason) // If our checks fail, we don't want it to prevent us from trying to publish our commit tx. - replyTo ! PreconditionsOk(ClaimLocalAnchorWithWitnessData(localAnchorTx, Nil, Nil)) + replyTo ! PreconditionsOk(ClaimLocalAnchorWithWitnessData(localAnchorTx)) Behaviors.stopped } } @@ -239,7 +228,7 @@ private class ReplaceableTxPrePublisher(nodeParams: NodeParams, commitment.changes.localChanges.all.collectFirst { case u: UpdateFulfillHtlc if Crypto.sha256(u.paymentPreimage) == tx.paymentHash => u.paymentPreimage } match { - case Some(preimage) => Some(HtlcSuccessWithWitnessData(tx, remoteSig, preimage, Nil, Nil)) + case Some(preimage) => Some(HtlcSuccessWithWitnessData(tx, remoteSig, preimage)) case None => log.error(s"preimage not found for htlcId=${tx.htlcId}, skipping...") None @@ -252,7 +241,7 @@ private class ReplaceableTxPrePublisher(nodeParams: NodeParams, commitment.localCommit.htlcTxsAndRemoteSigs.collectFirst { case HtlcTxAndRemoteSig(HtlcTimeoutTx(input, _, _, _), remoteSig) if input.outPoint == tx.input.outPoint => remoteSig } match { - case Some(remoteSig) => Some(HtlcTimeoutWithWitnessData(tx, remoteSig, Nil, Nil)) + case Some(remoteSig) => Some(HtlcTimeoutWithWitnessData(tx, remoteSig)) case None => log.error(s"remote signature not found for htlcId=${tx.htlcId}, skipping...") None diff --git a/eclair-core/src/test/scala/fr/acinq/eclair/channel/publish/ReplaceableTxFunderSpec.scala b/eclair-core/src/test/scala/fr/acinq/eclair/channel/publish/ReplaceableTxFunderSpec.scala index 431aa70526..e65a69e154 100644 --- a/eclair-core/src/test/scala/fr/acinq/eclair/channel/publish/ReplaceableTxFunderSpec.scala +++ b/eclair-core/src/test/scala/fr/acinq/eclair/channel/publish/ReplaceableTxFunderSpec.scala @@ -70,13 +70,13 @@ class ReplaceableTxFunderSpec extends TestKitBaseClass with AnyFunSuiteLike { paymentHash, 17, ConfirmationTarget.Absolute(BlockHeight(0)) - ), PlaceHolderSig, preimage, Nil, Nil) + ), PlaceHolderSig, preimage) val htlcTimeout = HtlcTimeoutWithWitnessData(HtlcTimeoutTx( InputInfo(OutPoint(commitTx, 1), commitTx.txOut.last, htlcTimeoutScript), Transaction(2, Seq(TxIn(OutPoint(commitTx, 1), ByteVector.empty, 0)), Seq(TxOut(4000 sat, Script.pay2wpkh(PlaceHolderPubKey))), 0), 12, ConfirmationTarget.Absolute(BlockHeight(0)) - ), PlaceHolderSig, Nil, Nil) + ), PlaceHolderSig) (htlcSuccess, htlcTimeout) } @@ -129,7 +129,7 @@ class ReplaceableTxFunderSpec extends TestKitBaseClass with AnyFunSuiteLike { test("adjust previous anchor transaction outputs") { val (commitTx, initialAnchorTx) = createAnchorTx() - val previousAnchorTx = ClaimLocalAnchorWithWitnessData(initialAnchorTx, Nil, Nil).updateTx(initialAnchorTx.tx.copy( + val previousAnchorTx = ClaimLocalAnchorWithWitnessData(initialAnchorTx).updateTx(initialAnchorTx.tx.copy( txIn = Seq( initialAnchorTx.tx.txIn.head, // The previous funding attempt added two wallet inputs: