Refactor pledge processing in AllOrNothing and KeepWhatsRaised contracts#36
Refactor pledge processing in AllOrNothing and KeepWhatsRaised contracts#36mahabubAlahi wants to merge 1 commit intooz-audit-fixesfrom
Conversation
mahabubAlahi
commented
Mar 3, 2026
- Updated the _pledge function to ensure pledgeAmount and shippingFee are always in the token's native decimals, removing the need for denormalization by the caller.
- Enhanced documentation for the _pledge function to clarify parameter requirements and processing logic.
- Adjusted related calculations and state updates to reflect the new handling of pledge amounts and fees.
- Updated the _pledge function to ensure pledgeAmount and shippingFee are always in the token's native decimals, removing the need for denormalization by the caller. - Enhanced documentation for the _pledge function to clarify parameter requirements and processing logic. - Adjusted related calculations and state updates to reflect the new handling of pledge amounts and fees.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6573537315
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| uint256 pledgeAmountInTokenDecimals = _denormalizeAmount(pledgeToken, pledgeAmount); | ||
| uint256 shippingFeeInTokenDecimals = _denormalizeAmount(pledgeToken, shippingFee); |
There was a problem hiding this comment.
Validate token before denormalizing reward pledge values
pledgeForAReward now denormalizes pledgeAmount and shippingFee before _pledge runs the INFO.isTokenAccepted guard, so an unaccepted token address that does not implement decimals() (e.g., EOA or non-ERC20 contract) will revert during _denormalizeAmount instead of returning the expected AllOrNothingTokenNotAccepted error. This changes failure semantics for invalid token inputs and can break callers/tests that depend on deterministic custom-error handling for unsupported tokens.
Useful? React with 👍 / 👎.
| pledgeAmount += tempReward.rewardValue; | ||
| } | ||
| _pledge(pledgeId, backer, pledgeToken, reward[0], pledgeAmount, tip, reward, tokenSource); | ||
| uint256 pledgeAmountInTokenDecimals = _denormalizeAmount(pledgeToken, pledgeAmount); |
There was a problem hiding this comment.
Check token acceptance before reward amount denormalization
The reward path now calls _denormalizeAmount(pledgeToken, pledgeAmount) before _pledge performs INFO.isTokenAccepted, so passing an unsupported token without a valid decimals() implementation reverts early with a low-level decode failure rather than KeepWhatsRaisedTokenNotAccepted. This is a regression in input-validation behavior for invalid tokens and makes error handling less predictable for integrations.
Useful? React with 👍 / 👎.