fix: Add validation for ERC20 premint amount to prevent "Literal too large" error #488
Conversation
…RC20 decimals. Added validation to prevent exceeding Solidity literal limits.
|
All contributors have signed the CLA ✍️ ✅ |
|
I have read the CLA Document and I hereby sign the CLA |
packages/core/solidity/src/erc20.ts
Outdated
| const exp = decimalPlace <= 0 ? 'decimals()' : `(decimals() - ${decimalPlace})`; | ||
|
|
||
| // Add 18 decimals to the number (standard ERC20 decimals) | ||
| const fullAmount = units + '000000000000000000'; |
There was a problem hiding this comment.
This number should not be hardcoded as 18 decimals. While 18 is the default, users who take the generated contract as a starting point could override the decimals() function of ERC20.sol to change the intended display decimals. The premint amount should take this value into consideration.
For example, if the user enters a Premint of 1000 in the UI, and they override decimals() to 2, then the actual premint calculation should result in 100000.
packages/core/solidity/src/erc20.ts
Outdated
| const fullAmount = units + '000000000000000000'; | ||
|
|
||
| // Check if the final number would be too large | ||
| if (fullAmount.length > 77) { // Solidity has a limit on number literals |
There was a problem hiding this comment.
Solidity's limit on literals is not based on character length. Rather, this must be less than 2**256. There is some documentation here which is for Yul, but it seems to be the same limit for Solidity.
Here is an example of a way to validate the size of a uint (that example is currently for Cairo but something similar could be done for Solidity).
packages/core/solidity/src/erc20.ts
Outdated
|
|
||
| c.addConstructorArgument({ type: 'address', name: 'recipient' }); | ||
|
|
||
| const mintLine = `_mint(recipient, ${units} * 10 ** ${exp});`; | ||
| // Generate the mint line with the full amount directly | ||
| const mintLine = `_mint(recipient, ${fullAmount});`; |
There was a problem hiding this comment.
The previous format should be used, so that decimals() from the user's contract is included in the calculation.
Overall, it seems that two checks should be made:
- Check that the value of
unitsis less than 2**256 - Check that the calculated value of
${units} * 10 ** ${exp}is less than 2**256, because2**256 - 1is the largest possible value of theuint256type as an argument for the_mintfunction.- While
decimals()could be overridden by the user and therefore${exp}cannot be definitely known, for the purposes of this check we can assumedecimals()is 18. Solidity would revert on overflow anyways if the actual calculation at runtime is larger than the max value ofuint256.
- While
|
hey @ericglau, I have tried to follow the approach you suggested. I had to add ES2020.BigInt as it is not available in ES2019 or ES2018. can you check if I am going in the right direction? |
|
@hashpalk Thanks for following up, this is looking good! I pushed some refactors, fixed the calculation to handle negative exponents, and added tests. It seems to work without adding ES2020.BigInt, where do you see the error if you don't add it? |
|
hey @ericglau, when I was using BigInt from ES2019, it was saying something like BigInt not found. I was also confused, as BigInt was used in other places. So I had to take it from ES2020. |
|
hi @CoveMB, just following up on this PR—whenever you have a chance, could you take a look and share any feedback? Let me know if you need any clarifications from my end. Appreciate your time! |
This solves issue #241 where the ERC20 token generator previously allowed arbitrarily large numbers in the premint field, which could lead to a "Literal too large" compilation error in Solidity. This occurred because very large numbers exceeded Solidity's number literal limit when combined with the token's 18 decimal places.
Changes:
This prevents contract compilation failures while still allowing reasonably large premint amounts up to Solidity's literal limits.