monero-project / meta

A Meta Repository for General Monero Project Matters
160 stars 69 forks source link

Current mymonero API #272

Closed vtnerd closed 4 years ago

vtnerd commented 5 years ago

@moneroexamples @paulshapiro

This is a description of the current API as implemented by mymonero and the current PR for the light wallet server. I would like to make some changes for the open source light wallet server, but that would be done in a separate PR for discussion.

moneroexamples commented 5 years ago

Hi. Thanks for that. Just quick question. Is it the same API as here: https://github.com/mymonero/hosted-monero-api-spec ?

If yes, then which one is the main repo where the discussions should be held?

vtnerd commented 5 years ago

Yes, the documents are identical. Only the commit message differs. The discussion should take place here.

moneroexamples commented 5 years ago

First thing I would like to clarify are the units for xmr amounts. Some are unit64-string and some are uint32 (e.g. get_random_outs; maybe mistake?). I think having everything in unit64, just like monero's codebase is using it, would be more natural and you would not need to keep converting between different units of xmr amounts. This has already been discussed in https://github.com/monero-project/monero/pull/2109, but not sure what was the outcome of it.

paulshapiro commented 5 years ago

So one problem I didn't see raised is that JSON can't represent uint64s afaik

I wouldn't say we'd have any problems with strings parsing to ints by locale on the client-side because we'd never use any digit group separator character .. i.e. these strings (edit: generally) get fed directly into JSBigInt.

moneroexamples commented 5 years ago

I see. Thanks for clarification. But then why amounts in get_random_outs would be sent in uint32?

paulshapiro commented 5 years ago

Heya @luigi1111 could we get this merged?

The other discussion about number formats is really more relevant for the communal standards doc that we hoped we could arrive at after mymonero and openmonero each commit their existing API version (so we have things documented)

paulshapiro commented 5 years ago

@moneroexamples I believe the linked docs are incorrect there. Our API server code reads those values in as an array of strings. Plus, amounts should technically be uint64s rather than 32s. If they were 32s then numbers should work.

paulshapiro commented 5 years ago

Actually sorry, @vtnerd can you rename this document "existing_mymonero_lightwallet_api" or something? Thanks..

moneroexamples commented 5 years ago

@paulshapiro

I will modify openmonero to use uint64-string for all xmr amounts, as in mymonero.

vtnerd commented 5 years ago

@moneroexamples I flipped the field types by accident in the document. mixin is a uint32 whereas the amount array is a uint64-string. This has been corrected with a force push (trying to keep commit log "clean").

@paulshapiro I'm don't quite understand the purpose. Are there going to be multiple versions of the document stored in this repo? The document can always mark things as proposals still in discussion and/or mark as not yet implemented or not going-to-implement in various projects, etc.

paulshapiro commented 5 years ago

yeah @vtnerd we currently have separate APIs, so the idea we had with openmonero was to commit our own versions, then we commit a version we agree to communally that we use going forward as what we have been calling the standard. Obviously your document doesn't document openmonero's API so we can't exactly call it the light wallet server.

vtnerd commented 5 years ago

But isn't this PR the perfect place for the communal discussion? Both documents already exist in the mymonero repo already for viewing, and @moneroexamples has already requested a change (and then decided to alter the openmonero version instead). Additional change requests before merge are welcomed, if there is a concern about flexibility on changes in the future. I could mark this document as "draft" it that helps any, to make it clear that future PRs are welcomed (in a reasonable time frame). For instance, I intended to propose the removal of some fields, and possibly propose a pagination scheme into this document after merge.

More importantly - I don't see why monero-project/meta would want to store some specific document for mymonero or openmonero when this could be stored elsewhere. Unless @luigi1111 thinks any extensions/alterations to the API should be stored here too? I'd likely create a sub-folder to contain the multiple documents if that was desired.

paulshapiro commented 5 years ago

But isn't this PR the perfect place for the communal discussion?

Depends on how you name the file that this PR is making.

More importantly - I don't see why monero-project/meta would want to store some specific document for mymonero or openmonero when this could be stored elsewhere.

