Rpc enhancements

[ghost]

depends/includes on https://github.com/mastercoin-MSC/mastercore/pull/196

[dexX7]

@faizkhan00: please help me to understand and correct me where I'm wrong:

1. metadex_obj are created by add_mdex_fields()
2. metadex_obj are used to describes or represents an offer
3. sort_mdex_obj() sorts an array of metadex_objs
4. metadex_objs are sorted based on metadex_obj[10]
5. value of metadex_obj[10] is the block height of an offer
6. sort_mdex_obj() therefore sorts metadex_objs based on their block height
7. metadex_objs at similar height are further sorted based on the lexical value of metadex_objs[1]
8. value of metadex_obj[1] is the txid of an offer
9. metadex_objs at similar height are therefore further lexically sorted based on txids of an offer
10. sort_mdex_obj() is used by gettradessince_MP and gettradehistory_MP
11. sort_mdex_obj() is also used by getorderbook_MP

I think I got this.

I'm wondering about three points:

• an orderbook should primarily be sorted by price, not chronologically based on height
• it seems as if this approach ignores position of an order within a block
• what's the purpose of sorting based on txids?

[ghost]

Dexx, your understanding is correct:

I'm wondering about three points:

*an orderbook should primarily be sorted by price, not chronologically based on height
*it seems as if this approach ignores position of an order within a block
*what's the purpose of sorting based on txids?

1 - An orderbook UI should be sorted by price, but the RPC layer is for integrators who almost always will use the RPC as a starting point for seeding their data management layer. I'm pretty much expecting that integrators add UI niceties on their own after they consume the data at the RPC, and thus the sort by blockheight seemed like a reasonable ordering to go with.

2 & 3 - I initially was looking to sort by the mp_tx_idx (mp tx index within a block) since that information is readily available, but ended up using lexical sorting because the mp_tx_idx data isn't always accurate (sometimes the same number is tagged per tx per block). I didn't try attaching more data to the CMPMetaDEx object for use (such as txindex) because the internals are already suffering from major changes, but could be changed down the line

[dexX7]

Ah, I see your point and reasoning and I believe it's based on terminology and how things are implemented at the moment.

gettradessince_MP, gettradehistory_MP and getorderbook_MP currently show offers, not trades. In fact they mostly differ by one or two lines.

There are three relevant things imho:

1. Information about trades - the result of matched and executed offers. Chronological order is the way to go. This is what gettradessince_MP, gettradehistory_MP should deliever.
2. Information about unmatched and active offers - a representation of the current market. An order by price seems more useful in this case. This is probably what getopenorders_MP aims for.
3. Information about historical offers - useful to reconstruct an "order book" of an earlier point in time. This is sort of what getorderbook_MP could be used for.

[dexX7]

sometimes the same number is tagged per tx per block

How is that possible? It the method broken?

[ghost]

How is that possible? It the method broken?

Perhaps not broken, I looked at the code a little closer and it may be that I was seeing something else at the time. I think sorting by idx is ideal so I'm all for revisiting this (possibly removing lexical sort?), because having some kind of determinism in the output is important, and I might have just confused myself looking at all the numbers :X

You can see the code to generate idx here: https://github.com/mastercoin-MSC/mastercore/blob/mscore-0.0.9/src/mastercore.cpp#L1539-L1544

[ghost]

gettradessince_MP, gettradehistory_MP and getorderbook_MP currently show offers, not trades. In fact they mostly differ by one or two lines.

This should change soon, there was a recent series of commits by Zathras that made historical trades accessible, I should be working on that over the weekend so these calls display trade data similar to the new gettrade_MP

good points on unmatched offers, currently there isn't any storage of active/inactive (orders are simply removed when filled) but there should be detection and exposure of the data on some level (not quite sure where but as you alluded to getopenorders_MP could be the place for it)

[dexX7]

Ah thanks.

I found something odd by the way while testing those calls, maybe it's just me, but you could try the following:

mastercore-cli gettradessince_MP 1407437168 | grep '"block" :'

This looks fine, an ordered list of block numbers from the trades. And then:

mastercore-cli gettradessince_MP 1407437167 | grep '"block" :'

Or any number less than 1407437168. All blocks have a height of -1 for me.

[zathras-crypto]

Yep good discussion - as per our Skype chats I only make completed trades available in tradelistdb which fits with DexX's comments...

