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

Resolve merge conflicts after meta DEx RPC enabler PR #61

Conversation

dexX7
Copy link

@dexX7 dexX7 commented Apr 9, 2015

OmniLayer#8 touched some parts, which caused merge conflicts for this branch.

dexX7 added 4 commits March 27, 2015 12:31
Should have no functional effect, as parsing transactions is guarded by the `isTransactionTypeAllowed` checks.
This commit adds all missing checks before executing "cancel-at-price",
"cancel-all-for-pair" and "cancel-everything".

Furthermore the behavior of "cancel-everything" was enhanced, such that
it cancels all open orders, if offered and desired properties are within
the same ecosystem, and otherwise it cancels all open orders for all
properties of both ecosystems.
CMPOffer:
- subaction was undefined

CMPMetaDex:
- still_left_for_sale is already in initializer list
- addr.empty() should be addr.clear() (addr.empty() is faulty here)
- std::string has default constructor
- therefore addr.empty() was removed
- other constructor should use initializer list
5e5be38 [Meta DEx] fix initializer list of CMPMetaDEx, CMPOffer (dexX7)
b720738 [Meta DEx] Add missing "cancel" checks + "cancel-everything" logic (dexX7)
774229c [Meta DEx] enable, but hide RPC calls (dexX7)
@dexX7
Copy link
Author

dexX7 commented Apr 9, 2015

Not yet finished. One moment..

OmniLayer#8 touched some parts, which caused merge conflicts for this branch.

Conflicts:
	src/mastercore_rpc.cpp
	src/rpcclient.cpp
	src/rpcserver.cpp
@dexX7 dexX7 force-pushed the oc-0.10-pr-13-resolve-mdex-merge-conflicts branch from 23f8791 to 07100c5 Compare April 9, 2015 10:33
@dexX7
Copy link
Author

dexX7 commented Apr 9, 2015

Alright. This seems to do it, even though you may just pull the updated omnicore-0.0.10 branch in yourself. For an easier comparison, the actual merge conflicts can be addressed by the changes of the following commit: dexX7@23f8791 - ... though cherry-picking this one alone doesn't seem to do the trick, because trade_MP for example was moved somewhere inbetween.

Sorry for the trouble!

zathras-crypto added a commit that referenced this pull request Apr 9, 2015
…nflicts

Resolve merge conflicts after meta DEx RPC enabler PR
@zathras-crypto zathras-crypto merged commit 2fb849c into zathras-crypto:0.0.10-Z-ClassC Apr 9, 2015
@zathras-crypto
Copy link
Owner

No trouble at all :)

@zathras-crypto
Copy link
Owner

Just a quick one to note test plan for MetaDEx passes so likely test plan is insufficient.

The UI throws up examples that are easy to spot, for example:
image

There should be matching going on here. Dumping the MetaDEx state we can pluck out a couple of offers that clearly should have been matched/traded:

0.20000000000000000000000000000000000000000000000000= 5.00000000000000000000000000000000000000000000000000:mpZATHupfCLqet5N1YL48ByCM1ZBfddbGJ in 311791/028, txid: ee5272132f , trade #1 2.00000000 for #12 10.00000000
5.00000000000000000000000000000000000000000000000000= 0.20000000000000000000000000000000000000000000000000:mpZATHupfCLqet5N1YL48ByCM1ZBfddbGJ in 306354/005, txid: 9572150174 , trade #12 91.00000000 for #1 18.20000000

Note they're the same address but this is permitted (quoting spec)

The protocol then finds existing sell orders that qualify (match), possibly including existing sell orders from that same address.

I'm itching to get into the nuts and bolts of MetaDEx but am really trying to get through the work I've been assigned first (which I think is getting there) so sorry I'm not being much help with specifics - figured it's best to note these things even if it's just for myself when I can circle back round to it :)

@dexX7
Copy link
Author

dexX7 commented Apr 10, 2015

Well, this was sort of expected.. :) I suggest you keep posting any flaws you find, and pin down the underlying issue (or issues).

@dexX7
Copy link
Author

dexX7 commented Apr 10, 2015

I'm confused. A unit price of 20'000'000? Shouldn't it be 0.2 TMSC? Sold 10x IndivisibleToken for 0.2 TMSC each (= 2.0 TMSC total).

dex_issues

@zathras-crypto
Copy link
Owner

Clearly something wrong there
image

@zathras-crypto
Copy link
Owner

Note listing sells via the UI is likely still broken (I haven't looked at refreshing sendTrade() from the old stuff from Dec14 yet)

@zathras-crypto
Copy link
Owner

Oh, you're in regtest mode nevermind hehe

@zathras-crypto
Copy link
Owner

Just sent the same transaction from the UI on testnet, 10 indivisible tokens for 0.2 each - looks OK in pending, will see what happens when it confirms:
image

@zathras-crypto
Copy link
Owner

Ahh nevermind, I thought I'd got all the divisibility display bugs out yesterday but seems a couple still remain... Working on it..

@dexX7
Copy link
Author

dexX7 commented Apr 10, 2015

Hehe, at some point you'll probably look back and wonder: "why didn't I use regtestnet earlier?" :)

Todo note to myself: tutorial for spock + some basic, manual regtest stuff.

@zathras-crypto
Copy link
Owner

at some point you'll probably look back and wonder: "why didn't I use regtestnet earlier?"

Hehe I have no doubt mate, I have no doubt :)

Think 0f5baed should sort that price display issue mate.

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

Successfully merging this pull request may close these issues.

2 participants