-
Notifications
You must be signed in to change notification settings - Fork 21
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
Created for reviewing the presale app #1
base: review
Are you sure you want to change the base?
Conversation
general/mrc20_presale.js
Outdated
|
||
let tokenPrice = toBaseUnit(token.price.toString(), 18) | ||
let finalMaxCap | ||
if (currentTime < PUBLIC_SALE) { |
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.
most of the codes in these 3 if, else if, else
block is repeated and can be out of if
.
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.
Fixed
general/mrc20_presale.js
Outdated
Number(Web3.utils.fromWei(sum, 'ether')) > | ||
usdMaxCap | ||
) | ||
throw { message: 'Amount is not valid' } |
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.
what is being checked here in the last else
block should also be checked for the previous phases before the public sale.
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.
Fixed
general/mrc20_presale.js
Outdated
Number(Web3.utils.fromWei(sum, 'ether')) > | ||
usdMaxCap | ||
) | ||
throw { message: 'Amount is not valid' } |
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.
what is being checked here in the last else
block should also be checked for the previous phases before the public sale.
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.
Fixed
} | ||
} | ||
} | ||
await this.writeNodeMem(memory, 5 * 60) |
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.
@sadeghte suggested that having such a delay can increase security.
https://github.com/muon-protocol/muon-apps/blob/master/custom/sample.js#L89
finalMaxCap = maxCap.sub(sum).toString() | ||
} | ||
} | ||
|
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.
It's safer to check if finalMaxCap > 0
and throw an error otherwise, instead of only relying on smart contract side checking because if in some rare cases it becomes negative, as it's defined as unsigned on, it might result in becoming an attack vector.
Add `stage_unitap` app
batchRequest issue
update recoverTypedSignature
Add pion_tss_reward_oracle app
pion_tss_reward_oracle bugfix
Add `random-words` method to `stage_unitap` app
factGPT dispute
Create thena_tc_testenv.js
LayerZeroDVN: testnets
change subgraph to squid
Update thena_tc.js
symmio pion
symmio_pion app
symmio_pion: bugfix
symmio_pion app
Rename symmio apps
No description provided.