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

Compile Against Official Repo #20

Closed
ctrlshp opened this issue Dec 27, 2018 · 29 comments
Closed

Compile Against Official Repo #20

ctrlshp opened this issue Dec 27, 2018 · 29 comments

Comments

@ctrlshp
Copy link

ctrlshp commented Dec 27, 2018

Hello, I'm interested in your work because I would like to run Monero address form validation in the browser and also a subaddress wallet in NodeJS and I would like to do that by compiling the official Monero C++ code into JavaScript with Emscripten.

I didn't know how to reach you, so I opened an issue here to ask my questions I hope you don't mind. Do not hesitate to let me know the preferred way to contact you for this, if Github doesn't suit you.

Here is my first question : could mymonero-core-cpp be compiled directly from the official Monero CPP repository ? If not what are the modification in monero-core-custom that are necessary from the official code to make it work ?

Thanks ! Merry Christmas and happy New Year.

@paulshapiro
Copy link
Contributor

Hey @ctrlshp!

Thanks for your note. Ordinarily you can reach me at [email protected] but this is a fine location for this topic, and it may benefit others in future as well.

If you want to do Monero address validation in the browser, mymonero-core-js can handle that handily with decode_address. However the entire build may be a bit heavy for you since it contains other things. One open issue we have is a way to select what methods and code are needed to strip that down. But the JS to do addr validation itself is quite straightforward – you can find it in the history of mymonero-core-js.

Here's a list of the changes I made to Monero official. I stripped it down considerably - and then made these modifications: https://github.com/mymonero/monero-core-custom/blob/master/changes%20when%20upgrading%20monero%20src.txt

Paul

@paulshapiro
Copy link
Contributor

(Of course, having these in patches would be better. The real goal is to improve Monero sourcecode so as to allow it to be embedded properly as a library but I fear that is a long ways off since the people doing the majority of development on Monero are only really interested in simplewallet and the daemon. Nevertheless, libraryfying Monero source code would greatly benefit those applications as well, not to mention the stability and clarity of the code. But that argument does not seem effective to them for some reason!)

@ctrlshp
Copy link
Author

ctrlshp commented Dec 27, 2018

Hi Paul. Thanks for your reply. I browsed through the diffs that you provided. I'm not sure I understand : did you submit a PR for these and it was rejected ? Are they otherwise breaking non-emscripten monero-core use cases like the cli executables and daemons ? For sure, I agree with you that it would be much better to have them merged with the official branch. I also agree that it doesn't make sense for the monero-core code base to keep on ingesting new functionnality forever (like the new Bimessage-based MMS system, ZeroMQ, etc.) and not extract the real "core" code into a new libmonero project of some kind.

I think that, in the coming days/weeks what I will do is this :

  1. See how easy it is (for me) to merge the latest commits to the upstream monero-core repository into (my own fork of) monero-core-custom.
  2. Try to compile mymonero-core-js on my own.
  3. Try to use mymonero-core-js in both my front-end (browser) and back-end (NodeJS) Typescript projects.

I'll be sending questions to the email you provided above, unless they are issues that warrant PRs and stuff, in which case I'll be asking them here as well.

Thanks again for taking the time to answer my questions and also putting the effort in the MyMonero wallet and keeping it all open source. It's awesome !

@ctrlshp
Copy link
Author

ctrlshp commented Dec 27, 2018

P.-S. : and after those 3 steps, I'll have a clearer idea about this whole libmonero thing, and I'll most likely want to investigate this further. But for now, I'll stick to try to understand what you did and if it works for me.

@paulshapiro
Copy link
Contributor

paulshapiro commented Dec 27, 2018

  1. The changes to monero-core-custom are not (edit: largely) specific to emscripten. mymonero-core-custom is used in our iOS and Android apps too. OpenSSL was removed because it's nice not to have to include it. But we do have a working OpenSSL compilation for emscripten if we come to need it.

  2. I did not submit a PR for the majority of these changes. I spoke extensively with community members and found significant verbal support from individuals like vtnerd, and to a lesser extent, hyc, and moneromooo, but no one is willing to work on it except me… I tried… then found that to make all these changes is an entire day job in and of itself (cannot do that as I wear many hats as it is). Many PRs should be made over time slowly factoring things. wallet2 is the big one. I did PR support for 13 word seeds but it was not merged yet because they couldn't decide on the best way to add support to simplewallet. Support for reading 16 byte / 13-word seeds monero-project/monero#3281 .. My changes do break some things such as mlocker support but I don't consider that necessary for our use-case. A PR to merge such things may need to have a switch to turn e.g. mlocker functionality on and off for stripped-down / embedded use-cases, or it will have to be made to work on platforms like emscripten, etc.... Largely, almost all of it works in emscripten though.

