Fix BIP32 secret key derivation (child key < 32 bytes) in ergo-wallet

[greenhat]

Ref #1627

This PR introduces the fix for incorrect BIP32 derivation in ergo-wallet lib only (child key less than 32 bytes, see #1627). #1841 is the PR enabling fixed derivation in the node's wallet.

Corresponding PR in ergo-appkit that uses updated ergo-wallet - ergoplatform/ergo-appkit#139

Overview

The ergo node's behavior is unchanged, i.e., it uses pre-1627 derivation in all cases (init, restore wallets).
In ergo-wallet lib, use previous(incorrect) derivation when loading existing wallets (JSON key file loading) without EncryptedSecret.usePre1627KeyDerivation option set, fixed key derivation for new wallets and set EncryptedSecret.usePre1627KeyDerivation, and explicitly ask for key derivation type to be used for the wallets restored from mnemonic.

Implementation details

The added ExtendedSecretKey.usePre1627KeyDerivation boolean option determines the behaviour of ExtendedSecretKey.deriveChildSecretKey(...) secret key derivation. It is set via ExtendedSecretKey.deriveMasterKey(...) which is a sort of a "constructor" method for ExtendedSecretKey class.
For user wallets stored in JsonSecretStorage a new usePre1627KeyDerivation argument added to restore and underlying init methods. It's effectively set to false for new wallets and expected to be set for restored wallets. The option is stored as EncryptedSecret.usePre1627KeyDerivation optional property, which is set to true on loading if it's not found in the JSON file, thus making existing wallets use previous(incorrect) key derivation.

Breaking changes(for release notes):

ergo-wallet

• ExtendedSecretKey.deriveMasterKey has added usePre1627KeyDerivation parameter;
• JsonSecretStorage.init and restore has added usePre1627KeyDerivation parameter;
• ExtendedSecretKey serialization always uses fixed BIP32 derivation for the parsed key;

[MrStahlfelge]

Change works well for me with all mnemonics tested. (No code review)

[aslesarenko]

@kushti this is probably important fix, please assign the milestone.

[kushti]

How the implementation is tested? What are possible effects for node users, in regards with p2pk and mining script addresses?

[kushti]

@kushti this is probably important fix, please assign the milestone.

@aslesarenko How have you tested it?

[aslesarenko]

@aslesarenko How have you tested it?

@kushti I'm not working on the issue, I just asked you to assign a milestone.

[kushti]

@aslesarenko How have you tested it?

@kushti I'm not working on the issue, I just asked you to assign a milestone.

@aslesarenko I need to understand how it is tested first

[greenhat]

How the implementation is tested?

I wrote unit tests for ExtendedSecretKey with known test vectors for both derivations.
For wallet loading, I wrote tests for JsonSecretStorage to test the old wallets are loaded with old derivation by default and new wallets are honoring an explicit flag usePre1627KeyDerivation`.Besides that, I set this flag to use fixed derivation in all tests that require this flag.

What are possible effects for node users, in regards with p2pk and mining script addresses?

Existing wallets should be loaded with old derivation. New and restored wallets (JsonSecretStorage.init and restore) should explicitly specify what derivation should be used.
Regarding the mining script addresses, I did not test anything since I don't see how key derivation algo affects them.

I planned to look into splitting this PR into wallet lib PR and node PR as was discussed in #core recently.

[MrStahlfelge]

I have tested it in linked ergo-appkit build both manually and by unit tests, and used that build in ergoplatform/ergo-wallet-app#94 testing the derivations manually.