search

XRPLF / xrpl.js

A JavaScript/TypeScript API for interacting with the XRP Ledger in Node.js and the browser
https://xrpl.org/
1.2k stars 512 forks source link

Library generates same wallet with any mnemonic generated #2042

Closed aditya-stb closed 2 years ago

aditya-stb commented 2 years ago

return xrpl.Wallet.fromMnemonic(secret, { mnemonicEncoding: 'rfc1751', algorithm: 'ecdsa-secp256k1', }) }

// light maze exit awful hire ramp trigger will response offer throw hobby // glue belt debris word submit clump provide organ waste group plunge glory //frgrfge reer rere reer frerf ergre reer reter erert erret erwre ertrt

SAME Wallet address generated for all these mnemonics :rGzzsoiXsM3RmHU9Fu9eDQ6hoNoouyQiNT

These mnemonics were created using xrpl-accountlib.generate.mnemonic({strength:128}) ( https://www.npmjs.com/package/xrpl-accountlib )

mvadari commented 2 years ago

Looks like fromMnemonic currently requires words to be all uppercase. This should probably be fixed - the code can just auto-capitalize. Because the lower-case words don't exist in the list of words, the code takes -1 as the index and uses that for all the math, which apparently works out to rGzzsoiXsM3RmHU9Fu9eDQ6hoNoouyQiNT. There should be some checks to ensure that this errors.

However, when these mnemonics are capitalized, it errors, because even the capitalized words are not in the list. I did some digging, and that's because xrpl-accountlib uses the bip39 mnemonic encoding, not the rfc1751 encoding. So that's the problem that you're having.

Are these the results you were expecting?

> secret = "light maze exit awful hire ramp trigger will response offer throw hobby"
> xrpl.Wallet.fromMnemonic(secret, {mnemonicEncoding: 'bip39', algorithm: 'secp256k1'})
Wallet {
  publicKey: '02862A0AEDA41CF3768D7FFA2EB9FA63BCF30E92688C7A977E0F29058C76AB3BB4',
  privateKey: '00F27165AA44A28328D3019943DF71C430D8373FB1050E3B6E7C065BAE21484E22',
  classicAddress: 'rGMY6pCthyTFuT8oW6DCE7v6nRYgiUoiT9',
  seed: undefined
}
###################################
> secret = "glue belt debris word submit clump provide organ waste group plunge glory"
> xrpl.Wallet.fromMnemonic(secret, {mnemonicEncoding: 'bip39', algorithm: 'secp256k1'})
Wallet {
  publicKey: '027F9E3E1B60524ABF02F062FCB09696D16F5401242E74299888CDCDF8127A06DD',
  privateKey: '00AC42D3D631D7602ED2812F03130844B643E9CEFED8F3D2F87446DF7CB0B572D3',
  classicAddress: 'rPWbEnrQr2syRfmb5kubcBukJ9cpjxnz7q',
  seed: undefined
}
###################################
> secret = "frgrfge reer rere reer frerf ergre reer reter erert erret erwre ertrt"
'frgrfge reer rere reer frerf ergre reer reter erert erret erwre ertrt'
> xrpl.Wallet.fromMnemonic(secret, {mnemonicEncoding: 'bip39', algorithm: 'secp256k1'})
Wallet {
  publicKey: '03B89D793961A48320424E95D6641D79102B458C2C19EC013C77CB36F9AC206FC6',
  privateKey: '00D186DB1B5FB757FE1F41BD79DFBCE81831C76AA312F3D3E486030EA085E2AA61',
  classicAddress: 'raLzQmiuDdgmoJzYPMSNck4sm2kXXttVS',
  seed: undefined
}

Action items to make this less opaque: [ ] Allow lower-case mnemonics [ ] Better error handling

aditya-stb commented 2 years ago

Hi Mayura Thanks for the detailed explanation. I had tried with capital letters already , but as you pointed out, it still didn't work. The returning of default XRPL address results in users putting money into that account( We put 15 XRP into it ) but lost that money forever. So IMHO , the library should throw an error and not return that address which gives a false impression that its a new wallet.

Moreover, the issue is we have people for whom we created mnemonics using RIPPLED server which gave RFC1751 type of mnemonics. Now, we want to generate in a similar format to maintain backward compatibility.

I found no library in XRPL JS to generate a mnemonic. If you can let us know a library which generates these mnemonics , our problem would be solved for good.

On Thu, 14 Jul 2022 at 17:35, Mayukha Vadari @.***> wrote:

Looks like fromMnemonic currently requires words to be all uppercase. This should probably be fixed - the code can just auto-capitalize. Because the lower-case words don't exist in the list of words, the code takes -1 as the index and uses that for all the math, which apparently works out to rGzzsoiXsM3RmHU9Fu9eDQ6hoNoouyQiNT. There should be some checks to ensure that this errors.

However, when these mnemonics are capitalized, it errors, because even the capitalized words are not in the list. I did some digging, and that's because xrpl-accountlib uses the bip39 mnemonic encoding, not the rfc1751 encoding. So that's the problem that you're having.

Are these the results you were expecting?

secret = "light maze exit awful hire ramp trigger will response offer throw hobby"> xrpl.Wallet.fromMnemonic(secret, {mnemonicEncoding: 'bip39', algorithm: 'secp256k1'})Wallet { publicKey: '02862A0AEDA41CF3768D7FFA2EB9FA63BCF30E92688C7A977E0F29058C76AB3BB4', privateKey: '00F27165AA44A28328D3019943DF71C430D8373FB1050E3B6E7C065BAE21484E22', classicAddress: 'rGMY6pCthyTFuT8oW6DCE7v6nRYgiUoiT9', seed: undefined} ###################################> secret = "glue belt debris word submit clump provide organ waste group plunge glory"> xrpl.Wallet.fromMnemonic(secret, {mnemonicEncoding: 'bip39', algorithm: 'secp256k1'})Wallet { publicKey: '027F9E3E1B60524ABF02F062FCB09696D16F5401242E74299888CDCDF8127A06DD', privateKey: '00AC42D3D631D7602ED2812F03130844B643E9CEFED8F3D2F87446DF7CB0B572D3', classicAddress: 'rPWbEnrQr2syRfmb5kubcBukJ9cpjxnz7q', seed: undefined} ###################################> secret = "frgrfge reer rere reer frerf ergre reer reter erert erret erwre ertrt"'frgrfge reer rere reer frerf ergre reer reter erert erret erwre ertrt'> xrpl.Wallet.fromMnemonic(secret, {mnemonicEncoding: 'bip39', algorithm: 'secp256k1'})Wallet { publicKey: '03B89D793961A48320424E95D6641D79102B458C2C19EC013C77CB36F9AC206FC6', privateKey: '00D186DB1B5FB757FE1F41BD79DFBCE81831C76AA312F3D3E486030EA085E2AA61', classicAddress: 'raLzQmiuDdgmoJzYPMSNck4sm2kXXttVS', seed: undefined}

Action items to make this less opaque: [ ] Allow lower-case mnemonics [ ] Better error handling

— Reply to this email directly, view it on GitHub https://github.com/XRPLF/xrpl.js/issues/2042#issuecomment-1184366167, or unsubscribe https://github.com/notifications/unsubscribe-auth/AUIXNT366FUOCA3KQRCDUDTVT767TANCNFSM53Q2A7YQ . You are receiving this because you authored the thread.Message ID: @.***>

JST5000 commented 2 years ago

Hey Aditya, you should be able to use keyToRFC1751Mnemonic to generate a Mnemonic in the same way rippled does. (It's in rfc1751.ts in xrpl.js - If that doesn't work, please let me know :)

I also created #2046 to address the issues around encoding/decoding using an incorrect method. It should now properly error out if you try decoding those keys using rfc1751 since those mnemonics don't follow that format.