Skip to content

fix: Add validation for ERC20 premint amount to prevent "Literal too large" error #488

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.

Sign up for GitHub

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

Merged
ericglau merged 8 commits into from
Mar 26, 2025
Merged

fix: Add validation for ERC20 premint amount to prevent "Literal too large" error #488

Changes from 1 commit
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
664c252
Enhance addPremint function to handle large amounts and standardize E…
hashpalk Mar 19, 2025
f25c4c9
enhanced ERC20 addPremint function with uint256 validation and overfl…
hashpalk Mar 20, 2025
5249990
Handle negative exponent, refactor
ericglau Mar 21, 2025
84c2549
Revert tsconfig since es2019 includes BigInt
ericglau Mar 21, 2025
0bb5698
Add tests
ericglau Mar 21, 2025
3b2f545
Fix lint
ericglau Mar 21, 2025
35a36db
Update changelog
ericglau Mar 21, 2025
e98385f
Apply review comments
ericglau Mar 25, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 12 additions & 2 deletions packages/core/solidity/src/erc20.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -179,11 +179,21 @@
const decimalPlace = decimals.length - exponent;
const zeroes = new Array(Math.max(0, -decimalPlace)).fill('0').join('');
const units = integer + decimals + zeroes;
const exp = decimalPlace <= 0 ? 'decimals()' : `(decimals() - ${decimalPlace})`;

Check failure on line 182 in packages/core/solidity/src/erc20.ts

View workflow job for this annotation

GitHub Actions / lint 

Delete `······`
// Add 18 decimals to the number (standard ERC20 decimals)
const fullAmount = units + '000000000000000000';
Copy link
Member

@ericglau ericglau Mar 19, 2025

Choose a reason for hiding this comment

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

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.


Check failure on line 185 in packages/core/solidity/src/erc20.ts

View workflow job for this annotation

GitHub Actions / lint 

Delete `······`
// Check if the final number would be too large
if (fullAmount.length > 77) { // Solidity has a limit on number literals

Check failure on line 187 in packages/core/solidity/src/erc20.ts

View workflow job for this annotation

GitHub Actions / lint 

Insert `⏎·······`
Copy link
Member

@ericglau ericglau Mar 19, 2025

Choose a reason for hiding this comment

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

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).

throw new OptionsError({
premint: 'Amount too large. Please use a smaller number to avoid Solidity literal limits',
});
}

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});`;
Copy link
Member

@ericglau ericglau Mar 19, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

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:

  1. Check that the value of units is less than 2**256
  2. Check that the calculated value of ${units} * 10 ** ${exp} is less than 2**256, because 2**256 - 1 is the largest possible value of the uint256 type as an argument for the _mint function.
    • While decimals() could be overridden by the user and therefore ${exp} cannot be definitely known, for the purposes of this check we can assume decimals() is 18. Solidity would revert on overflow anyways if the actual calculation at runtime is larger than the max value of uint256.


if (crossChainBridging) {
if (premintChainId === '') {
Expand Down
Loading