cosmos / cosmjs

The Swiss Army knife to power JavaScript based client solutions ranging from Web apps/explorers over browser extensions to server-side clients like faucets/scrapers.
https://cosmos.github.io/cosmjs/
Apache License 2.0
646 stars 331 forks source link

proto-signing serialize() error #1429

Closed atmoner closed 1 year ago

atmoner commented 1 year ago

I have a small problem on the use of DirectSecp256k1HdWallet from the lib @cosmjs/proto-signing 0.30.1 When I want to encode a mnenomic with the serialize() function I have an error on the lib libsodium:

TypeError: libsodium_wrappers_1.default.crypto_pwhash is not a function

Here is a simple script to reproduce the error:

import { DirectSecp256k1HdWallet } from "@cosmjs/proto-signing"

const password = '420420420'
const wallet = await DirectSecp256k1HdWallet.generate(12)
const original = await DirectSecp256k1HdWallet.fromMnemonic(wallet.mnemonic, { 
  prefix: 'bcna',
});     
const serialized = await original.serialize(password);

console.log(wallet.mnemonic)
console.log(serialized)

Result: image

Tested on:

Node.js v18.16.0
Node.js v16.15.0

Reproduct:

git clone https://gist.github.com/atmoner/16d00040cef99f4f6b30392aa92e454c bug-serialize
cd  bug-serialize
npm i

Run:

node app
atmoner commented 1 year ago

I found the problem and I know how to fix it!

The problem is that your @cosmjs/crypto lib uses the libsoduim-wrapper lib while it and as indicated in the documentation, crypto_pwhash_* is only used in the sumo version See the explanation here: https://github.com/jedisct1/libsodium.js#standard-vs-sumo-version

So I installed the libsodium-wrappers-sumo package in your @cosmjs/crypto and I replaced the import of libsodium-wrappers in libsodium.ts

And it works! :+1:

I don't make pull requests since I don't know your workflow, I let you do your tests!

webmaster128 commented 1 year ago

Thank you for debugging this! It seems like crypto_pwhash was removed between 0.7.10 and 0.7.11 of the non-sumo version. See https://github.com/jedisct1/libsodium.js/issues/313. Since we pin an older version in the CosmJS repo, we don't see the problem here.

However, it was documented for a long time that the function is not available: https://github.com/jedisct1/libsodium.js/commit/57599d25165196e1fa66b33481b4f72cb3f0dc9a https://github.com/jedisct1/libsodium.js/commit/d297e49dd804bfb7c664cd58be1cdf012e0f3aea

atmoner commented 1 year ago

@webmaster128 you are welcome o/

Yes you have to use the libsodium-wrappers-sumo version! It is possible to update the crypto lib of cosmJs? because currently the serialize() function does not work on cosmJs

webmaster128 commented 1 year ago

Yeah, this should fix it: https://github.com/cosmos/cosmjs/pull/1430

webmaster128 commented 1 year ago

The fix is shipped as part of the pre-release 0.31.0-alpha.1

atmoner commented 1 year ago

Perfect! thank you very much for your speed

atmoner commented 1 year ago

@webmaster128 0.31.0-alpha.1 tested and works perfectly!! Thanks again

mihail727 commented 1 year ago

when release 0.31 version?

atmoner commented 1 year ago

when release 0.31 version?

use 0.31.0-alpha.1 of @cosmjs/crypto