Handle mutual close in state DATA_WAIT_FOR_FUNDING_CONFIRMED
#358
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…T_FOR_FUNDING_CONFIRMED` state
|
Still haven't figured out how to properly add a test for this but at least it works fine in testnet. |
pm47
changed the title
DATA_WAIT_FOR_FUNDING_CONFIRMED
Private channels need specific handling because we can't rely on `WatchEventSpentBasic` event for tracking whether they are still up.
Implementation should guarantee that in case of a `BITCOIN_FUNDING_PUBLISH_FAILED` event, the funding tx will *never* be published (see `commit` method in trait `EclairWallet`). With that in mind, there is no need for a `ERR_FUNDING_PUBLISH_FAILED` state, instead we can just permanently close the channel. Note that the user will receive an error "couldn't publish funding tx".
* simplified blacklisting of nodes in case of payment failures and fixed `transformForUser` * Add a test for PaymentLifeCycle.transformForUser()
And specify that we will soon drop support for older (< 0.16) versions of Bitcoin Core
* added copyright notice to all files * updated date in LICENSE
* set a configurable `maxPaymentFee` as safety Sending a payment will not be attempted if the cheapest route found is more expensive than this value. Default value is 3%. This is meant as a protection mechanism, to protect against an intermediate well-connected node to set outrageous fees. * reduced default `fee-base-msat` to 1 sat * (gui) using max fee from node params when sending payment from ui
* added a serializer for `UInt64` class * added a serializer for `OutPoint` class * use `txid` instead of `txhash` when serializing `OutPoint` * added a simple serializer for class `InputInfo`
* remove useless hex methods for bitcoin tx
* re-enabled test
* `BinaryData("") -> `BinaryData.empty`
* removed invalid comment
add electrum router, which manages a set of client primary client is the one on the longest chain, when a secondary client has a chain that is longer than master + 2 it becomes the new master. this will allow us to move away from "dead" electrum servers that are not up to date with the blockchain. electrum client: add 'get header' method electrum: maintain a chain of headers for each client and switch to the one with the longest chain electrum: handle case when received header connects to our first header it can happen when we connect to a server and ask for the last N headers electrum: automatic reconnection when we lose connection to our master client, we consider that we are disconnected. when we lose connection to a secondary client, we try to find an address that is not used and connect to it. If there is no such address we wait a bit and try to reconnect to the failing client electrum: check that incoming header proof of work is valid electrum router: migrate to Akka's FSM electrum: fix match error for `ready` message add missing copyright headers move Blockchain to electrum package electrum: simplify router electrum client: fix handling of dead subscribers electrum: move blockchain validation into electrum client client will now send `ready` notifications and subscriptions messages only when it has a valid and long enough blockchain pass execution context as argument reworked `ElectrumRouter` following 3dc1c2b61c82debeaed8d0d92238c0b5ee5c49c8 renamed `ElectrumRouter` -> `ElectrumMultiClient` reformat `var` -> `val` electrum: rename multiclient -> pool + cleanup
* Add api call to check invoice/paymentRequest and return details in json * Adding in API call to allow user to check route before making send call * Added a serializer for route response * Update README to include new checkinvoice and findroute API calls
This regression caused in 438d8e3 is what caused flaky tests during the past few days. Calling `sender()` inside a `Props()` leads to undefined behavior.
* switch to bitcoin core 0.16.0 * check that we are using bitcoin core 0.16 and check that there are no p2pkh UTXOs * update readme [ci skip]
Added a serializer for Throwable (local failures), and a serializer for FailureMessage (remote failures)
* Update funding tx id value upon receiving ChannelStateChanged to WAIT_FOR_FUNIDNG_CONFIRMED Currently the only time the "Funding tx id" text in GUI is updated is upon receiving the ChannelRestored event, which means that if the GUI is not restarted the value keeps showing "N/A".
Previously we were only stealing the remote's main output when they publish a revoked commit, and were relying on a sufficiently high `channel_reserve` do deincentivize cheating. In order to also steal the htlc outputs, we need to handle both cases: - they only publish their revoked commit tx => we claim the htlc outputs directly from the commit tx - they publish their revoked commit tx, and their 2nd-stage HTLCSuccessTx and HtlcTimeout txes => we claim the output of these htlcs tx To do that, we need to be able to reconstruct htlc scripts (`htlcOffered` and `htlcReceived`), therefore we need to store `paymentHash` and `cltvExpiry` for each htlc we sign. Note that this won't be needed in the future when we have MAST. * store `paymentHash` and `cltvExpiry` for each signed htlc * spend htlc outputs with penalty txs * added full integration tests on revoked scenario
…utual-close # Conflicts: # eclair-core/src/main/scala/fr/acinq/eclair/channel/Channel.scala # eclair-core/src/main/scala/fr/acinq/eclair/channel/ChannelTypes.scala
|
hope I rebased it right |
Hmmm, I see very weird things from my end! 218 files changed, a review by an unknown user, and unrelated comments on some files lol |
|
I've issued the following commands: and then did some editing on top of that. Still learning git! |
|
I think the actual result should be like this branch: https://github.com/ACINQ/eclair/tree/pm-mutual-close-funding-confirmed Here is what I did: git checkout master
git branch pm-mutual-close-funding-confirmed
git checkout pm-mutual-close-funding-confirmed
git cherry-pick 55314e8ed15230a9e6f13d4aad0a3475f5b42d5855314e8 is your rebased commit |
|
This is related to #300. To be honest I didn't think it was worth it until this was supported other implementations, and it seems |
|
Yes, LND does support it. It's mostly useful to prevent negative user experience when they decide to close it anyway right now (maybe because fees have risen and funding was hanging for days). |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
…T_FOR_FUNDING_CONFIRMED` state