Totally agree that the core code needs to be extracted. It's not sustainable. Been saying this for a long time. This is just a fact of software entropy.

My pleasure to assist. Thank you very much for your interest!

@paulshapiro
Copy link
Contributor

By the way please note that the PR i made for 13 word support is a bit different than the version that I have got in monero-core-custom -- the one in core-custom is probably a bit better, and incorporates some recent changes to monero-core.

@paulshapiro
Copy link
Contributor

I think I might have changed the word "coerce" as well 😅

@ctrlshp
Copy link
Author

ctrlshp commented Dec 27, 2018

Got it (I think 😎). I will fork everything to my Github account and commit there so you can see what I'm doing.

@paulshapiro
Copy link
Contributor

oh hey I forgot to say happy holidays back, thanks!

@ctrlshp
Copy link
Author

ctrlshp commented Jan 4, 2019

Hello. I didn't get the chance to look at this until today. I tested the address_and_keys_from_seed example from the README in a simple Node.JS index.js (attached here). Besides the atexit() called, but EXIT_RUNTIME is not set, so atexits() will not be called. set EXIT_RUNTIME to 1 (see the FAQ) you already know about, it works. But I have some questions, if you don't mind :

  1. Which of these files is/are required in a browser/web environment, and then in a Node.JS environment ? MyMoneroCoreCpp_ASMJS.asm.js, MyMoneroCoreCpp_ASMJS.js, MyMoneroCoreCpp_WASM.js, MyMoneroCoreCpp_WASM.wasm

  2. A couple of days ago Knaccc posted a javascript subaddress generator and I think MyMonero used to rely on those same dependencies before the Emscripten code (some BigInteger lib, elliptic, keccak or TweetNaCl, I'm not sure exactly). Can you ELI5 to me why you made the switch to Emscripten, besides the performance boost ?

  3. I didn't attempt to update the merge the latest commits to the upstream monero-core repository. I'm getting an instant migraine just thinking about it. Would it be feasible to maintain just a simple diff patch that would get applied to the monero-core code prior to compilation ?

  4. At the moment, my first integration would be to have an exhaustive form validation function for Monero addresses that can handle all possible kind of supported Monero addresses (main address, subaddress, integrated address, etc.). Of course would need to write my custom mymonero-core-cpp with just that function or tweak the CMake script to exclude/include functions and/or files at pre-compile configuration time. Do you think this is the way to go ? Is Emscripten good at optimizating output binaries to include only the required "symbols" (or whatever they are called in asm.js world) ?

That's it for now. This is exciting. I didn't expect it to work right away ! ;-)

Thanks!

@ctrlshp
Copy link
Author

ctrlshp commented Jan 4, 2019

It didn't want my .js attachment. Here it is in plaintext :

const mymonero = require("./MyMoneroCoreBridge")();

const nettype_utils = require("./cryptonote_utils/nettype");

mymonero.then(monero_utils => {
let nettype = nettype_utils.network_type.MAINNET;
const decoded = monero_utils.address_and_keys_from_seed("....", nettype);
console.log(decoded.address_string);
});

@paulshapiro
Copy link
Contributor

Hey @ctrlshp ,
Glad to hear it worked out of the gate!

  1. the ASM.js fallbacks are for older browsers (like old Firefox versions that some of our users still use) … the code will detect WASM support and fall back if needed. If you're just running under Node.JS then you should be fine including only the WASM.

  2. We switched to emscripten because at the time, we had no help to write bulletproofs transactions, etc., and as a matter of fact, mymonero has used emscripten ever since 2014 - but it was only crypto-ops at the time. We wanted to share the core codebase among all our apps, and weren't able to keep rebuilding Monero's cryptography in JS by hand.

  3. I totally think a diff patch would be a good idea. I'd do it but have so much on my plate right now.

  4. At the moment, much more code is included in mymonero-core-js than is needed for your application of just decoding addresses. So to accomplish what you're looking for where it only includes the symbols/files you want, we'd have to complete something like this: Idea: build-time capability to select functions to actually include in bridge - may be a space optimization mymonero-core-js#58
    Alternatively you could just manually maintain your own fork where you select the functions you want and include only the necessary files in a CMakeLists.txt. Of course, if you could generalize this for others so that the functions could be selected at build time, that would be a major contribution and I'm sure a ton of people would love you for it :)

