Meta DEx progress #9
Comments
|
@dexX7 this is fantastic work (as always) :) thanks a lot! Agreed a more in depth test plan should be devised, I remember when I first looked at it I found it difficult to accept something as complex as MetaDEx could be tested (including all possible fringe cases) with just 10 scenarios comprising under 100 steps in total across them all. But since I'm not experienced in software testing, I can't speak with any authority on this and will simply say, yep I support more in depth testing 100%. Regarding the "all tests pass" this highlights the test plan is insufficient furthermore. There are known bugs in the math according to @m21 (he said we should see them pretty easily) so if everything is passing, those math issues are not being picked up in the testing. P.S. The spreadsheet shows a bunch of failure results - these now pass is that correct? Thanks again dude |
It would be incredibly useful, if those were documented.
@faizkhan00 spent a lot of good work on testing the plan on testnet, but the plan was changed in the meantime here and there, if I recall correctly. But if you were referring to the rows with Good tests don't require millions of lines, but rather breaking down the problem, ideally starting with unit tests. I agree nevertheless, and what I currently miss are tests that tackle rounding issues. |
Agreed, I recall @m21 saying they should be easy to spot, but paging @m21 - do you have any notes on those MetaDEx math bugs you mentioned bud?
Personally I didn't like the way the spreadsheet gave results. Hover over most of those 'invalid' cells to see the tooltip - you'll note it's a fail result on a good percentage of them, eg:
Understood, and that's why I'm happy people like yourself, Marv and Sean are handling the testing aspects because I just don't have the experience sadly. Infrastructure testing leverages a lot of experience, I can only assume software testing is a similar situ. So my thanks there to you guys :) |
Expected result: invalid, actual result: fail => test passed.. ;) Probably not very intuitive, yes. But as mentioned, the test plan was changed here and there, so I wouldn't give much about those transactions. Row 21 to 131 are coded, with some minor differences at the very beginning ( The STO tests (and using spock in general!) was a game changer, because the way they were created, it's possible to extend the tests, simply by adding new lines to sto-testplan.tsv. Even though each line represents an atomic test, something similar could be done for whole test sequences. Another awesome aspect of bitcoin-spock is that specifications can basically be coded line by line, here is an example: DexSpec.groovy + the test report. Most of the quoted lines were directly taken from spec#sell-mastercoins-for-bitcoins. Anyway, sorry for the OT, and back to the actual topic. :) |
Yeah, that's - hmm hehe. I've received many, many test reports over my years in infra and when a test result is a fail, the test is a fail. Getting into semantics now but surely "Expected outcome vs actual outcome => result" or similar would be better? Again perhaps it's just my lack of SW testing experience, but when I see "result: fail" on test reports, that leads me to believe the test has failed. It's probably just the way I'm used to working :) I did however for example look up the first transaction, which is supposed to be 2.00000000 TMSC for 2.00000000 TDiv1 which is invalidated because the TX version is >1. But when we look at the transaction the amounts don't seem right and the version appears to be fine: However that version display is clearly inaccurate, and there are some interesting things happening on the back end... Black holing??? What!!! haha EDIT: In case not clear, I thought we had abandoned the idea of black holing. |
Hehe yeah, I agree, the more expressive and intuitive, the better.
Sort of.. it actually has no effect at the moment, see src/mastercore.cpp#L974-L982. I do have some ideas in this context, but this is probably material for another thread. |
|
Hm, been a while, I can't seem to find the specifics of the issue I was thinking about, but some general notes:
|
|
Yeah I was surprised to see it still there - IIRC black holing was dropped in favour of an alert system that could shutdown outdated clients (one of the alert types leverages Note this can be considered sensitive, so I did add a CLI option to override the forced shutdown (lest we be accused of having a centralized kill switch). |
|
Hey @m21 :) thanks for the info! |
|
Quick one, regarding the transaction we were discussing in the meeting - I've dumped a debug parse here http://pastie.org/pastes/10097075/text?key=zyhapet9kp6npfj93w9grg for anyone interested - about to start looking through it. |
|
Hmm.. I just noticed my remote testnet node (running 0.9) went into safe mode four days ago (and remains in that state after restarting). Probably when you mentioned the crazy number of new blocks: "Safe mode: Warning: The network does not appear to fully agree! Some miners appear to be experiencing issues." Let's see, if the 0.10 based Omni Core can handle it. :) |
|
Somewhat shorter log: The bad state was appearingly created via 84ceae5eec2b933d170f63955fa19533da0a3ea02fdfa1ee51dc23e9f5b61e32: Edit: when executing this trade alone it matches and the order of A1 is filled completely.. maybe I read the log wrong. |
|
As long as there's still a non-zero amount for sale, it shouldn't matter that the amount desired is zero. It's perfectly ok for a seller to receive more than the desired amount. And, the matching is done based on the original unit price, not the remaining amount desired. As for this log entry:
It appears to be related, and it's not clear why the unit price and reciprocal have changed from their original values. An order's unit price never changes. From the spec (highlighted for emphasis):
There is a test to sell the largest possible amount of indiv for the smallest possible amount of div desired. So, a unit price can never be calculated as zero, using at least 27 decimal digits. |
|
@marv-engine: I'm still in the phase where I figure out the implementation details, but after the very first trade: There is this debug line:
... and this slightly off unit price is due to the ratio of Edit: I think there might be the underlying issue: the original unit price is not honored 100 %, but only approximated by a representable ratio, which carries through the following trades. I'm further not sure, if an offer of |
|
@dexX7: The unit price is calculated once, based on the original amounts, in this case "A1 offers 100.00000000 SPX for 10.00000000 MSC". After the match and transfers, the remaining amount for sale "92.72727273 SPX" is still offered at the original unit price (0.10). Can you see why/where the unit price is recalculated? That's not supposed to happen. |
|
Sorry, I actually flipped the initial trade in the post before. It's: ...and not vice versa. This makes a difference, and in this order the unit price is "updated". @marv-engine: as far as I can see there is no concept of an original unit price, or original amounts desired and offered, but only those two amounts, which are updated. My test setup for this is a bit hacky right now, but I think it would help to have a base test template for bitcoin spock, which ideally uses a TSV file as input, and simply dumps results. |
It appears to me that things started falling apart the transaction before that in 8aeb5cdbd282ea2a49e92d94c7f7aa201195ca6377ddb1808b62c46b1a359362 http://pastie.org/pastes/10100648/text?key=9cueyytgdv61v79sad8xw |
|
How do you define "falling apart" in this context?
|
I may be wrong - that was from a very brief look. As you probably guessed by now I'm working on some extra code to help us identify where and when things go wrong - my comment above was just based on that (nothing in-depth). Specifically I see so I look and see 75b687ca67 was not parsed in that block, so other trades must have affected it - so looked at the trades in block 307607 and see two trades (8aeb5cdbd2 & 84ceae5ee). Both trades are flagged as '`PROBLEM!`` and 8aeb5cdbd2 was first in the sequence. Hence the idea that things started going wrong in 8aeb5cdbd2. As I say not thorough at all, I've somewhat segwayed for a day or two to build this auditing thing because I believe overall it'll make tracking down these bugs easier/faster. |
|
Just a quick note, the unit price does in fact change - often. Using the auditor: Need to really up the precision to see it sometimes though (eg 120th digit). |
|
@zathras-crypto: as far as I can see this is, because the original unit price isn't stored at all. I don't feel very comfortable with using floating numbers, and with the unexpected behavior of STO at the beginning, I can only assume this is worse for the meta DEx, given that there are even more calculations, which are further mixed with string -> number conversions, for example here: cpp_dec_float_100 seller_amount_stilldesired = (cpp_dec_float_100) seller_amountLeft * sellers_price;
seller_amount_stilldesired += (cpp_dec_float_100) 0.5; // ROUND UP
std::string str_amount_stilldesired = seller_amount_stilldesired.str(INTERNAL_PRECISION_LEN, std::ios_base::fixed);
std::string str_stilldesired_int_part = str_amount_stilldesired.substr(0, str_amount_stilldesired.find_first_of("."));
CMPMetaDEx seller_replacement = *p_older;
seller_replacement.setAmountForSale(seller_amountLeft, "seller_replacement");
seller_replacement.setAmountDesired(boost::lexical_cast<int64_t>(str_stilldesired_int_part), "seller_replacement");But that said, I haven't really looked at the code yet, and this just looks scarry.. :) I'm currently trying to extend bitcoin-spock for additional test support. |
Yikes... I'd just been kind of feeding off some of the testing discussion - one of those discussions related to unit prices should not be allowed to change so I codified that in the auditor - I see that an existing trade is replaced with new values after a part trade so I guess this is where the problem lies...
I'm not too knowledgeable on this topic, Faiz & Michael talked together a fair bit about integer vs float but I was working on different aspects and didn't get too involved. I believe the end outcome was that our token precision is a maximum of 8 decimal digits, so using such a large float (
Same, I'm by no means fully familiar with the MetaDEx stuff yet either |
Essentially this, and that code for the metadex had to separate the fraction and whole part in calculation, which is why some of those calculations look so strange. I think the simplification is to |
Holy Hallucination Batman!!! Could that really be Faiz!?!?! Hello mate :) Been too long, hope things are cool for you and things are going well :) |
Heh, yea- If I have time this week I'll certainly investigate that library in an attempt to fix the code, I know it looks hiary, I was meaning to slap a big TODO on it, but never had the chance... :) |
|
Wow, the old team's back :)
|
|
Haha, welcome back! :)
I'm wondering, if 128 bit wide integer could be used, similar to the plain integer STO logic, as described here: OmniLayer/spec#301 (comment) mastercoin-MSC#275 mastercoin-MSC#273 Further, it would be very helpful, if @m21 a question to you: That leftover of 3 units, causing an update to |
|
@faizkhan00 - awesome to have you taking a peek back into the guts! |
|
There is clearly a pattern ... :) Here is the trade sequence of the testnet trade and some others I tested to trigger trading of zero amounts:
The section on the left describes the trade, the sections to the right describe the match and the results, whereby the labels and values were pulled from the log file. |
|
Looking at the math here, is it even possible to have the unit price unchanged? Let's take an example. I setup the following trade Then let's say Now, as far as I can tell, there is no possible (valid) amount desired value that can apply this same price. To apply the original price, the amount desired would need to be Since this amount desired is not valid, the closest valid amount desired is TL:DR; after a partial trade, is it even possible to conduct all remaining trades at exactly the same unit price? |
|
|
|
`Looking at the math here, is it even possible to have the unit price unchanged?`` To Dexx: 128 bits didn't work for us. Even 50 digits after the decimal point wasn't enough. That's why 100 digits is the correct number. |
This is a mega thread, with a lot of noise, and probably false information, but the linked post basically summarizes the essence: when calculating the amounts to trade, the unit price is rounded up, to the next tradable unit, constrained by the amounts available, however, for the matching and partially filled orders, the original unit price should prevail. Right now it looks like the price could slowly drift, because the original unit price is not stored. Nevertheless, and re:
I'm not convinced that the drifting of unit prices causes zero trades, and the issue probably starts, when offers, which have some amount left for sale, but no more desire, are added back to the book: Here is an example, where A2 ends up with RPC output with some extra info (using Debug log, with some extra output for the desired amount:
Edit: actually, the desired amount is indeed calculated based on unit price and amount available, but it basically comes down to: |
diff --git a/src/mastercore_dex.cpp b/src/mastercore_dex.cpp
index 0e855ce..4e8bf25 100644
--- a/src/mastercore_dex.cpp
+++ b/src/mastercore_dex.cpp
@@ -281,7 +281,7 @@ const XDOUBLE desprice = (1/buyersprice); // inverse, to be matched against that
if (bBuyerSatisfied)
{
// insert the updated one in place of the old
- if (0 < seller_replacement.getAmountForSale())
+ if (0 < seller_replacement.getAmountForSale() && 0 < seller_replacement.getAmountDesired())
{
file_log("++ inserting seller_replacement: %s\n", seller_replacement.ToString());
indexes->insert(seller_replacement); |
|
Side note, it seems that Boost comes with a rational number class (http://www.boost.org/doc/libs/1_58_0/libs/rational/rational.html) which might simplify the metadex calculations a fair bit- I'll need to do some testing to see if the implementation can support a unlimited precision type on the backend, but given that- we might have a shot at a simplier implmentation of the math |
|
Hey @faizkhan00: this is an interesting library. May I ask in which cases you guys faced trouble with precision? I'm currently looking at the core math, and it looks like there isn't really an incredible number of values and operations involved. Each offer, in it's most basic form (without property identifiers, or block positions), has four properties, and we need to calculate three values on each side, and further, determine whether an offer is cheaper than another offer. I haven't tested it yet with any values, and a second eye would be apprechiated: But if that is correct, and the most complex calculation is basically |
|
@dexX7 those values seem mostly correct - the key trouble area would be: how are you handling the fractional part of those calculations? recall that the orderbook calculations need to retain a certain precision for there not to be rounding errors- this is mainly why just 128 bit integers alone dont suffice, you would need (IIRC) two arbitrary precision integers to have the whole part calcluated to some high accuracy , and the same for the fractional part. You can see an example of this here: https://github.com/OmniLayer/omnicore/blob/omnicore-0.0.10/src/mastercore.cpp#L698-L733 basically, it makes sense to use 128bit ints for some calculations, and its incorrect for use in other areas, depending on the level of precision needed for A) the calculation intermediaries and B) the end result of the calculations |
Please correct me, if I'm wrong, but all we need to know is, whether some order is cheaper (or more expensive) than another order, to find a match. Further, what I forgot earlier: we also need to determine, whether some order's price equals another order's price, for the cancel logic. |
|
Sure, multiplying 2 64bit numbers yields at most a 128bit number. |
Yeah, and looking over the MetaDex code again (whew, its been awhile!) I want to roll back the amount I thought we could optimize/clean up those calcs - they are pretty good as-is, except for the rational number handling, and I'm reasonably confident in saying that it probably isn't worth including new dependencies/build changes for only approximately a 20 line refactor (basically I think a couple of lines of comments achieves the goal of code clarity, more or less) |
Aside from whether it's possible or not, having concrete numbers here would be extremely useful for testing, and should become part of the test plan, so please, also in general, and not limited to the exchange, publish any edge cases or unexpected scenarios, whenever you stumble upon one. :) |
|
I unfortunately lost many of my notes recently. |
Switching from It's an open question though, whether all this may actually have an effect for the numbers we use.
Ah, thanks, I see. Here we go: /**
* price_lhs = lhs_desired / lhs_forsale
* price_rhs = rhs_desired / lhs_forsale
*
* price_lhs < price_rhs
* =>
* int128_t(lhs_desired) * int128_t(rhs_forsale) <
* int128_t(rhs_desired) * int128_t(lhs_forsale)
*
* price_lhs != price_rhs:
* =>
* int128_t(lhs_desired) * int128_t(rhs_forsale) !=
* int128_t(rhs_desired) * int128_t(lhs_forsale)
*/
BOOST_CHECK(int128_t(1) * int128_t(INT64_MAX) < int128_t(2) * int128_t(INT64_MAX));
BOOST_CHECK(int128_t(1) * int128_t(INT64_MAX) != int128_t(2) * int128_t(INT64_MAX));Edit: see here for the DEx related calculations (untested). |
|
Hm, replacing division by multiplication? |
Yes, to avoid fractions and the hassle with floating point numbers altogether.
This works for other operations as well, which might be handy to compare orders: Now in the context of the DEx, we have for example: // ..this is equal to (XDOUBLE) seller_amountGot / sellers_price;
XDOUBLE x_buyer_got = (XDOUBLE) seller_amountGot / ((XDOUBLE) seller_amountWanted / (XDOUBLE) seller_amountForSale);
x_buyer_got += (XDOUBLE) 0.5; // ROUND UP
std::string str_buyer_got = x_buyer_got.str(INTERNAL_PRECISION_LEN, std::ios_base::fixed);
std::string str_buyer_got_int_part = str_buyer_got.substr(0, str_buyer_got.find_first_of("."));
const int64_t buyer_amountGot = boost::lexical_cast<int64_t>(str_buyer_got_int_part);... which can be replaced by (tested): // for integer rounding up: ceil(num / denom) => 1 + (num - 1) / denom
int128_t x_buyer_got = 1 + ((int128_t) seller_amountGot * (int128_t) seller_amountForSale - 1) / (int128_t) seller_amountWanted;
const int64_t buyer_amountGot = x_buyer_got.convert_to<int64_t>();This is basically a similar trick as used for send-to-owners, to get rid of those issues with precision, which caused some trouble for this STO test Marv created.. :) I'm more and more confident that it's possible, in similar fashion, to avoid any use of |
|
Re storing original prices, my initial thought would be to expand the It looks like there may have been some thought down this road as the If we expand the class to keep those two original values, we'll always have the original price available. |
I support this direction. This likely requires an extension in the context of data persistence, as well, which I haven't looked into yet. |
|
This thread became very large, and several issues and topics are disscued at the same time. If there are no objections, I'd like to close this issue (edit: or use it for general discussion related to the meta DEx), and suggest to move to seperated threads from here, to focus on one point at the time. So far we have:
I labeled the related issues, but it is also thinkable to tag them with a milestone. |
|
@dexX7, thanks for consolidating the discussion issues into individual ones. I have no objections to closing this thread, unless as you said, others wish to leave it open for discussion. In any event, let’s at least move the discussion for the items listed above in their individual issues. |
|
^^^ what he said :) |
|
Well then. If there is need for any off-topic discussion, for example unexpected trade sequences, we may rename and reopen, but until then ... |
fac04cb refactor: Add lock annotations to Active* methods (MacroFake) fac15ff Fix logical race in rest_getutxos (MacroFake) fa97a52 Fix UB/data-race in RPCNotifyBlockChange (MacroFake) fa530bc Add ChainstateManager::GetMutex(), an alias for ::cs_main (MacroFake) Pull request description: This fixes two issues: * A data race in `ActiveChain`, which returns a reference to the chain (a `std::vector`), which is not thread safe. See also below traceback. * A corrupt rest response, which returns a blockheight and blockhash, which are unrelated to each other and to the result, as the chain might advance between each call without cs_main held. The issues are fixed by taking cs_main and holding it for the required time. ``` ================== WARNING: ThreadSanitizer: data race (pid=32335) Write of size 8 at 0x7b3c000008f0 by thread T22 (mutexes: write M131626, write M151, write M131553): #0 std::__1::enable_if<(is_move_constructible<CBlockIndex**>::value) && (is_move_assignable<CBlockIndex**>::value), void>::type std::__1::swap<CBlockIndex**>(CBlockIndex**&, CBlockIndex**&) /usr/lib/llvm-13/bin/../include/c++/v1/__utility/swap.h:39:7 (bitcoind+0x501239) OmniLayer#1 std::__1::vector<CBlockIndex*, std::__1::allocator<CBlockIndex*> >::__swap_out_circular_buffer(std::__1::__split_buffer<CBlockIndex*, std::__1::allocator<CBlockIndex*>&>&) /usr/lib/llvm-13/bin/../include/c++/v1/vector:977:5 (bitcoind+0x501239) OmniLayer#2 std::__1::vector<CBlockIndex*, std::__1::allocator<CBlockIndex*> >::__append(unsigned long) /usr/lib/llvm-13/bin/../include/c++/v1/vector:1117:9 (bitcoind+0x501239) OmniLayer#3 std::__1::vector<CBlockIndex*, std::__1::allocator<CBlockIndex*> >::resize(unsigned long) /usr/lib/llvm-13/bin/../include/c++/v1/vector:2046:15 (bitcoind+0x4ffe29) OmniLayer#4 CChain::SetTip(CBlockIndex*) src/chain.cpp:19:12 (bitcoind+0x4ffe29) OmniLayer#5 CChainState::ConnectTip(BlockValidationState&, CBlockIndex*, std::__1::shared_ptr<CBlock const> const&, ConnectTrace&, DisconnectedBlockTransactions&) src/validation.cpp:2748:13 (bitcoind+0x475d00) OmniLayer#6 CChainState::ActivateBestChainStep(BlockValidationState&, CBlockIndex*, std::__1::shared_ptr<CBlock const> const&, bool&, ConnectTrace&) src/validation.cpp:2884:18 (bitcoind+0x47739e) OmniLayer#7 CChainState::ActivateBestChain(BlockValidationState&, std::__1::shared_ptr<CBlock const>) src/validation.cpp:3011:22 (bitcoind+0x477baf) OmniLayer#8 node::ThreadImport(ChainstateManager&, std::__1::vector<fs::path, std::__1::allocator<fs::path> >, ArgsManager const&) src/node/blockstorage.cpp:883:30 (bitcoind+0x23cd74) OmniLayer#9 AppInitMain(node::NodeContext&, interfaces::BlockAndHeaderTipInfo*)::$_7::operator()() const src/init.cpp:1657:9 (bitcoind+0x15863e) OmniLayer#10 decltype(static_cast<AppInitMain(node::NodeContext&, interfaces::BlockAndHeaderTipInfo*)::$_7&>(fp)()) std::__1::__invoke<AppInitMain(node::NodeContext&, interfaces::BlockAndHeaderTipInfo*)::$_7&>(AppInitMain(node::NodeContext&, interfaces::BlockAndHeaderTipInfo*)::$_7&) /usr/lib/llvm-13/bin/../include/c++/v1/type_traits:3918:1 (bitcoind+0x15863e) OmniLayer#11 void std::__1::__invoke_void_return_wrapper<void, true>::__call<AppInitMain(node::NodeContext&, interfaces::BlockAndHeaderTipInfo*)::$_7&>(AppInitMain(node::NodeContext&, interfaces::BlockAndHeaderTipInfo*)::$_7&) /usr/lib/llvm-13/bin/../include/c++/v1/__functional/invoke.h:61:9 (bitcoind+0x15863e) OmniLayer#12 std::__1::__function::__alloc_func<AppInitMain(node::NodeContext&, interfaces::BlockAndHeaderTipInfo*)::$_7, std::__1::allocator<AppInitMain(node::NodeContext&, interfaces::BlockAndHeaderTipInfo*)::$_7>, void ()>::operator()() /usr/lib/llvm-13/bin/../include/c++/v1/__functional/function.h:171:16 (bitcoind+0x15863e) OmniLayer#13 std::__1::__function::__func<AppInitMain(node::NodeContext&, interfaces::BlockAndHeaderTipInfo*)::$_7, std::__1::allocator<AppInitMain(node::NodeContext&, interfaces::BlockAndHeaderTipInfo*)::$_7>, void ()>::operator()() /usr/lib/llvm-13/bin/../include/c++/v1/__functional/function.h:345:12 (bitcoind+0x15863e) OmniLayer#14 std::__1::__function::__value_func<void ()>::operator()() const /usr/lib/llvm-13/bin/../include/c++/v1/__functional/function.h:498:16 (bitcoind+0x88891f) OmniLayer#15 std::__1::function<void ()>::operator()() const /usr/lib/llvm-13/bin/../include/c++/v1/__functional/function.h:1175:12 (bitcoind+0x88891f) OmniLayer#16 util::TraceThread(char const*, std::__1::function<void ()>) src/util/thread.cpp:18:9 (bitcoind+0x88891f) OmniLayer#17 decltype(static_cast<void (*>(fp)(static_cast<char const*>(fp0), static_cast<AppInitMain(node::NodeContext&, interfaces::BlockAndHeaderTipInfo*)::$_7>(fp0))) std::__1::__invoke<void (*)(char const*, std::__1::function<void ()>), char const*, AppInitMain(node::NodeContext&, interfaces::BlockAndHeaderTipInfo*)::$_7>(void (*&&)(char const*, std::__1::function<void ()>), char const*&&, AppInitMain(node::NodeContext&, interfaces::BlockAndHeaderTipInfo*)::$_7&&) /usr/lib/llvm-13/bin/../include/c++/v1/type_traits:3918:1 (bitcoind+0x157e6a) OmniLayer#18 void std::__1::__thread_execute<std::__1::unique_ptr<std::__1::__thread_struct, std::__1::default_delete<std::__1::__thread_struct> >, void (*)(char const*, std::__1::function<void ()>), char const*, AppInitMain(node::NodeContext&, interfaces::BlockAndHeaderTipInfo*)::$_7, 2ul, 3ul>(std::__1::tuple<std::__1::unique_ptr<std::__1::__thread_struct, std::__1::default_delete<std::__1::__thread_struct> >, void (*)(char const*, std::__1::function<void ()>), char const*, AppInitMain(node::NodeContext&, interfaces::BlockAndHeaderTipInfo*)::$_7>&, std::__1::__tuple_indices<2ul, 3ul>) /usr/lib/llvm-13/bin/../include/c++/v1/thread:280:5 (bitcoind+0x157e6a) OmniLayer#19 void* std::__1::__thread_proxy<std::__1::tuple<std::__1::unique_ptr<std::__1::__thread_struct, std::__1::default_delete<std::__1::__thread_struct> >, void (*)(char const*, std::__1::function<void ()>), char const*, AppInitMain(node::NodeContext&, interfaces::BlockAndHeaderTipInfo*)::$_7> >(void*) /usr/lib/llvm-13/bin/../include/c++/v1/thread:291:5 (bitcoind+0x157e6a) Previous read of size 8 at 0x7b3c000008f0 by main thread: #0 std::__1::vector<CBlockIndex*, std::__1::allocator<CBlockIndex*> >::size() const /usr/lib/llvm-13/bin/../include/c++/v1/vector:680:61 (bitcoind+0x15179d) OmniLayer#1 CChain::Tip() const src/./chain.h:449:23 (bitcoind+0x15179d) OmniLayer#2 ChainstateManager::ActiveTip() const src/./validation.h:927:59 (bitcoind+0x15179d) OmniLayer#3 AppInitMain(node::NodeContext&, interfaces::BlockAndHeaderTipInfo*) src/init.cpp:1841:35 (bitcoind+0x15179d) OmniLayer#4 AppInit(node::NodeContext&, int, char**) src/bitcoind.cpp:231:43 (bitcoind+0x133fd2) OmniLayer#5 main src/bitcoind.cpp:275:13 (bitcoind+0x133fd2) Location is heap block of size 232 at 0x7b3c00000870 allocated by main thread: #0 operator new(unsigned long) <null> (bitcoind+0x132668) OmniLayer#1 ChainstateManager::InitializeChainstate(CTxMemPool*, std::__1::optional<uint256> const&) src/validation.cpp:4851:21 (bitcoind+0x48e26b) OmniLayer#2 node::LoadChainstate(bool, ChainstateManager&, CTxMemPool*, bool, Consensus::Params const&, bool, long, long, long, bool, bool, std::__1::function<bool ()>, std::__1::function<void ()>) src/node/chainstate.cpp:31:14 (bitcoind+0x24de07) OmniLayer#3 AppInitMain(node::NodeContext&, interfaces::BlockAndHeaderTipInfo*) src/init.cpp:1438:32 (bitcoind+0x14e994) OmniLayer#4 AppInit(node::NodeContext&, int, char**) src/bitcoind.cpp:231:43 (bitcoind+0x133fd2) OmniLayer#5 main src/bitcoind.cpp:275:13 (bitcoind+0x133fd2) Mutex M131626 (0x7b3c00000898) created at: #0 pthread_mutex_lock <null> (bitcoind+0xda898) OmniLayer#1 std::__1::mutex::lock() <null> (libc++.so.1+0x49f35) OmniLayer#2 node::ThreadImport(ChainstateManager&, std::__1::vector<fs::path, std::__1::allocator<fs::path> >, ArgsManager const&) src/node/blockstorage.cpp:883:30 (bitcoind+0x23cd74) OmniLayer#3 AppInitMain(node::NodeContext&, interfaces::BlockAndHeaderTipInfo*)::$_7::operator()() const src/init.cpp:1657:9 (bitcoind+0x15863e) OmniLayer#4 decltype(static_cast<AppInitMain(node::NodeContext&, interfaces::BlockAndHeaderTipInfo*)::$_7&>(fp)()) std::__1::__invoke<AppInitMain(node::NodeContext&, interfaces::BlockAndHeaderTipInfo*)::$_7&>(AppInitMain(node::NodeContext&, interfaces::BlockAndHeaderTipInfo*)::$_7&) /usr/lib/llvm-13/bin/../include/c++/v1/type_traits:3918:1 (bitcoind+0x15863e) OmniLayer#5 void std::__1::__invoke_void_return_wrapper<void, true>::__call<AppInitMain(node::NodeContext&, interfaces::BlockAndHeaderTipInfo*)::$_7&>(AppInitMain(node::NodeContext&, interfaces::BlockAndHeaderTipInfo*)::$_7&) /usr/lib/llvm-13/bin/../include/c++/v1/__functional/invoke.h:61:9 (bitcoind+0x15863e) OmniLayer#6 std::__1::__function::__alloc_func<AppInitMain(node::NodeContext&, interfaces::BlockAndHeaderTipInfo*)::$_7, std::__1::allocator<AppInitMain(node::NodeContext&, interfaces::BlockAndHeaderTipInfo*)::$_7>, void ()>::operator()() /usr/lib/llvm-13/bin/../include/c++/v1/__functional/function.h:171:16 (bitcoind+0x15863e) OmniLayer#7 std::__1::__function::__func<AppInitMain(node::NodeContext&, interfaces::BlockAndHeaderTipInfo*)::$_7, std::__1::allocator<AppInitMain(node::NodeContext&, interfaces::BlockAndHeaderTipInfo*)::$_7>, void ()>::operator()() /usr/lib/llvm-13/bin/../include/c++/v1/__functional/function.h:345:12 (bitcoind+0x15863e) OmniLayer#8 std::__1::__function::__value_func<void ()>::operator()() const /usr/lib/llvm-13/bin/../include/c++/v1/__functional/function.h:498:16 (bitcoind+0x88891f) OmniLayer#9 std::__1::function<void ()>::operator()() const /usr/lib/llvm-13/bin/../include/c++/v1/__functional/function.h:1175:12 (bitcoind+0x88891f) OmniLayer#10 util::TraceThread(char const*, std::__1::function<void ()>) src/util/thread.cpp:18:9 (bitcoind+0x88891f) OmniLayer#11 decltype(static_cast<void (*>(fp)(static_cast<char const*>(fp0), static_cast<AppInitMain(node::NodeContext&, interfaces::BlockAndHeaderTipInfo*)::$_7>(fp0))) std::__1::__invoke<void (*)(char const*, std::__1::function<void ()>), char const*, AppInitMain(node::NodeContext&, interfaces::BlockAndHeaderTipInfo*)::$_7>(void (*&&)(char const*, std::__1::function<void ()>), char const*&&, AppInitMain(node::NodeContext&, interfaces::BlockAndHeaderTipInfo*)::$_7&&) /usr/lib/llvm-13/bin/../include/c++/v1/type_traits:3918:1 (bitcoind+0x157e6a) OmniLayer#12 void std::__1::__thread_execute<std::__1::unique_ptr<std::__1::__thread_struct, std::__1::default_delete<std::__1::__thread_struct> >, void (*)(char const*, std::__1::function<void ()>), char const*, AppInitMain(node::NodeContext&, interfaces::BlockAndHeaderTipInfo*)::$_7, 2ul, 3ul>(std::__1::tuple<std::__1::unique_ptr<std::__1::__thread_struct, std::__1::default_delete<std::__1::__thread_struct> >, void (*)(char const*, std::__1::function<void ()>), char const*, AppInitMain(node::NodeContext&, interfaces::BlockAndHeaderTipInfo*)::$_7>&, std::__1::__tuple_indices<2ul, 3ul>) /usr/lib/llvm-13/bin/../include/c++/v1/thread:280:5 (bitcoind+0x157e6a) OmniLayer#13 void* std::__1::__thread_proxy<std::__1::tuple<std::__1::unique_ptr<std::__1::__thread_struct, std::__1::default_delete<std::__1::__thread_struct> >, void (*)(char const*, std::__1::function<void ()>), char const*, AppInitMain(node::NodeContext&, interfaces::BlockAndHeaderTipInfo*)::$_7> >(void*) /usr/lib/llvm-13/bin/../include/c++/v1/thread:291:5 (bitcoind+0x157e6a) Mutex M151 (0x55aacb8ea030) created at: #0 pthread_mutex_init <null> (bitcoind+0xbed2f) OmniLayer#1 std::__1::recursive_mutex::recursive_mutex() <null> (libc++.so.1+0x49fb3) OmniLayer#2 __libc_start_main <null> (libc.so.6+0x29eba) Mutex M131553 (0x7b4c000042e0) created at: #0 pthread_mutex_init <null> (bitcoind+0xbed2f) OmniLayer#1 std::__1::recursive_mutex::recursive_mutex() <null> (libc++.so.1+0x49fb3) OmniLayer#2 std::__1::__unique_if<CTxMemPool>::__unique_single std::__1::make_unique<CTxMemPool, CBlockPolicyEstimator*, int const&>(CBlockPolicyEstimator*&&, int const&) /usr/lib/llvm-13/bin/../include/c++/v1/__memory/unique_ptr.h:728:32 (bitcoind+0x15c81d) OmniLayer#3 AppInitMain(node::NodeContext&, interfaces::BlockAndHeaderTipInfo*) src/init.cpp:1426:24 (bitcoind+0x14e7b4) OmniLayer#4 AppInit(node::NodeContext&, int, char**) src/bitcoind.cpp:231:43 (bitcoind+0x133fd2) OmniLayer#5 main src/bitcoind.cpp:275:13 (bitcoind+0x133fd2) Thread T22 'b-loadblk' (tid=32370, running) created by main thread at: #0 pthread_create <null> (bitcoind+0xbd5bd) OmniLayer#1 std::__1::__libcpp_thread_create(unsigned long*, void* (*)(void*), void*) /usr/lib/llvm-13/bin/../include/c++/v1/__threading_support:443:10 (bitcoind+0x155e06) OmniLayer#2 std::__1::thread::thread<void (*)(char const*, std::__1::function<void ()>), char const (&) [8], AppInitMain(node::NodeContext&, interfaces::BlockAndHeaderTipInfo*)::$_7, void>(void (*&&)(char const*, std::__1::function<void ()>), char const (&) [8], AppInitMain(node::NodeContext&, interfaces::BlockAndHeaderTipInfo*)::$_7&&) /usr/lib/llvm-13/bin/../include/c++/v1/thread:307:16 (bitcoind+0x155e06) OmniLayer#3 AppInitMain(node::NodeContext&, interfaces::BlockAndHeaderTipInfo*) src/init.cpp:1656:29 (bitcoind+0x150164) OmniLayer#4 AppInit(node::NodeContext&, int, char**) src/bitcoind.cpp:231:43 (bitcoind+0x133fd2) OmniLayer#5 main src/bitcoind.cpp:275:13 (bitcoind+0x133fd2) SUMMARY: ThreadSanitizer: data race /usr/lib/llvm-13/bin/../include/c++/v1/__utility/swap.h:39:7 in std::__1::enable_if<(is_move_constructible<CBlockIndex**>::value) && (is_move_assignable<CBlockIndex**>::value), void>::type std::__1::swap<CBlockIndex**>(CBlockIndex**&, CBlockIndex**&) ================== ``` From https://cirrus-ci.com/task/5612886578954240?logs=ci#L4868 ACKs for top commit: achow101: re-ACK fac04cb theStack: Code-review ACK fac04cb Tree-SHA512: 9d619f99ff6373874c7ffe1db20674575605646b4b54b692fb54515a4a49f110a770026d7320ed6dfeaa7976be4cd89e93f821acdbf22c7662bd1c5be0cedcd2
As continuation of mastercoin-MSC#303.
In short: I coded the whole meta DEx test plan, and some other tests, as Python based RPC tests some time ago and the current omnicore-0.0.10 + wallet fix + cancel logic passes all tests.
Rather sooner than later all tests should be converted into spock tests, which are much, much nicer, and once a test base stands, all tests can hopefully be added via a spreadsheet, similar to sto-testplan.tsv, which is literally processed line by line.
However, if someone likes to dig into it, no setup or bitcoin.conf is required, but only:
This executes MetaDexPlanTest, MetaDexCancelAtPriceTest, MetaDexCancelPairAndLookupTest, DexCrossEcosystemSideEffectsTest, MetaDexCancelEverythingInSameEcosystemTest, MetaDexCancelEverythingIgnorePropertyTest and MetaDexCancelEverythingScopeTest.
Orderbook states and trades were added as comments for the MetaDexPlanTest.
So what's missing? First, the test plan is available here:
I'm actually not convinced this covers everything, and I'm sceptical regarding the rounding behavior of the current implementation, see mega thread for the general discussion: OmniLayer/spec#170.
Rounding is only barely tackled by the test plan, and further, the current test plan doesn't add up, because the actors have insufficient amounts of TMSC at the end of the last rows. This is a minor issue though.
At this point the most valuable missing pieces are probably addtional test sequences, preferably in a format such as:
Where state infos could be expected balances, open offers, or simliar, like in sto-testplan.tsv.
The text was updated successfully, but these errors were encountered: