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

V0.12.1.x govobj sync #864

Merged
merged 5 commits into from
Jun 8, 2016

Conversation

eduffield222
Copy link

  • disable fCached values
  • use two maps for storing votes, by hash and parent-hash/type
  • disable part of flatdb.dump (still overwriting)
  • fixed govobj/votes relay and sync

@eduffield222 eduffield222 force-pushed the v0.12.1.x-govobj-sync branch from 320adc4 to e558daf Compare June 3, 2016 22:08
- disable fCached values
- use two maps for storing votes, by hash and parent-hash/type
- disable part of flatdb.dump (still overwriting)
- fixed govobj/votes relay and sync
@eduffield222 eduffield222 force-pushed the v0.12.1.x-govobj-sync branch from e558daf to 487674f Compare June 3, 2016 22:12
static const int MIN_MASTERNODE_PAYMENT_PROTO_VERSION_1 = 70066;
static const int MIN_MASTERNODE_PAYMENT_PROTO_VERSION_2 = 70103;
static const int MIN_MASTERNODE_PAYMENT_PROTO_VERSION_1 = 70103;
static const int MIN_MASTERNODE_PAYMENT_PROTO_VERSION_2 = 70200;

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldnt it be '70201'? '70200' was a internal testnet version that will never make it to mainnet.

@schinzelh
Copy link

some nits but utACK


// NEW GOVERNANCE OBJECT

if (strCommand == NetMsgType::MNGOVERNANCEOBJECT) {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should be else if to follow the same style as in other ProcessMessage

Evan Duffield added 2 commits June 6, 2016 16:06
- Uncommented sync block
- Protocol min 70201
- Fixed bug which flags invalid votes incorrectly
- Formatting
if(vote.nTime - mapVotes[hash2].nTime < GOVERNANCE_UPDATE_MIN){
strError = strprintf("time between votes is too soon - %s - %lli", vote.GetHash().ToString(), vote.nTime - mapVotes[hash2].nTime);
if(vote.nTime - mapVotesByType[nTypeHash].nTime < GOVERNANCE_UPDATE_MIN){
strError = strprintf("time between votes is too soon - %s - %lli", nTypeHash.ToString(), vote.nTime - mapVotesByType[nHash].nTime);
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

typo mapVotesByType[nHash] -> mapVotesByType[nTypeHash] ?

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

in general: looks much cleaner now so after this ^^^ is addressed imo we are good to merge

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nope, that is correct, but upon further review there was a bug in GetTypeHash. I've added some comments to the code to make the intention clearer. We're trying to track when masternodes update their votes on governance objects. With 12.1 there's various types of votes (funding, valid, delete, etc), so this is the deterministic hash that will collide with the previous vote and allow the system to update.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm.. ok, I missed the one in GetTypeHash() :) but shouldn't mapVotesByType be indexed via nTypeHash and not nHash here anyway?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ha, yes. updated... I think we're good to go now?

Evan Duffield added 2 commits June 7, 2016 06:50
- Should not collide based on the outcome
@UdjinM6
Copy link

UdjinM6 commented Jun 8, 2016

ACK squash merge 732a8a3 :)
@schinzelh

@schinzelh schinzelh merged commit 95ba715 into dashpay:v0.12.1.x Jun 8, 2016
@crowning-
Copy link

@UdjinM6 @schinzelh : I noted "squash merge" in a couple of PRs recently. Does this mean I don't have to squash all my commits into one myself anymore?

@schinzelh
Copy link

@crowning- Yep, github introduced this wonderfull "squash and merge" option

image

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.

4 participants