-
Notifications
You must be signed in to change notification settings - Fork 0
Commit
- Loading branch information
There are no files selected for viewing
4 comments
on commit bcc43c2
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For some reason this commit triggers a failure in the test plan, because this cancel fails:
https://github.com/dexX7/mastercore-rpc-tests/blob/master/test_meta_dex_plan.py#L1021
It's caused by a price mismatch, which was introduced by the trade that happened right before the cancel operation:
https://github.com/dexX7/mastercore-rpc-tests/blob/master/test_meta_dex_plan.py#L994
Here is the log with the relevant parts, where "correct" refers to the test run, which doesn't fail, based on a build of one commit before this one, and "faulty" refers to the test run, which fails, based on this commit:
It's known that there are issues with the numbers, but this is what makes it complicated now. So maybe you have an idea, and it would be great to know, whether you are certain, if this commit is fine?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As far as I can see in the old version neworder_buyersprice
is inserted, while in the new version new_mdex.effectivePrice()
is inserted.
In the old version, neworder_buyersprice
equals the price the offer had before the trade.
In the new version, new_mdex.effectivePrice()
equals the price the offer has after the trade.
So if I'm not completely mistaken, this is correct and expected behavior, and the failure is caused, because the original unit price isn't honored.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
First, rather messy, take on honoring the original price, build upon this branch:
bitcoin/bitcoin@bcc43c2...dexX7:wip-oc-0.10-mdex-prices-dirty
RPC fields have to be adjusted, and I haven't tested, whether persistence works, but I really wanted to test it, and it passes the tests.. :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In the new version, new_mdex.effectivePrice() equals the price the offer has after the trade.
That's right yep, I felt it was better to fix the CMPMetaDEx object so it always returned the original price.
First, rather messy, take on honoring the original price, build upon this branch
Had a quick skim through this - looks awesome thanks dude, will spin it up when I'm back after the weekend
Quick note: this should be
const CMPMetaDEx& objMetaDEx
, because otherwise a clone/copy of the object is created, which isn't necessary.