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

Add capability to generate seed blocks over RPC #325

Merged
merged 5 commits into from Jan 4, 2016

Conversation

Projects
None yet
2 participants
@zathras-crypto
Copy link

zathras-crypto commented Jan 4, 2016

This PR adds the capability to generate seed blocks via the RPC interface, in order to provide a repeatable process for future seed block usage. The data is retrieved from LevelDB.

Please note due to the fact Exodus crowdsale purchases are not currently stored in LevelDB, it is only valid for generating seed blocks after block 255365.

Example usage:

$ src/omnicore-cli omni_getseedblocks 370000 370020
[
    {
        "seedblock" : 370002
    },
    {
        "seedblock" : 370010
    },
    {
        "seedblock" : 370014
    }
]

For easier inclusion in seedblocks.cpp the output can be stringified by appending true to the request:

$ src/omnicore-cli omni_getseedblocks 370000 370020 true
{
    "seedblocks" : "370002, 370010, 370014"
}

Tested by generating seed blocks from block 370000 to 390000 via this new call and adding them into seedblocks.cpp, then running a fresh parse with the upgraded consensus hashing changes which successfully passed all checkpoints.

zathras-crypto added some commits Jan 4, 2016

Add capability to generate seed blocks over RPC
- Example use: omni_getseedblocks 370000 380000 true
for (it->SeekToFirst(); it->Valid(); it->Next()) {
std::string itData = it->value().ToString();
std::vector<std::string> vstr;
boost::split(vstr, itData, boost::is_any_of(":"), token_compress_on);

This comment has been minimized.

@dexX7

dexX7 Jan 4, 2016

Member

Should be boost::token_compress_on.

{
if (fHelp || params.size() < 2 || params.size() > 3)
throw runtime_error(
"omni_getseedblocks startblock endblock\n"

This comment has been minimized.

@dexX7

dexX7 Jan 4, 2016

Member

Should be: omni_getseedblocks startblock endblock ( stringify )

RequireHeightInChain(startHeight);
RequireHeightInChain(endHeight);

std::set<int> setSeedBlocks = p_txlistdb->GetSeedBlocks(startHeight, endHeight);

This comment has been minimized.

@dexX7

dexX7 Jan 4, 2016

Member

p_txlistdb->GetSeedBlocks() should be surrounded with LOCK(cs_tally);.

This comment has been minimized.

@zathras-crypto

zathras-crypto Jan 4, 2016

Should we do it here, or in the function itself (GetSeedBlocks)?

This comment has been minimized.

@dexX7

dexX7 Jan 4, 2016

Member

Let's do it here, to guard the pointer p_txlistdb too. Not sure, if this is the best way, but it's similar to other places and it might come handy in case of a shutdown (where the pointer is cleared).

@dexX7

This comment has been minimized.

Copy link
Member

dexX7 commented Jan 4, 2016

That's pretty nice! Do you end up with the same seedblocks as currently used?

@dexX7 dexX7 added the api label Jan 4, 2016

@dexX7 dexX7 added this to the 0.0.10.1 milestone Jan 4, 2016

@dexX7

This comment has been minimized.

Copy link
Member

dexX7 commented Jan 4, 2016

I'm wondering about the format: do we need the version with false at all? Would it be handier to just return a plain array with the values, e.g.: [370002, 370010, 370014], without stringification etc.?

@zathras-crypto

This comment has been minimized.

Copy link

zathras-crypto commented Jan 4, 2016

That's pretty nice! Do you end up with the same seedblocks as currently used?

I actually didn't compare directly, just tested them against checkpoints - let me fiddle with the formats and do a direct compare :)

I'm wondering about the format: do we need the version with false at all? Would it be handier to just return a plain array with the values, e.g.: [370002, 370010, 370014], without stringification etc.?

I'm not really sure - I guess I was just trying to cover all bases as it were in case there were other uses, but you're probably right it'll likely only be us using it so perhaps we only need the one (string) format...

@dexX7

This comment has been minimized.

Copy link
Member

dexX7 commented Jan 4, 2016

Well, if a simple array is returned with numbers, then it should probably be easy to just copy and paste the results. And if a proper array with numbers is returned, then it's easier to process with external applications (e.g. some script to get and format the results).

@zathras-crypto

This comment has been minimized.

Copy link

zathras-crypto commented Jan 4, 2016

Actually the reason I "stringified" was to simplify the copy/paste action to seedblocks.cpp :)

A plain array is returned as follows by the RPC client:

$ src/omnicore-cli omni_getseedblocks 370000 370020
[
    370002,
    370010,
    370014
]

Which means all the line breaks and extra spaces need to be stripped first. Where as a "stringified" output comes back as

$ src/omnicore-cli omni_getseedblocks 370000 370020 true
{
    "seedblocks" : "370002, 370010, 370014"
}

The "stringified" data thus just needs pasting into a text editor and the appropriate line breaks (every 15 entries) added to get it ready to paste into seedblocks.cpp.

However with that said it's also pretty easy to strip line breaks in the shell (eg tr -d '\n' < input) and we can do the same with the extra padding - I guess I saw it as being a little easier without these steps but it's not exactly a hardship hehe so I'm happy either way mate :)

@zathras-crypto

This comment has been minimized.

Copy link

zathras-crypto commented Jan 4, 2016

Do you end up with the same seedblocks as currently used?

I just did a comparison for 370000 to 390000 between my seed block PR (basically generated from 'Chests DB this time) and the output of omni_getseedblocks 370000 390000 and yep they're identical :)

I'm thinking I might go the whole hog and compare the entire set from seedblocks.cpp to omni_getseedblocks 255365 390000...

@zathras-crypto

This comment has been minimized.

Copy link

zathras-crypto commented Jan 4, 2016

FYI comparing the current seed block list to the output from RPC for everything post 255365 actually turns up a number of differences. No blocks from the RPC output are missing from the current seed block list, but quite a few (~50) are in the seed block list but missing from the RPC output.

Digging in now...

@dexX7

This comment has been minimized.

Copy link
Member

dexX7 commented Jan 4, 2016

However with that said it's also pretty easy to strip line breaks in the shell (eg tr -d '\n' < input) and we can do the same with the extra padding - I guess I saw it as being a little easier without these steps but it's not exactly a hardship hehe so I'm happy either way mate :)

If there are some ninja tricks to do the formatting via the console, then I'd prefer to use plain arrays with numbers, because the effort seems somewhat similar, and blocks are indeed numbers.

Digging in now...

Hmm.. interesting. In this case, should I hold #324 back?

@zathras-crypto

This comment has been minimized.

Copy link

zathras-crypto commented Jan 4, 2016

Hmm.. interesting. In this case, should I hold #324 back?

Nope, no need - the seed blocks for 370K to 390K are definitely good :)

Also thankfully was a nice and quick analysis of the problem blocks, every single discrepancy fell into one of two categories:

interpret_CreatePropertyVariable(): rejected: malformed string value(s)
!!! interpretPacket() returned -9002 !!!

or

                 type: {various integers} (* unknown type *)
!!! interpretPacket() returned -9002 !!!

So there are actually quite a few blocks that can be safely removed from the current seed block list (since it was never intended to include structurally invalid transactions).

@zathras-crypto

This comment has been minimized.

Copy link

zathras-crypto commented Jan 4, 2016

Sorry for the close/reopen, clicked wrong comment button d'oh!

@zathras-crypto

This comment has been minimized.

Copy link

zathras-crypto commented Jan 4, 2016

If there are some ninja tricks to do the formatting via the console, then I'd prefer to use plain arrays with numbers, because the effort seems somewhat similar, and blocks are indeed numbers.

Sure, I'll change that out now :)

@zathras-crypto

This comment has been minimized.

Copy link

zathras-crypto commented Jan 4, 2016

Switched to a plain array as requested :)