Best
Paul

@paulshapiro
Copy link
Contributor

As for #3, could you make an issue on mymonero-core-cpp for whichever one of us gets around to it?

@ctrlshp
Copy link
Author

ctrlshp commented Jan 6, 2019

Hi Paul ! I didn't mean to rush either of us into anything just yet, ha ! I might indeed try it (no. 3) myself after I'm done exploring your work. Only then can I maybe pretend to know what is the best course of action. What is certain now is that both of our business interests overlap quite a bit over everything JavaScript Monero. Open source collaboration of some magnitude seems to be in order here. And I feel it's only right I eventually contribute something back to the Monero software ecosystem. But I don't want it to be garbage, so I might be a little slow compared to a seasoned Monero contributor like you.

Now that I have tried Node.JS, I will see if I'm having the same kind of success in the browser. That's what I'll do whenever I can in the next couple of days. Please note that I will have to include this into transpiled TypeScript code, so I don't think it's going to be as straightforward as NodeJs. We'll see.

Speaking of browser and my nos. 2 and 4, since I will not want to, on top of the Monero Emscripten stuff, include cryptographic primitive javascript libraries (Keccak, elliptic curve and BigInteger arithmetics and such), I would like to expose those from monero-core. I didn't see them made available in your bridge nor in the mymonero-core-cpp C++ proxy code. Did I overlook anything ? Also, do you think I can avoid to including what I assume is redundant code like tweetnacl-js to do the kind of encryption-decryption-signing-verification that I'm sure is the behind the basic Monero operations ?

Good night !

@paulshapiro
Copy link
Contributor

paulshapiro commented Jan 6, 2019

Hey @ctrlshp

Have to say I am quite possibly far from a seasoned Monero contributor but I appreciate your kind words :) I know a little. Happy to help where I can. Most of my experience comes from software engineering, application-, and library dev itself. All in all, we can collaborate over time, so no worries about getting things exactly perfect.

Re: TypeScript, that just gets compiled down to vanilla aiui so it should end up being basically the same. I know that MyCryptoHQ has done some TypeScript work based on the old pre-bulletproofs mymonero-core-js before we began to move the majority of the core-js impl into C++. So you may find some inspiration and help there. Though I hear the project was given up on for internal planning reasons and they're interested in moving to the emscripten version. So your TypeScript work may end up being quite interesting to them.

There is no way to do BigInteger operations in JS-land exposed via C++ without using e.g. JSBigInt. The C++ just uses uint64s but no such thing exists in JS. As for tweetnacl I'd suggest bridging to C++ and doing it within the C++ (compiled to WASM).

We do expose a variety of utility functions via the bridge code so if you find anything missing just let me know and we can figure out how to bridge it!

Best regards

Paul

@ctrlshp
Copy link
Author

ctrlshp commented Jan 6, 2019

Hello ! Integrating this into a web TypeScript project is a very humbling process for me, right now. Lol. I got it to work, but it's a hack. I am able to load the WASM file if the MyMoneroCoreCpp_WASM.js file is loaded through a <script> tag and a type declaration for the MyMoneroCoreCpp global constant (and its ready Promise). Question : where can I get more information on how the MyMoneroCoreCpp_WASM.js is generated ? I couldn't find anything in the Emscripten docs. I would like to find a more elegant way to import it into TypeScript code.

Also, I didn't find anything relevant in the public MyCryptoHQ code.

Good evening !

@paulshapiro
Copy link
Contributor

MyMoneroCoreCpp_WASM.js is generated by mymonero-core-js config lines such as https://github.com/mymonero/mymonero-core-js/blob/529fc93194d166a3e7d2430e3b083252aea4fde9/CMakeLists.txt#L136 and https://github.com/mymonero/mymonero-core-js/blob/529fc93194d166a3e7d2430e3b083252aea4fde9/CMakeLists.txt#L190 etc …… and this gets run via this: https://github.com/mymonero/mymonero-core-js/blob/master/bin/build-emcpp.sh (by way of archive-emcpp.sh) - instructions in readme at https://github.com/mymonero/mymonero-core-js#building-mymonerocorecpp-from-scratch

