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

Back port fixes & improvements from develop #449

Merged
merged 16 commits into from Jan 24, 2017

Conversation

zathras-crypto
Copy link

@zathras-crypto zathras-crypto commented Jan 5, 2017

This PR backports fixes and improvements from the 0.2 development branch to 0.0.11.

The changes backported are as follows:

#436 - Improve parsing performance
#439 - Fix incorrect value from getTotalTokens when fees are cached
#447 - Set minimum fee distribution threshold and protect against empty distributions
#448 - Check for fee distribution when total number of tokens is changed

Note #436 has been partially backported, as some of the work is based on changes in Bitcoin 0.13.

Thanks
Z

@dexX7
Copy link
Member

dexX7 commented Jan 5, 2017

Thanks for preparing this!

You proposed to merge this into omnicore-0.0.10, instead of master. Are you suggesting to create 0.0.11.3 based on that branch?

@dexX7
Copy link
Member

dexX7 commented Jan 5, 2017

Not sure what's up with Travis CI, but I tested it locally with:

./src/test/test_omnicore
./qa/pull-tester/omnicore-rpc-tests.sh true

And both test suits pass. :)

Would you say, this is ready?

@dexX7
Copy link
Member

dexX7 commented Jan 6, 2017

Can you please also reference the pull requests you ported back?

@zathras-crypto
Copy link
Author

I wasn't sure what branch to pull against, I guessed we would work 'omnicore-0.0.10' until ready and then push to master, but I'm open to whatever approach you want to use dude...

Re refs, sure I can do that though to note not "all" of the performance PR was ported back (since some of it relies on 0.13 Bitcoin stuff).

@dexX7
Copy link
Member

dexX7 commented Jan 7, 2017

I wasn't sure what branch to pull against, I guessed we would work 'omnicore-0.0.10' until ready and then push to master, but I'm open to whatever approach you want to use dude...

I'm not really sure either. If we leave master as it is and build 0.0.11.3 based on omnicore-0.0.10, we can at some point merge the develop branch and pick up all the stuff anyway, but this seems a bit messy.

@zathras-crypto
Copy link
Author

I'm not really sure either. If we leave master as it is and build 0.0.11.3 based on omnicore-0.0.10, we can at some point merge the develop branch and pick up all the stuff anyway, but this seems a bit messy.

Can you let me know how you'd prefer to do it mate? I genuinely don't mind how we do it :)

Can you please also reference the pull requests you ported back?

Added following to description:

The changes backported are as follows:

#436 - Improve parsing performance
#439 - Fix incorrect value from getTotalTokens when fees are cached
#447 - Set minimum fee distribution threshold and protect against empty distributions
#448 - Check for fee distribution when total number of tokens is changed

Note #436 has been partially backported, as some of the work is based on changes in Bitcoin 0.13.

@dexX7
Copy link
Member

dexX7 commented Jan 9, 2017

Can you let me know how you'd prefer to do it mate?

For the sake of making a decision, I'd say, let's merge against the master branch, and hope that this won't create any incompatibilities, when later trying to merge develop into it. :)

@dexX7 dexX7 modified the milestones: 0.0.11.2, 0.0.11.3 Jan 9, 2017
@dexX7 dexX7 removed this from the 0.0.11.2 milestone Jan 9, 2017
@zathras-crypto zathras-crypto changed the base branch from omnicore-0.0.10 to master January 23, 2017 19:40
@zathras-crypto
Copy link
Author

I've changed the base to master...

@dexX7
Copy link
Member

dexX7 commented Jan 23, 2017

Hmm, looks like I'm unable to merge:

remote: Resolving deltas: 100% (76/76), completed with 16 local objects.
remote: error: GH006: Protected branch update failed for refs/heads/master.
remote: error: You're not authorized to push to this branch. Visit https://help.github.com/articles/about-protected-branches/ for more information.
To git@github.com:OmniLayer/omnicore
 ! [remote rejected] master -> master (protected branch hook declined)

We need to check with @achamely how the master branch is protected (in contrast to develop), I guess it's due to the failing Travis CI checks.

@dexX7 dexX7 merged commit 48ac1cb into OmniLayer:master Jan 24, 2017
dexX7 added a commit that referenced this pull request Jan 24, 2017
48ac1cb Test for fee distribution when number of tokens (and thus threshold) changes (Zathras)
2bc7ea2 Pass block into NotifyTotalTokensChanged() (Zathras)
dae6a2d Protect against fee distribution when the cache is empty (Zathras)
8600316 Protect against zero valued fee distribution thresholds (Zathras)
e1887e8 Clean up following @dexX7's feedback (thanks!) (zathras-crypto)
1a404df Fix incorrect value from getTotalTokens when fees are cached (zathras-crypto)
88d9690 Narrow scope of UpdateDistributionThresholds() (zathras-crypto)
aedf526 Add back in zero check dropped while prepping commits (zathras-crypto)
c655dd2 Check whether a property ID is valid by inferring from next available ID, instead of fetching the SP (zathras-crypto)
ebe318a Only generate the SHA256 obfuscation hashes we need instead of 255 every time (zathras-crypto)
c268e83 Drop non-Omni transactions quicker by looking for marker/Exodus bytes directly in scriptPubKey hex (zathras-crypto)
ba9ac74 Skip calling HandleExodusPurchase() on mainnet after Exodus crowdsale closed (zathras-crypto)
8732e15 Add consensus hash for block 440,000 (zathras-crypto)
a50c30d Add seed blocks for 430,000 to 440,000 (zathras-crypto)
fc78462 Update MAX_SEED_BLOCKS to include seed blocks above 390,000 (zathras-crypto)
d411286 Return immediately from VerifyCheckpoint if block isn't a multiple of 10K (zathras-crypto)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants