Skip to content

Commit

Permalink
Limit how far we look into the blockchain (#2731)
Browse files Browse the repository at this point in the history
If we're unable to find the spending tx in the mempool, we start looking
for it in the blockchain. Unfortunately, there are cases where we end up
in that code path even though the spending tx is not confirmed:

- a timeout on the `getTransaction` call
- the spending tx gets dropped from our mempool for some reason
- bitcoind is malicious or buggy

Fetching the whole blockchain isn't really useful: after enough time has
passed, we can be pretty sure that a potential attacker would have claimed
the transaction's outputs already and we can't punish them.
  • Loading branch information
t-bast committed Sep 21, 2023
1 parent 4e339aa commit 96ebbfe
Show file tree
Hide file tree
Showing 8 changed files with 24 additions and 10 deletions.
4 changes: 4 additions & 0 deletions eclair-core/src/main/resources/reference.conf
Original file line number Diff line number Diff line change
Expand Up @@ -137,6 +137,10 @@ eclair {
min-final-expiry-delta-blocks = 30 // Bolt 11 invoice's min_final_cltv_expiry; must be strictly greater than fulfill-safety-before-timeout-blocks
max-block-processing-delay = 30 seconds // we add a random delay before processing blocks, capped at this value, to prevent herd effect
max-tx-publish-retry-delay = 60 seconds // we add a random delay before retrying failed transaction publication
// When a channel has been spent while we were offline, we limit how many blocks in the past we scan, otherwise we
// may scan the entire blockchain (which is very costly). It doesn't make sense to scan too far in the past, as an
// attacker will already have swept the funds if we didn't detect a channel close that happened a long time ago.
max-channel-spent-rescan-blocks = 720

// The default strategy, when we encounter an unhandled exception or internal error, is to locally force-close the
// channel. Not only is there a delay before the channel balance gets refunded, but if the exception was due to some
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -510,6 +510,7 @@ object NodeParams extends Logging {
minFinalExpiryDelta = minFinalExpiryDelta,
maxBlockProcessingDelay = FiniteDuration(config.getDuration("channel.max-block-processing-delay").getSeconds, TimeUnit.SECONDS),
maxTxPublishRetryDelay = FiniteDuration(config.getDuration("channel.max-tx-publish-retry-delay").getSeconds, TimeUnit.SECONDS),
maxChannelSpentRescanBlocks = config.getInt("channel.max-channel-spent-rescan-blocks"),
unhandledExceptionStrategy = unhandledExceptionStrategy,
revocationTimeout = FiniteDuration(config.getDuration("channel.revocation-timeout").getSeconds, TimeUnit.SECONDS),
requireConfirmedInputsForDualFunding = config.getBoolean("channel.require-confirmed-inputs-for-dual-funding"),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -396,9 +396,11 @@ private class ZmqWatcher(nodeParams: NodeParams, blockHeight: AtomicLong, client
case None =>
// no luck, we have to do it the hard way...
log.warn(s"${w.txId}:${w.outputIndex} has already been spent, spending tx not in the mempool, looking in the blockchain...")
client.lookForSpendingTx(None, w.txId, w.outputIndex).map { spendingTx =>
client.lookForSpendingTx(None, w.txId, w.outputIndex, nodeParams.channelConf.maxChannelSpentRescanBlocks).map { spendingTx =>
log.warn(s"found the spending tx of ${w.txId}:${w.outputIndex} in the blockchain: txid=${spendingTx.txid}")
context.self ! ProcessNewTransaction(spendingTx)
}.recover {
case _ => log.warn(s"could not find the spending tx of ${w.txId}:${w.outputIndex} in the blockchain, funds are at risk")
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -183,19 +183,20 @@ class BitcoinCoreClient(val rpcClient: BitcoinJsonRPCClient, val onChainKeyManag

/**
* Iterate over blocks to find the transaction that has spent a given output.
* NB: only call this method when you're sure the output has been spent, otherwise this will iterate over the whole
* blockchain history.
* It isn't useful to look at the whole blockchain history: if the transaction was confirmed long ago, an attacker
* will have already claimed all possible outputs and there's nothing we can do about it.
*
* @param blockhash_opt hash of a block *after* the output has been spent. If not provided, we will use the blockchain tip.
* @param txid id of the transaction output that has been spent.
* @param outputIndex index of the transaction output that has been spent.
* @param limit maximum number of previous blocks to scan.
* @return the transaction spending the given output.
*/
def lookForSpendingTx(blockhash_opt: Option[ByteVector32], txid: ByteVector32, outputIndex: Int)(implicit ec: ExecutionContext): Future[Transaction] = {
lookForSpendingTx(blockhash_opt.map(KotlinUtils.scala2kmp), KotlinUtils.scala2kmp(txid), outputIndex)
def lookForSpendingTx(blockhash_opt: Option[ByteVector32], txid: ByteVector32, outputIndex: Int, limit: Int)(implicit ec: ExecutionContext): Future[Transaction] = {
lookForSpendingTx(blockhash_opt.map(KotlinUtils.scala2kmp), KotlinUtils.scala2kmp(txid), outputIndex, limit)
}

def lookForSpendingTx(blockhash_opt: Option[fr.acinq.bitcoin.ByteVector32], txid: fr.acinq.bitcoin.ByteVector32, outputIndex: Int)(implicit ec: ExecutionContext): Future[Transaction] =
def lookForSpendingTx(blockhash_opt: Option[fr.acinq.bitcoin.ByteVector32], txid: fr.acinq.bitcoin.ByteVector32, outputIndex: Int, limit: Int)(implicit ec: ExecutionContext): Future[Transaction] =
for {
blockhash <- blockhash_opt match {
case Some(b) => Future.successful(b)
Expand All @@ -205,8 +206,9 @@ class BitcoinCoreClient(val rpcClient: BitcoinJsonRPCClient, val onChainKeyManag
block <- rpcClient.invoke("getblock", blockhash, 0).collect { case JString(b) => Block.read(b) }
prevblockhash = block.header.hashPreviousBlock.reversed()
res <- block.tx.asScala.find(tx => tx.txIn.asScala.exists(i => i.outPoint.txid == txid && i.outPoint.index == outputIndex)) match {
case None => lookForSpendingTx(Some(prevblockhash), txid, outputIndex)
case Some(tx) => Future.successful(KotlinUtils.kmp2scala(tx))
case None if limit > 0 => lookForSpendingTx(Some(prevblockhash), txid, outputIndex, limit - 1)
case None => Future.failed(new RuntimeException(s"couldn't find tx spending $txid:$outputIndex in the blockchain"))
}
} yield res

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,7 @@ object Channel {
minFinalExpiryDelta: CltvExpiryDelta,
maxBlockProcessingDelay: FiniteDuration,
maxTxPublishRetryDelay: FiniteDuration,
maxChannelSpentRescanBlocks: Int,
unhandledExceptionStrategy: UnhandledExceptionStrategy,
revocationTimeout: FiniteDuration,
requireConfirmedInputsForDualFunding: Boolean,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -117,6 +117,7 @@ object TestConstants {
minFinalExpiryDelta = CltvExpiryDelta(18),
maxBlockProcessingDelay = 10 millis,
maxTxPublishRetryDelay = 10 millis,
maxChannelSpentRescanBlocks = 144,
htlcMinimum = 0 msat,
minDepthBlocks = 3,
toRemoteDelay = CltvExpiryDelta(144),
Expand Down Expand Up @@ -278,6 +279,7 @@ object TestConstants {
minFinalExpiryDelta = CltvExpiryDelta(18),
maxBlockProcessingDelay = 10 millis,
maxTxPublishRetryDelay = 10 millis,
maxChannelSpentRescanBlocks = 144,
htlcMinimum = 1000 msat,
minDepthBlocks = 3,
toRemoteDelay = CltvExpiryDelta(144),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1288,10 +1288,12 @@ class BitcoinCoreClientSpec extends TestKitBaseClass with BitcoindService with A
bitcoinClient.isTransactionOutputSpendable(tx1.txid, 0, includeMempool = true).pipeTo(sender.ref)
sender.expectMsg(true)

generateBlocks(1)
generateBlocks(10)
bitcoinClient.lookForMempoolSpendingTx(tx1.txIn.head.outPoint.txid, tx1.txIn.head.outPoint.index.toInt).pipeTo(sender.ref)
sender.expectMsgType[Failure]
bitcoinClient.lookForSpendingTx(None, tx1.txIn.head.outPoint.txid, tx1.txIn.head.outPoint.index.toInt).pipeTo(sender.ref)
bitcoinClient.lookForSpendingTx(None, tx1.txIn.head.outPoint.txid, tx1.txIn.head.outPoint.index.toInt, limit = 5).pipeTo(sender.ref)
sender.expectMsgType[Failure]
bitcoinClient.lookForSpendingTx(None, tx1.txIn.head.outPoint.txid, tx1.txIn.head.outPoint.index.toInt, limit = 15).pipeTo(sender.ref)
sender.expectMsg(tx1)
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -577,7 +577,7 @@ class StandardChannelIntegrationSpec extends ChannelIntegrationSpec {
generateBlocks(3)
awaitCond(stateListener.expectMsgType[ChannelStateChanged](max = 60 seconds).currentState == CLOSED, max = 60 seconds)

bitcoinClient.lookForSpendingTx(None, fundingOutpoint.txid, fundingOutpoint.index.toInt).pipeTo(sender.ref)
bitcoinClient.lookForSpendingTx(None, fundingOutpoint.txid, fundingOutpoint.index.toInt, limit = 10).pipeTo(sender.ref)
val closingTx = sender.expectMsgType[Transaction]
assert(closingTx.txOut.map(_.publicKeyScript).toSet == Set(finalPubKeyScriptC, finalPubKeyScriptF))

Expand Down

0 comments on commit 96ebbfe

Please sign in to comment.