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

new Wallet(privateKey) throws "invalid hexlify value" when privateKey is not 0x prefixed #1166

Closed
matYang opened this issue Nov 20, 2020 · 30 comments
Labels
enhancement New feature or improvement. fixed/complete This Bug is fixed or Enhancement is complete and published.

Comments

@matYang
Copy link

matYang commented Nov 20, 2020

While upgrading to Ethers V5, we ran into

Error: invalid hexlify value (argument="value", value="[not copied here]", code=INVALID_ARGUMENT, version=bytes/5.0.6)
I20201120-11:39:47.012(-5)?     at Logger.makeError (/Users/matthewyang/Desktop/blockchain/tokenfunder/tokenfunderapp/node_modules/@ethersproject/logger/src.ts/index.ts:205:28)
I20201120-11:39:47.013(-5)?     at Logger.throwError (/Users/matthewyang/Desktop/blockchain/tokenfunder/tokenfunderapp/node_modules/@ethersproject/logger/src.ts/index.ts:217:20)
I20201120-11:39:47.013(-5)?     at Logger.throwArgumentError (/Users/matthewyang/Desktop/blockchain/tokenfunder/tokenfunderapp/node_modules/@ethersproject/logger/src.ts/index.ts:221:21)
I20201120-11:39:47.013(-5)?     at Object.hexlify (/Users/matthewyang/Desktop/blockchain/tokenfunder/tokenfunderapp/node_modules/@ethersproject/bytes/src.ts/index.ts:242:19)
I20201120-11:39:47.013(-5)?     at new SigningKey (/Users/matthewyang/Desktop/blockchain/tokenfunder/tokenfunderapp/node_modules/@ethersproject/signing-key/src.ts/index.ts:35:44)
I20201120-11:39:47.032(-5)?     at new Wallet (/Users/matthewyang/Desktop/blockchain/tokenfunder/tokenfunderapp/node_modules/@ethersproject/wallet/src.ts/index.ts:81:36)

This issue was brought up in
https://www.gitmemory.com/issue/ethers-io/ethers.js/994/674504533

I assume this is not an intended feature, as all other tools accept non 0x prefixed private keys, and prefixing 0x to the private key causes issues with bunch of other libraries, so it's a change specific for Ethers V5

Regardless of it's intended or not, this should have been included in the breaking change log https://docs.ethers.io/v5/migration/ethers-v4/#migration-v4--contracts

@ricmoo ricmoo added the discussion Questions, feedback and general information. label Nov 20, 2020
@ricmoo
Copy link
Member

ricmoo commented Nov 20, 2020

Heya!

Ethers 100% requires all hex strings to be 0x-prefixed and this is intentional.

This was also required in v4 (I just installed v4 to verify). So, it is not a breaking change since v4.

/tmp/test-0x> npm install ethers@4.0.0
/tmp/test-0x> node
> e = require("ethers")
> new e.Wallet("123456789012345678901234567890123456789012345678901234567890123")
Thrown:
{ Error: hex string must have 0x prefix (arg="value", value="123456789012345678901234567890123456789012345678901234567890123")
    at Object.throwError (/private/tmp/test-0x/node_modules/ethers/errors.js:74:17)
    at Object.arrayify (/private/tmp/test-0x/node_modules/ethers/utils/bytes.js:56:20)
    at new SigningKey (/private/tmp/test-0x/node_modules/ethers/utils/signing-key.js:35:39)
    at new Wallet (/private/tmp/test-0x/node_modules/ethers/wallet.js:44:62)
  reason: 'hex string must have 0x prefix',
  code: 'INVALID_ARGUMENT',
  arg: 'value',
  value:
   '123456789012345678901234567890123456789012345678901234567890123' }

In fact, if I do the same with ethers@2.0.0 The same thing happens. I believe the requirement was present since 0.0.1, but my memory gets a little foggy that far back, cut I seem to recall people at the time complaining, but I believe in explicitly declaring intentions, and since strings are used for myriad things in JavaScript, it is the least worst way to indicate the string's intention as an octet-string.

@matYang
Copy link
Author

matYang commented Nov 20, 2020

I'm very certain this is not an issue in Ethers V4, as I personally have experienced, and the author of the linked post also experienced that, it was perfectly ok to use non-0x-prefixed private key in V4 to construct a wallet object.

The issue only showed up as we upgraded form v4 to v5. I was using 4.0.43.

@ricmoo
Copy link
Member

ricmoo commented Nov 20, 2020

Perhaps there is somewhere else the hex string is being generated that used to 0x-prefix it for you in your pre-upgrade code?

You can install v4 to try it out, maybe there is some other aspect that has changed I am not thinking of, but you can install v4 direct via a specific version or use the tag legacy (npm install ethers@legacy) and try it out... I can't (I don't think?) fiddle with the versions in npm, so you can verify/investigate for yourself...

@ricmoo
Copy link
Member

ricmoo commented Nov 20, 2020

(I just installed 4.0.43 and verified it throws the same error)

@matYang
Copy link
Author

matYang commented Nov 20, 2020

image

@matYang
Copy link
Author

matYang commented Nov 20, 2020

using a valid random private key and ethers@4.0.43, a wallet object was returned

@ricmoo
Copy link
Member

ricmoo commented Nov 20, 2020

What is the result of console.log(e.version)?

That seems very odd, because I know I have code in there to check the regex of the string coming in... :)

@matYang
Copy link
Author

matYang commented Nov 20, 2020

image

@matYang
Copy link
Author

matYang commented Nov 20, 2020

so as in the screenshot, it is Ethers 4.0.43. We had that piece of code working in production since last year so pretty sure it was working fine, and we know it was not 0x prefixed because the other libraries we use would throw

