Skip to content
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.

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

Test preimage extraction from mempool #1387

Merged
merged 2 commits into from
Apr 27, 2020
Merged

Conversation

t-bast
Copy link
Member

@t-bast t-bast commented Apr 22, 2020

Instead of waiting for htlc-success txs to be confirmed, eclair also looks at mempool txs to detect preimages as soon as possible.

This has been the case for a very long time, but our integration tests didn't showcase this correctly.

@t-bast t-bast requested a review from sstone April 22, 2020 15:13
@pm47
Copy link
Member

pm47 commented Apr 22, 2020

We should test both cases: a tx that we haven't seen in the mempool may appear in a block.

@t-bast
Copy link
Member Author

t-bast commented Apr 22, 2020

We should test both cases: a tx that we haven't seen in the mempool may appear in a block.

Indeed. But I think we won't be able to test that in the IntegrationSpec.
I'll check what we already have in the non E2E tests, we probably already test that.

@t-bast
Copy link
Member Author

t-bast commented Apr 23, 2020

I added tests (and test helpers) to ZmqWatcher in 09a308d. The ClosingStateSpec already has tests for preimage extraction from WatchEventSpents so we should be covering most of it now.

sstone
sstone previously approved these changes Apr 24, 2020
t-bast added 2 commits April 27, 2020 11:34
Instead of waiting for htlc-success txs to be confirmed, eclair also looks
at mempool txs to detect preimages as soon as possible.

This has been the case for a very long time, but our integration tests
didn't showcase this correctly.
Refactor common helpers.
Add tests to ZmqWatcherSpec.
@codecov-io
Copy link

codecov-io commented Apr 27, 2020

Codecov Report

Merging #1387 into master will increase coverage by 0.01%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master    #1387      +/-   ##
==========================================
+ Coverage   86.68%   86.69%   +0.01%     
==========================================
  Files         123      123              
  Lines        9456     9456              
  Branches      388      388              
==========================================
+ Hits         8197     8198       +1     
+ Misses       1259     1258       -1     
Impacted Files Coverage Δ
...q/eclair/blockchain/electrum/ElectrumWatcher.scala 53.60% <0.00%> (-1.61%) ⬇️
...in/scala/fr/acinq/eclair/channel/Commitments.scala 90.96% <0.00%> (-0.34%) ⬇️
...clair/blockchain/electrum/ElectrumClientPool.scala 82.79% <0.00%> (+4.30%) ⬆️

@t-bast t-bast requested a review from sstone April 27, 2020 16:12
@t-bast t-bast merged commit 3809b02 into master Apr 27, 2020
@t-bast t-bast deleted the test-preimage-from-mempool branch April 27, 2020 16:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants