Skip to content

Commit

Permalink
Update funding timeout fundee (#1692)
Browse files Browse the repository at this point in the history 
Our previous timeout was based on timestamps, mostly because blockCount
could be 0 on mobile using Electrum until a new block was received.
Now that we're diverging from the mobile wallet codebase, we can use block
heights instead which is more accurate.

See lightning/bolts#839
  • Loading branch information
@t-bast
t-bast authored Feb 19, 2021
1 parent ab89851 commit 0835150
Show file tree
Hide file tree
Showing 7 changed files with 114 additions and 71 deletions.
10 changes: 6 additions & 4 deletions eclair-core/src/main/scala/fr/acinq/eclair/Setup.scala
View file
Original file line number Diff line number Diff line change
Expand Up @@ -66,10 +66,10 @@ import scala.util.{Failure, Success}
*
* Created by PM on 25/01/2016.
*
* @param datadir directory where eclair-core will write/read its data.
* @param pluginParams parameters for all configured plugins.
* @param seeds_opt optional seeds, if set eclair will use them instead of generating them and won't create a node_seed.dat and channel_seed.dat files.
* @param db optional databases to use, if not set eclair will create the necessary databases
* @param datadir directory where eclair-core will write/read its data.
* @param pluginParams parameters for all configured plugins.
* @param seeds_opt optional seeds, if set eclair will use them instead of generating them and won't create a node_seed.dat and channel_seed.dat files.
* @param db optional databases to use, if not set eclair will create the necessary databases
*/
class Setup(datadir: File,
pluginParams: Seq[PluginParams],
Expand Down Expand Up @@ -185,6 +185,8 @@ class Setup(datadir: File,
assert(!initialBlockDownload, s"bitcoind should be synchronized (initialblockdownload=$initialBlockDownload)")
assert(progress > 0.999, s"bitcoind should be synchronized (progress=$progress)")
assert(headers - blocks <= 1, s"bitcoind should be synchronized (headers=$headers blocks=$blocks)")
logger.info(s"current blockchain height=$blocks")
blockCount.set(blocks)
Bitcoind(bitcoinClient)
case ELECTRUM =>
val addresses = config.hasPath("electrum") match {
Expand Down
83 changes: 51 additions & 32 deletions eclair-core/src/main/scala/fr/acinq/eclair/channel/Channel.scala
View file

Large diffs are not rendered by default.

6 changes: 3 additions & 3 deletions eclair-core/src/main/scala/fr/acinq/eclair/channel/ChannelTypes.scala
View file
Original file line number Diff line number Diff line change
Expand Up @@ -335,7 +335,7 @@ final case class DATA_WAIT_FOR_FUNDING_SIGNED(channelId: ByteVector32,
final case class DATA_WAIT_FOR_FUNDING_CONFIRMED(commitments: Commitments,
fundingTx: Option[Transaction],
initialRelayFees_opt: Option[(MilliSatoshi, Int)],
waitingSince: Long, // how long have we been waiting for the funding tx to confirm
waitingSinceBlock: Long, // how long have we been waiting for the funding tx to confirm
deferred: Option[FundingLocked],
lastSent: Either[FundingCreated, FundingSigned]) extends Data with HasCommitments
final case class DATA_WAIT_FOR_FUNDING_LOCKED(commitments: Commitments, shortChannelId: ShortChannelId, lastSent: FundingLocked, initialRelayFees_opt: Option[(MilliSatoshi, Int)]) extends Data with HasCommitments
Expand All @@ -353,11 +353,11 @@ final case class DATA_NEGOTIATING(commitments: Commitments,
closingTxProposed: List[List[ClosingTxProposed]], // one list for every negotiation (there can be several in case of disconnection)
bestUnpublishedClosingTx_opt: Option[Transaction]) extends Data with HasCommitments {
require(closingTxProposed.nonEmpty, "there must always be a list for the current negotiation")
require(!commitments.localParams.isFunder || closingTxProposed.forall(_.nonEmpty), "funder must have at least one closing signature for every negotation attempt because it initiates the closing")
require(!commitments.localParams.isFunder || closingTxProposed.forall(_.nonEmpty), "funder must have at least one closing signature for every negotiation attempt because it initiates the closing")
}
final case class DATA_CLOSING(commitments: Commitments,
fundingTx: Option[Transaction], // this will be non-empty if we are funder and we got in closing while waiting for our own tx to be published
waitingSince: Long, // how long since we initiated the closing
waitingSinceBlock: Long, // how long since we initiated the closing
mutualCloseProposed: List[Transaction], // all exchanged closing sigs are flattened, we use this only to keep track of what publishable tx they have
mutualClosePublished: List[Transaction] = Nil,
localCommitPublished: Option[LocalCommitPublished] = None,
Expand Down
20 changes: 10 additions & 10 deletions eclair-core/src/test/scala/fr/acinq/eclair/channel/HelpersSpec.scala
View file
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ class HelpersSpec extends AnyFunSuite {
DATA_CLOSING(
commitments = commitments,
fundingTx = None,
waitingSince = 0,
waitingSinceBlock = 0,
mutualCloseProposed = tx1 :: tx2 :: tx3 :: Nil,
mutualClosePublished = tx2 :: tx3 :: Nil,
localCommitPublished = None,
Expand All @@ -75,7 +75,7 @@ class HelpersSpec extends AnyFunSuite {
DATA_CLOSING(
commitments = commitments,
fundingTx = None,
waitingSince = 0,
waitingSinceBlock = 0,
mutualCloseProposed = tx1 :: Nil,
mutualClosePublished = tx1 :: Nil,
localCommitPublished = Some(LocalCommitPublished(
Expand All @@ -97,7 +97,7 @@ class HelpersSpec extends AnyFunSuite {
DATA_CLOSING(
commitments = commitments,
fundingTx = None,
waitingSince = 0,
waitingSinceBlock = 0,
mutualCloseProposed = tx1 :: Nil,
mutualClosePublished = tx1 :: Nil,
localCommitPublished = Some(LocalCommitPublished(
Expand All @@ -121,7 +121,7 @@ class HelpersSpec extends AnyFunSuite {
DATA_CLOSING(
commitments = commitments,
fundingTx = None,
waitingSince = 0,
waitingSinceBlock = 0,
mutualCloseProposed = Nil,
mutualClosePublished = Nil,
localCommitPublished = Some(LocalCommitPublished(
Expand Down Expand Up @@ -149,7 +149,7 @@ class HelpersSpec extends AnyFunSuite {
DATA_CLOSING(
commitments = commitments,
fundingTx = None,
waitingSince = 0,
waitingSinceBlock = 0,
mutualCloseProposed = tx1 :: Nil,
mutualClosePublished = tx1 :: Nil,
localCommitPublished = Some(LocalCommitPublished(
Expand Down Expand Up @@ -179,7 +179,7 @@ class HelpersSpec extends AnyFunSuite {
DATA_CLOSING(
commitments = commitments.copy(remoteNextCommitInfo = Left(WaitingForRevocation(commitments.remoteCommit, null, 7L))),
fundingTx = None,
waitingSince = 0,
waitingSinceBlock = 0,
mutualCloseProposed = tx1 :: Nil,
mutualClosePublished = tx1 :: Nil,
localCommitPublished = Some(LocalCommitPublished(
Expand Down Expand Up @@ -215,7 +215,7 @@ class HelpersSpec extends AnyFunSuite {
DATA_CLOSING(
commitments = commitments,
fundingTx = None,
waitingSince = 0,
waitingSinceBlock = 0,
mutualCloseProposed = Nil,
mutualClosePublished = Nil,
localCommitPublished = None,
Expand All @@ -236,7 +236,7 @@ class HelpersSpec extends AnyFunSuite {
DATA_CLOSING(
commitments = commitments,
fundingTx = None,
waitingSince = 0,
waitingSinceBlock = 0,
mutualCloseProposed = Nil,
mutualClosePublished = Nil,
localCommitPublished = None,
Expand All @@ -259,7 +259,7 @@ class HelpersSpec extends AnyFunSuite {
DATA_CLOSING(
commitments = commitments,
fundingTx = None,
waitingSince = 0,
waitingSinceBlock = 0,
mutualCloseProposed = Nil,
mutualClosePublished = Nil,
localCommitPublished = Some(LocalCommitPublished(
Expand Down Expand Up @@ -306,7 +306,7 @@ class HelpersSpec extends AnyFunSuite {
DATA_CLOSING(
commitments = commitments,
fundingTx = None,
waitingSince = 0,
waitingSinceBlock = 0,
mutualCloseProposed = Nil,
mutualClosePublished = Nil,
localCommitPublished = Some(LocalCommitPublished(
Expand Down
43 changes: 42 additions & 1 deletion ...re/src/test/scala/fr/acinq/eclair/channel/states/c/WaitForFundingConfirmedStateSpec.scala
View file
Original file line number Diff line number Diff line change
Expand Up @@ -106,13 +106,54 @@ class WaitForFundingConfirmedStateSpec extends TestKitBaseClass with FixtureAnyF
awaitCond(alice.stateName == CLOSED)
}

test("recv BITCOIN_FUNDING_TIMEOUT") { f =>
test("recv BITCOIN_FUNDING_TIMEOUT (funder)") { f =>
import f._
alice ! BITCOIN_FUNDING_TIMEOUT
alice2bob.expectMsgType[Error]
awaitCond(alice.stateName == CLOSED)
}

test("recv BITCOIN_FUNDING_TIMEOUT (fundee)") { f =>
import f._
bob ! BITCOIN_FUNDING_TIMEOUT
bob2alice.expectMsgType[Error]
awaitCond(bob.stateName == CLOSED)
}

test("recv CurrentBlockCount (funder)") { f =>
import f._
val initialState = alice.stateData.asInstanceOf[DATA_WAIT_FOR_FUNDING_CONFIRMED]
alice ! CurrentBlockCount(initialState.waitingSinceBlock + Channel.FUNDING_TIMEOUT_FUNDEE + 1)
alice2bob.expectNoMsg(100 millis)
}

test("recv CurrentBlockCount (funding timeout not reached)") { f =>
import f._
val initialState = bob.stateData.asInstanceOf[DATA_WAIT_FOR_FUNDING_CONFIRMED]
bob ! CurrentBlockCount(initialState.waitingSinceBlock + Channel.FUNDING_TIMEOUT_FUNDEE - 1)
bob2alice.expectNoMsg(100 millis)
}

test("recv CurrentBlockCount (funding timeout reached)") { f =>
import f._
val initialState = bob.stateData.asInstanceOf[DATA_WAIT_FOR_FUNDING_CONFIRMED]
bob ! CurrentBlockCount(initialState.waitingSinceBlock + Channel.FUNDING_TIMEOUT_FUNDEE + 1)
bob2alice.expectMsgType[Error]
awaitCond(bob.stateName == CLOSED)
}

test("migrate waitingSince to waitingSinceBlocks") { f =>
import f._
// Before version 0.5.1, eclair used an absolute timestamp instead of a block height for funding timeouts.
val beforeMigration = bob.stateData.asInstanceOf[DATA_WAIT_FOR_FUNDING_CONFIRMED].copy(waitingSinceBlock = System.currentTimeMillis().milliseconds.toSeconds)
bob.setState(WAIT_FOR_INIT_INTERNAL, Nothing)
bob ! INPUT_RESTORED(beforeMigration)
awaitCond(bob.stateName == OFFLINE)
// We reset the waiting period to the current block height when starting up after updating eclair.
val currentBlockHeight = bob.underlyingActor.nodeParams.currentBlockHeight
assert(bob.stateData.asInstanceOf[DATA_WAIT_FOR_FUNDING_CONFIRMED].waitingSinceBlock === currentBlockHeight)
}

test("recv BITCOIN_FUNDING_SPENT (remote commit)") { f =>
import f._
// bob publishes his commitment tx
Expand Down
21 changes: 1 addition & 20 deletions eclair-core/src/test/scala/fr/acinq/eclair/channel/states/h/ClosingStateSpec.scala
View file
Original file line number Diff line number Diff line change
Expand Up @@ -243,32 +243,13 @@ class ClosingStateSpec extends TestKitBaseClass with FixtureAnyFunSuiteLike with
bob2blockchain.expectMsgType[WatchConfirmed] // claim-main-delayed

// test starts here
bob.setState(stateData = bob.stateData.asInstanceOf[DATA_CLOSING].copy(waitingSince = System.currentTimeMillis.milliseconds.toSeconds - 15.days.toSeconds))
bob.setState(stateData = bob.stateData.asInstanceOf[DATA_CLOSING].copy(waitingSinceBlock = bob.underlyingActor.nodeParams.currentBlockHeight - Channel.FUNDING_TIMEOUT_FUNDEE - 1))
bob ! GetTxWithMetaResponse(fundingTx.txid, None, System.currentTimeMillis.milliseconds.toSeconds)
bob2alice.expectMsgType[Error]
bob2blockchain.expectNoMsg(200 millis)
assert(bob.stateName == CLOSED)
}

test("recv GetTxResponse (fundee, tx not found, timeout, blockchain lags)", Tag("funding_unconfirmed")) { f =>
import f._
val sender = TestProbe()
val fundingTx = alice.stateData.asInstanceOf[DATA_WAIT_FOR_FUNDING_CONFIRMED].fundingTx.get
bob ! CMD_FORCECLOSE(sender.ref)
awaitCond(bob.stateName == CLOSING)
bob2alice.expectMsgType[Error]
bob2blockchain.expectMsgType[PublishAsap]
bob2blockchain.expectMsgType[PublishAsap] // claim-main-delayed
bob2blockchain.expectMsgType[WatchConfirmed] // commitment
bob2blockchain.expectMsgType[WatchConfirmed] // claim-main-delayed

// test starts here
bob ! GetTxWithMetaResponse(fundingTx.txid, None, System.currentTimeMillis.milliseconds.toSeconds - 3.hours.toSeconds)
bob2alice.expectNoMsg(200 millis)
bob2blockchain.expectNoMsg(200 millis)
assert(bob.stateName == CLOSING) // the above expectNoMsg will make us wait, so this checks that we are still in CLOSING
}

test("recv CMD_ADD_HTLC") { f =>
import f._
mutualClose(alice, bob, alice2bob, bob2alice, alice2blockchain, bob2blockchain)
Expand Down
2 changes: 1 addition & 1 deletion eclair-core/src/test/scala/fr/acinq/eclair/wire/ChannelCodecsSpec.scala
View file
Original file line number Diff line number Diff line change
Expand Up @@ -335,7 +335,7 @@ class ChannelCodecsSpec extends AnyFunSuite {
// let's decode the old data (this will use the old codec that provides default values for new fields)
val data_new = stateDataCodec.decode(bin_old.toBitVector).require.value
assert(data_new.asInstanceOf[DATA_WAIT_FOR_FUNDING_CONFIRMED].fundingTx === None)
assert(System.currentTimeMillis.milliseconds.toSeconds - data_new.asInstanceOf[DATA_WAIT_FOR_FUNDING_CONFIRMED].waitingSince < 3600) // we just set this timestamp to current time
assert(System.currentTimeMillis.milliseconds.toSeconds - data_new.asInstanceOf[DATA_WAIT_FOR_FUNDING_CONFIRMED].waitingSinceBlock < 3600) // we just set this timestamp to current time
// and re-encode it with the new codec
val bin_new = ByteVector(stateDataCodec.encode(data_new).require.toByteVector.toArray)
// data should now be encoded under the new format
Expand Down

0 comments on commit 0835150

Please sign in to comment.