To recap that discussion: Trades are matched offers that have actually traded tokens Offers are expressions of willingness to trade that may or may not have been traded (open / closed)

Thus gettradessince and gettradehistory are reflective of trades and thus can reference the historic tradelistdb, but getorderbook are reflective of offers so must reference the current metadex map.

Hope that helps :) On 08/11/2014 3:56 PM, "Faiz Khan" notifications@github.com wrote:

gettradessince_MP, gettradehistory_MP and getorderbook_MP currently show offers, not trades. In fact they mostly differ by one or two lines.

This should change soon, there was a recent series of commits by Zathras that made historical trades accessible, I should be working on that over the weekend so these calls display trade data similar to the new gettrade_MP

good points on unmatched offers, currently there isn't any storage of active/inactive (orders are simply removed when filled) but there should be detection and exposure of the data on some level (not quite sure where but as you alluded to getopenorders_MP could be the place for it)

— Reply to this email directly or view it on GitHub https://github.com/mastercoin-MSC/mastercore/pull/197#issuecomment-62245868 .

[ghost]

Yep good discussion - as per our Skype chats I only make *completed* trades
available in tradelistdb which fits with DexX's comments...

Are there any plans to make offers available anywhere? from Dexx's comments that information will be needed eventually (and not having it would be an oversight), just wondering about your 0.02c there...

edit: I guess there won't be getoffershistory? Or a historical offer list (including empty/unmatched). This seems like an oversight in my view, but then again what do I know :)

[zathras-crypto]

Offers are always available from the blockchain so no need to store a second time - I already modified your metadex case in populateRPCTransactionObject to use parsing off the wire - tl:dr you can get any metadex offer past/present just with regular gettransction_MP like any other tx :)

Thanks Z On 08/11/2014 4:20 PM, "Faiz Khan" notifications@github.com wrote:

Yep good discussion - as per our Skype chats I only make completed trades available in tradelistdb which fits with DexX's comments...

Are there any plans to make offers available anywhere? from Dexx's comments that information will be needed eventually (and not having it would be an oversight), just wondering about your 0.02c there...

— Reply to this email directly or view it on GitHub https://github.com/mastercoin-MSC/mastercore/pull/197#issuecomment-62246454 .

[dexX7]

What's your interpretation of an offer? I actually assumed the results of gettradessince_MP, gettradehistory_MP and getorderbook_MP are offers, at least at the moment.

[zathras-crypto]

I see an offer as a message offering trade terms, but it does not become an actual 'trade' until tokens are exchanged.

Trade history for example should show your history of trades not history of offers - offers aren't trades and thus shouldn't be part of any gettrade calls imo.

Thanks Z On 08/11/2014 5:00 PM, "dexX7" notifications@github.com wrote:

What's your interpretation of an offer? I actually assumed the results of gettradessince_MP, gettradehistory_MP and getorderbook_MP are offers, at least at the moment.

— Reply to this email directly or view it on GitHub https://github.com/mastercoin-MSC/mastercore/pull/197#issuecomment-62247301 .

[dexX7]

Well yeah, I was just wondering about "Are there any plans to make offers available anywhere?".

Or it's because I'm not aware of the whole picture and there is an obstacle, for example the permanent deletion of offers that were matched.

@zathras-crypto: the reason why I'm actually not such a mega huge fan of gettransaction_MP is because parsing and interpretation are done on the fly. "Parse once, verify and seal" seems more robust.

Regarding this PR: Sorting of CMPMetaDEx in vectors can be done with bool MetaDEx_compare::operator() in combination with std::sort:

std::vector<CMPMetaDEx> vMetaDexObjs;  // holds unordered data
...
MetaDEx_compare compareByHeight;       // in mastercore_dex.h
std::sort (vMetaDexObjs.begin(), vMetaDexObjs.end(), compareByHeight);  // blk height + tx pos

[ghost]

Dexx, can you remake your PR onto this repo directly? I'm not sure the merge of your code + my code makes sense now that I look at it ( from dexX7/mcore-order-last-since-by-position to mastercore/0.0.9)

Thank you sir :)

[dexX7]

Yup, I can. Are you fine with the MetaDexObjectToJSON function and the rest?

[ghost]

I think so, I came here to review the fully merged code and it was a mess, so once we have something clean it'll be easier to tell if something needs to be fixed :)