After the mutually agreed upon standard doc which merges the APIs gets added and after we update our own APIs then there'd be no reason to keep around the old API documents on this repository.

I'm just doing two things: 1) relaying the original suggestion in the conversations we all had about how this would proceed and 2) just making a suggestion based on project management experience which you're completely free to ignore. We're already talking, yes, but it's nice to have a record of separate APIs that are actually in play as well as actual versions of documents we come up with.

Whichever, let's just agree on something :)

moneroexamples commented 5 years ago

@vtnerd @paulshapiro

I will change openmonero xmr values to uint64-string because now I know that this is better than using uint64 after discussing them with you guys. There are many other things to discuss regarding the api though. For example I think get_address_info and get_address_txs could be merged into one api call. And this could be potentially much greater change than changing uint64 to uint64-string. Other example is the usage of legacy payment_ids for importing wallet in mymonero. I already implemented integrated_addresses for that in openmonero, but integrated_addresses are deprecated anyway these days. Probably would need to start thinking about subaddresses and api around them.

paulshapiro commented 5 years ago

@moneroexamples that is exactly why I suggest we split this up into stages where we first normalize our APIs then enhance them by agreeing on enhancements.

moneroexamples commented 5 years ago

@paulshapiro

Yes, fully agree.

vtnerd commented 5 years ago

Renamed.

For example I think get_address_info and get_address_txs could be merged into one api call.

It might be worth keeping for bandwidth purposes. With some tweaks the client can use it to poll for changes and then call the more bandwidth expensive get_address_txs when a change occurs. The client can mostly do this already, but a reorg (timestamp changes) wouldn't necessarily be detected. Pushing behavior is more desirable anyway, so might be a consideration if HTTP-only scenarios are needed (behind a firewall/proxy, no socket availability).

Other example is the usage of legacy payment_ids for importing wallet in mymonero. I already implemented integrated_addresses for that in openmonero, but integrated_addresses are deprecated anyway these days. Probably would need to start thinking about subaddresses and api around them.

Have both payment id types been deprecated, or just the unencrypted type? I've been hesitant to implement subaddresses on the mymonero backend.

vtnerd commented 5 years ago

For example I think get_address_info and get_address_txs could be merged into one api call.

It might be worth keeping for bandwidth purposes. With some tweaks the client can use it to poll for changes and then call the more bandwidth expensive get_address_txs when a change occurs. The client can mostly do this already, but a reorg (timestamp changes) wouldn't necessarily be detected. Pushing behavior is more desirable anyway, so might be a consideration if HTTP-only scenarios are needed (behind a firewall/proxy, no socket availability).

The other technique would be to combine polling / pagination, which is likely the best since you can have one endpoint with significant bandwidth reduction. get_address_info wouldn't have much of a use case, if clients implemented the poll from last-block-seen via pagination technique.

moneroexamples commented 5 years ago

@vtnerd Pagination seems as a good choice. My understating in this case is that the client would be keeping all the get_address_txs data between requests to the backend, and was just getting new tx data, instead of eveyrthing each time?

subaddresses are expected to replace payments IDs as indicated https://github.com/monero-project/monero/pull/2056 and here https://github.com/monero-project/monero/issues/3679#issuecomment-383371036

This won't happen anytime soon, but I think slowly have to start thinking about supporting them. And this probably will be a major resource hog for the backend. For now, I just added ability to send sub-addresses in openmonero frontend, but adding them to backend is major change I haven't looked into yet.

paulshapiro commented 5 years ago

@vtnerd if we rename the file, then imo it makes sense for record keeping purposes to rename the PR

If we do so, we get to merge this PR w/o having to do all the work to agree on the standard yet. Luckily it's probably not a long discussion. But technically we'll need another PR for the actual agreed upon API doc (which may incur the same conversation) and then additional PRs for the discussions you're having about new features.

So for clarity for other people who want to follow these developments plus record keeping purposes you may like to consider having these discussions on new PRs which indicate that they're about (a) normalizing the API and (b) new features on top of the normalized API instead of on the PR for the initial commit of the mymonero document.

