Updated hdkey dependency from v1.1.1 to v2.0.1

[holgerd77]

$ npm ls secp256k1
[email protected] PATH/ethereumjs-wallet
├─┬ [email protected]
│ └── [email protected]
└─┬ [email protected]
  └── [email protected]  deduped

Yay. 😄 🎉

[ryanio]

hm that's not good, 3 tests failing (ci link):

  53 passing (3m)
  3 failing

  1) .derivePath()
       should work with m:

      AssertionError [ERR_ASSERTION]: Input A expected to strictly equal input B:
+ expected - actual

- 'xprv9s21ZrQH143K4KqQx9Zrf1eN8EaPQVFxM2Ast8mdHn7GKiDWzNEyNdduJiQLetNUY7PDEYqnGLkhdzLLUSsrnzEMQBrJoTXMsbyAfkNJveM'
+ 'xprv9s21ZrQH143K4KqQx9Zrf1eN8EaPQVFxM2Ast8mdHn7GKiDWzNEyNdduJhWXToy8MpkGcKjxeFWd8oBSvsz4PCYamxR7TX49pSpp3bmHVAY'
      + expected - actual

      -xprv9s21ZrQH143K4KqQx9Zrf1eN8EaPQVFxM2Ast8mdHn7GKiDWzNEyNdduJiQLetNUY7PDEYqnGLkhdzLLUSsrnzEMQBrJoTXMsbyAfkNJveM
      +xprv9s21ZrQH143K4KqQx9Zrf1eN8EaPQVFxM2Ast8mdHn7GKiDWzNEyNdduJhWXToy8MpkGcKjxeFWd8oBSvsz4PCYamxR7TX49pSpp3bmHVAY
      
      at Context.<anonymous> (test/hdkey.spec.ts:78:12)

  2) .derivePath()
       should work with m/44'/0'/0/1:

      AssertionError [ERR_ASSERTION]: Input A expected to strictly equal input B:
+ expected - actual

- 'xprv9zr2Dxaspc1uXaXjwifQSoiBysxq1Bf987enSQrJgAEYWbVLp9Rahgn8Yd2nXqdhjhNJqKE6D8RiqBpsSvpezEyMZg7ht7ByCmSmmTbsiUf'
+ 'xprvA1ErCzsuXhpB8iDTsbmgpkA2P8ggu97hMZbAXTZCdGYeaUrDhyR8fEw47BNEgLExsWCVzFYuGyeDZJLiFJ9kwBzGojQ6NB718tjVJrVBSrG'
      + expected - actual

      -xprv9zr2Dxaspc1uXaXjwifQSoiBysxq1Bf987enSQrJgAEYWbVLp9Rahgn8Yd2nXqdhjhNJqKE6D8RiqBpsSvpezEyMZg7ht7ByCmSmmTbsiUf
      +xprvA1ErCzsuXhpB8iDTsbmgpkA2P8ggu97hMZbAXTZCdGYeaUrDhyR8fEw47BNEgLExsWCVzFYuGyeDZJLiFJ9kwBzGojQ6NB718tjVJrVBSrG
      
      at Context.<anonymous> (test/hdkey.spec.ts:85:12)

  3) .getWallet()
       should work:

      AssertionError [ERR_ASSERTION]: Input A expected to strictly equal input B:
+ expected - actual

- '0xc97a4a9ed38c8da3fde103e96465bc2542a5e40367df1aefd65b7d84fb0c70e9'
+ '0x26cc9417b89cd77c4acdbe2e3cd286070a015d8e380f9cd1244ae103b7d89d81'
      + expected - actual

      -0xc97a4a9ed38c8da3fde103e96465bc2542a5e40367df1aefd65b7d84fb0c70e9
      +0x26cc9417b89cd77c4acdbe2e3cd286070a015d8e380f9cd1244ae103b7d89d81
      
      at Context.<anonymous> (test/hdkey.spec.ts:94:12)

[alcuadrado]

