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

0.0.10 RPC changes #105

Merged
merged 76 commits into from
Jul 4, 2015

Conversation

zathras-crypto
Copy link

Primarily brings in the new RPC population function to centralize data retrieval.

WIP.

- listtransactions_MP will no longer spam all STO recipients
- getsto_MP will now use populateRPCTransactionObject to add extended details
- Additional minor formatting
Status is enumerated in multiple places - this function can thus provide for these duplications to be removed
… errors logged

Iterating STO receipts always comes up with an empty receipt at the end due to extra trailing comma
Note - any feedback on discussion comments at top of function welcome
Note - should be pretty much functional now, all that should be remaining is to sort the response array by confirmations
Note, not passed `const` as step_x stages need to modify the object
Note: see discussion comments about gettradehistory_MP - will become:
gettradehistoryforaddress_OMNI
gettradehistoryforpair_OMNI
…t_OMNI

Notes:
Experimenting with applying a similar concept per the UI of 'market'.
Since one side of the pair must always be MSC/TMSC, and we enforce the usage of MSC unit prices in RPC, it is therefore feasible to extend the "buyer/seller" concept to the RPC layer for simplicity.
@dexX7
Copy link
Member

dexX7 commented Jul 3, 2015

Really great progress! :) Thanks for addressing all nits and remarks.

@zathras-crypto
Copy link
Author

OK, I think that's about it for this RPC PR before it gets totally out of control hehe! The Meta DEx calls seem better now and I think a lot of stuff is cleaned up.

The last bit I'd like to do is now rename everything _MP to _OMNI and alias in the old names - what do you think? Time to reopen the suffix or no suffix debate? hehe

@dexX7
Copy link
Member

dexX7 commented Jul 4, 2015

Yup, great. So this is basically ready?

I don't like changing the suffix though.. but mostly because of _OMNI itself.

Time to reopen the suffix or no suffix debate?

Hehe.. I think so, but I'm still out of ideas. In the "worst case", I'd choose omni_xxxx.

@zathras-crypto
Copy link
Author

Yup, great. So this is basically ready?

Yeah I think so :)

I don't like changing the suffix though

It's just the idea of introducing new users to the _MP concept I don't like - if we've moved away from "Master Protocol" I see no real reason to maintain it, and the "half the calls end in _MP and half end in _OMNI" thing just seems unclean.

We'd always keep the calls aliased for backwards compatibility, but I think standardizing is the right way to go - just standardizing on what seems to be the difficult question haha

@dexX7
Copy link
Member

dexX7 commented Jul 4, 2015

I think standardizing is the right way to go - just standardizing on what seems to be the difficult question haha

I agree, it should be updated. I really, really hoped there were a few more opinions or suggestions though. :)

@zathras-crypto
Copy link
Author

sendtrade_OMNI
sendcanceltradesbyprice_OMNI
sendcanceltradesbypair_OMNI
sendcancelalltrades_OMNI
sendtoowners_OMNI
sendrawtx_OMNI
sendtrade_omni
sendcanceltradesbyprice_omni
sendcanceltradesbypair_omni
sendcancelalltrades_omni
sendtoowners_omni
sendrawtx_omni
OMNI_sendtrade
OMNI_sendcanceltradesbyprice
OMNI_sendcanceltradesbypair
OMNI_sendcancelalltrades
OMNI_sendtoowners
OMNI_sendrawtx
omni_sendtrade
omni_sendcanceltradesbyprice
omni_sendcanceltradesbypair
omni_sendcancelalltrades
omni_sendtoowners
omni_sendrawtx
sendtrade_OL
sendcanceltradesbyprice_OL
sendcanceltradesbypair_OL
sendcancelalltrades_OL
sendtoowners_OL
sendrawtx_OL
olsendtrade
olsendcanceltradesbyprice
olsendcanceltradesbypair
olsendcancelalltrades
olsendtoowners
olsendrawtx
omnisendtrade
omnisendcanceltradesbyprice
omnisendcanceltradesbypair
omnisendcancelalltrades
omnisendtoowners
omnisendrawtx
om_sendtrade
om_sendcanceltradesbyprice
om_sendcanceltradesbypair
om_sendcancelalltrades
om_sendtoowners
om_sendrawtx