hope this helps!

I'd have to dig for MyCryptoHQ's fork of mymonero-core-js … I'm not sure why it's not available

You should never have to include MyMoneroCoreCpp_WASM directly - it's taken care of by MyMoneroCoreBridge, fwiw.

@ctrlshp
Copy link
Author

ctrlshp commented Jan 7, 2019

Thanks for your pointers.

I made a gist of my last attempt so you can see what I mean. This would require to complete the MyMoneroCoreCppModule type, and I agree with you it's not the best way to do it. First, I wanted to load the WASM file directly with the WebAssembly API but I needed the "info" object that is in MyMoneroCoreCpp_WASM.js, hence my question about how it is generated, to see if I could isolate that by configuring the binaryen generator. I'm glad I did all this because it makes me understand and appreciate your work a little better.

I think I found the MyCryptoHQ code you were talking about. Based on what I can see, they didn't use MyMoneroCoreBridge.js. Nonetheless, later this week, I will try to import MyMoneroCoreBridge.js into TypeScript with as little changes as possible. Clearly, I'd rather use as much of your code as-is. Ideally, as an rpm package with native TypeScript type definitions. Anyway, I'll let you know how it goes as soon as I can get something working using MyMoneroCoreBridge.js.

Take care.

@ctrlshp
Copy link
Author

ctrlshp commented Jan 7, 2019

Hello ! Just to let you know that I couldn't wait the end of the week to try this. So I bundled everything into a mymonero-core.js file using the provided mymonero-core-js/bin/package_browser_js script, put everything in its right place using a <script> tag in the TypeScript web application and wrote types for the top-level mymonero-core-js/index.js file going all the way down to its callable elements.

This will only work for the Web, of course, so I want to see how it translates to a TypeScript NodeJS environment (within the Serverless framework) and how to reuse the same type definitions there.

Then, I might want to find a way to include mymonero-core.js in the TypeScript code using a proper import statement with matching .d.ts types instead of a <script> tag and dangling type definitions. It'll probably require a separate webpack configuration and script. What do you think about an official public npm packaging for mymonero-core-js with inluded types (using the types attribute in package.json) ?

@paulshapiro
Copy link
Contributor

I'm more or less ok with using npm but I'm not crazy about it as a point of failure. It's not much worse than pointing directly to the git repo of course, but then again, it's not much better, and convenience is somewhat an enemy of safety in this case (as a critical crypto repo). Open to suggestions and other points of view.

I'm not very familiar with TypeScript, so I can't weigh in on the types field. If it doesn't affect anything related to the vanilla code and if it's properly implemented then yep I'll merge it.

MyCryptoHQ didn't use MyMoneroCoreBridge b/c they did that work at the beginning of 2018 before we made a bunch of simplifications and refactors.

@ctrlshp
Copy link
Author

ctrlshp commented Jan 7, 2019

re: npm, I'm just trying to find a convenient way to a) distribute mymonero-core-js and b) have the type definitions follow along. Another possible solution is to leave the whole mymonero-core-js project as it is and commit the type definitions alone to the DefinitelyTyped project.

For now, when I'm done with the NodeJS integration and the type definitions themselves, I'll send you a PR for a index.d.ts and a modified package.json once I get a npm install working on a Github fork.

@paulshapiro
Copy link
Contributor

I was just gonna say as well, how about we commit everything to the repo? Or, we could do them as releases. In any case, npm still works by pointing to the repo so, agreed, we can add things to package.json and go from there.

@ctrlshp
Copy link
Author

ctrlshp commented Jan 7, 2019

Yes, that's the plan. I'll fork ctrlshp/mymonero-core-js into ctrlshp/mymonero-core-js and make changes there towards a npm i ctrlshp/mymonero-core-js that works for both Web and Node TypeScript projects.

On top of the types, I think I need to find a way to pass a custom location for the .wasm file to MyMoneroCoreBridge.js via the options parameter.

@ctrlshp
Copy link
Author

ctrlshp commented Jan 14, 2019

Hi,

First of all, congrats for your website launch, it looks goooooood.

So I tested mymonero-core-js as a npm plugin package with type definitions in a Serverless AWS Lambda context. It works as-is. But my lambda went from a <1s cold-start to a >15s cold-start. The execution time per se is fine with <1s from <100ms, I can live with that. But the cold-start time makes it unusable in my current production use cases. I will stick with the current wallet+monerod RPC VPS infrastructure for the time being.

