mymonero / mymonero-core-js

The JS library containing the Monero crypto plus lightwallet functions behind the official MyMonero apps
BSD 3-Clause "New" or "Revised" License
101 stars 103 forks source link

create_address -- no longer there in #cpp branch #55

Closed bityogi closed 6 years ago

bityogi commented 6 years ago

Hi,

I am adding this as a separate issue here and realize that this is not related to the master branch. But for our intents we are relying on the cpp branch as that branch can be packaged by the latest version of react-scripts.

However, earlier using the code in the master branch, we were able to generate a public address and private spend key, by providing a seed (and networkType) to the mymoneroCoreJs.monero_utils.create_address function like so:


this.account = mymonero.monero_utils.create_address(this.moneroSeed, this.netType);

and then elsewhere use this.account to get:

let pubAddress = moneroAccount.account.public_addr
let privateKey = moneroAccount.account.spend.sec

With the #cpp branch, the create_address function is no longer available. So how do we go about getting the public address and the private spend key?

paulshapiro commented 6 years ago

Hi, the function for creating a wallet on master was in monero_wallet_utils, NewlyCreatedWallet or similar. Nowadays it's on monero_utils called newly_created_wallet, and it returns all the info you requested.

bityogi commented 6 years ago

Hey @paulshapiro ,

The function that seems to meet our needs is seed_and_keys_from_mnemonic.

