Skip to content

Use CLTV as tie-breaker for offered htlc output sorting #790

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
araspitzu merged 6 commits into from
Mar 30, 2020
Merged

Use CLTV as tie-breaker for offered htlc output sorting #790

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
d4703fc
Sort commit transaction outputs using BIP69 + CLTV as tie-breaker for…
araspitzu Mar 26, 2020
f59111a
Type DirectedHtlc:
araspitzu Mar 27, 2020
49d3e9f
fixup! Match on correct htlc type in 'claimRemoteCommitTxOutput'
araspitzu Mar 27, 2020
a3dcaa1
Address PR feedback: formatting
araspitzu Mar 27, 2020
60ba2e4
Address PR feedback: refactor code, add non-reg test for new HTLC codec
araspitzu Mar 30, 2020
e6d9182
Extract non-reg test for htlc codecs in a separate test.
araspitzu Mar 30, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion eclair-core/src/main/scala/fr/acinq/eclair/blockchain/bitcoind/BitcoinCoreWallet.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,7 @@ class BitcoinCoreWallet(rpcClient: BitcoinJsonRPCClient)(implicit ec: ExecutionC
// now let's sign the funding tx
SignTransactionResponse(fundingTx, true) <- signTransactionOrUnlock(unsignedFundingTx)
// there will probably be a change output, so we need to find which output is ours
outputIndex = Transactions.findPubKeyScriptIndex(fundingTx, pubkeyScript, outputsAlreadyUsed = Set.empty, amount_opt = None)
outputIndex = Transactions.findPubKeyScriptIndex(fundingTx, pubkeyScript, amount_opt = None)
_ = logger.debug(s"created funding txid=${fundingTx.txid} outputIndex=$outputIndex fee=$fee")
} yield MakeFundingTxResponse(fundingTx, outputIndex, fee)
}
Expand Down
7 changes: 3 additions & 4 deletions eclair-core/src/main/scala/fr/acinq/eclair/channel/Channel.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -736,10 +736,9 @@ class Channel(val nodeParams: NodeParams, val wallet: EclairWallet, remoteNodeId
// we persist htlc data in order to be able to claim htlc outputs in case a revoked tx is published by our
// counterparty, so only htlcs above remote's dust_limit matter
val trimmedHtlcs = Transactions.trimOfferedHtlcs(d.commitments.remoteParams.dustLimit, nextRemoteCommit.spec) ++ Transactions.trimReceivedHtlcs(d.commitments.remoteParams.dustLimit, nextRemoteCommit.spec)
trimmedHtlcs collect {
case DirectedHtlc(_, u) =>
log.info(s"adding paymentHash=${u.paymentHash} cltvExpiry=${u.cltvExpiry} to htlcs db for commitNumber=$nextCommitNumber")
nodeParams.db.channels.addHtlcInfo(d.channelId, nextCommitNumber, u.paymentHash, u.cltvExpiry)
trimmedHtlcs.map(_.add).foreach { htlc =>
log.info(s"adding paymentHash=${htlc.paymentHash} cltvExpiry=${htlc.cltvExpiry} to htlcs db for commitNumber=$nextCommitNumber")
nodeParams.db.channels.addHtlcInfo(d.channelId, nextCommitNumber, htlc.paymentHash, htlc.cltvExpiry)
}
if (!Helpers.aboveReserve(d.commitments) && Helpers.aboveReserve(commitments1)) {
// we just went above reserve (can't go below), let's refresh our channel_update to enable/disable it accordingly
Expand Down
26 changes: 20 additions & 6 deletions eclair-core/src/main/scala/fr/acinq/eclair/channel/Commitments.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -571,27 +571,41 @@ object Commitments {
}
}

def makeLocalTxs(keyManager: KeyManager, channelVersion: ChannelVersion, commitTxNumber: Long, localParams: LocalParams, remoteParams: RemoteParams, commitmentInput: InputInfo, localPerCommitmentPoint: PublicKey, spec: CommitmentSpec): (CommitTx, Seq[HtlcTimeoutTx], Seq[HtlcSuccessTx]) = {
def makeLocalTxs(keyManager: KeyManager,
channelVersion: ChannelVersion,
commitTxNumber: Long,
localParams: LocalParams,
remoteParams: RemoteParams,
commitmentInput: InputInfo,
localPerCommitmentPoint: PublicKey,
spec: CommitmentSpec): (CommitTx, Seq[HtlcTimeoutTx], Seq[HtlcSuccessTx]) = {
val channelKeyPath = keyManager.channelKeyPath(localParams, channelVersion)
val localDelayedPaymentPubkey = Generators.derivePubKey(keyManager.delayedPaymentPoint(channelKeyPath).publicKey, localPerCommitmentPoint)
val localHtlcPubkey = Generators.derivePubKey(keyManager.htlcPoint(channelKeyPath).publicKey, localPerCommitmentPoint)
val remotePaymentPubkey = Generators.derivePubKey(remoteParams.paymentBasepoint, localPerCommitmentPoint)
val remoteHtlcPubkey = Generators.derivePubKey(remoteParams.htlcBasepoint, localPerCommitmentPoint)
val localRevocationPubkey = Generators.revocationPubKey(remoteParams.revocationBasepoint, localPerCommitmentPoint)
val commitTx = Transactions.makeCommitTx(commitmentInput, commitTxNumber, keyManager.paymentPoint(channelKeyPath).publicKey, remoteParams.paymentBasepoint, localParams.isFunder, localParams.dustLimit, localRevocationPubkey, remoteParams.toSelfDelay, localDelayedPaymentPubkey, remotePaymentPubkey, localHtlcPubkey, remoteHtlcPubkey, spec)
val (htlcTimeoutTxs, htlcSuccessTxs) = Transactions.makeHtlcTxs(commitTx.tx, localParams.dustLimit, localRevocationPubkey, remoteParams.toSelfDelay, localDelayedPaymentPubkey, localHtlcPubkey, remoteHtlcPubkey, spec)
val outputs = makeCommitTxOutputs(localParams.isFunder, localParams.dustLimit, localRevocationPubkey, remoteParams.toSelfDelay, localDelayedPaymentPubkey, remotePaymentPubkey, localHtlcPubkey, remoteHtlcPubkey, spec)
val commitTx = Transactions.makeCommitTx(commitmentInput, commitTxNumber, keyManager.paymentPoint(channelKeyPath).publicKey, remoteParams.paymentBasepoint, localParams.isFunder, outputs)
val (htlcTimeoutTxs, htlcSuccessTxs) = Transactions.makeHtlcTxs(commitTx.tx, localParams.dustLimit, localRevocationPubkey, remoteParams.toSelfDelay, localDelayedPaymentPubkey, spec.feeratePerKw, outputs)
(commitTx, htlcTimeoutTxs, htlcSuccessTxs)
}

def makeRemoteTxs(keyManager: KeyManager, channelVersion: ChannelVersion, commitTxNumber: Long, localParams: LocalParams, remoteParams: RemoteParams, commitmentInput: InputInfo, remotePerCommitmentPoint: PublicKey, spec: CommitmentSpec): (CommitTx, Seq[HtlcTimeoutTx], Seq[HtlcSuccessTx]) = {
def makeRemoteTxs(keyManager: KeyManager,
channelVersion: ChannelVersion,
commitTxNumber: Long, localParams: LocalParams,
remoteParams: RemoteParams, commitmentInput: InputInfo,
remotePerCommitmentPoint: PublicKey,
spec: CommitmentSpec): (CommitTx, Seq[HtlcTimeoutTx], Seq[HtlcSuccessTx]) = {
val channelKeyPath = keyManager.channelKeyPath(localParams, channelVersion)
val localPaymentPubkey = Generators.derivePubKey(keyManager.paymentPoint(channelKeyPath).publicKey, remotePerCommitmentPoint)
val localHtlcPubkey = Generators.derivePubKey(keyManager.htlcPoint(channelKeyPath).publicKey, remotePerCommitmentPoint)
val remoteDelayedPaymentPubkey = Generators.derivePubKey(remoteParams.delayedPaymentBasepoint, remotePerCommitmentPoint)
val remoteHtlcPubkey = Generators.derivePubKey(remoteParams.htlcBasepoint, remotePerCommitmentPoint)
val remoteRevocationPubkey = Generators.revocationPubKey(keyManager.revocationPoint(channelKeyPath).publicKey, remotePerCommitmentPoint)
val commitTx = Transactions.makeCommitTx(commitmentInput, commitTxNumber, remoteParams.paymentBasepoint, keyManager.paymentPoint(channelKeyPath).publicKey, !localParams.isFunder, remoteParams.dustLimit, remoteRevocationPubkey, localParams.toSelfDelay, remoteDelayedPaymentPubkey, localPaymentPubkey, remoteHtlcPubkey, localHtlcPubkey, spec)
val (htlcTimeoutTxs, htlcSuccessTxs) = Transactions.makeHtlcTxs(commitTx.tx, remoteParams.dustLimit, remoteRevocationPubkey, localParams.toSelfDelay, remoteDelayedPaymentPubkey, remoteHtlcPubkey, localHtlcPubkey, spec)
val outputs = makeCommitTxOutputs(!localParams.isFunder, remoteParams.dustLimit, remoteRevocationPubkey, localParams.toSelfDelay, remoteDelayedPaymentPubkey, localPaymentPubkey, remoteHtlcPubkey, localHtlcPubkey, spec)
val commitTx = Transactions.makeCommitTx(commitmentInput, commitTxNumber, remoteParams.paymentBasepoint, keyManager.paymentPoint(channelKeyPath).publicKey, !localParams.isFunder, outputs)
val (htlcTimeoutTxs, htlcSuccessTxs) = Transactions.makeHtlcTxs(commitTx.tx, remoteParams.dustLimit, remoteRevocationPubkey, localParams.toSelfDelay, remoteDelayedPaymentPubkey, spec.feeratePerKw, outputs)
(commitTx, htlcTimeoutTxs, htlcSuccessTxs)
}

Expand Down
33 changes: 16 additions & 17 deletions eclair-core/src/main/scala/fr/acinq/eclair/channel/Helpers.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -602,6 +602,9 @@ object Helpers {
val localHtlcPubkey = Generators.derivePubKey(keyManager.htlcPoint(channelKeyPath).publicKey, remoteCommit.remotePerCommitmentPoint)
val remoteHtlcPubkey = Generators.derivePubKey(remoteParams.htlcBasepoint, remoteCommit.remotePerCommitmentPoint)
val remoteRevocationPubkey = Generators.revocationPubKey(keyManager.revocationPoint(channelKeyPath).publicKey, remoteCommit.remotePerCommitmentPoint)
val remoteDelayedPaymentPubkey = Generators.derivePubKey(remoteParams.delayedPaymentBasepoint, remoteCommit.remotePerCommitmentPoint)
val localPaymentPubkey = Generators.derivePubKey(keyManager.paymentPoint(channelKeyPath).publicKey, remoteCommit.remotePerCommitmentPoint)
val outputs = makeCommitTxOutputs(!localParams.isFunder, remoteParams.dustLimit, remoteRevocationPubkey, localParams.toSelfDelay, remoteDelayedPaymentPubkey, localPaymentPubkey, remoteHtlcPubkey, localHtlcPubkey, remoteCommit.spec)

// we need to use a rather high fee for htlc-claim because we compete with the counterparty
val feeratePerKwHtlc = feeEstimator.getFeeratePerKw(target = 2)
Expand All @@ -610,23 +613,21 @@ object Helpers {
val preimages = commitments.localChanges.all.collect { case u: UpdateFulfillHtlc => u.paymentPreimage }

// remember we are looking at the remote commitment so IN for them is really OUT for us and vice versa
var outputsAlreadyUsed = Set.empty[Int] // this is needed to handle cases where we have several identical htlcs
val txes = remoteCommit.spec.htlcs.collect {
// incoming htlc for which we have the preimage: we spend it directly
case DirectedHtlc(OUT, add: UpdateAddHtlc) if preimages.exists(r => sha256(r) == add.paymentHash) => generateTx("claim-htlc-success")(Try {
// incoming htlc for which we have the preimage: we spend it directly.
// NB: we are looking at the remote's commitment, from its point of view it's an outgoing htlc.
case OutgoingHtlc(add: UpdateAddHtlc) if preimages.exists(r => sha256(r) == add.paymentHash) => generateTx("claim-htlc-success")(Try {
val preimage = preimages.find(r => sha256(r) == add.paymentHash).get
val txinfo = Transactions.makeClaimHtlcSuccessTx(remoteCommitTx.tx, outputsAlreadyUsed, localParams.dustLimit, localHtlcPubkey, remoteHtlcPubkey, remoteRevocationPubkey, localParams.defaultFinalScriptPubKey, add, feeratePerKwHtlc)
outputsAlreadyUsed = outputsAlreadyUsed + txinfo.input.outPoint.index.toInt
val txinfo = Transactions.makeClaimHtlcSuccessTx(remoteCommitTx.tx, outputs, localParams.dustLimit, localHtlcPubkey, remoteHtlcPubkey, remoteRevocationPubkey, localParams.defaultFinalScriptPubKey, add, feeratePerKwHtlc)
val sig = keyManager.sign(txinfo, keyManager.htlcPoint(channelKeyPath), remoteCommit.remotePerCommitmentPoint)
Transactions.addSigs(txinfo, sig, preimage)
})

// (incoming htlc for which we don't have the preimage: nothing to do, it will timeout eventually and they will get their funds back)

// outgoing htlc: they may or may not have the preimage, the only thing to do is try to get back our funds after timeout
case DirectedHtlc(IN, add: UpdateAddHtlc) => generateTx("claim-htlc-timeout")(Try {
val txinfo = Transactions.makeClaimHtlcTimeoutTx(remoteCommitTx.tx, outputsAlreadyUsed, localParams.dustLimit, localHtlcPubkey, remoteHtlcPubkey, remoteRevocationPubkey, localParams.defaultFinalScriptPubKey, add, feeratePerKwHtlc)
outputsAlreadyUsed = outputsAlreadyUsed + txinfo.input.outPoint.index.toInt
case IncomingHtlc(add: UpdateAddHtlc) => generateTx("claim-htlc-timeout")(Try {
val txinfo = Transactions.makeClaimHtlcTimeoutTx(remoteCommitTx.tx, outputs, localParams.dustLimit, localHtlcPubkey, remoteHtlcPubkey, remoteRevocationPubkey, localParams.defaultFinalScriptPubKey, add, feeratePerKwHtlc)
val sig = keyManager.sign(txinfo, keyManager.htlcPoint(channelKeyPath), remoteCommit.remotePerCommitmentPoint)
Transactions.addSigs(txinfo, sig)
})
Expand Down Expand Up @@ -697,19 +698,19 @@ object Helpers {
val remotePerCommitmentPoint = remotePerCommitmentSecret.publicKey
val remoteDelayedPaymentPubkey = Generators.derivePubKey(remoteParams.delayedPaymentBasepoint, remotePerCommitmentPoint)
val remoteRevocationPubkey = Generators.revocationPubKey(keyManager.revocationPoint(channelKeyPath).publicKey, remotePerCommitmentPoint)
val localPubkey = Generators.derivePubKey(keyManager.paymentPoint(channelKeyPath).publicKey, remotePerCommitmentPoint)
val localHtlcPubkey = Generators.derivePubKey(keyManager.htlcPoint(channelKeyPath).publicKey, remotePerCommitmentPoint)
val remoteHtlcPubkey = Generators.derivePubKey(remoteParams.htlcBasepoint, remotePerCommitmentPoint)
val localPaymentPubkey = Generators.derivePubKey(keyManager.paymentPoint(channelKeyPath).publicKey, remotePerCommitmentPoint)
val localHtlcPubkey = Generators.derivePubKey(keyManager.htlcPoint(channelKeyPath).publicKey, remotePerCommitmentPoint)

val feeratePerKwMain = feeEstimator.getFeeratePerKw(feeTargets.claimMainBlockTarget)
// we need to use a high fee here for punishment txes because after a delay they can be spent by the counterparty
val feeratePerKwPenalty = feeEstimator.getFeeratePerKw(target = 2)

// first we will claim our main output right away
val mainTx = generateTx("claim-p2wpkh-output")(Try {
val claimMain = Transactions.makeClaimP2WPKHOutputTx(tx, localParams.dustLimit, localPubkey, localParams.defaultFinalScriptPubKey, feeratePerKwMain)
val claimMain = Transactions.makeClaimP2WPKHOutputTx(tx, localParams.dustLimit, localPaymentPubkey, localParams.defaultFinalScriptPubKey, feeratePerKwMain)
val sig = keyManager.sign(claimMain, keyManager.paymentPoint(channelKeyPath), remotePerCommitmentPoint)
Transactions.addSigs(claimMain, localPubkey, sig)
Transactions.addSigs(claimMain, localPaymentPubkey, sig)
})

// then we punish them by stealing their main output
Expand All @@ -726,16 +727,14 @@ object Helpers {
htlcInfos.map { case (paymentHash, cltvExpiry) => Scripts.htlcReceived(remoteHtlcPubkey, localHtlcPubkey, remoteRevocationPubkey, Crypto.ripemd160(paymentHash), cltvExpiry) } ++
htlcInfos.map { case (paymentHash, _) => Scripts.htlcOffered(remoteHtlcPubkey, localHtlcPubkey, remoteRevocationPubkey, Crypto.ripemd160(paymentHash)) }
)
.map(redeemScript => (Script.write(pay2wsh(redeemScript)) -> Script.write(redeemScript)))
.map(redeemScript => Script.write(pay2wsh(redeemScript)) -> Script.write(redeemScript))
.toMap

// and finally we steal the htlc outputs
var outputsAlreadyUsed = Set.empty[Int] // this is needed to handle cases where we have several identical htlcs
val htlcPenaltyTxs = tx.txOut.collect { case txOut if htlcsRedeemScripts.contains(txOut.publicKeyScript) =>
val htlcPenaltyTxs = tx.txOut.zipWithIndex.collect { case (txOut, outputIndex) if htlcsRedeemScripts.contains(txOut.publicKeyScript) =>
val htlcRedeemScript = htlcsRedeemScripts(txOut.publicKeyScript)
generateTx("htlc-penalty")(Try {
val htlcPenalty = Transactions.makeHtlcPenaltyTx(tx, outputsAlreadyUsed, htlcRedeemScript, localParams.dustLimit, localParams.defaultFinalScriptPubKey, feeratePerKwPenalty)
outputsAlreadyUsed = outputsAlreadyUsed + htlcPenalty.input.outPoint.index.toInt
val htlcPenalty = Transactions.makeHtlcPenaltyTx(tx, outputIndex, htlcRedeemScript, localParams.dustLimit, localParams.defaultFinalScriptPubKey, feeratePerKwPenalty)
val sig = keyManager.sign(htlcPenalty, keyManager.revocationPoint(channelKeyPath), remotePerCommitmentSecret)
Transactions.addSigs(htlcPenalty, sig, remoteRevocationPubkey)
})
Expand Down
25 changes: 23 additions & 2 deletions eclair-core/src/main/scala/fr/acinq/eclair/transactions/CommitmentSpec.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,25 @@ case object IN extends Direction { def opposite = OUT }
case object OUT extends Direction { def opposite = IN }
// @formatter:on

case class DirectedHtlc(direction: Direction, add: UpdateAddHtlc)
sealed trait CommitmentOutput
object CommitmentOutput {
case object ToLocal extends CommitmentOutput
case object ToRemote extends CommitmentOutput
case class InHtlc(incomingHtlc: IncomingHtlc) extends CommitmentOutput
Comment thread
t-bast marked this conversation as resolved.
case class OutHtlc(outgoingHtlc: OutgoingHtlc) extends CommitmentOutput
}

sealed trait DirectedHtlc {
def direction: Direction
Comment thread
t-bast marked this conversation as resolved.
val add: UpdateAddHtlc
def opposite: DirectedHtlc = this match {
case IncomingHtlc(_) => OutgoingHtlc(add)
case OutgoingHtlc(_) => IncomingHtlc(add)
}
}

case class IncomingHtlc(add: UpdateAddHtlc) extends DirectedHtlc { override def direction: Direction = IN }
case class OutgoingHtlc(add: UpdateAddHtlc) extends DirectedHtlc { override def direction: Direction = OUT }

final case class CommitmentSpec(htlcs: Set[DirectedHtlc], feeratePerKw: Long, toLocal: MilliSatoshi, toRemote: MilliSatoshi) {

Expand All @@ -45,7 +63,10 @@ object CommitmentSpec {
}

def addHtlc(spec: CommitmentSpec, direction: Direction, update: UpdateAddHtlc): CommitmentSpec = {
Comment thread
t-bast marked this conversation as resolved.
val htlc = DirectedHtlc(direction, update)
val htlc = direction match {
case IN => IncomingHtlc(update)
case OUT => OutgoingHtlc(update)
}
direction match {
case OUT => spec.copy(toLocal = spec.toLocal - htlc.add.amountMsat, htlcs = spec.htlcs + htlc)
case IN => spec.copy(toRemote = spec.toRemote - htlc.add.amountMsat, htlcs = spec.htlcs + htlc)
Expand Down
Loading