I still needed a replacement for the regex that I currently use to validate the Monero refund address on the browser client side. In this scenario, refactoring mymonero-core, both C++ and JavaScript (with ES6) for modularity, would solve the two problems currently preventing me from using it for simple address validation : 1) it would make it compabible with the Rollup bundler I currently use and 2) such modules would allow targeted imports of only the required code, so if they are done correctly, I should be able to bundle only the code required for a isValidMoneroAddress(address : string) => boolean method, which, hopefully, is significantly lighter than just having the full wallet. For now, I replaced the regex with with Base58, BigInteger and Keccak code that I refactored in TypeScript from JavaScript.

So I haven't been able to integrate your mymonero-core code into my stuff for these two things. I still hope to be able to use it some day for 100% serverless address generation and/or payment verification via remote full node monerod RPC access to the blockchain. I have looked at Emsripten options such as MODULARIZE=1, SINGLE_FILE=1, EXPORT_ES6=1 and I think something can be worked out to make the code smaller and more compatible with TypeScript. I will also try to have a patch system over the official monero-core code base working as I do that.

However, I have more urgent things to do before I can work on this again. My time frame is end of February. But before that, planning for the long term, I will initiate a conversation with you regarding multisig, on the issue about that.

@paulshapiro
Copy link
Contributor

The startup time can be significantly reduced by us implementing the system which selects only the core-cpp code which is needed by the custom core-js build.

@paulshapiro
Copy link
Contributor

Excellent progress otherwise though. Glad to hear.

@ndorf
Copy link
Contributor

ndorf commented Jan 18, 2019

Hi @ctrlshp. You might find this helpful: https://github.com/mymonero/monero/commits/core-custom-mymonero

It is a branch with content identical to that in monero-core-custom, but with the full Monero project source code as an ancestor in its git history -- specifically, tag v0.13.0.3. This makes it possibly to merge any newer upstream Monero commits automatically using git, instead of applying each change individually by hand.

The automatic merge still leaves a bit of work to be done, however. This is still a work in progress, and any suggestions to improve this process would be most welcome. In the meantime, I am able to merge newer master commits with the following extra steps:

  1. Changes to files that MyMonero has removed always result in conflicts. It's easy enough to select our deleted "versions" for all of them at once: git status --porcelain | awk '$1 == "DU" { print $2 }' | xargs git rm -f

  2. New files should mostly be skipped. git diff --cached --diff-filter=A | xargs git rm -f will remove them all, but watch out for any files that might actually be needed. In this case, you can use the same command without the -f flag, then git add the required files back in, and finally git mv them to the right location (e.g., from src/ to ./, and from contrib/epee/ to ./epee/).

  3. The majority of the remaining files should merge without conflicts, but there will undoubtedly be a couple that need to be resolved by hand. There's no way around this, but fortunately they're relatively trivial to resolve.

  4. Lastly, new code from upstream that merged cleanly might still have to be modified if it attempts to reference something that MyMonero has removed. For instance, new lines that invoke the various PERF_TIMER_* macros will have to be removed manually. Fortunately, the compiler will do the work of finding these for you.

As scary as all that might sound, it was actually pretty easy to do such a merge, especially compared to the alternative of doing it manually. As an example, here is a merge I've made into this branch of a slightly more recent upstream master commit: https://github.com/mymonero/monero/commits/core-custom-master. This is still experimental, and isn't yet meant to be directly used, but it does seem to be on the right track (it builds with mymonero-core-cpp and passes the tests.)

On a final note, because both of these branches are based on the upstream Monero repository, cloning them will require as much space as that -- around 300MB at present. If you already have a Monero clone locally, you can check these branches out there to avoid downloading a second copy. If you don't want to mess with your existing Monero working directory, you can use the git-new-workdir script to add a second one that shares the metadata (.git subdirectory) with the main one but doesn't affect its working directory. It's very handy in general for working with large repos such as the Monero project. If you're on a Debian-based system, the script should be in /usr/share/doc/git/contrib/workdir/git-new-workdir.

I hope this helps you at least a little bit. Please don't hesitate to let me know if you have any questions about any of it!

@ctrlshp
Copy link
Author

ctrlshp commented Jan 28, 2019

Hi @ndorf sorry for the delay. Thanks for that. Will look into it very soon.

@paulshapiro
Copy link
Contributor

Closing this now

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

No branches or pull requests

3 participants