Skip to content

Commit

Permalink
Add a funding fee budget (#2808)
Browse files Browse the repository at this point in the history
When opening an important channel we may be willing to pay a high feerate that we wouldn't want to use when consolidating UTXOs. However because we delegate the transaction creation to bitcoin core, we can sometimes be surprised by funding transactions that are much bigger than expected. This PR protects us from such cases by refusing to open the channel if the transaction fee is too high.

Based on original work from @thomash-acinq.
  • Loading branch information
pm47 authored Jan 10, 2024
1 parent 3bd3d07 commit c37df26
Show file tree
Hide file tree
Showing 40 changed files with 197 additions and 139 deletions.
2 changes: 2 additions & 0 deletions docs/release-notes/eclair-vnext.md
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,8 @@ This feature leaks a bit of information about the balance when the channel is al
### API changes

- `bumpforceclose` can be used to make a force-close confirm faster, by spending the anchor output (#2743)
- `open` now takes an optional parameter `--fundingFeeBudgetSatoshis` to define the maximum acceptable value for the mining fee of the funding transaction. This mining fee can sometimes be unexpectedly high depending on available UTXOs in the wallet. Default value is 0.1% of the funding amount (#2808)
- `rbfopen` now takes a mandatory parameter `--fundingFeeBudgetSatoshis`, with the same semantics as for `open` (#2808)

### Miscellaneous improvements and bug fixes

Expand Down
18 changes: 10 additions & 8 deletions eclair-core/src/main/scala/fr/acinq/eclair/Eclair.scala
Original file line number Diff line number Diff line change
Expand Up @@ -30,9 +30,8 @@ import fr.acinq.eclair.balance.CheckBalance.GlobalBalance
import fr.acinq.eclair.balance.{BalanceActor, ChannelsListener}
import fr.acinq.eclair.blockchain.OnChainWallet.OnChainBalance
import fr.acinq.eclair.blockchain.bitcoind.rpc.BitcoinCoreClient
import fr.acinq.eclair.blockchain.bitcoind.rpc.BitcoinCoreClient.WalletTx
import fr.acinq.eclair.blockchain.fee.{ConfirmationTarget, FeeratePerByte, FeeratePerKw}
import fr.acinq.eclair.blockchain.bitcoind.rpc.BitcoinCoreClient.{Descriptors, WalletTx}
import fr.acinq.eclair.blockchain.fee.{ConfirmationTarget, FeeratePerByte, FeeratePerKw}
import fr.acinq.eclair.channel._
import fr.acinq.eclair.crypto.Sphinx
import fr.acinq.eclair.db.AuditDb.{NetworkFee, Stats}
Expand Down Expand Up @@ -87,9 +86,9 @@ trait Eclair {

def disconnect(nodeId: PublicKey)(implicit timeout: Timeout): Future[String]

def open(nodeId: PublicKey, fundingAmount: Satoshi, pushAmount_opt: Option[MilliSatoshi], channelType_opt: Option[SupportedChannelType], fundingFeeratePerByte_opt: Option[FeeratePerByte], announceChannel_opt: Option[Boolean], openTimeout_opt: Option[Timeout])(implicit timeout: Timeout): Future[OpenChannelResponse]
def open(nodeId: PublicKey, fundingAmount: Satoshi, pushAmount_opt: Option[MilliSatoshi], channelType_opt: Option[SupportedChannelType], fundingFeerate_opt: Option[FeeratePerByte], fundingFeeBudget_opt: Option[Satoshi], announceChannel_opt: Option[Boolean], openTimeout_opt: Option[Timeout])(implicit timeout: Timeout): Future[OpenChannelResponse]

def rbfOpen(channelId: ByteVector32, targetFeerate: FeeratePerKw, lockTime_opt: Option[Long])(implicit timeout: Timeout): Future[CommandResponse[CMD_BUMP_FUNDING_FEE]]
def rbfOpen(channelId: ByteVector32, targetFeerate: FeeratePerKw, fundingFeeBudget: Satoshi, lockTime_opt: Option[Long])(implicit timeout: Timeout): Future[CommandResponse[CMD_BUMP_FUNDING_FEE]]

def spliceIn(channelId: ByteVector32, amountIn: Satoshi, pushAmount_opt: Option[MilliSatoshi])(implicit timeout: Timeout): Future[CommandResponse[CMD_SPLICE]]

Expand Down Expand Up @@ -205,26 +204,29 @@ class EclairImpl(appKit: Kit) extends Eclair with Logging {
(appKit.switchboard ? Peer.Disconnect(nodeId)).mapTo[Peer.DisconnectResponse].map(_.toString)
}

override def open(nodeId: PublicKey, fundingAmount: Satoshi, pushAmount_opt: Option[MilliSatoshi], channelType_opt: Option[SupportedChannelType], fundingFeeratePerByte_opt: Option[FeeratePerByte], announceChannel_opt: Option[Boolean], openTimeout_opt: Option[Timeout])(implicit timeout: Timeout): Future[OpenChannelResponse] = {
override def open(nodeId: PublicKey, fundingAmount: Satoshi, pushAmount_opt: Option[MilliSatoshi], channelType_opt: Option[SupportedChannelType], fundingFeerate_opt: Option[FeeratePerByte], fundingFeeBudget_opt: Option[Satoshi], announceChannel_opt: Option[Boolean], openTimeout_opt: Option[Timeout])(implicit timeout: Timeout): Future[OpenChannelResponse] = {
// we want the open timeout to expire *before* the default ask timeout, otherwise user will get a generic response
val openTimeout = openTimeout_opt.getOrElse(Timeout(20 seconds))
// if no budget is provided for the mining fee of the funding tx, we use a default of 0.1% of the funding amount as a safety measure
val fundingFeeBudget = fundingFeeBudget_opt.getOrElse(fundingAmount * 0.001)
for {
_ <- Future.successful(0)
open = Peer.OpenChannel(
remoteNodeId = nodeId,
fundingAmount = fundingAmount,
channelType_opt = channelType_opt,
pushAmount_opt = pushAmount_opt,
fundingTxFeerate_opt = fundingFeeratePerByte_opt.map(FeeratePerKw(_)),
fundingTxFeerate_opt = fundingFeerate_opt.map(FeeratePerKw(_)),
fundingTxFeeBudget_opt = Some(fundingFeeBudget),
channelFlags_opt = announceChannel_opt.map(announceChannel => ChannelFlags(announceChannel = announceChannel)),
timeout_opt = Some(openTimeout))
res <- (appKit.switchboard ? open).mapTo[OpenChannelResponse]
} yield res
}

override def rbfOpen(channelId: ByteVector32, targetFeerate: FeeratePerKw, lockTime_opt: Option[Long])(implicit timeout: Timeout): Future[CommandResponse[CMD_BUMP_FUNDING_FEE]] = {
override def rbfOpen(channelId: ByteVector32, targetFeerate: FeeratePerKw, fundingFeeBudget: Satoshi, lockTime_opt: Option[Long])(implicit timeout: Timeout): Future[CommandResponse[CMD_BUMP_FUNDING_FEE]] = {
sendToChannelTyped(channel = Left(channelId),
cmdBuilder = CMD_BUMP_FUNDING_FEE(_, targetFeerate, lockTime_opt.getOrElse(appKit.nodeParams.currentBlockHeight.toLong)))
cmdBuilder = CMD_BUMP_FUNDING_FEE(_, targetFeerate, fundingFeeBudget, lockTime_opt.getOrElse(appKit.nodeParams.currentBlockHeight.toLong)))
}

override def spliceIn(channelId: ByteVector32, amountIn: Satoshi, pushAmount_opt: Option[MilliSatoshi])(implicit timeout: Timeout): Future[CommandResponse[CMD_SPLICE]] = {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ trait OnChainChannelFunder {
* Fund the provided transaction by adding inputs (and a change output if necessary).
* Callers must verify that the resulting transaction isn't sending funds to unexpected addresses (malicious bitcoin node).
*/
def fundTransaction(tx: Transaction, feeRate: FeeratePerKw, replaceable: Boolean, externalInputsWeight: Map[OutPoint, Long] = Map.empty)(implicit ec: ExecutionContext): Future[FundTransactionResponse]
def fundTransaction(tx: Transaction, feeRate: FeeratePerKw, replaceable: Boolean, externalInputsWeight: Map[OutPoint, Long] = Map.empty, feeBudget_opt: Option[Satoshi])(implicit ec: ExecutionContext): Future[FundTransactionResponse]

/**
* Sign a PSBT. Result may be partially signed: only inputs known to our bitcoin wallet will be signed. *
Expand All @@ -55,7 +55,7 @@ trait OnChainChannelFunder {
def publishTransaction(tx: Transaction)(implicit ec: ExecutionContext): Future[TxId]

/** Create a fully signed channel funding transaction with the provided pubkeyScript. */
def makeFundingTx(pubkeyScript: ByteVector, amount: Satoshi, feeRate: FeeratePerKw)(implicit ec: ExecutionContext): Future[MakeFundingTxResponse]
def makeFundingTx(pubkeyScript: ByteVector, amount: Satoshi, feeRate: FeeratePerKw, feeBudget_opt: Option[Satoshi])(implicit ec: ExecutionContext): Future[MakeFundingTxResponse]

/**
* Committing *must* include publishing the transaction on the network.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ import fr.acinq.eclair.blockchain.OnChainWallet.{FundTransactionResponse, MakeFu
import fr.acinq.eclair.blockchain.bitcoind.ZmqWatcher.{GetTxWithMetaResponse, UtxoStatus, ValidateResult}
import fr.acinq.eclair.blockchain.fee.{FeeratePerKB, FeeratePerKw}
import fr.acinq.eclair.crypto.keymanager.OnChainKeyManager
import fr.acinq.eclair.json.SatoshiSerializer
import fr.acinq.eclair.transactions.Transactions
import fr.acinq.eclair.wire.protocol.ChannelAnnouncement
import fr.acinq.eclair.{BlockHeight, TimestampSecond, TxCoordinates}
Expand Down Expand Up @@ -233,7 +234,10 @@ class BitcoinCoreClient(val rpcClient: BitcoinJsonRPCClient, val onChainKeyManag

//------------------------- FUNDING -------------------------//

def fundTransaction(tx: Transaction, options: FundTransactionOptions)(implicit ec: ExecutionContext): Future[FundTransactionResponse] = {
/**
* @param feeBudget max allowed fee, if the transaction returned by bitcoin core has a higher fee a funding error is returned.
*/
def fundTransaction(tx: Transaction, options: FundTransactionOptions, feeBudget_opt: Option[Satoshi])(implicit ec: ExecutionContext): Future[FundTransactionResponse] = {
rpcClient.invoke("fundrawtransaction", tx.toString(), options).flatMap(json => {
val JString(hex) = json \ "hex"
val JInt(changePos) = json \ "changepos"
Expand All @@ -243,21 +247,23 @@ class BitcoinCoreClient(val rpcClient: BitcoinJsonRPCClient, val onChainKeyManag

val walletInputs = fundedTx.txIn.map(_.outPoint).toSet -- tx.txIn.map(_.outPoint).toSet
val addedOutputs = fundedTx.txOut.size - tx.txOut.size
val feeSat = toSatoshi(fee)
Try {
require(addedOutputs <= 1, "more than one change output added")
require(addedOutputs == 0 || changePos >= 0, "change output added, but position not returned")
require(options.changePosition.isEmpty || changePos_opt.isEmpty || changePos_opt == options.changePosition, "change output added at wrong position")
feeBudget_opt.foreach(feeBudget => require(feeSat <= feeBudget, s"mining fee is higher than budget ($feeSat > $feeBudget)"))

FundTransactionResponse(fundedTx, toSatoshi(fee), changePos_opt)
FundTransactionResponse(fundedTx, feeSat, changePos_opt)
} match {
case Success(response) => Future.successful(response)
case Failure(error) => unlockOutpoints(walletInputs.toSeq).flatMap(_ => Future.failed(error))
}
})
}

def fundTransaction(tx: Transaction, feeRate: FeeratePerKw, replaceable: Boolean, externalInputsWeight: Map[OutPoint, Long] = Map.empty)(implicit ec: ExecutionContext): Future[FundTransactionResponse] = {
fundTransaction(tx, FundTransactionOptions(feeRate, replaceable, inputWeights = externalInputsWeight.map { case (outpoint, weight) => InputWeight(outpoint, weight) }.toSeq))
def fundTransaction(tx: Transaction, feeRate: FeeratePerKw, replaceable: Boolean, externalInputsWeight: Map[OutPoint, Long] = Map.empty, feeBudget_opt: Option[Satoshi] = None)(implicit ec: ExecutionContext): Future[FundTransactionResponse] = {
fundTransaction(tx, FundTransactionOptions(feeRate, replaceable, inputWeights = externalInputsWeight.map { case (outpoint, weight) => InputWeight(outpoint, weight) }.toSeq), feeBudget_opt = feeBudget_opt)
}

private def processPsbt(psbt: Psbt, sign: Boolean = true, sighashType: Option[Int] = None)(implicit ec: ExecutionContext): Future[ProcessPsbtResponse] = {
Expand Down Expand Up @@ -302,7 +308,7 @@ class BitcoinCoreClient(val rpcClient: BitcoinJsonRPCClient, val onChainKeyManag
}
}

def makeFundingTx(pubkeyScript: ByteVector, amount: Satoshi, targetFeerate: FeeratePerKw)(implicit ec: ExecutionContext): Future[MakeFundingTxResponse] = {
def makeFundingTx(pubkeyScript: ByteVector, amount: Satoshi, targetFeerate: FeeratePerKw, feeBudget_opt: Option[Satoshi] = None)(implicit ec: ExecutionContext): Future[MakeFundingTxResponse] = {

def verifyAndSign(tx: Transaction, fees: Satoshi, requestedFeeRate: FeeratePerKw): Future[MakeFundingTxResponse] = {
import KotlinUtils._
Expand Down Expand Up @@ -338,7 +344,7 @@ class BitcoinCoreClient(val rpcClient: BitcoinJsonRPCClient, val onChainKeyManag
// TODO: we should check that mempoolMinFee is not dangerously high
feerate <- mempoolMinFee().map(minFee => FeeratePerKw(minFee).max(targetFeerate))
// we ask bitcoin core to add inputs to the funding tx, and use the specified change address
FundTransactionResponse(tx, fee, _) <- fundTransaction(partialFundingTx, FundTransactionOptions(feerate))
FundTransactionResponse(tx, fee, _) <- fundTransaction(partialFundingTx, FundTransactionOptions(feerate), feeBudget_opt = feeBudget_opt)
lockedUtxos = tx.txIn.map(_.outPoint)
signedTx <- unlockIfFails(lockedUtxos)(verifyAndSign(tx, fee, feerate))
} yield signedTx
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,7 @@ case class INPUT_INIT_CHANNEL_INITIATOR(temporaryChannelId: ByteVector32,
dualFunded: Boolean,
commitTxFeerate: FeeratePerKw,
fundingTxFeerate: FeeratePerKw,
fundingTxFeeBudget_opt: Option[Satoshi],
pushAmount_opt: Option[MilliSatoshi],
requireConfirmedInputs: Boolean,
localParams: LocalParams,
Expand Down Expand Up @@ -207,7 +208,7 @@ final case class CMD_CLOSE(replyTo: ActorRef, scriptPubKey: Option[ByteVector],
final case class CMD_FORCECLOSE(replyTo: ActorRef) extends CloseCommand
final case class CMD_BUMP_FORCE_CLOSE_FEE(replyTo: akka.actor.typed.ActorRef[CommandResponse[CMD_BUMP_FORCE_CLOSE_FEE]], confirmationTarget: ConfirmationTarget) extends Command

final case class CMD_BUMP_FUNDING_FEE(replyTo: akka.actor.typed.ActorRef[CommandResponse[CMD_BUMP_FUNDING_FEE]], targetFeerate: FeeratePerKw, lockTime: Long) extends Command
final case class CMD_BUMP_FUNDING_FEE(replyTo: akka.actor.typed.ActorRef[CommandResponse[CMD_BUMP_FUNDING_FEE]], targetFeerate: FeeratePerKw, fundingFeeBudget: Satoshi, lockTime: Long) extends Command
case class SpliceIn(additionalLocalFunding: Satoshi, pushAmount: MilliSatoshi = 0 msat)
case class SpliceOut(amount: Satoshi, scriptPubKey: ByteVector)
final case class CMD_SPLICE(replyTo: akka.actor.typed.ActorRef[CommandResponse[CMD_SPLICE]], spliceIn_opt: Option[SpliceIn], spliceOut_opt: Option[SpliceOut]) extends Command {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -212,7 +212,7 @@ trait ChannelOpenDualFunded extends DualFundingHandlers with ErrorHandlers {
targetFeerate = open.fundingFeerate,
requireConfirmedInputs = RequireConfirmedInputs(forLocal = open.requireConfirmedInputs, forRemote = accept.requireConfirmedInputs)
)
val purpose = InteractiveTxBuilder.FundingTx(open.commitmentFeerate, open.firstPerCommitmentPoint)
val purpose = InteractiveTxBuilder.FundingTx(open.commitmentFeerate, open.firstPerCommitmentPoint, feeBudget_opt = None)
val txBuilder = context.spawnAnonymous(InteractiveTxBuilder(
randomBytes32(),
nodeParams, fundingParams,
Expand Down Expand Up @@ -275,7 +275,7 @@ trait ChannelOpenDualFunded extends DualFundingHandlers with ErrorHandlers {
targetFeerate = d.lastSent.fundingFeerate,
requireConfirmedInputs = RequireConfirmedInputs(forLocal = accept.requireConfirmedInputs, forRemote = d.lastSent.requireConfirmedInputs)
)
val purpose = InteractiveTxBuilder.FundingTx(d.lastSent.commitmentFeerate, accept.firstPerCommitmentPoint)
val purpose = InteractiveTxBuilder.FundingTx(d.lastSent.commitmentFeerate, accept.firstPerCommitmentPoint, feeBudget_opt = d.init.fundingTxFeeBudget_opt)
val txBuilder = context.spawnAnonymous(InteractiveTxBuilder(
randomBytes32(),
nodeParams, fundingParams,
Expand Down Expand Up @@ -547,7 +547,7 @@ trait ChannelOpenDualFunded extends DualFundingHandlers with ErrorHandlers {
randomBytes32(),
nodeParams, fundingParams,
channelParams = d.commitments.params,
purpose = InteractiveTxBuilder.PreviousTxRbf(d.commitments.active.head, 0 msat, 0 msat, previousTransactions = d.allFundingTxs.map(_.sharedTx)),
purpose = InteractiveTxBuilder.PreviousTxRbf(d.commitments.active.head, 0 msat, 0 msat, previousTransactions = d.allFundingTxs.map(_.sharedTx), feeBudget_opt = None),
localPushAmount = d.localPushAmount, remotePushAmount = d.remotePushAmount,
wallet))
txBuilder ! InteractiveTxBuilder.Start(self)
Expand Down Expand Up @@ -585,7 +585,7 @@ trait ChannelOpenDualFunded extends DualFundingHandlers with ErrorHandlers {
randomBytes32(),
nodeParams, fundingParams,
channelParams = d.commitments.params,
purpose = InteractiveTxBuilder.PreviousTxRbf(d.commitments.active.head, 0 msat, 0 msat, previousTransactions = d.allFundingTxs.map(_.sharedTx)),
purpose = InteractiveTxBuilder.PreviousTxRbf(d.commitments.active.head, 0 msat, 0 msat, previousTransactions = d.allFundingTxs.map(_.sharedTx), feeBudget_opt = Some(cmd.fundingFeeBudget)),
localPushAmount = d.localPushAmount, remotePushAmount = d.remotePushAmount,
wallet))
txBuilder ! InteractiveTxBuilder.Start(self)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -186,7 +186,7 @@ trait ChannelOpenSingleFunded extends SingleFundingHandlers with ErrorHandlers {
log.info("remote will use fundingMinDepth={}", accept.minimumDepth)
val localFundingPubkey = keyManager.fundingPublicKey(init.localParams.fundingKeyPath, fundingTxIndex = 0)
val fundingPubkeyScript = Script.write(Script.pay2wsh(Scripts.multiSig2of2(localFundingPubkey.publicKey, accept.fundingPubkey)))
wallet.makeFundingTx(fundingPubkeyScript, init.fundingAmount, init.fundingTxFeerate).pipeTo(self)
wallet.makeFundingTx(fundingPubkeyScript, init.fundingAmount, init.fundingTxFeerate, init.fundingTxFeeBudget_opt).pipeTo(self)
val params = ChannelParams(init.temporaryChannelId, init.channelConfig, channelFeatures, init.localParams, remoteParams, open.channelFlags)
goto(WAIT_FOR_FUNDING_INTERNAL) using DATA_WAIT_FOR_FUNDING_INTERNAL(params, init.fundingAmount, init.pushAmount_opt.getOrElse(0 msat), init.commitTxFeerate, accept.fundingPubkey, accept.firstPerCommitmentPoint, d.initFunder.replyTo)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -169,7 +169,7 @@ object InteractiveTxBuilder {
def localHtlcs: Set[DirectedHtlc]
def htlcBalance: MilliSatoshi = localHtlcs.toSeq.map(_.add.amountMsat).sum
}
case class FundingTx(commitTxFeerate: FeeratePerKw, remotePerCommitmentPoint: PublicKey) extends Purpose {
case class FundingTx(commitTxFeerate: FeeratePerKw, remotePerCommitmentPoint: PublicKey, feeBudget_opt: Option[Satoshi]) extends Purpose {
override val previousLocalBalance: MilliSatoshi = 0 msat
override val previousRemoteBalance: MilliSatoshi = 0 msat
override val previousFundingAmount: Satoshi = 0 sat
Expand All @@ -194,7 +194,7 @@ object InteractiveTxBuilder {
* only one of them ends up confirming. We guarantee this by having the latest transaction
* always double-spend all its predecessors.
*/
case class PreviousTxRbf(replacedCommitment: Commitment, previousLocalBalance: MilliSatoshi, previousRemoteBalance: MilliSatoshi, previousTransactions: Seq[InteractiveTxBuilder.SignedSharedTransaction]) extends Purpose {
case class PreviousTxRbf(replacedCommitment: Commitment, previousLocalBalance: MilliSatoshi, previousRemoteBalance: MilliSatoshi, previousTransactions: Seq[InteractiveTxBuilder.SignedSharedTransaction], feeBudget_opt: Option[Satoshi]) 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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -189,7 +189,12 @@ private class InteractiveTxFunder(replyTo: ActorRef[InteractiveTxFunder.Response
*/
private def fund(txNotFunded: Transaction, currentInputs: Seq[OutgoingInput], unusableInputs: Set[UnusableInput]): Behavior[Command] = {
val sharedInputWeight = fundingParams.sharedInput_opt.toSeq.map(i => i.info.outPoint -> i.weight.toLong).toMap
context.pipeToSelf(wallet.fundTransaction(txNotFunded, fundingParams.targetFeerate, replaceable = true, externalInputsWeight = sharedInputWeight)) {
val feeBudget_opt = purpose match {
case p: FundingTx => p.feeBudget_opt
case p: PreviousTxRbf => p.feeBudget_opt
case _ => None
}
context.pipeToSelf(wallet.fundTransaction(txNotFunded, fundingParams.targetFeerate, replaceable = true, externalInputsWeight = sharedInputWeight, feeBudget_opt = feeBudget_opt)) {
case Failure(t) => WalletFailure(t)
case Success(result) => FundTransactionResult(result.tx, result.changePosition)
}
Expand Down
Loading

0 comments on commit c37df26

Please sign in to comment.