@@ -129,6 +129,9 @@ static const CRPCConvertParam vRPCConvertParams[] =
{ "omni_listblocktransactions", 0 },
{ "omni_getorderbook", 0 },
{ "omni_getorderbook", 1 },
{ "omni_getseedblocks", 0 },
{ "omni_getseedblocks", 1 },
{ "omni_getseedblocks", 2 },

This comment has been minimized.

@dexX7

dexX7 Jan 4, 2016

Member

This line is no longer needed.

This comment has been minimized.

@zathras-crypto

zathras-crypto Jan 4, 2016

Thanks dude, just testing without these and then will push the change :)

This comment has been minimized.

@zathras-crypto

zathras-crypto Jan 4, 2016

Sorry, think I misread your comment - thought you meant all three weren't required (which I wanted to test because I didn't think it would work).

Now rereading I realize you meant just the last line hehe - all sorted.

@dexX7

This comment has been minimized.

Copy link
Member

dexX7 commented Jan 4, 2016

I tested it. :) Other then the remaining nit, this is good to go.

@zathras-crypto

This comment has been minimized.

Copy link

zathras-crypto commented Jan 4, 2016

Fixed up nit as per @dexX7 comment, and as per good to go comment will merge.

@dexX7 dexX7 merged commit 45da456 into OmniLayer:omnicore-0.0.10 Jan 4, 2016

1 check was pending

continuous-integration/travis-ci/pr The Travis CI build is in progress
Details

dexX7 added a commit that referenced this pull request Jan 4, 2016

Merge pull request #325
45da456 Remove unused parameter for omni_getseedblocks from rpcclient.cpp (zathras-crypto)
d3308bf Update help for omni_getseedblocks (zathras-crypto)
55fa47a Update omni_getseedblocks following @dexX7's feedback (zathras-crypto)
4b472fd Slices no longer used, remove unused refs (zathras-crypto)
50ce158 Add capability to generate seed blocks over RPC (zathras-crypto)

@zathras-crypto zathras-crypto deleted the zathras-crypto:0.0.10.1-Z-RPCGetSeedBlocks branch May 8, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment