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

Module to handle mnemonics #3

Closed
ligi opened this issue Oct 18, 2017 · 12 comments
Closed

Module to handle mnemonics #3

ligi opened this issue Oct 18, 2017 · 12 comments

Comments

@ligi
Copy link
Member

ligi commented Oct 18, 2017

No description provided.

@issueth
Copy link

issueth bot commented Oct 18, 2017

This issue now has a bounty-address via issuETH.

Your bounty-address is 1568df7d85915941bdcab8e2f2d5119154adff63
Watch on rinkeby
Watch on main

@issueth
Copy link

issueth bot commented Oct 18, 2017

new token-transfer transaction on rinkeby with value 4.2WALL

@issueth
Copy link

issueth bot commented Oct 25, 2017

Token-transfer transaction on rinkeby with value 4.2WALL

1 similar comment
@issueth
Copy link

issueth bot commented Oct 25, 2017

Token-transfer transaction on rinkeby with value 4.2WALL

@mirceanis
Copy link
Member

Hello,

I want to submit a PR for this but have a question regarding the project layout.
Handling mnemonics is basically supporting bip32 and bip39, but it also ties in to the already existing module BIP44.
IMO the best place for this new code to sit is in the crypto module because it involves working with some code from Keys.kt and Sign.kt but because of the above mentioned link it would also make the crypto module depend on BIP44.

Of course, it can add it as its own module but it just seems to be missing its spot.

How flexible is the project layout?

@ligi
Copy link
Member Author

ligi commented Jan 11, 2018

I want to submit a PR for this

YAY

Handling mnemonics is basically supporting bip32 and bip39, but it also ties in to the already existing module BIP44.
IMO the best place for this new code to sit is in the crypto module because it involves working with some code from Keys.kt and Sign.kt but because of the above mentioned link it would also make the crypto module depend on BIP44.

Did not yet look that deep into mnemonics - but I think there should be a module BIP-32 that depends on BIP-44 and crypto - don't you think this would work? Would love to avoid putting it in crypto as I like small modules - unix style

How flexible is the project layout?

can you elaborate what you mean? I want it to be as flexible and modular as possible - if you have ideas - shoot!

@mirceanis
Copy link
Member

Thanks,
I'll add it as its own module then, with a sprinkle of changes to the other modules as needed.

@ligi
Copy link
Member Author

ligi commented Jan 11, 2018

Sounds great - happy coding!-)

@ligi
Copy link
Member Author

ligi commented Jan 14, 2018

@mirceanis totally forgot: might also be interesting to @rmeissner and @fmrsabino - they already have a implementation with kethereum dependencies in their repo: https://github.com/gnosis/heimdall-android/tree/master/mnemonic - just not a big fan of having the wordlists in memory all the time there - but perhaps it could share some tests or ideas.

@rmeissner
Copy link
Contributor

Yeah, we planned to refactor and test our implementation and then also provide a PR. But of course we are happy to take another implementation ;).

Concerning dependencies to bip32/bip44: imo bip39 is only to generate a seed from a mnemonic. That seed then is used to generate the root node that you can use to derive the children nodes with bip32/bip44. We kept these in different modules in our project, so that we can move small parts to other projects.

@issueth
Copy link

issueth bot commented Jan 18, 2018

This is the private key encrypted for the assignee:

-----BEGIN PGP MESSAGE-----
Version: BCPG v1.54

hQEMAwcp0UUbdQeOAQf/drX3LI6q8p3LvfwG4RJIfsGN3A2F4qhx1KhqrjZtmqnQ
oEDG+oVz+G6i+cEkLnBgdYOxOzRu5fnAzVtuo8ayOvfeI1EKDfjmAPZOlvKbtC1u
TxysE06fwb1rQIhi4Wijvvkl2ngunwK5cswFrVF+yw4uhqAEyY9hzEdFXaqYakpB
vHzlyzCfS7TGgw4CUzZ+zJutYfDdZH/i8jyvQOG/OD42ZCpJmXLoBhIamzdbb1jP
lOuFvCWc+b2cl7vzpYWFi/vqgO7couFbny2R/UjSGeW/qeqkSSaujxiC3flAyI41
tBzBKvP9MGHfnY6mjM6X/hV8V3jy/PjuB/v5v6KxOclcAY2U0uqFaRuF6n++RJxs
J0YUi2lB6haIi+zWhGShewEWXz1CTKemqESJTApG33tNH/bIBLyfs+ijEzfzegGB
dMPQdWvE3bDHJzAATNlM0IpqHSDp2/Bsu3A4OMI=
=YJE4
-----END PGP MESSAGE-----

@rmeissner
Copy link
Contributor

rmeissner commented Jan 19, 2018

Currently the only open issue is to load the wordlist only when needed, right?

I would propose to make bip32 and bip39 two different modules (since bip32 is not about mnemonics).

Another question for me is what is the best way to handle spongycastle with proguard. Since some of the crypto algorithms used by bip39 are only available through spongycastle and we use a provider to load them we need to exclude everything from spongycastle from proguard (which I really don't want to do). In our project we directly use the spongy castle implementations (therefore we can use proguard and obfuscation since they are directly referenced). I know this might not make sense for other platforms (and maybe upcoming android versions where this is not needed).

EDIT:
I just saw that spongycastle is directly referenced for some hash algorithms already. In this case I would recommend doing the same for HmacWithSHA512

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants