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

Created for reviewing the presale app #1

Open
wants to merge 550 commits into
base: review
Choose a base branch
from
Open

Created for reviewing the presale app #1

wants to merge 550 commits into from

Conversation

abramsymons
Copy link
Contributor

No description provided.

@abramsymons abramsymons self-assigned this Jul 31, 2022
general/mrc20_presale.js Outdated Show resolved Hide resolved
general/mrc20_presale.js Outdated Show resolved Hide resolved
general/mrc20_presale.js Outdated Show resolved Hide resolved
general/mrc20_presale.js Outdated Show resolved Hide resolved

let tokenPrice = toBaseUnit(token.price.toString(), 18)
let finalMaxCap
if (currentTime < PUBLIC_SALE) {
Copy link
Contributor Author

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.

Copy link
Contributor

Choose a reason for hiding this comment

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

Fixed

Number(Web3.utils.fromWei(sum, 'ether')) >
usdMaxCap
)
throw { message: 'Amount is not valid' }
Copy link
Contributor Author

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.

Copy link
Contributor

Choose a reason for hiding this comment

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

Fixed

Number(Web3.utils.fromWei(sum, 'ether')) >
usdMaxCap
)
throw { message: 'Amount is not valid' }
Copy link
Contributor Author

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.

Copy link
Contributor

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)
Copy link
Contributor Author

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()
}
}

Copy link
Contributor Author

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.

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.