Refactor out local/remote/shared amounts from Input.Shared and Output…

….Shared

This change primarily impacted:
 - computing `localFees` (used for tests and reporting command success during dual funding)
 - logic of `shouldSignFirst` (I also noticed that this already deviates from initial draft splice spec)
 - `InteractiveTxBuilderSpec` needs further review to make sure local/remote fees are properly checked
 - I am not certain local/remote pushes of value are properly accounted for everywhere; needs more tests

eclair-core/src/main/scala/fr/acinq/eclair/channel/fsm/ChannelOpenDualFunded.scala

@@ -334,7 +334,7 @@ trait ChannelOpenDualFunded extends DualFundingHandlers with ErrorHandlers {
  case InteractiveTxBuilder.SendMessage(msg) => stay() sending msg
  case InteractiveTxBuilder.Succeeded(status, commitSig) =>
  d.deferred.foreach(self ! _)
- d.replyTo_opt.foreach(_ ! OpenChannelResponse.Created(d.channelId, status.fundingTx.txId, status.fundingTx.tx.localFees.truncateToSatoshi))
+ d.replyTo_opt.foreach(_ ! OpenChannelResponse.Created(d.channelId, status.fundingTx.txId, status.fundingTx.tx.fees))
  val d1 = DATA_WAIT_FOR_DUAL_FUNDING_SIGNED(d.channelParams, d.secondRemotePerCommitmentPoint, d.localPushAmount, d.remotePushAmount, status, None)
  goto(WAIT_FOR_DUAL_FUNDING_SIGNED) using d1 storing() sending commitSig
  case f: InteractiveTxBuilder.Failed =>
@@ -656,7 +656,7 @@ trait ChannelOpenDualFunded extends DualFundingHandlers with ErrorHandlers {
  msg match {
  case InteractiveTxBuilder.SendMessage(msg) => stay() sending msg
  case InteractiveTxBuilder.Succeeded(signingSession, commitSig) =>
- cmd_opt.foreach(cmd => cmd.replyTo ! RES_BUMP_FUNDING_FEE(rbfIndex = d.previousFundingTxs.length, signingSession.fundingTx.txId, signingSession.fundingTx.tx.localFees.truncateToSatoshi))
+ cmd_opt.foreach(cmd => cmd.replyTo ! RES_BUMP_FUNDING_FEE(rbfIndex = d.previousFundingTxs.length, signingSession.fundingTx.txId, signingSession.fundingTx.tx.fees))
  remoteCommitSig_opt.foreach(self ! _)
  val d1 = d.copy(rbfStatus = RbfStatus.RbfWaitingForSigs(signingSession))
  stay() using d1 storing() sending commitSig

eclair-core/src/main/scala/fr/acinq/eclair/channel/fsm/DualFundingHandlers.scala

@@ -50,7 +50,8 @@ trait DualFundingHandlers extends CommonFundingHandlers {
  // Note that we don't use wallet.commit because we don't want to rollback on failure, since our peer may be able
  // to publish and we may be able to RBF.
  wallet.publishTransaction(fundingTx.signedTx).onComplete {
- case Success(_) => context.system.eventStream.publish(TransactionPublished(dualFundedTx.fundingParams.channelId, remoteNodeId, fundingTx.signedTx, fundingTx.tx.localFees.truncateToSatoshi, "funding"))
+ // TODO: is it ok to return aggregate instead of local fees here?
+ case Success(_) => context.system.eventStream.publish(TransactionPublished(dualFundedTx.fundingParams.channelId, remoteNodeId, fundingTx.signedTx, fundingTx.tx.fees, "funding"))
  case Failure(t) => log.warning("error while publishing funding tx: {}", t.getMessage) // tx may be published by our peer, we can't fail-fast
  }
  }

eclair-core/src/main/scala/fr/acinq/eclair/channel/fund/InteractiveTxBuilder.scala

@@ -29,7 +29,7 @@ import fr.acinq.eclair.channel.Helpers.Closing.MutualClose
 import fr.acinq.eclair.channel.Helpers.Funding
 import fr.acinq.eclair.channel._
 import fr.acinq.eclair.channel.fund.InteractiveTxBuilder.Purpose
-import fr.acinq.eclair.channel.fund.InteractiveTxSigningSession.UnsignedLocalCommit
+import fr.acinq.eclair.channel.fund.InteractiveTxSigningSession.{UnsignedLocalCommit, shouldSignFirst}
 import fr.acinq.eclair.crypto.keymanager.ChannelKeyManager
 import fr.acinq.eclair.transactions.Transactions.{CommitTx, HtlcTx, InputInfo, TxOwner}
 import fr.acinq.eclair.transactions.{CommitmentSpec, DirectedHtlc, Scripts, Transactions}
@@ -159,7 +159,8 @@ object InteractiveTxBuilder {
  sealed trait Purpose {
  def previousLocalBalance: MilliSatoshi
  def previousRemoteBalance: MilliSatoshi
- def previousFundingAmount: Satoshi
+ // Note that the truncation is a no-op: the sum of balances in a channel must be a satoshi amount.
+ lazy val previousFundingAmount: Satoshi = (previousLocalBalance + previousRemoteBalance + previousHtlcsBalance).truncateToSatoshi
  def localCommitIndex: Long
  def remoteCommitIndex: Long
  def remotePerCommitmentPoint: PublicKey
@@ -173,7 +174,6 @@ object InteractiveTxBuilder {
  case class FundingTx(commitTxFeerate: FeeratePerKw, remotePerCommitmentPoint: PublicKey) extends Purpose {
  override val previousLocalBalance: MilliSatoshi = 0 msat
  override val previousRemoteBalance: MilliSatoshi = 0 msat
- override val previousFundingAmount: Satoshi = 0 sat
  override val localCommitIndex: Long = 0
  override val remoteCommitIndex: Long = 0
  override val fundingTxIndex: Long = 0
@@ -182,7 +182,6 @@ object InteractiveTxBuilder {
  case class SpliceTx(parentCommitment: Commitment) extends Purpose {
  override val previousLocalBalance: MilliSatoshi = parentCommitment.localCommit.spec.toLocal
  override val previousRemoteBalance: MilliSatoshi = parentCommitment.remoteCommit.spec.toLocal
- override val previousFundingAmount: Satoshi = parentCommitment.capacity
  override val localCommitIndex: Long = parentCommitment.localCommit.index
  override val remoteCommitIndex: Long = parentCommitment.remoteCommit.index
  override val remotePerCommitmentPoint: PublicKey = parentCommitment.remoteCommit.remotePerCommitmentPoint
@@ -196,8 +195,6 @@ object InteractiveTxBuilder {
  * always double-spend all its predecessors.
  */
  case class PreviousTxRbf(replacedCommitment: Commitment, previousLocalBalance: MilliSatoshi, previousRemoteBalance: MilliSatoshi, previousTransactions: Seq[InteractiveTxBuilder.SignedSharedTransaction]) extends Purpose {
- // Note that the truncation is a no-op: the sum of balances in a channel must be a satoshi amount.
- override val previousFundingAmount: Satoshi = (previousLocalBalance + previousRemoteBalance).truncateToSatoshi
  override val localCommitIndex: Long = replacedCommitment.localCommit.index
  override val remoteCommitIndex: Long = replacedCommitment.remoteCommit.index
  override val remotePerCommitmentPoint: PublicKey = replacedCommitment.remoteCommit.remotePerCommitmentPoint
@@ -231,7 +228,7 @@ object InteractiveTxBuilder {
  case class Remote(serialId: UInt64, outPoint: OutPoint, txOut: TxOut, sequence: Long) extends Input with Incoming
 
  /** The shared input can be added by us or by our peer, depending on who initiated the protocol. */
- case class Shared(serialId: UInt64, outPoint: OutPoint, sequence: Long, localAmount: MilliSatoshi, remoteAmount: MilliSatoshi) extends Input with Incoming with Outgoing
+ case class Shared(serialId: UInt64, outPoint: OutPoint, sequence: Long, amount: Satoshi) extends Input with Incoming with Outgoing
  }
 
  sealed trait Output {
@@ -254,10 +251,7 @@ object InteractiveTxBuilder {
  case class Remote(serialId: UInt64, amount: Satoshi, pubkeyScript: ByteVector) extends Output with Incoming
 
  /** The shared output can be added by us or by our peer, depending on who initiated the protocol. */
- case class Shared(serialId: UInt64, pubkeyScript: ByteVector, localAmount: MilliSatoshi, remoteAmount: MilliSatoshi) extends Output with Incoming with Outgoing {
- // Note that the truncation is a no-op: the sum of balances in a channel must be a satoshi amount.
- override val amount: Satoshi = (localAmount + remoteAmount).truncateToSatoshi
- }
+ case class Shared(serialId: UInt64, pubkeyScript: ByteVector, amount: Satoshi) extends Output with Incoming with Outgoing
  }
 
  type OutgoingInput = Input with Outgoing
@@ -283,14 +277,13 @@ object InteractiveTxBuilder {
  localInputs: List[Input.Local], remoteInputs: List[Input.Remote],
  localOutputs: List[Output.Local], remoteOutputs: List[Output.Remote],
  lockTime: Long) {
- val localAmountIn: MilliSatoshi = sharedInput_opt.map(_.localAmount).getOrElse(0 msat) + localInputs.map(i => i.previousTx.txOut(i.previousTxOutput.toInt).amount).sum
- val remoteAmountIn: MilliSatoshi = sharedInput_opt.map(_.remoteAmount).getOrElse(0 msat) + remoteInputs.map(_.txOut.amount).sum
- val localAmountOut: MilliSatoshi = sharedOutput.localAmount + localOutputs.map(_.amount).sum
- val remoteAmountOut: MilliSatoshi = sharedOutput.remoteAmount + remoteOutputs.map(_.amount).sum
- val localFees: MilliSatoshi = localAmountIn - localAmountOut
- val remoteFees: MilliSatoshi = remoteAmountIn - remoteAmountOut
- // Note that the truncation is a no-op: sub-satoshi balances are carried over from inputs to outputs and cancel out.
- val fees: Satoshi = (localFees + remoteFees).truncateToSatoshi
+ val localInputsAmount: Satoshi = localInputs.map(i => i.previousTx.txOut(i.previousTxOutput.toInt).amount).sum
+ val remoteInputsAmount: Satoshi = remoteInputs.map(_.txOut.amount).sum
+ val localOutputsAmount: Satoshi = localOutputs.map(_.amount).sum
+ val remoteOutputsAmount: Satoshi = remoteOutputs.map(_.amount).sum
+ val localContribution: Satoshi = localInputsAmount - localOutputsAmount
+ val remoteContribution: Satoshi = remoteInputsAmount - remoteOutputsAmount
+ val fees: Satoshi = (localInputsAmount + remoteInputsAmount + sharedInput_opt.map(_.amount).getOrElse(0 sat)) - (localOutputsAmount + remoteOutputsAmount + sharedOutput.amount)
 
  def buildUnsignedTx(): Transaction = {
  val sharedTxIn = sharedInput_opt.map(i => (i.serialId, TxIn(i.outPoint, ByteVector.empty, i.sequence))).toSeq
@@ -350,8 +343,8 @@ object InteractiveTxBuilder {
  Behaviors.withMdc(Logs.mdc(remoteNodeId_opt = Some(channelParams.remoteParams.nodeId), channelId_opt = Some(fundingParams.channelId))) {
  Behaviors.receiveMessagePartial {
  case Start(replyTo) =>
- val nextLocalFundingAmount = purpose.previousLocalBalance + purpose.incomingHtlcsBalance + fundingParams.localContribution
- val nextRemoteFundingAmount = purpose.previousRemoteBalance + purpose.outgoingHtlcsBalance + fundingParams.remoteContribution
+ val nextLocalFundingAmount = purpose.previousLocalBalance + fundingParams.localContribution
+ val nextRemoteFundingAmount = purpose.previousRemoteBalance + fundingParams.remoteContribution
  if (fundingParams.fundingAmount < fundingParams.dustLimit || fundingParams.fundingAmount < purpose.previousHtlcsBalance.truncateToSatoshi) {
  replyTo ! LocalFailure(FundingAmountTooLow(channelParams.channelId, fundingParams.fundingAmount, fundingParams.dustLimit + purpose.previousHtlcsBalance.truncateToSatoshi))
  Behaviors.stopped
@@ -485,7 +478,7 @@ private class InteractiveTxBuilder(replyTo: ActorRef[InteractiveTxBuilder.Respon
  case None =>
  (addInput.sharedInput_opt, fundingParams.sharedInput_opt) match {
  case (Some(outPoint), Some(sharedInput)) if outPoint == sharedInput.info.outPoint =>
- Input.Shared(addInput.serialId, outPoint, addInput.sequence, purpose.previousLocalBalance + purpose.incomingHtlcsBalance, purpose.previousRemoteBalance + purpose.outgoingHtlcsBalance)
+ Input.Shared(addInput.serialId, outPoint, addInput.sequence, purpose.previousFundingAmount)
  case _ =>
  return Left(PreviousTxMissing(fundingParams.channelId, addInput.serialId))
  }
@@ -511,8 +504,7 @@ private class InteractiveTxBuilder(replyTo: ActorRef[InteractiveTxBuilder.Respon
  } else if (!MutualClose.isValidFinalScriptPubkey(addOutput.pubkeyScript, allowAnySegwit = true)) {
  Left(InvalidSpliceOutputScript(fundingParams.channelId, addOutput.serialId, addOutput.pubkeyScript))
  } else if (addOutput.pubkeyScript == fundingPubkeyScript) {
- val htlcsAmount = fundingParams.sharedInput_opt.map(_.info.txOut.amount - purpose.previousLocalBalance - purpose.previousRemoteBalance).getOrElse(0 msat)
- Right(Output.Shared(addOutput.serialId, addOutput.pubkeyScript, purpose.previousLocalBalance + purpose.incomingHtlcsBalance + fundingParams.localContribution, purpose.previousRemoteBalance + purpose.outgoingHtlcsBalance + fundingParams.remoteContribution))
+ Right(Output.Shared(addOutput.serialId, addOutput.pubkeyScript, fundingParams.fundingAmount))
  } else {
  Right(Output.Remote(addOutput.serialId, addOutput.amount, addOutput.pubkeyScript))
  }
@@ -656,11 +648,12 @@ private class InteractiveTxBuilder(replyTo: ActorRef[InteractiveTxBuilder.Respon
  return Left(InvalidCompleteInteractiveTx(fundingParams.channelId))
  }
 
+ val toRemote = purpose.previousRemoteBalance - remotePushAmount + localPushAmount + remoteInputs.map(i => i.txOut.amount).sum - remoteOutputs.map(i => i.amount).sum
  val sharedInput_opt = fundingParams.sharedInput_opt.map(_ => {
  val remoteReserve = channelParams.remoteChannelReserveForCapacity(fundingParams.fundingAmount)
  // We ignore the reserve requirement if we are splicing funds into the channel, which increases the size of the reserve.
- if (sharedOutput.remoteAmount < remoteReserve && remoteOutputs.nonEmpty && localInputs.isEmpty) {
- log.warn("invalid interactive tx: peer takes too much funds out and falls below the channel reserve ({} < {})", sharedOutput.remoteAmount, remoteReserve)
+ if (toRemote < remoteReserve && remoteOutputs.nonEmpty && localInputs.isEmpty) {
+ log.warn("invalid interactive tx: peer takes too much funds out and falls below the channel reserve ({} < {})", toRemote, remoteReserve)
  return Left(InvalidCompleteInteractiveTx(fundingParams.channelId))
  }
  if (sharedInputs.length > 1) {
@@ -677,8 +670,9 @@ private class InteractiveTxBuilder(replyTo: ActorRef[InteractiveTxBuilder.Respon
 
  val sharedTx = SharedTransaction(sharedInput_opt, sharedOutput, localInputs.toList, remoteInputs.toList, localOutputs.toList, remoteOutputs.toList, fundingParams.lockTime)
  val tx = sharedTx.buildUnsignedTx()
- if (sharedTx.localAmountIn < sharedTx.localAmountOut || sharedTx.remoteAmountIn < sharedTx.remoteAmountOut) {
- log.warn("invalid interactive tx: input amount is too small (localIn={}, localOut={}, remoteIn={}, remoteOut={})", sharedTx.localAmountIn, sharedTx.localAmountOut, sharedTx.remoteAmountIn, sharedTx.remoteAmountOut)
+ val toLocal = purpose.previousLocalBalance - localPushAmount + remotePushAmount + localInputs.map(i => i.previousTx.txOut(i.previousTxOutput.toInt).amount).sum - localOutputs.map(i => i.amount).sum
+ if (toLocal < 0.msat || toRemote < 0.msat) {
+ log.warn("invalid interactive tx: input amount is too small (localIn={}, localOut={}, remoteIn={}, remoteOut={})", sharedTx.localInputsAmount, sharedTx.localOutputsAmount, sharedTx.remoteInputsAmount, sharedTx.remoteOutputsAmount)
  return Left(InvalidCompleteInteractiveTx(fundingParams.channelId))
  }
 
@@ -732,8 +726,8 @@ private class InteractiveTxBuilder(replyTo: ActorRef[InteractiveTxBuilder.Respon
  val fundingOutputIndex = fundingTx.txOut.indexWhere(_.publicKeyScript == fundingPubkeyScript)
  Funding.makeCommitTxs(keyManager, channelParams,
  fundingAmount = fundingParams.fundingAmount,
- toLocal = completeTx.sharedOutput.localAmount - localPushAmount + remotePushAmount - purpose.incomingHtlcsBalance,
- toRemote = completeTx.sharedOutput.remoteAmount - remotePushAmount + localPushAmount - purpose.outgoingHtlcsBalance,
+ toLocal = purpose.previousLocalBalance + fundingParams.localContribution - localPushAmount + remotePushAmount,
+ toRemote = purpose.previousRemoteBalance + fundingParams.remoteContribution - remotePushAmount + localPushAmount,
  purpose.commitTxFeerate,
  fundingTxIndex = purpose.fundingTxIndex,
  fundingTx.hash, fundingOutputIndex,
@@ -758,7 +752,8 @@ private class InteractiveTxBuilder(replyTo: ActorRef[InteractiveTxBuilder.Respon
  Behaviors.receiveMessagePartial {
  case SignTransactionResult(signedTx) =>
  log.info(s"interactive-tx txid=${signedTx.txId} partially signed with {} local inputs, {} remote inputs, {} local outputs and {} remote outputs", signedTx.tx.localInputs.length, signedTx.tx.remoteInputs.length, signedTx.tx.localOutputs.length, signedTx.tx.remoteOutputs.length)
- replyTo ! Succeeded(InteractiveTxSigningSession.WaitingForSigs(fundingParams, purpose.fundingTxIndex, signedTx, Left(localCommit), remoteCommit), commitSig)
+ val signFirst = shouldSignFirst(channelParams, localAmountIn = signedTx.tx.localInputsAmount + purpose.previousLocalBalance, remoteAmountIn = signedTx.tx.remoteInputsAmount + purpose.previousRemoteBalance)
+ replyTo ! Succeeded(InteractiveTxSigningSession.WaitingForSigs(fundingParams, purpose.fundingTxIndex, signedTx, Left(localCommit), remoteCommit, signFirst), commitSig)
  Behaviors.stopped
  case WalletFailure(t) =>
  log.error("could not sign funding transaction: ", t)
@@ -845,13 +840,13 @@ object InteractiveTxSigningSession {
  /** A local commitment for which we haven't received our peer's signatures. */
  case class UnsignedLocalCommit(index: Long, spec: CommitmentSpec, commitTx: CommitTx, htlcTxs: List[HtlcTx])
 
- private def shouldSignFirst(channelParams: ChannelParams, tx: SharedTransaction): Boolean = {
- if (tx.localAmountIn == tx.remoteAmountIn) {
+ def shouldSignFirst(channelParams: ChannelParams, localAmountIn: MilliSatoshi, remoteAmountIn: MilliSatoshi): Boolean = {
+ if (localAmountIn == remoteAmountIn) {
  // When both peers contribute the same amount, the peer with the lowest pubkey must transmit its `tx_signatures` first.
  LexicographicalOrdering.isLessThan(channelParams.localNodeId.value, channelParams.remoteNodeId.value)
  } else {
  // Otherwise, the peer with the lowest total of input amount must transmit its `tx_signatures` first.
- tx.localAmountIn < tx.remoteAmountIn
+ localAmountIn < remoteAmountIn
  }
  }
 
@@ -906,7 +901,8 @@ object InteractiveTxSigningSession {
  fundingTxIndex: Long,
  fundingTx: PartiallySignedSharedTransaction,
  localCommit: Either[UnsignedLocalCommit, LocalCommit],
- remoteCommit: RemoteCommit) extends InteractiveTxSigningSession {
+ remoteCommit: RemoteCommit,
+ signFirst: Boolean) extends InteractiveTxSigningSession {
  val commitInput: InputInfo = localCommit.fold(_.commitTx.input, _.commitTxAndRemoteSig.commitTx.input)
 
  def receiveCommitSig(nodeParams: NodeParams, channelParams: ChannelParams, remoteCommitSig: CommitSig)(implicit log: LoggingAdapter): Either[ChannelException, InteractiveTxSigningSession] = {
@@ -919,7 +915,7 @@ object InteractiveTxSigningSession {
  case Failure(_) => Left(InvalidCommitmentSignature(fundingParams.channelId, signedLocalCommitTx.tx.txid))
  case Success(_) =>
  val signedLocalCommit = LocalCommit(unsignedLocalCommit.index, unsignedLocalCommit.spec, CommitTxAndRemoteSig(unsignedLocalCommit.commitTx, remoteCommitSig.signature), htlcTxsAndRemoteSigs = Nil)
- if (shouldSignFirst(channelParams, fundingTx.tx)) {
+ if (signFirst) {
  val fundingStatus = LocalFundingStatus.DualFundedUnconfirmedFundingTx(fundingTx, nodeParams.currentBlockHeight, fundingParams)
  val commitment = Commitment(fundingTxIndex, fundingParams.remoteFundingPubKey, fundingStatus, RemoteFundingStatus.NotLocked, signedLocalCommit, remoteCommit, None)
  Right(SendingSigs(fundingStatus, commitment, fundingTx.localSigs))