@dexX7
Copy link
Member

dexX7 commented Jul 4, 2015

My preference:

  • om_sendtrade
  • omni_sendtrade
  • omnisendtrade

@dexX7 dexX7 merged commit 5ed8141 into OmniLayer:omnicore-0.0.10 Jul 4, 2015
dexX7 added a commit that referenced this pull request Jul 4, 2015
5ed8141 RPC: Make gettradehistoryfor*_OMNI calls threadsafe (zathras-crypto)
5473bd9 RPC: fix param & response ordering for gettradehistoryforpair_OMNI (zathras-crypto)
ac2cd99 RPC: Use count param and order correctly in getTradesForPair() (zathras-crypto)
6b31eb2 RPC: lock in gettradehistoryforpair_OMNI (zathras-crypto)
8e567b5 RPC: minor syntax change to gettradehistoryforpair_OMNI help (zathras-crypto)
d88d09e RPC: missed fixing title (zathras-crypto)
e8b3538 RPC: Fix help for gettradehistoryforpair_OMNI (zathras-crypto)
dd6610f RPC: minor fixes to params (zathras-crypto)
d4292bf RPC: clean up getsto_MP & gettrade_MP (zathras-crypto)
53cc85e RPC: Add populateFailure & clean up gettransaction_MP (zathras-crypto)
f7ab52c TRADEDB: Clear both types of record in a reorg (zathras-crypto)
386e13c RPC: remove unused sort compare (zathras-crypto)
a4456d2 RPC: clean up gettradehistoryforpair_OMNI (zathras-crypto)
9377a8c RPC: Clean up getorderbook_MP (zathras-crypto)
b7f27eb RPC: Remove old leftover gettradessince_MP call (zathras-crypto)
222af43 Fix getTradesForAddress should filter on both property fields (zathras-crypto)
2237056 RPC: Update help for gettradehistoryforaddress_OMNI (zathras-crypto)
a265b8e DB: Use trade database to record new trades for history purposes NOTE: also rewritten gettradehistoryforaddress_OMNI() accordingly (zathras-crypto)
3b54b3d UI: display received amount not total amount for STOs we didn't send (zathras-crypto)
5b1545a RPC: Minor fixes to gettradehistoryforaddress_OMNI (zathras-crypto)
5c19c20 getTradesForAddress - use reference instead of pointer (thanks @dexX7) (zathras-crypto)
f472382 FetchWallet - Add missing lock (zathras-crypto)
14aa571 UI: Move txhistory to use the new history fetcher (zathras-crypto)
139a3c6 RPC: Add support for our pending transactions to RPC populater (zathras-crypto)
c62d594 RPC: Rewrite listtransctions_MP to use new history fetcher (zathras-crypto)
41b0ba1 WALLET: Move wallet transaction retrieval into FetchWalletOmniTransactions() (zathras-crypto)
99702eb RPC: Remove use of boost::to_string (zathras-crypto)
2bb24e2 RPC: Rewrite listtransactions_MP sort & STO handling (zathras-crypto)
ea000f5 RPC: Add locks to refactored RPC populator (zathras-crypto)
b15678a Remove temp performance counters (zathras-crypto)
c449879 RPC: rewrite gettradehistoryformarket > gettradehistoryforpair (zathras-crypto)
85319ae RPC: cleanup getMatchingTrades() + rename amountBought > amountReceived (zathras-crypto)
064a528 MDEX: Remove unneeded line for zero amounts remaining (zathras-crypto)
3bbe965 RPC: Fix parameters passed to FormatMP (thanks @dexX7) (zathras-crypto)
2fa8ad0 MetaDEx: Add getAmountToFill() to CMPMetaDEx and RPC (zathras-crypto)
bdfddb1 RPC: remove leftover HEAD tag & fix minor build issues from merge (zathras-crypto)
164d61a Temp perf (zathras-crypto)
01b773b Temporary Perf Testing Commit (zathras-crypto)
803e488 RPC: rename "subaction" to "action" in DEx RPC tx objects (zathras-crypto)
183aa28 RPC: add missing std:: & use sort instead of stable_sort (zathras-crypto)
e8ac82d RPC: sort response for gettradehistoryformarket_OMNI (zathras-crypto)
d8fcfd2 RPC: replace gettradehistoryforpair_OMNI with gettradehistoryformarket_OMNI (zathras-crypto)
b109a9e RPC: add initial gettradesforpair_OMNI (WIP) (zathras-crypto)
0821ec7 RPC: renmae gettradehistory and remove getopenorders from rpcserver.cpp Note: see discussion comments about gettradehistory_MP - will become: gettradehistoryforaddress_OMNI gettradehistoryforpair_OMNI (zathras-crypto)
864499d RPC: discussion commentary for gettradessince_MP (zathras-crypto)
303c994 RPC: remove non-implemented getopenorders_MP call (use getorderbook_MP) (zathras-crypto)
96f2a92 RPC: clean up getorderbook_MP (zathras-crypto)
e07d2b8 UI: Use extended functions of RPC to populate tx info dialogs (zathras-crypto)
8fed3d8 RPC: Replace pointer with reference for txobj (zathras-crypto)
6f584e1 RPC: add forward dec for CTransaction class after removal of tx.h include (zathras-crypto)
3afaca1 RPC: use references instead of copying CMPTransaction objects Note, not passed `const` as step_x stages need to modify the object (zathras-crypto)
bbfdd56 RPC: use forward dec for CMPTransaction instead of including tx.h (zathras-crypto)
1903147 RPC: Make define for rpctxobject more unique (zathras-crypto)
ba9f677 RPC: reorder valid attribute position to fix crowdsale reporting (zathras-crypto)
ce3bbb9 RPC: Refactor populateRPCTransactionObject() (zathras-crypto)
734014e TRADEDB: minor comment fix (zathras-crypto)
e21cc75 RPC: fix up gettradehistory_MP Note - any feedback on discussion comments at top of function welcome Note - should be pretty much functional now, all that should be remaining is to sort the response array by confirmations (zathras-crypto)
c4e261e TRADEDB: Add getTradesForAddress() function for historical lookup (zathras-crypto)
dde5e96 RPC: gettradehistory should return trade objects with status & matches (zathras-crypto)
a092c8a RPC: fix numerous "STODB Error - number of tokens is not as expected" errors logged Iterating STO receipts always comes up with an empty receipt at the end due to extra trailing comma (zathras-crypto)
dcaf11e RPC: Move trade extended details into populateRPCTransactionObject (zathras-crypto)
d719462 METADEX: Ensure getMatchingTrades zeros totals before starting eval (zathras-crypto)
127bee1 METADEX: Add MetaDEx_getStatus() function Status is enumerated in multiple places - this function can thus provide for these duplications to be removed (zathras-crypto)
d8c1665 RPC: Move STO extended details into populateRPCTransactionObject - listtransactions_MP will no longer spam all STO recipients - getsto_MP will now use populateRPCTransactionObject to add extended details - Additional minor formatting (zathras-crypto)
6b07b48 RPC: clean up populateRPCTransactionObject formatting a little Note no functional change (zathras-crypto)
aa01899 RPC: Start rewriting gettradehistory_MP - **WIP** (zathras-crypto)
@zathras-crypto
Copy link
Author

My preference:
om_sendtrade
omni_sendtrade
omnisendtrade

Thinking about it since it's an API I don't really see the need to shorten omni_ to om_ so I'll put omni_ as my preference.

We don't really seem to get much other feedback on this so and since omni_ is your second preference also I went ahead with it to get the standardization moving. Please see PR #113

@dexX7 dexX7 modified the milestone: 0.0.10.0 Dec 30, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants