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

Bring back send_step2__try_create_transaction #92

Closed gutenye closed 5 years ago

gutenye commented 5 years ago

Alought we have the new async_send method, this PR bring back send_step2__try_create_transaction (was removed in https://github.com/mymonero/mymonero-core-js/commit/1ba894aed3e3fff246cdd790c75f53068603e3c9)

async_send is a full feature all-in-one solution for sending XMR, it uses send_step1_.. and send_step2_.. behind the scene.

send_step2__try_create_transaction is more like a construct_tx_and_get_tx_key util method, it gives you fine control over how to get utxos, calculate fee, select utxos, etc. That's why I think to bringing it back is a great idea.

paulshapiro commented 5 years ago

Hi @gutenye ,

I accidentally merged this but then reverted it because I mistakenly thought it should be in the Essentials class.. but then realized that it should not actually be in the Essentials class.

Can I trouble you to PR again?

Thanks

paulshapiro commented 5 years ago

Actually I can probably revert the revert, so nevermind

gutenye commented 5 years ago

Thanks for the merge! BTW, Which kind of functions should be in the Essentials class?

paulshapiro commented 5 years ago

Hi,

Good question, and I must say that the need for the question indicates an insufficiency in the naming of the class (at least in the ideal case) … basically, the Essentials class contains everything which libapp-js also uses. It's not preferable that we have this hard separation between things which core-js exposes in its bridge and those which are only exposed by libapp-js (for example, libapp-js uses its own send function since libapp-cpp now encapsulates the entire form handler), but in the absence of a better solution, such as one where we can specify which functions we need in the bridge code, inheritance has been the best option so far.

–Paul

paulshapiro commented 5 years ago

Related: https://github.com/mymonero/mymonero-core-js/issues/58