vtnerd commented 5 years ago

Pagination seems as a good choice. My understating in this case is that the client would be keeping all the get_address_txs data between requests to the backend, and was just getting new tx data, instead of eveyrthing each time?

Yes, the client would send a height/hash pair to the server. The server can then filter below that height and report an error if that block reorg'ed off the primary chain. Somewhat similar to monerod.

moneroexamples commented 5 years ago

I don't know if this is the right place to ask, but are there any changes in API planned with regard to booletproofs? Also, is it known when mymonero.com will support making booletproof txs?

paulshapiro commented 5 years ago

I happened to see it so I guess I can answer here. I would suggest we could open an issue for it on this repo instead or even add a note to the future standards document about it.

We have a few things to clear off before jumping to bulletproofs support. I don't think any changes are necessary to the APIs for this but am not 100% sure yet. An earlier version of bulletproofs is ready on the client-side, but I need to update the C++ behind it given recent changes and then run the rebuild script on the JS lib so that the web wallet can use it. Once the fork rolls around, I can update the hardcoded fork version in the C++. (hard-coded since the server is not supplying it - normally the daemon does)

paulshapiro commented 5 years ago

By the way I'm not super clear on where we landed on this PR, but I'm pretty sure we can merge this so we can move on to PRing the shared API format. MyMonero is looking forward to PRing improvement proposals we've been cooking up!

vtnerd commented 5 years ago

Nothing needs to be changed for bulletproofs. The broadcast tx API takes the entire transaction as hex-ascii bytes - so the client needs to support computing the bulletproof values and encoding it in the monero specific format.

moneroexamples commented 5 years ago

@paulshapiro @vtnerd Yep, i think it can be merged. For now I don't have further comments or questions.

moneroexamples commented 5 years ago

Quick question. mymonero-app-js is using same api? Or this api is specific to only mymonero.com website?

paulshapiro commented 5 years ago

Same API (app-ios too)

moneroexamples commented 5 years ago

Was wondiring if it would be possible to extend mymonero api specs with actual examples of the requests and responses? For example, for some public stagenet account?

paulshapiro commented 5 years ago

Wanted to share this as we get the process going for the lightwallet standard. Seems to be working pretty well for them..

https://github.com/WebAssembly/proposals

https://github.com/WebAssembly/design

anonimal commented 5 years ago

This PR was sent to the wrong repo. MyMonero has nothing to do with the meta functionality of Monero Project. MyMonero is a private enterprise with a purported CEO @paulshapiro and hired guns who contribute to Monero code when it suits MyMonero's need.

This discussion should be in the Monero code repo or in the MyMonero portion of github, not here. Otherwise, this looks like a shift of the overton window toward accepting Monero Project privatization.

paulshapiro commented 5 years ago

This PR was sent to the wrong repo.

Well, @anonimal, aside from this PR being able to be marked as a "Do not merge" because its function (which has been fulfilled) was to facilitate establishing the details of standardizing the first common lightwallet API…

MyMonero is a private enterprise with a purported CEO @paulshapiro and hired guns who contribute to Monero code when it suits MyMonero's need.

Are you saying that OpenMonero doesn't exist, that wallet2 does not contain lightwallet functionality to this very API, and that there is not an apparently valid lightwallet server PR to the Monero project at the moment? The "privatization" you accuse us of consists of other people dumping a lot of money into open sourcing a bunch of software that we had virtually zero short or medium term incentive to open source. What is your deal, man?

paulshapiro commented 5 years ago

I mean if the issue is that you're getting spammed with email notifications then there are solutions for that.

anonimal commented 5 years ago

Hey Shappy, try reading what people actually write and take more than a few minutes to think of a response. You'll find that the answers are already there.

HIPERLENDARIO commented 5 years ago

Hi, I'm trying to create a Miner of Monero with JavaScript and Ajax, and HTML5, I wanted to know if it's okay to use 32-bit variables either for the signed integer or for the unsigned integer, and how they will be sent by JSON to the Network. Not has problem right????

paulshapiro commented 4 years ago

Can we close this PR or …? @vtnerd @ndorf