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

Update handling of crowdsales and missing bonus amounts #253

Merged

Conversation

dexX7
Copy link
Member

@dexX7 dexX7 commented Sep 20, 2015

This pull request

The "inflation" of indivisible amounts for crowdsales is directly moved into calculateFundraiser, where 256 bit wide integers are used, to avoid constraints related to the max. representable amounts.

Instead of recalculating the credited amounts to determine the missing issuer bonus, the totals, which are available via the crowdsale objects, are directly used.

The FP math was removed and replaced with plain 256 bit wide integers calculations.

This update triggers reparsing, to clear the bad persistence data (see #249).

zathras-crypto and others added 2 commits September 20, 2015 23:52
…t256

Original submission:
zathras-crypto@33f3114

Note:
The calculation for missing issuer bonus tokens must be updated.
There is no need to pass all values manually to calculateFractional,
except for testing maybe, so it was updated for convenience.
Instead of recalculating the credited amounts, the total is directly
used.

The FP math was removed and replaced with plain 256 bit wide integers
calculations.
Due to the SP related changes, transactions should be reparsed.

Bumping the DB version initiates a fresh start.
@dexX7
Copy link
Member Author

dexX7 commented Sep 20, 2015

Updated to "on progress", because this change should not be rushed, due to it's potential impact.

@zathras-crypto
Copy link

Reviewed, looks good to me - tested, seems fine, passes all checkpoints.

Nice work :)

@dexX7
Copy link
Member Author

dexX7 commented Sep 22, 2015

Thanks for the review.

I confirmed that the results on Windows and other plattforms are the same (with 32000 autogenerated test cases). This doesn't necessarily imply the calculations are correct, but that's a different story anyway. At least I'm more or less convinced that issues such as #124 or #222 are history. :)

@dexX7 dexX7 merged commit a4d3e9c into OmniLayer:omnicore-0.0.10 Sep 22, 2015
dexX7 added a commit that referenced this pull request Sep 22, 2015
a4d3e9c Update unit test for crowdsale calculation (dexX7)
301b23e Bump DB version to 3, to trigger reparsing of transactions (dexX7)
575d4c7 Replace calculation for missing issuer bonus (dexX7)
1437b27 Add alternative format for calculateFractional (dexX7)
354b678 Move inflation of indivisible amounts into calculateFundraiser as uint256 (zathras-crypto)
@dexX7 dexX7 modified the milestone: 0.0.10.0 Dec 30, 2015
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.

2 participants