From what I can see in the code, This function expects the mnemonic (instead of the seed that the removed create_address used.

However, when I call the

this.account = mymonero.monero_utils.seed_and_keys_from_mnemonic(this.mnemonic, this.netType);

it is complaining that

You must call OnceModuleReady to wait for _CNCrypto to be ready

Adding this line before I call seed_and_keys_from_mnemonic: mymonero.monero_utils.OnceModuleReady(); does not help.

TypeError: mymonero.monero_utils.OnceModuleReady is not a function
paulshapiro commented 6 years ago

Hi @bityogi ah yes thanks for pointing this out. It's a regression from one of the last things I did.

bityogi commented 6 years ago

Hey @paulshapiro ,

Could you specify how I should be using this function, or does this require a change in the code base for us to be able to use it?

For now, I can't create wallets using monero_utils.seed_and_keys_from_mnemonic(this.mnemonic, this.netType); because of the You must call OnceModuleReady to wait for _CNCrypto to be ready error.

Thanks so much for all your help on this.

paulshapiro commented 6 years ago

In a few moments I'll push my fix and update you

paulshapiro commented 6 years ago

Hi @bityogi, the issue was merely that OnceModuleReady was missing from the bridge between the interface to monero_utils and the functions in cryptonote_utils. These files are going to get renamed soon for clarity. You can go ahead and call OnceModuleReady now, and I've clarified the tests to show you how to do it. You just go

monero_utils.OnceModuleReady(function() { 
    monero_utils.create_address(…) … 
})

Eventually I would like to promisify this stuff so that someone has the option of doing async-await to roll these two steps into one. If they do, the usage of the function loaded_CNCrypto within cryptonote_utils can be removed since blocking until load would no longer be necessary.

paulshapiro commented 6 years ago

And for what it's worth I'm also going to be able to remove the whole bridging code in monero_utils.js once I get mymonero-core-cpp going as a Node.JS native addon which works with Electron. That will be mymonero-core-nodejs.

bityogi commented 6 years ago

the issue was merely that OnceModuleReady was missing from the bridge between the interface to monero_utils and the functions in cryptonote_utils.

I thought that was the case, and I tried it with a local fork, but it didn't work for me:

https://github.com/bityogi/mymonero-core-js/commit/a0a8ee8f5f3c41290d05d9e1140a2c01573fd0cd

Nevertheless, I'll try your version and see if it works. I definitely wasn't using OnceModuleReady as you specified it.

And yes, promisifying things would go a long way in simplifying usage of this library.

bityogi commented 6 years ago

Hi @paulshapiro ,

I'm not sure I quiet understand this usage:

I tried something like this:

     this.mnemonic = mnemonic;

     this.netType = network === 'test' ? mymonero.nettype_utils.network_type.STAGENET : mymonero.nettype_utils.network_type.MAINNET;

        let account;

        mymonero.monero_utils.OnceModuleReady(() => {
            account = mymonero.monero_utils.seed_and_keys_from_mnemonic(this.mnemonic, this.netType);
            console.log('seed_and_keys executed! account: ', account);
        });
        this.account = account;

From what you have in your tests:

mymonero.monero_utils.OnceModuleReady(function(Module)
{
    var decoded = mymonero.monero_utils.decode_address(
        "49qwWM9y7j1fvaBK684Y5sMbN8MZ3XwDLcSaqcKwjh5W9kn9qFigPBNBwzdq6TCAm2gKxQWrdZuEZQBMjQodi9cNRHuCbTr",
        nettype,
    );
    console.log("decoded", decoded)
})

What is this Module that you are sending in as an argument? (I noticed in the code that you are calling methods of of this Module), but your example in the comment above and my code don't have this is an argument.

paulshapiro commented 6 years ago

Just ignore Module

paulshapiro commented 6 years ago

Promisifying won't really simplify much. It's already functioning in the same way. You have to wait for monero_utils itself to become ready before you call stuff on it. That's what OnceModuleReady is for.

paulshapiro commented 6 years ago

One reason I haven't promisified the API is that combining the async module load with each call makes each of them asynchronous; in other words, it sacrifices the ability to do any calls synchronously. Meanwhile one could instead simply front-load the wait with the present method.

bityogi commented 6 years ago
mymonero.monero_utils.OnceModuleReady(() => {
           console.log('OnceModuleReady!');
            account = mymonero.monero_utils.seed_and_keys_from_mnemonic(this.mnemonic, this.netType);
            console.log('seed_and_keys executed! account: ', account);
        });

In the above code, the line console.log('OnceModuleReady') is NEVER called.

paulshapiro commented 6 years ago

Can you trace it out and see where it's getting stuck? I'm not able to reproduce this. Run tests/index.node.js.

paulshapiro commented 6 years ago

Just dealing with an emscripten issue #5820 related to promise/await'd loading

bityogi commented 6 years ago

Hey @paulshapiro ,

When I load this library in a standalone node.js app, I am able to execute the code in the function I send to OnceModuleReady.

However, it never executes the function passed to OnceModuleReady in our electron app. I know you load the monero_utils through some other way if its an electron app and wondering if that is an issue.

On a separate note, I'm wondering if it would be better to do the necessarily loading in some sort of a constructor which would make it easy for the people using this library down-stream.

i.e,

const MoneroCore = require(`mymonero-core-js`);

let myMonero = new MoneroCore(); //Do what OnceModuleReady does in here

// use library without having to call any other extraneous code or wrap it inside some other function
myMonero.monero_utils.seed_and_keys_from_mnemonic();
bityogi commented 6 years ago

Just dealing with an emscripten issue #5820 related to promise/await'd loading

OK, I missed this comment. This might be the issue then.

paulshapiro commented 6 years ago

So if you never had to call OnceModuleReady and could still call methods synchronously (so that async code doesn't have to pollute your app code), how do you feel about an API like this?

require("monero_utils/monero_utils").then(function(monero_utils) { });

Then you just call your monero_utils function in there.

paulshapiro commented 6 years ago

Hm it, of course, still disturbs the previously synchronous functions which must import monero_utils. Will need to keep fiddling with this.

paulshapiro commented 6 years ago

So I think the way I'm going to do this is require that every function which (a) must remain synchronous and (b) wants to use monero_utils must take a monero_utils as an argument. Things like requestURI parsing, key image generation, etc.

paulshapiro commented 6 years ago

Hi @bityogi please look at the updated cpp branch. It functions as I mentioned just before. Just require monero_utils and it will handle doing the IPC to your electron remote for you.

bityogi commented 6 years ago

Hi @paulshapiro ,

We were primarily using this library to generate addresses for our monero cold wallets by providing our own mnemonic.

I have for now been able to integrate source code from this site, and it seems to be working. It has added a lot of size to our code bundle but it seems to be working.

Nevertheless, I'll try it over the weekend as well to let you know if this code base is also compatible.

bityogi commented 6 years ago

Hey @paulshapiro ,

Actually, the code in the site may also not work, but neither would the function we were using in the cpp branch -- seed_and_keys_from_mnemonic().

We are using extended pub/priv keys to generate multiple wallets using the same mnemonic.

i.e, we'll create the privateKey ourselves

Something akin to this: mnemonic -> extenedPrivateKey --> PrivateKey at a particular index

Then use this privateKey as the seed to generate the account information (address, viewKeys, etc) for the monero wallet.

The code in the master branch accomplished this using create_address(seed, networkType) (the seed was the privateKey we generated above.

Is there anything like this create_address function that we could possibly use in the cpp branch?

paulshapiro commented 6 years ago

Yeah, as you could see in MyMoneroCoreBridge, if you have the mnemonic already, you can just pass it to seed_and_keys_from_mnemonic. The only difference between doing that and the old create_address is that you would need to convert the seed to obtain the mnemonic to call seed_and_keys_from_mnemonic. So I guess I could add an additional function for you if there is a clear, appropriate, unmet use-case. Edit: but it sounds like you're actually starting from the mnemonic and not the seed so it's ok.

paulshapiro commented 6 years ago

Why do you have to generate the seed on your own? Scratch this, now I get what you're saying.

bityogi commented 6 years ago

Scratch this, now I get what you're saying.

Thanks for your understanding. Just to re-iterate, we are creating multiple wallets (with different address, view/spend keys)... but we use the same mnemonic.

Basically the same as using one extended private key to generate child private-keys/public-keys/addresses.

This is why the create_address was perfect for us, because it just accepted whatever privateKey we gave it as a seed.

Do you think this will be implemented in cpp?

paulshapiro commented 6 years ago

I probably don't have an issue adding create_address to my list of new functions to add but let me be clear that you can already do what you want – you just need to take whatever "child seed" you're generating, convert it to mnemonic, then call seed_and_keys_from_mnemonic, so at best you're saving the computational overhead of converting a seed to a mnemonic… but it sounds to me like the work you're doing to to generate child keys should actually already be in the core... do you know if it is or isn't, and if not, why it's not ?

bityogi commented 6 years ago

I'll try the code in the cpp branch based on the test you have for decode and let you know of my findings.

bityogi commented 6 years ago

Hello @paulshapiro , I've tried this code as well, but the problem remains the same:

here is my code:

async function getMoneroMnemonic(seed) {
    let deferred = Q.defer();
    try {
        console.log('getting monero mnemonic using seed: ', seed);
        let mnemonic = (await mymonero.monero_utils_promise).mnemonic_from_seed(seed, 'english');
        console.log('mnemonic from monero: ', mnemonic);
        deferred.resolve(mnemonic);
    } catch (error) {
        console.error('error in getMoneroMnemonic: ', error);
    }
    return deferred.promise;
}

The code hits the line: console.log('getting monero mnemonic using seed: ', seed);

But never hits the line: console.log('mnemonic from monero: ', mnemonic);

paulshapiro commented 6 years ago

That's not the same issue. Change 'english' to 'English'. You're hitting an exception in the emscripten/C++ runtime due to no support for inexact wallet languages as arguments. The issue is https://github.com/mymonero/mymonero-core-js/issues/52

paulshapiro commented 5 years ago

Hey, so I've just pushed create_address as requested, however, just note that I've changed the name and slightly changed the return format. I'm going to document the JS API soon. But, the documentation for the return format can at least be found in the JSON API documentation on mymonero-core-cpp.

New name for create_address is address_and_keys_from_seed.

Returns an object containing: err_msg: String OR address_string: String, pub_spendKey_string: String, pub_viewKey_string: String, sec_viewKey_string: String, and sec_spendKey_string: String

Test https://github.com/mymonero/mymonero-core-js/blob/develop/tests/monero_utils.spec.js#L45

Added in https://github.com/mymonero/mymonero-core-js/commit/299c0c4bd65e29a5983361bbf478ddcc14d9ecd8

bityogi commented 5 years ago

Thanks @paulshapiro , I've reached out to the team about this.

We've almost finished implementing monero in our solution and for this functionality we jerry-rigged what is used on this website

paulshapiro commented 5 years ago

Yep that code is shared with mymonero.com, and was the precursor to the original mymonero-core-js which had create_address. Enjoy!