I had very similar errors lately. In my case, the problem was that in [email protected] some methods modify the buffers you pass them, while that doesn't happen in 3.x.

[alcuadrado]

I fixed it upstream.

[coveralls]

Coverage remained the same at 86.601% when pulling 5f504fd on update-hdkey-dependency into 23c7106 on master.

[alcuadrado]

The upstream fix has already been published. We should just change the minimum version in the package.json.

[ryanio]

great, passing! ✅

[alcuadrado]

LGTM

[ryanio]

great, I'll merge in

[alcuadrado]

I didn't know that GH prevented merges until reviewers that requested changes approve a PR, sorry about that 😅

[ryanio]

haha no worries, yeah i think it is a repo setting

[holgerd77]

@alcuadrado @ryanio Great guys that you figured this out here, Patricio special thanks for the upstream fix, think these things are always the ones where the ecosystem benefits most at the end! 👍

this we are now really closing in on the v1.0.0 release. 😄

[holgerd77]

I had very similar errors lately. In my case, the problem was that in [email protected] some methods modify the buffers you pass them, while that doesn't happen in 3.x.

I will circle in @fanatid here since I wonder if this should this really be the case? And woudn't the root-addressing fix rather be in the secp256k1 library not modifying the buffer values?

[fanatid]

Hi. Which methods modify buffers?

[holgerd77]

Likely @alcuadrado can give an answer (or a hint) here, you'll have to wait for Argentinian time zone morning though! 😄

[fanatid]

ok, just checked code/readme, probably I'll not need wait morning in Argentinian time zone :D

This 4 functions do in-place operations:

• privateKeyNegate
• privateKeyTweakAdd
• privateKeyTweakMul
• signatureNormalize

I changed these functions for 4.x because bitcoin-core/secp256k1 do in-place. API.md have note about this. Such functions can be filtered from list by searching output and functions which will not match but change data included in list above.

Should I make API.md more clear about this? Should I put some dangerous note to README.md? (Not sure how much people read API.md)

[holgerd77]

Thanks for the quick check 😄 , I'll nevertheless let an answer open to @alcuadrado since he normally has some deeper rooted opinion than me on questions like these.

[alcuadrado]

I'd modify this part of API.md "In place operations. Some functions doing in place operations" to include a list of all of them.

In general, I'd try to avoid changing the semantics of a function without changing its name and/or type, as people tend to just bump version numbers without much care.

[alcuadrado]

In general, I'd try to avoid changing the semantics of a function without changing its name and/or type, as people tend to just bump version numbers without much care.

This can still be done by deprecating the functions with a warning, and expose them with another name. Something like:

import * as secp256k1 from  "secp256k1";

export function privateKeyTweakAdd(privateKey: Uint8Array, tweak: Uint8Array): Uint8Array {
  console.warn("privateKeyTweakAdd is deprecated. Please use privateKeyTweakAddInPlace");
  return privateKeyTweakAddInPlace(privateKey, tweak);
}

export function privateKeyTweakAddInPlace(privateKey: Uint8Array, tweak: Uint8Array): Uint8Array {
  return secp256k1.privateKeyTweakAddInPlace(privateKey, tweak);
}

[holgerd77]

Yes, I would also agree that it is a good suggestion to deprecate the existing functions, otherwise this can easily lead to serious problems if people do their usual upgrading and don't have a close-enough look.

[holgerd77]

Eventually I would even be a bit more explicit on the warning message, like:

import * as secp256k1 from  "secp256k1";

export function privateKeyTweakAdd(privateKey: Uint8Array, tweak: Uint8Array): Uint8Array {
  console.warn("privateKeyTweakAdd() is deprecated. Please use privateKeyTweakAddInPlace().");
  console.warn("Attention: both versions of the function changed to do operations in-place.");
  return privateKeyTweakAddInPlace(privateKey, tweak);
}

export function privateKeyTweakAddInPlace(privateKey: Uint8Array, tweak: Uint8Array): Uint8Array {
  return secp256k1.privateKeyTweakAddInPlace(privateKey, tweak);
}