Deterministic channel keypath #1026

araspitzu · 2019-06-04T16:04:00Z

This PR introduces deterministic keypath construction for the derivation of the keys used in the channel operations, the construction is made in such a way that we guarantee we never reuse the same keypath in order to prevent reusing keys across channels.The schema can be summarized as follow:

Funder scenario:

keyPath = m/47'/2'/SHA256(funding_tx.input[0].outpoint)/0

Fundee scenario:

fundingPubkey.keyPath = m/47'/2'/SHA256(blockchain_height || counter)/2
keyPath = m/47'/2'/SHA256(funding_tx.output[channel_output_index].scriptPubkey)/1

The fundee scenario involves using 2 different keypaths, one to derive the funding pubkey and another to derive all the other points required to construct accept_channel message. The usage of 2 different keypaths is required to compute the scriptPubkey of the funding transaction which is later used to construct the second keypath which is then used to derive all the other points for the channel.
As side effect of this PR all the static channel data, as in the data derived from the channel points exchanged in the open/accept messages, can now be derived only from the seed.

Credits: @armelinw

…ding input

…nt with test vectors

…HANNEL

sstone · 2019-06-12T14:49:59Z

eclair-core/src/main/scala/fr/acinq/eclair/wire/ChannelCodecs.scala

+      (version match {
+        case CommitmentV0 => ("channelPath" | keyPathCodec.xmap[Either[KeyPath, KeyPathFundee]](Left(_), {
+          case Left(kp) => kp
+          case Right(_) => throw new IllegalArgumentException(s"Found unexpected value for version=$version")


This will not work for "fundee" channels, but it should be possible to use the V0 key path we read from the database

Yes, this has been fixed in 847eb06

sstone · 2019-06-12T14:51:37Z

eclair-core/src/main/scala/fr/acinq/eclair/io/Peer.scala

@@ -588,7 +588,7 @@ object Peer {
  def makeChannelParams(nodeParams: NodeParams, defaultFinalScriptPubKey: ByteVector, isFunder: Boolean, fundingSatoshis: Long, channelKeyPath: DeterministicWallet.KeyPath): LocalParams = {


If unused it should be removed ?

Done in 5ded959

sstone · 2019-06-12T14:52:32Z

eclair-core/src/main/scala/fr/acinq/eclair/db/sqlite/SqliteChannelsDb.scala

+    }
+  }
+
+


It is possible to infer the counter value from the data we read from the database (check all channels fro a given peer, then get their block heights from the short channel id) ?

The alternative is to have an atomic counter stored in volatile memory, however while this is a more elegant solution it opens up to new attacker scenarios.

sstone · 2019-06-12T14:53:00Z

eclair-core/src/main/scala/fr/acinq/eclair/crypto/KeyManager.scala

+  def deterministicCommitmentSecret(localParams: LocalParams, index: Long): Crypto.Scalar
+
+  def deterministicCommitmentPoint(localParams: LocalParams, index: Long): Crypto.Point
+


Can we keep the old names instead ?

Yes these changes have been reverted

# Conflicts: # eclair-core/src/main/scala/fr/acinq/eclair/channel/Channel.scala

# Conflicts: # eclair-core/src/main/scala/fr/acinq/eclair/channel/Channel.scala # eclair-core/src/main/scala/fr/acinq/eclair/channel/ChannelTypes.scala # eclair-core/src/main/scala/fr/acinq/eclair/channel/Commitments.scala # eclair-core/src/main/scala/fr/acinq/eclair/channel/Helpers.scala # eclair-core/src/main/scala/fr/acinq/eclair/crypto/LocalKeyManager.scala # eclair-core/src/main/scala/fr/acinq/eclair/wire/ChannelCodecs.scala # eclair-core/src/test/scala/fr/acinq/eclair/TestConstants.scala # eclair-core/src/test/scala/fr/acinq/eclair/crypto/LocalKeyManagerSpec.scala

…date test vectors

…k while wait_for_funding_signed_internal

…FIRMED -> WAIT_FOR_FUNDING_LOCKED in funder scenario

…KED -> NORMAL in funder scenario

# Conflicts: # eclair-core/src/main/scala/fr/acinq/eclair/channel/Channel.scala # eclair-core/src/main/scala/fr/acinq/eclair/wire/ChannelCodecs.scala # eclair-core/src/test/scala/fr/acinq/eclair/channel/states/c/WaitForFundingConfirmedStateSpec.scala

codecov-io · 2019-07-04T10:42:59Z

Codecov Report

Merging #1026 into master will increase coverage by 0.1%.
The diff coverage is 90.9%.

@@            Coverage Diff            @@
##           master    #1026     +/-   ##
=========================================
+ Coverage   82.47%   82.58%   +0.1%     
=========================================
  Files          99       99             
  Lines        7594     7682     +88     
  Branches      293      292      -1     
=========================================
+ Hits         6263     6344     +81     
- Misses       1331     1338      +7

Impacted Files	Coverage Δ
...air/blockchain/electrum/ElectrumEclairWallet.scala	`0% <0%> (ø)`	⬆️
...a/fr/acinq/eclair/db/sqlite/SqliteChannelsDb.scala	`96.92% <100%> (+1.08%)`	⬆️
...n/scala/fr/acinq/eclair/channel/ChannelTypes.scala	`100% <100%> (ø)`	⬆️
...scala/fr/acinq/eclair/crypto/LocalKeyManager.scala	`91.3% <100%> (+1.83%)`	⬆️
...ain/scala/fr/acinq/eclair/wire/ChannelCodecs.scala	`100% <100%> (ø)`	⬆️
...c/main/scala/fr/acinq/eclair/channel/Helpers.scala	`94.44% <100%> (+0.01%)`	⬆️
...nq/eclair/blockchain/electrum/ElectrumWallet.scala	`80.66% <100%> (+0.14%)`	⬆️
...-core/src/main/scala/fr/acinq/eclair/io/Peer.scala	`73.94% <100%> (-0.99%)`	⬇️
...a/fr/acinq/eclair/wire/LightningMessageTypes.scala	`100% <100%> (ø)`	⬆️
...in/scala/fr/acinq/eclair/channel/Commitments.scala	`86.92% <100%> (ø)`	⬆️
... and 7 more

araspitzu · 2019-07-04T11:23:18Z

Summary of the changes

Key path generation

Eclair uses BIP32 key paths to derive all the various keys needed for channel operation, instead of generating a new random keypath when negotiating a channel we now use an external source of entropy to feed randomness in the path construction. The entropy is taken from the funding input's outpoint and in the case we're fundee we use a combination of block height and an incrementing counter to ensure we never reuse keys, the full construction is highlighted below:

Funder scenario:

keyPath = m/47'/2'/SHA256(funding_tx.input[0].outpoint)/0'

Fundee scenario:

fundingPubkey.keyPath = m/47'/2'/SHA256(blockchain_height || counter)/2'
keyPath = m/47'/2'/SHA256(funding_tx.output[channel_output_index].scriptPubkey)/1'

Channel creation changes

Funder scenario : in order to send an open_channel we need to compute the basepoints for the channel operation and indeed we need the key path to do so. This means that before starting the negotiation of the channel opening we must ask the wallet to provide an input for the channel funding, we will use the input outpoint to construct the channel key path. Such particular flow modified the Channel FSM to have some new intermediate states, namely where we ask the wallet to create a funding transaction WAIT_FOR_FUNDING_INTERNAL_CREATED and where we ask the wallet to sign it WAIT_FOR_FUNDING_SIGNED_INTERNAL. Note that after we ask the wallet to make a funding transaction we strip the signatures from it and we later query the wallet again to add new signatures, the wallet will sign the updated transaction with the script containing the funding pubkey from the remote peer (we have the remote funding pubkey only after we receive accept_channel).
Fundee scenario: this scenario uses 2 different keypaths to operate the channel, one for the funding pubkey and one for all the other points. This is necessary because we want to use the scriptPubkey from the funding transaction to construct the keypath but to do so we need to derive out funding pubkey first, in order to do that we use a combination of block height and a counter. This scenario doesn't alter the channel opening flow.

Reference: funder construction and fundee construction

Codecs and Commitments upgrades

The usage of 2 different kinds of keypaths (one for funder scenario, two for fundee) forced to upgrade the LocalParams class to store an Either[KeyPath, KeyPathFundee], we also need to upgrade the codecs to store these new objects and we must do it in a backward compatible manner.
The codec has been modified to support commitment versioning and depending on the version we'll fetch the correct key paths.

Database layer upgrades

There is no db migration needed here but we add a new table to channel db, block_key_counter table. This is used to emulate an atomic auto incrementing counter used for the fundee key path construction, the alternative implementation using the AtomicLong has been ditched because of security concerns.

Testing

The PR has been manual tested against eclair master, there are also new codec tests and backward compatibility test for fundee scenario in DATA_NORMAL: here. To ensure previous version with yet-to-open channels can safely upgrade there are some test where we assert we can do FSM transition like "WAIT_FOR_FUNDING_CONFIRMED -> WAIT_FOR_FUNDING_LOCKED", test here and here.

…_channel`

# Conflicts: # eclair-core/src/main/scala/fr/acinq/eclair/wire/ChannelCodecs.scala # eclair-core/src/test/scala/fr/acinq/eclair/db/ChannelStateSpec.scala # eclair-core/src/test/scala/fr/acinq/eclair/wire/ChannelCodecsSpec.scala

# Conflicts: # eclair-core/src/main/scala/fr/acinq/eclair/channel/ChannelTypes.scala # eclair-core/src/test/scala/fr/acinq/eclair/wire/ChannelCodecsSpec.scala

araspitzu · 2019-07-22T13:28:48Z

Replaced by #1086

araspitzu added 14 commits May 21, 2019 14:41

Use deterministic key channel path for LOCAL -> REMOTE channel opening

6451bb4

Factor out function to compute the deterministic keypath from the fun…

6c3c306

…ding input

WIP funder channel opening flow

9436c78

WIP funder channel opening flow

3fd125e

Remove unnecessary logs

9992581

Use different states/events to handle creation/signing of the funding tx

2235c6f

Fix test for internal funding created state

0810f12

Revert changes to use old derivation path for REMOTE -> LOCAL

54a0e59

Use bech32 test address

f77e1f8

Use deterministic derivation

132035b

Bump commitment version for scodecs

3980fa0

Sort scodecs properly, update test

2a329d6

Use new derivation scheme for fundee scenario

3b25ea5

Add test vectors for key derivation

b041d94

araspitzu force-pushed the deterministic_channel_keypath branch from 3d60a90 to b041d94 Compare June 6, 2019 14:33

araspitzu added 4 commits June 7, 2019 15:27

Store counter on channelDb

596bc0c

Update test vectors

004a2a6

Do not use hardened index for shaSeed and test the shaSeed public poi…

92ee96c

…nt with test vectors

Add test for fundee/funder keypath choice

64f4b33

viaj3ro mentioned this pull request Jun 8, 2019

Feature request: better (manual) fee control #1028

Closed

Add test to electrum wallet for SignTransactionResponse

3da0894

ACINQ deleted a comment from codecov-io Jun 11, 2019

araspitzu added 3 commits June 11, 2019 17:50

Use dummy script in MakeFundingTx

a3db46f

Add test for WAIT_FOR_FUNDING_CREATED_INTERNAL

169c1b3

Remove empty lines

a4f32f9

ACINQ deleted a comment from codecov-io Jun 12, 2019

Rollback funding tx if the flow is aborted in state WAIT_FOR_ACCEPT_C…

726c5d9

…HANNEL

sstone requested changes Jun 12, 2019

View reviewed changes

araspitzu added 2 commits June 12, 2019 17:22

Remove unused functions

5ded959

Fix race condition in flaky test WaitForFundingCreatedInternalStateSpec

9e4db87

araspitzu marked this pull request as ready for review June 14, 2019 15:21

araspitzu added 6 commits June 14, 2019 17:28

Merge branch 'master' into deterministic_channel_keypath

c1a976d

# Conflicts: # eclair-core/src/main/scala/fr/acinq/eclair/channel/Channel.scala

Finish merging master

f78f2f3

Do not alter root derivation paths from previous eclair's version, up…

0d5d15d

…date test vectors

Use hardened BIP32 indexes also for the the internal keypaths

602e73b

Remove unnecessary println, merge master, improve handling of rollbac…

922072f

…k while wait_for_funding_signed_internal

araspitzu force-pushed the deterministic_channel_keypath branch from 5183bc9 to 922072f Compare June 18, 2019 09:02

araspitzu added 6 commits June 18, 2019 11:29

Wait for correct state in test

79a8284

Use a non returing call to avoid quick transition during test

a693df5

Add counter table to migration v1 -> v2

4b24e03

Test backward compatibility for state transition WAIT_FOR_FUNDING_CON…

f95bd00

…FIRMED -> WAIT_FOR_FUNDING_LOCKED in funder scenario

Test backward compatibility for state transition WAIT_FOR_FUNDING_LOC…

08395e8

…KED -> NORMAL in funder scenario

Merge branch 'master' into deterministic_channel_keypath

8a3cd4a

ACINQ deleted a comment from codecov-io Jun 25, 2019

araspitzu added 2 commits July 4, 2019 11:01

Finish merging master

bdcd0ab

araspitzu force-pushed the deterministic_channel_keypath branch from c12c706 to bdcd0ab Compare July 4, 2019 10:21

araspitzu added 2 commits July 4, 2019 13:24

Renaming

259748b

Move the publishing of ChannelCreated close to the sending of `open…

5c1698e

…_channel`

pm47 mentioned this pull request Jul 12, 2019

Replace traits by bitfield for ChannelVersion #1073

Merged

araspitzu added 4 commits July 15, 2019 09:03

Merge branch 'master' into deterministic_channel_keypath

49355a1

# Conflicts: # eclair-core/src/main/scala/fr/acinq/eclair/wire/ChannelCodecs.scala # eclair-core/src/test/scala/fr/acinq/eclair/db/ChannelStateSpec.scala # eclair-core/src/test/scala/fr/acinq/eclair/wire/ChannelCodecsSpec.scala

Merge branch 'master' into deterministic_channel_keypath

f26791e

# Conflicts: # eclair-core/src/main/scala/fr/acinq/eclair/channel/ChannelTypes.scala # eclair-core/src/test/scala/fr/acinq/eclair/wire/ChannelCodecsSpec.scala

Use transitions callbacks to avoid race condition in test

6136adf

Do not send message to remote (bob) during fixture setup

2b00b14

araspitzu closed this Jul 22, 2019

araspitzu deleted the deterministic_channel_keypath branch July 22, 2019 13:29

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Deterministic channel keypath #1026

Deterministic channel keypath #1026

araspitzu commented Jun 4, 2019

sstone Jun 12, 2019

araspitzu Jun 14, 2019

sstone Jun 12, 2019

araspitzu Jun 14, 2019

sstone Jun 12, 2019

araspitzu Jun 14, 2019

sstone Jun 12, 2019

araspitzu Jun 14, 2019

codecov-io commented Jul 4, 2019 •

edited

Loading

araspitzu commented Jul 4, 2019 •

edited

Loading

araspitzu commented Jul 22, 2019

		@@ -588,7 +588,7 @@ object Peer {
		def makeChannelParams(nodeParams: NodeParams, defaultFinalScriptPubKey: ByteVector, isFunder: Boolean, fundingSatoshis: Long, channelKeyPath: DeterministicWallet.KeyPath): LocalParams = {

		def deterministicCommitmentSecret(localParams: LocalParams, index: Long): Crypto.Scalar

		def deterministicCommitmentPoint(localParams: LocalParams, index: Long): Crypto.Point

Deterministic channel keypath #1026

Deterministic channel keypath #1026

Conversation

araspitzu commented Jun 4, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

codecov-io commented Jul 4, 2019 • edited Loading

Codecov Report

araspitzu commented Jul 4, 2019 • edited Loading

Summary of the changes

Key path generation

Channel creation changes

Codecs and Commitments upgrades

Database layer upgrades

Testing

araspitzu commented Jul 22, 2019

codecov-io commented Jul 4, 2019 •

edited

Loading

araspitzu commented Jul 4, 2019 •

edited

Loading