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

Users are not incentivized to mint last tokens of any of the generations #921

Closed
howlbot-integration bot opened this issue Aug 9, 2024 · 1 comment
Labels
2 (Med Risk) Assets not at direct risk, but function/availability of the protocol could be impacted or leak value bug Something isn't working downgraded by judge Judge downgraded the risk level of this issue duplicate-564 🤖_03_group AI based duplicate group recommendation satisfactory satisfies C4 submission criteria; eligible for awards sufficient quality report This report is of sufficient quality

Comments

@howlbot-integration
Copy link

Lines of code

https://github.com/code-423n4/2024-07-traitforge/blob/279b2887e3d38bc219a05d332cbcb0655b2dc644/contracts/TraitForgeNft/TraitForgeNft.sol#L345-L355

Vulnerability details

Impact

When a use tries to mint a TraitForgeNft, the calculateMintPrice() function is used to get the cost price of the NFT. We will easily note that first token of 1st gen costs 0.005 ETH, while last token of 1st gen costs 0.25 ETH, which means 50 times the cost of the first token with no guarantees of having better features. This happens because the function takes into account how many NFTs of this gen (up to 10000) have been minted to get the price for the next NFT.

The cost of 1st token of 2nd gen will be 0.005005 ETH, so the conclusion is that nobody will be willing to pay for last tokens of a generation and will wait for next generation to start to mint cheap NFTs, leading to a unexpected behaviour of the system or even reaching a locked situation.

Tools Used

Manual review.

Recommended Mitigation Steps

A good way to solve this problem would be to make that expensive tokens have better properties than cheap ones, but this looks complex as it would require to modify great part of the protocol.

The recommended way is to make tokens more and more expensive as they are minted, this way users will be incentivized to mint as soon as possible. Update startPrice to currentPrice when gen is about to be increased, so that next time calculateMintPrice() is called, the resulting price is higher:

 function _incrementGeneration() private {
    require(
      generationMintCounts[currentGeneration] >= maxTokensPerGen,
      'Generation limit not yet reached'
    );
+   startPrice = startPrice + maxTokensPerGen * priceIncrement;
    currentGeneration++;
    generationMintCounts[currentGeneration] = 0;
    priceIncrement = priceIncrement + priceIncrementByGen;
    entropyGenerator.initializeAlphaIndices();
    emit GenerationIncremented(currentGeneration);
  }

Assessed type

Payable

@howlbot-integration howlbot-integration bot added 2 (Med Risk) Assets not at direct risk, but function/availability of the protocol could be impacted or leak value 🤖_03_group AI based duplicate group recommendation bug Something isn't working duplicate-210 sufficient quality report This report is of sufficient quality labels Aug 9, 2024
howlbot-integration bot added a commit that referenced this issue Aug 9, 2024
@c4-judge c4-judge added satisfactory satisfies C4 submission criteria; eligible for awards 3 (High Risk) Assets can be stolen/lost/compromised directly upgraded by judge Original issue severity upgraded from QA/Gas by judge 2 (Med Risk) Assets not at direct risk, but function/availability of the protocol could be impacted or leak value downgraded by judge Judge downgraded the risk level of this issue and removed 2 (Med Risk) Assets not at direct risk, but function/availability of the protocol could be impacted or leak value 3 (High Risk) Assets can be stolen/lost/compromised directly upgraded by judge Original issue severity upgraded from QA/Gas by judge labels Aug 20, 2024
@c4-judge
Copy link
Contributor

c4-judge commented Sep 5, 2024

koolexcrypto marked the issue as duplicate of #564

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
2 (Med Risk) Assets not at direct risk, but function/availability of the protocol could be impacted or leak value bug Something isn't working downgraded by judge Judge downgraded the risk level of this issue duplicate-564 🤖_03_group AI based duplicate group recommendation satisfactory satisfies C4 submission criteria; eligible for awards sufficient quality report This report is of sufficient quality
Projects
None yet
Development

No branches or pull requests

1 participant