-
Notifications
You must be signed in to change notification settings - Fork 2
Fix requiredETH calculation in buyWithETH function to account for decimal scaling #31
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
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Copilot wasn't able to review any files in this pull request.
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
Fix errors |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Review
Co-authored-by: sonnyquinn24 <[email protected]>
Co-authored-by: sonnyquinn24 <[email protected]>
|
||
contract SEQToken is ERC20, Ownable { | ||
constructor( | ||
uint256 totalSupply, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
'Total supply is 1,750,000
Problem
The
buyWithETH
function incontracts/SEQICO.sol
had an incorrect calculation forrequiredETH
that didn't account for decimal scaling whenpricePerTokenETH
uses 18 decimals (standard for ERC20 tokens).Before:
This resulted in astronomically large ETH requirements. For example, with a price of 0.01 ETH per token, purchasing 1 SEQ token would require 10,000,000,000,000,000 ETH instead of the expected 0.01 ETH.
Solution
Updated the calculation to include division by
1e18
to properly handle decimal scaling:After:
This brings the
buyWithETH
function in line with the existing correct implementations inbuyWithUSDT
andbuyWithUSDC
functions, which already include the/1e18
division.Verification
buyWithETH
,buyWithUSDT
,buyWithUSDC
)Additional Changes
Ownable
constructor compatibility issues in bothSEQICO.sol
andSEQToken.sol
to work with the newer version of OpenZeppelin contractsThe core fix is minimal and surgical, changing only the problematic calculation line while maintaining all existing functionality.
This pull request was created as a result of the following prompt from Copilot chat.
✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.