@ricmoo
Copy link
Member

ricmoo commented Nov 20, 2020

Can you include the npm install you used for that? Or your package.json? Because now I am super confused... Staring at this, my eyes are close to falling out. :p

@matYang
Copy link
Author

matYang commented Nov 20, 2020

image

@ricmoo
Copy link
Member

ricmoo commented Nov 20, 2020

Right, but I know that code has been there for years, and just now installed 4.0.43 locally from npm. So there is some disconnect. It was never supported, so the fact it is working for you means something we are not thinking of is going on...

Can you verify in the node_modules/ethers the following code:

    // Line 17 of ethers-wallet/signing-key.js
    privateKey = utils.arrayify(privateKey);
    // Line 41 of ethers-utils/convert.js
    if (isHexString(value)) {

    // Line 17 of ethers-utils/convert.js
    if (typeof(value) !== 'string' || !value.match(/^0x[0-9A-Fa-f]*$/)) {

@matYang
Copy link
Author

matYang commented Nov 20, 2020

I assume it should be reproducible with valid private keys, like

9573fa33a57fee662a23bf60f1b1674364d99fb8dd2166b4ae470ce7ab20ed9f
79ae42fce836c603bc3d6d90421fa3ab0b946e339735e5df3433d328ac870755
147fe9be399879bdeefbecb239e4dc37ffa1e01b2eaae151e77e9babba28cf88

@ricmoo
Copy link
Member

ricmoo commented Nov 20, 2020

Something weird just happened... It started doing what you have.

I will investigate further, because this is quite odd behaviour. Gimme a few minutes. I'll update this issue shortly.

@matYang
Copy link
Author

matYang commented Nov 20, 2020

The file seems different

signing-keys.js looks different
image

and I can't find converts.js

just to clarify, with @4.0.43, I do see the same error if an invalid private key is supplied, but no error if the private key is valid

@ricmoo
Copy link
Member

ricmoo commented Nov 20, 2020

Sorry, I was looking at the wrong files when I sent those line numbers. That was the v2 code I'd installed to compare.

One sec. Adding some tracing statements...

@ricmoo
Copy link
Member

ricmoo commented Nov 20, 2020

You are absolutely correct! So sorry! My sample private key above was only 63 nibbles long, but the error indicated missing 0x, so that was why my examples were not working.

I just found a section in v4:

// A lot of common tools do not prefix private keys with a 0x
if (typeof (privateKey) === 'string' && privateKey.match(/^[0-9a-f]*$/i) && privateKey.length === 64) {
    privateKey = '0x' + privateKey;
}

And now that I found it, I mentioned it to my partner and she has a slight recollection of that change because the version of Ganache at the time dumped out private keys without a 0x prefix.

Where are you getting your private key from? Is it still common for any recent tools to not 0x-prefix? I will document this change in the migration guide.

Again, huge apologies for the inconvenience and my mistake. Thanks for your patience. :)

@ricmoo ricmoo added documentation Documentation related issue. on-deck This Enhancement or Bug is currently being worked on. and removed discussion Questions, feedback and general information. labels Nov 20, 2020
@ricmoo
Copy link
Member

ricmoo commented Nov 20, 2020

(I see now that you had linked to that same portion of code above; thanks!)

@matYang
Copy link
Author

matYang commented Nov 20, 2020

No worries! We love the Ethers library and really appreciate your work, glad could be helpful.

The private keys I posted were from the latest Ganache
image

Quick check on Metamask also exports keys without '0x'

example the PrivateKeyWalletSubprovider module from the 0x project throws if private key starts with '0x'

@ricmoo
Copy link
Member

ricmoo commented Nov 20, 2020

@yuetloo just noted the same thing re: Metamask...

I'm contemplating adding support for 0x-prefix-less private keys... The ContractFactory also allows no 0x prefix since solc doesn't spit it out... So it's not without precedent.

@ricmoo ricmoo added enhancement New feature or improvement. fixed/complete This Bug is fixed or Enhancement is complete and published. and removed documentation Documentation related issue. on-deck This Enhancement or Bug is currently being worked on. labels Nov 24, 2020
@ricmoo
Copy link
Member

ricmoo commented Nov 24, 2020

This has been changed in 5.0.22. It should not reflect the behaviour in v4. Try it out and let me know if there are any issues.

Hopefully in a years time, I don't have the same debate with someone else that it doesn't work that way. :p

Thanks again for your patience! :)

@ricmoo
Copy link
Member

ricmoo commented Dec 14, 2020

Closing this now. If you have any further issues, please feel free to re-open.

Thanks!! :)

@apollo0102
Copy link

hi.
I had the same errors
But there is not exist utils directory in my node module.
what's the matter?

@apollo0102
Copy link

My ether node version 5.5.4 latest version.
what's wrong with it?

@sriharsh1104
Copy link

as per above chat my conclusion came that it require 4 version.

@apollo0102
Copy link

apollo0102 commented May 4, 2022 via email

@sriharsh1104
Copy link

me fine what's about you . in this issue i am getting same error while i tried to buy a nft from my unity game .

@sriharsh1104
Copy link

i think u should change the version of npm ether to lower to 4 . i don't how to do it . i am also trying that

@ceerlepy
Copy link

I am still getting the same error? is there any plan to resolve it?

@zemse
Copy link
Collaborator

zemse commented Jun 26, 2022

If installing latest version of ethers is not helping npm install ethers@latest, can you try removing the package lock file and node modules and then npm install. I just confirmed that this was fixed and works in latest ethers.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or improvement. fixed/complete This Bug is fixed or Enhancement is complete and published.
Projects
None yet
Development

No branches or pull requests

6 participants