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

contracts: early check that the fee is correct. #26

Merged
merged 2 commits into from Jun 13, 2022

Conversation

dbadoy
Copy link
Contributor

@dbadoy dbadoy commented Jun 12, 2022

// Validate `symbol`, `name` and `description` to ensure generateJSON() creates a valid JSON
if(!StringUtils.validJSONValue(name)) revert DixelClubV2Factory__NameContainedMalicious();
if(!StringUtils.validJSONValue(symbol)) revert DixelClubV2Factory__SymbolContainedMalicious();
if(!StringUtils.validJSONValue(description)) revert DixelClubV2Factory__DescriptionContainedMalicious();
// Neutralize minting starts date
if (metaData.mintingBeginsFrom < block.timestamp) {
metaData.mintingBeginsFrom = uint40(block.timestamp);
}
if (creationFee > 0) {
if(msg.value != creationFee) revert DixelClubV2Factory__InvalidCreationFee();
// Send fee to the beneficiary
(bool sent, ) = beneficiary.call{ value: creationFee }("");
require(sent, "CREATION_FEE_TRANSFER_FAILED");
}

function validJSONValue(string calldata haystack) internal pure returns (bool) {
bytes memory haystackBytes = bytes(haystack);
uint256 length = haystackBytes.length;
for (uint256 i; i != length;) {
bytes1 char = haystackBytes[i];
if (char < 0x20 || char == 0x22 || char == 0x5c || char == 0x7f) {
return false;
}
unchecked {
++i;
}
}
return true;
}

I think validJSONValue() is pretty heavy method. ㅡespecially in check the description
So, It would be nice that early check the creation fee before validJSONValue().

@dbadoy
Copy link
Contributor Author

dbadoy commented Jun 12, 2022

Additional) To express the flow, this logic seems good.

if(msg.value != creationFee) revert DixelClubV2Factory__InvalidCreationFee();

// Validate `symbol`, `name` and `description` to ensure generateJSON() creates a valid JSON
if(!StringUtils.validJSONValue(name)) revert DixelClubV2Factory__NameContainedMalicious();
if(!StringUtils.validJSONValue(symbol)) revert DixelClubV2Factory__SymbolContainedMalicious();
if(!StringUtils.validJSONValue(description)) revert DixelClubV2Factory__DescriptionContainedMalicious();

// Neutralize minting starts date
if (metaData.mintingBeginsFrom < block.timestamp) {
    metaData.mintingBeginsFrom = uint40(block.timestamp);
}

if(creationFee > 0) {
    (bool sent, ) = beneficiary.call{ value: creationFee }("");
    require(sent, "CREATION_FEE_TRANSFER_FAILED");
}

@sydneyitguy sydneyitguy self-assigned this Jun 13, 2022
@sydneyitguy sydneyitguy added enhancement New feature or request minor labels Jun 13, 2022
@sydneyitguy
Copy link
Member

LGTM 👍🏻

@sydneyitguy sydneyitguy merged commit 6d158d8 into Steemhunt:main Jun 13, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request minor
Development

Successfully merging this pull request may close these issues.

None yet

2 participants