mirror of
https://github.com/ACINQ/eclair.git
synced 2025-02-23 14:40:34 +01:00
Accept closing fee above commit fee (#2662)
When performing a mutual close, we initially rejected fees that were higher to the commit tx fees. This was removed from the specification for anchor output channels, and doesn't make a lot of sense for standard channels either: even at a higher fee, it makes sense to do a mutual close to avoid waiting for relative delays on our outputs. Fixes #2646
This commit is contained in:
Bastien Teinturier
GitHub
parent
4713a541b6
commit
aaad2e1d61
2 changed files with 27 additions and 31 deletions
|
|
@ -64,13 +64,13 @@ object Helpers {
|
|||
}
|
||||
}
|
||||
|
||||
def extractShutdownScript(channelId: ByteVector32, localFeatures: Features[InitFeature], remoteFeatures: Features[InitFeature], upfrontShutdownScript_opt: Option[ByteVector]): Either[ChannelException, Option[ByteVector]] = {
|
||||
private def extractShutdownScript(channelId: ByteVector32, localFeatures: Features[InitFeature], remoteFeatures: Features[InitFeature], upfrontShutdownScript_opt: Option[ByteVector]): Either[ChannelException, Option[ByteVector]] = {
|
||||
val canUseUpfrontShutdownScript = Features.canUseFeature(localFeatures, remoteFeatures, Features.UpfrontShutdownScript)
|
||||
val canUseAnySegwit = Features.canUseFeature(localFeatures, remoteFeatures, Features.ShutdownAnySegwit)
|
||||
extractShutdownScript(channelId, canUseUpfrontShutdownScript, canUseAnySegwit, upfrontShutdownScript_opt)
|
||||
}
|
||||
|
||||
def extractShutdownScript(channelId: ByteVector32, hasOptionUpfrontShutdownScript: Boolean, allowAnySegwit: Boolean, upfrontShutdownScript_opt: Option[ByteVector]): Either[ChannelException, Option[ByteVector]] = {
|
||||
private def extractShutdownScript(channelId: ByteVector32, hasOptionUpfrontShutdownScript: Boolean, allowAnySegwit: Boolean, upfrontShutdownScript_opt: Option[ByteVector]): Either[ChannelException, Option[ByteVector]] = {
|
||||
(hasOptionUpfrontShutdownScript, upfrontShutdownScript_opt) match {
|
||||
case (true, None) => Left(MissingUpfrontShutdownScript(channelId))
|
||||
case (true, Some(script)) if script.isEmpty => Right(None) // but the provided script can be empty
|
||||
|
|
@ -304,7 +304,7 @@ object Helpers {
|
|||
* @param remoteFeeratePerKw remote fee rate per kiloweight
|
||||
* @return true if the remote fee rate is too small
|
||||
*/
|
||||
def isFeeTooSmall(remoteFeeratePerKw: FeeratePerKw): Boolean = {
|
||||
private def isFeeTooSmall(remoteFeeratePerKw: FeeratePerKw): Boolean = {
|
||||
remoteFeeratePerKw < FeeratePerKw.MinimumFeeratePerKw
|
||||
}
|
||||
|
||||
|
|
@ -605,9 +605,6 @@ object Helpers {
|
|||
|
||||
object MutualClose {
|
||||
|
||||
// used only to compute tx weights and estimate fees
|
||||
lazy val dummyPublicKey: PublicKey = PrivateKey(ByteVector32(ByteVector.fill(32)(1))).publicKey
|
||||
|
||||
def isValidFinalScriptPubkey(scriptPubKey: ByteVector, allowAnySegwit: Boolean): Boolean = {
|
||||
Try(Script.parse(scriptPubKey)) match {
|
||||
case Success(OP_DUP :: OP_HASH160 :: OP_PUSHDATA(pubkeyHash, _) :: OP_EQUALVERIFY :: OP_CHECKSIG :: Nil) if pubkeyHash.size == 20 => true
|
||||
|
|
@ -622,7 +619,7 @@ object Helpers {
|
|||
def firstClosingFee(commitment: FullCommitment, localScriptPubkey: ByteVector, remoteScriptPubkey: ByteVector, feerates: ClosingFeerates)(implicit log: LoggingAdapter): ClosingFees = {
|
||||
// this is just to estimate the weight, it depends on size of the pubkey scripts
|
||||
val dummyClosingTx = Transactions.makeClosingTx(commitment.commitInput, localScriptPubkey, remoteScriptPubkey, commitment.localParams.isInitiator, Satoshi(0), Satoshi(0), commitment.localCommit.spec)
|
||||
val closingWeight = Transaction.weight(Transactions.addSigs(dummyClosingTx, dummyPublicKey, commitment.remoteFundingPubKey, Transactions.PlaceHolderSig, Transactions.PlaceHolderSig).tx)
|
||||
val closingWeight = Transaction.weight(Transactions.addSigs(dummyClosingTx, Transactions.PlaceHolderPubKey, commitment.remoteFundingPubKey, Transactions.PlaceHolderSig, Transactions.PlaceHolderSig).tx)
|
||||
log.info(s"using feerates=$feerates for initial closing tx")
|
||||
feerates.computeFees(closingWeight)
|
||||
}
|
||||
|
|
@ -666,21 +663,15 @@ object Helpers {
|
|||
}
|
||||
|
||||
def checkClosingSignature(keyManager: ChannelKeyManager, commitment: FullCommitment, localScriptPubkey: ByteVector, remoteScriptPubkey: ByteVector, remoteClosingFee: Satoshi, remoteClosingSig: ByteVector64)(implicit log: LoggingAdapter): Either[ChannelException, (ClosingTx, ClosingSigned)] = {
|
||||
val lastCommitFeeSatoshi = commitment.commitInput.txOut.amount - commitment.localCommit.commitTxAndRemoteSig.commitTx.tx.txOut.map(_.amount).sum
|
||||
if (remoteClosingFee > lastCommitFeeSatoshi && commitment.params.commitmentFormat == DefaultCommitmentFormat) {
|
||||
log.error(s"remote proposed a commit fee higher than the last commitment fee: remote closing fee=${remoteClosingFee.toLong} last commit fees=$lastCommitFeeSatoshi")
|
||||
Left(InvalidCloseFee(commitment.channelId, remoteClosingFee))
|
||||
} else {
|
||||
val (closingTx, closingSigned) = makeClosingTx(keyManager, commitment, localScriptPubkey, remoteScriptPubkey, ClosingFees(remoteClosingFee, remoteClosingFee, remoteClosingFee))
|
||||
if (checkClosingDustAmounts(closingTx)) {
|
||||
val signedClosingTx = Transactions.addSigs(closingTx, keyManager.fundingPublicKey(commitment.localParams.fundingKeyPath, commitment.fundingTxIndex).publicKey, commitment.remoteFundingPubKey, closingSigned.signature, remoteClosingSig)
|
||||
Transactions.checkSpendable(signedClosingTx) match {
|
||||
case Success(_) => Right(signedClosingTx, closingSigned)
|
||||
case _ => Left(InvalidCloseSignature(commitment.channelId, signedClosingTx.tx.txid))
|
||||
}
|
||||
} else {
|
||||
Left(InvalidCloseAmountBelowDust(commitment.channelId, closingTx.tx.txid))
|
||||
val (closingTx, closingSigned) = makeClosingTx(keyManager, commitment, localScriptPubkey, remoteScriptPubkey, ClosingFees(remoteClosingFee, remoteClosingFee, remoteClosingFee))
|
||||
if (checkClosingDustAmounts(closingTx)) {
|
||||
val signedClosingTx = Transactions.addSigs(closingTx, keyManager.fundingPublicKey(commitment.localParams.fundingKeyPath, commitment.fundingTxIndex).publicKey, commitment.remoteFundingPubKey, closingSigned.signature, remoteClosingSig)
|
||||
Transactions.checkSpendable(signedClosingTx) match {
|
||||
case Success(_) => Right(signedClosingTx, closingSigned)
|
||||
case _ => Left(InvalidCloseSignature(commitment.channelId, signedClosingTx.tx.txid))
|
||||
}
|
||||
} else {
|
||||
Left(InvalidCloseAmountBelowDust(commitment.channelId, closingTx.tx.txid))
|
||||
}
|
||||
}
|
||||
|
||||
|
|
@ -995,7 +986,7 @@ object Helpers {
|
|||
*
|
||||
* This function returns the per-commitment secret in the first case, and None in the other cases.
|
||||
*/
|
||||
def getRemotePerCommitmentSecret(keyManager: ChannelKeyManager, params: ChannelParams, remotePerCommitmentSecrets: ShaChain, commitTx: Transaction)(implicit log: LoggingAdapter): Option[(Long, PrivateKey)] = {
|
||||
def getRemotePerCommitmentSecret(keyManager: ChannelKeyManager, params: ChannelParams, remotePerCommitmentSecrets: ShaChain, commitTx: Transaction): Option[(Long, PrivateKey)] = {
|
||||
import params._
|
||||
// a valid tx will always have at least one input, but this ensures we don't throw in tests
|
||||
val sequence = commitTx.txIn.headOption.map(_.sequence).getOrElse(0L)
|
||||
|
|
|
|||
|
|
@ -26,6 +26,7 @@ import fr.acinq.eclair.channel.fsm.Channel
|
|||
import fr.acinq.eclair.channel.publish.TxPublisher.{PublishFinalTx, PublishTx}
|
||||
import fr.acinq.eclair.channel.states.{ChannelStateTestsBase, ChannelStateTestsTags}
|
||||
import fr.acinq.eclair.transactions.Transactions
|
||||
import fr.acinq.eclair.transactions.Transactions.ZeroFeeHtlcTxAnchorOutputsCommitmentFormat
|
||||
import fr.acinq.eclair.wire.protocol.ClosingSignedTlv.FeeRange
|
||||
import fr.acinq.eclair.wire.protocol.{ClosingSigned, Error, Shutdown, TlvStream, Warning}
|
||||
import fr.acinq.eclair.{CltvExpiry, Features, MilliSatoshiLong, TestConstants, TestFeeEstimator, TestKitBaseClass, randomBytes32}
|
||||
|
|
@ -415,17 +416,21 @@ class NegotiatingStateSpec extends TestKitBaseClass with FixtureAnyFunSuiteLike
|
|||
bob2alice.expectNoMessage(100 millis)
|
||||
}
|
||||
|
||||
test("recv ClosingSigned (fee too high)") { f =>
|
||||
test("recv ClosingSigned (fee higher than commit tx fee)", Tag(ChannelStateTestsTags.AnchorOutputsZeroFeeHtlcTxs)) { f =>
|
||||
import f._
|
||||
bobClose(f)
|
||||
val commitment = alice.stateData.asInstanceOf[DATA_NORMAL].commitments.latest
|
||||
val commitFee = Transactions.commitTxFeeMsat(commitment.localParams.dustLimit, commitment.localCommit.spec, ZeroFeeHtlcTxAnchorOutputsCommitmentFormat)
|
||||
aliceClose(f)
|
||||
val aliceCloseSig = alice2bob.expectMsgType[ClosingSigned]
|
||||
val tx = bob.stateData.asInstanceOf[DATA_NEGOTIATING].commitments.latest.localCommit.commitTxAndRemoteSig.commitTx.tx
|
||||
alice2bob.forward(bob, aliceCloseSig.copy(feeSatoshis = 99000 sat)) // sig doesn't matter, it is checked later
|
||||
val error = bob2alice.expectMsgType[Error]
|
||||
assert(new String(error.data.toArray).startsWith("invalid close fee: fee_satoshis=99000 sat"))
|
||||
assert(bob2blockchain.expectMsgType[PublishFinalTx].tx.txid == tx.txid)
|
||||
bob2blockchain.expectMsgType[PublishTx]
|
||||
bob2blockchain.expectMsgType[WatchTxConfirmed]
|
||||
assert(aliceCloseSig.feeSatoshis > commitFee.truncateToSatoshi)
|
||||
alice2bob.forward(bob, aliceCloseSig)
|
||||
val bobCloseSig = bob2alice.expectMsgType[ClosingSigned]
|
||||
assert(bobCloseSig.feeSatoshis == aliceCloseSig.feeSatoshis)
|
||||
awaitCond(bob.stateName == CLOSING)
|
||||
val closingTx = bob.stateData.asInstanceOf[DATA_CLOSING].mutualClosePublished.head.tx
|
||||
assert(bob2blockchain.expectMsgType[PublishFinalTx].tx.txid == closingTx.txid)
|
||||
bob2alice.forward(alice, bobCloseSig)
|
||||
assert(alice2blockchain.expectMsgType[PublishFinalTx].tx.txid == closingTx.txid)
|
||||
}
|
||||
|
||||
test("recv ClosingSigned (invalid sig)") { f =>
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue