mastercoin-MSC / mastercore

mastercore info
mastercoin.org
MIT License
24 stars 11 forks source link

Checkpoints + Get snapshot of state for block n #143

Open dexX7 opened 9 years ago

dexX7 commented 9 years ago

I think it would be great to get a representation of global state for a specific block / time / whatever.

While this doesn't replace tests*, checkpoints should be used to compare builds and ensure history remains intact and no side effects were introduced.

This may replace some tests, if edge case transactions are intentionally created. This would actually be nice to have as well and should be documented, if done so.

m21 commented 9 years ago

Very nice Dexx. It's something I've wanted to complete for a long time now. Since realizing our consensus testing was slowly going away. Started out as "breakout" feature: https://github.com/mastercoin-MSC/mastercore/blob/0.0.7/src/mastercore.cpp#L79

But I could never have the time to properly figure out the user side of it. Ideas I had:

Thanks for giving it a thought; I consider it quite an important verification step.

dexX7 commented 9 years ago

Yes, parsing from zero to the specific point was what I had in mind as well. Performance shouldn't be an issue here, because this is a rather specific action where (potential long) parsing time is acceptable imho, but as you also suggested: dumping every n blocks might be possible as well.

Using raw balances is a too crude approach and pending actions such as accepted purchases, ongoing crowdsales etc. must be included as well, but the actual data structure is probably a different topic.

That being said, but as a side note nevertheless and what I once mentioned to @zathras: maybe we should start to think with a mindset such as "what if mastercoin were an altcoin with it's own blockchain and blocks". In some cases this might help to come up with an answer and in this particular case: if you come up with an elegant solution how such blocks would look like, you also found an answer to the data format question.

I'm very happy to see consensus about this (imho indeed important) topic. :)

What do you mean with "figure out the user side of it"?

dexX7 commented 9 years ago

Right now I see a few options on different levels how to achive this goal. First it needs to be determined which data is used as basis.

Raw Mastercoin data alone is not sufficient and information about "reference outputs" and other things is relevant, too. A data entry could include:

  1. Mastercoin transaction index (1, 2, 3, ...)
  2. Bitcoin transaction reference as hash
  3. Mastercoin version
  4. Encoding class (A, B, ...)
  5. Decoded Mastercoin raw data
  6. BTC amount sent to reference
  7. Reference

The "version" of Mastercoin should provide information whether a feature is enabled and anytime a new feature is added, e.g. the traditional DEx, crowdsales, meta DEx, ... there should be a version bump.

Checkpoints based on this data would tackle the encoding layer, but not the logical one. The logical layer involves roughly:

m21 commented 9 years ago

A crude approach would be to create a list of all valid Mastercoin transactions

We have that: MP_txlist, also includes invalid ones.

A finer approach would be to create a list of all valid Mastercoin transactions and store decoded, raw Mastercoin data as well.

We have that as well -- using the above MP_txlist + blockchain that's how gettransaction_MP roughly works. So that raw data is available now, right? For the "encoding layer".

dexX7 commented 9 years ago

Yup, I tested dumping the raw transaction data from interpretPacket (started from scratch). I'm not sure, if it would be side effect free to go this route from somewhere else - it looks like there are data manipulations (update_tally_map). Is this the case?

dexX7 commented 9 years ago

Actually I sort of missed one other important aspect: basically nothing from mastercore.cpp/.h and the others is testable at the moment due to the heavy cross-dependencies.

zathras-crypto commented 9 years ago

Agree 100% on the need for this. Just thinking off the top of my head here but comment from skype:

[8:30:15 AM] zathrasc: everytime UPDATE_TALLY_MAP calls an action [8:30:20 AM] zathrasc: write that to a seperate logfile - so just contains list of lines like: [8:30:35 AM] zathrasc: UPDATE_TALLY_MAP 1Address 1OtherAddress TALLY_TYPE AMOUNT [8:30:45 AM] zathrasc: diff that seperate logfile across versions and platforms [8:30:56 AM] zathrasc: and can see any funds movement from transactions including reserved txs like dex/metadex (since we update_tally to reserve funds) [8:31:17 AM] zathrasc: if funds get moved to different place (eg send to self instead of send to P2SH) those logs would be different across builds [8:31:52 AM] zathrasc: it's not the "whole answer" but it's a very easy to implement (few lines of code) way to start off maybe?

zathras-crypto commented 9 years ago

Just a further note here from the Skype chat, Michael has pointed out we can get something to this effect already - another quick paste eg:

[8:55:11 AM] zathrasc: 
rm -rf ~/.bitcoin/MP_*
rm ~/.bitcoin/mastercore.log
./mastercored-0.0.8 --daemon --server --checkblocks=1 
---wait---
grep update_tally ~/.bitcoin/mastercore.log >0.8log

rm -rf ~/.bitcoin/MP_*
rm ~/.bitcoin/mastercore.log
./mastercored-0.0.9 --daemon --server --checkblocks=1 
---wait---
grep update_tally ~/.bitcoin/mastercore.log >0.9log

diff 0.8log 0.9log

something like that?
m21 commented 9 years ago

[4:56:42 PM] Michael: and they should match up to the the P2SH block [4:56:49 PM] Michael: or STO block on TestNet [4:56:51 PM] Michael: etc. [4:57:07 PM] Michael: between 7 & 8 [4:57:11 PM] zathrasc: right nice [4:57:18 PM] Michael: and 8 & 9 must match up to Metadex diversions

and so on forever between Version N & Version N-1

dexX7 commented 9 years ago

Testing based on logs is actually a neat and creative solution. Sounds like a fast and very effective route for comparing builds where log output can be expected to differ only in values - that should be given, if one build is compared against another where a small change was added etc.

On the long run:

I started with the first ones which are probably not optimal, but serve as example nevertheless:

I really haven't explored what's possible, but this was fairly simple:

  1. clone one of those other test files in src/test/
  2. replace name in functions (e.g. mastercore_strtoint64_tests), add new, remove other
  3. write tests, use BOOST_CHECK(should-be-true) or [BOOST_CHECK_EQUAL(expected, actual)](https://github.com/dexX7/bitcoin/blob/mcore-add-convert-test-round/src/test/mastercore_rounduint64_tests.cpp#L86-L88
  4. add the new file to src/test/Makefile.am,
  5. build
  6. run src/test/test_bitcoin

Unit tests are rather difficult at this point though, because most functions (and files, headers) appear to be not self-contained and depend on or influence other parts (e.g. parseTransaction that requires LevelDB stored transactions, interpretPackage that can't be used to get results, but also updates state etc.). And an include of solely mastercore.h completely breaks right at the start.

As already suggested, I think comparing the content of MP_txlist would actually indeed be a good starting point, too.

I think it would also help, if there were a breakout feature as mentioned at the beginning, basically something like mastercored --stop-at-height nnnn.