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

Segregate the node seed from the channel seed #1584

Merged
merged 13 commits into from Nov 5, 2020

Conversation

dariuskramer
Copy link
Contributor

Segregate the node seed from the channel seed

We want to have 2 separated seeds:

  • one for the node (to sign protocol messages, etc.)
  • one for the channels (to sign commit txs, etc.)

This PR creates 2 new files, nodeSeed.dat & channelSeed.dat, both stored in $datadir.

Migration from existing setup

This flow is executed twice, for nodeSeed.dat and channelSeed.dat:

+----------------------------+   Yes   +--------------------+
| Does `newSeed.dat` exists? +-------->+ Use that seed file |
+------------+---------------+         +---+----------------+
             |                             ^
             | No                          |
             v                             |
+------------+---------------+   Yes   +---+----------------------+
| Does `seed.dat` exists?    +-------->+ Copy it to `newSeed.dat` |
+------------+---------------+         +---+----------------------+
             |                             ^
             | No                          |
             v                             |
+------------+-------------+               |
| Generate a random seed   |               |
| into a file              +---------------+
+--------------------------+

The old seed.dat is not removed after the migration.

Donovan Jean added 2 commits October 30, 2020 15:08
- change getSeed signature
- change LocalKeyManager signature
- implement getSeeds to return both nodeSeed and channelSeed
- use separate seeds for node and channels
- update README
- create NodeKeyManager & ChannelKeyManager
- add migration tests

Signed-off-by: Donovan Jean <donovan@acinq.fr>
Signed-off-by: Donovan Jean <donovan@acinq.fr>
@sstone sstone requested review from t-bast, pm47 and sstone October 30, 2020 16:24
@codecov-io
Copy link

codecov-io commented Oct 30, 2020

Codecov Report

Merging #1584 into master will increase coverage by 0.03%.
The diff coverage is 92.47%.

@@            Coverage Diff             @@
##           master    #1584      +/-   ##
==========================================
+ Coverage   87.21%   87.25%   +0.03%     
==========================================
  Files         140      141       +1     
  Lines       10929    10951      +22     
  Branches      460      449      -11     
==========================================
+ Hits         9532     9555      +23     
+ Misses       1397     1396       -1     
Impacted Files Coverage Δ
...air/crypto/keymanager/LocalChannelKeyManager.scala 85.71% <68.75%> (ø)
...ir-core/src/main/scala/fr/acinq/eclair/Setup.scala 70.96% <80.00%> (+0.15%) ⬆️
...re/src/main/scala/fr/acinq/eclair/NodeParams.scala 91.12% <96.00%> (+0.86%) ⬆️
...r-core/src/main/scala/fr/acinq/eclair/Eclair.scala 55.14% <100.00%> (ø)
...c/main/scala/fr/acinq/eclair/channel/Channel.scala 85.89% <100.00%> (-0.08%) ⬇️
...in/scala/fr/acinq/eclair/channel/Commitments.scala 93.91% <100.00%> (ø)
...c/main/scala/fr/acinq/eclair/channel/Helpers.scala 96.27% <100.00%> (+0.06%) ⬆️
...q/eclair/crypto/keymanager/ChannelKeyManager.scala 87.50% <100.00%> (ø)
...eclair/crypto/keymanager/LocalNodeKeyManager.scala 100.00% <100.00%> (ø)
...-core/src/main/scala/fr/acinq/eclair/io/Peer.scala 88.58% <100.00%> (ø)
... and 8 more

sstone
sstone previously approved these changes Nov 2, 2020
Donovan Jean added 6 commits November 3, 2020 09:49
Signed-off-by: Donovan Jean <donovan@acinq.fr>
Signed-off-by: Donovan Jean <donovan@acinq.fr>
Signed-off-by: Donovan Jean <donovan@acinq.fr>
…lKeyManagerSpec

Signed-off-by: Donovan Jean <donovan@acinq.fr>
Signed-off-by: Donovan Jean <donovan@acinq.fr>
Signed-off-by: Donovan Jean <donovan@acinq.fr>
@dariuskramer dariuskramer force-pushed the keymanager/feature/segregated-seeds branch 3 times, most recently from 4411ebc to 25d3279 Compare November 3, 2020 12:56
Donovan Jean added 2 commits November 3, 2020 13:57
Signed-off-by: Donovan Jean <donovan@acinq.fr>
Signed-off-by: Donovan Jean <donovan@acinq.fr>
Copy link
Member

@pm47 pm47 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would move all the *KeyManager files to a sub-package of crypto.

Otherwise seems ok to me.

Copy link
Member

@t-bast t-bast left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, tiny nit

@dariuskramer dariuskramer force-pushed the keymanager/feature/segregated-seeds branch from 11cdaec to 5f723d3 Compare November 3, 2020 16:27
dariuskramer and others added 3 commits November 3, 2020 17:28
Co-authored-by: Pierre-Marie Padiou <pm47@users.noreply.github.com>
Signed-off-by: Donovan Jean <donovan@acinq.fr>
Signed-off-by: Donovan Jean <donovan@acinq.fr>
Copy link
Member

@t-bast t-bast left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@dariuskramer dariuskramer merged commit f32e75b into master Nov 5, 2020
@dariuskramer dariuskramer deleted the keymanager/feature/segregated-seeds branch November 5, 2020 10:34
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.

None yet

5 participants