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

(Fix) Investors incorrect max cap #1072

Closed
wants to merge 9 commits into from

Conversation

fernandomg
Copy link
Contributor

Closes #1067

This PR fixes all the related issues with whitelisted addresses and tier's supply. Scenarios that I took into account for both strategies (MintedCapped and DutchAuction):

  • Whitelists addresses added (from CSV, manually or combined), will be limited to the sum of maxCap that is not greater than tier's supply. i.e.: for addresses with maxCap of [100, 200, 300] if supply is 400, then only the first two addresses will be added.
  • If after adding whitelist addresses, the supply value will be validated against the sum of the maxCaps. It should not be less than the maxCap sum.
  • In the manage screen supply cannot be edited, so whitelist addresses will be checked.

Note
The scenario that I didn't fix, due to the complexity of the fix itself (it would require several modifications and would be better to address it in a separate PR), has to be with manage screen and an address values modification.

As you can see in the screenshot
image
Despite having 1011 tokens available, and requiring only 28 extra tokens to change 1972 to 2000, it's giving the error as it considers that the address is new and not a modification of an existing one.


Also:

  • tests where updated and fixed
  • linting errors where solved

 - Prevents adding more addresses if the sum of the maxCaps exceeds the tiers' supply
 - triggers an error for maxCap if it exceeds the remaining tier's supply
 - prevent displaying error messages if field wasn't modified yet
 - update maxCap validation if supply has been modified
 - Validate supply against sum of whitelist's maxCap
 - Make validation active only when whitelist is enabled
 - Fix warning message when trying to read an array index out of bound
 - used bigNumber's eq() when === cannot be used
 - removed unused variables
 - replaced == with ===
 - disabled linting warnings
  - stepTwo/index.js:17
  - DeploymentStore.js:80
  - blockchainHelpers.js:380
@dennis00010011b
Copy link

@fernandomg
Manage page: allowed to add whitelisted investors with maxCap greater than remained token's supply. Steps:

  1. Create Minted crowdsale, 1 tier:
  • supply =1000
  • whitelisted address with maxCap=300
  1. As whitelisted investor buy maxCap
  2. As an owner open manage page
  3. Add another whitelisted investor with maxCap=supply

Expected result:

  • whitelisted maxCap should be restricted by supply-300=700

Actual result:

  • it is possible to add whitelisted address wjth maxCap >700

screen shot 2018-08-07 at 10 25 20

@fernandomg
Copy link
Contributor Author

@dennis00010011b that was quick! thanks for the catch, will review it

@BlackDuckCoPilot
Copy link

Black Duck Security Report

Merging #1072 into 2.0 will not change security risk.

Removed Components

Clean: 2

Click here to see full report

@fernandomg
Copy link
Contributor Author

Ok, so after reviewing the different scenarios that will arise with this PR and have a chat with @vbaranov. It was decided to close this PR and create a new solution for #1067, which is #1067 (comment)

@fernandomg fernandomg closed this Aug 8, 2018
@ghost ghost removed the awaiting for review label Aug 8, 2018
@gabitoesmiapodo gabitoesmiapodo deleted the csv-investors-incorrect-maxCap-#1067 branch November 15, 2018 13:50
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.

None yet

3 participants