Skip to content

Refactor pledge processing in AllOrNothing and KeepWhatsRaised contracts #36

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.

Sign up for GitHub

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

Open
mahabubAlahi wants to merge 1 commit into
base: oz-audit-fixes
Choose a base branch
from
Open

Refactor pledge processing in AllOrNothing and KeepWhatsRaised contracts #36

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
40 changes: 18 additions & 22 deletions src/treasuries/AllOrNothing.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -299,7 +299,9 @@ contract AllOrNothing is IReward, BaseTreasury, TimestampChecker, ReentrancyGuar
}
pledgeAmount += tempReward.rewardValue;
}
_pledge(backer, pledgeToken, reward[0], pledgeAmount, shippingFee, reward);
uint256 pledgeAmountInTokenDecimals = _denormalizeAmount(pledgeToken, pledgeAmount);
uint256 shippingFeeInTokenDecimals = _denormalizeAmount(pledgeToken, shippingFee);
Comment on lines +302 to +303
Copy link

@chatgpt-codex-connector chatgpt-codex-connector bot Mar 3, 2026

Choose a reason for hiding this comment

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

P2 Badge 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 👍 / 👎.

_pledge(backer, pledgeToken, reward[0], pledgeAmountInTokenDecimals, shippingFeeInTokenDecimals, reward);
}

/**
Expand Down Expand Up @@ -394,6 +396,15 @@ contract AllOrNothing is IReward, BaseTreasury, TimestampChecker, ReentrancyGuar
return INFO.getTotalRaisedAmount() >= INFO.getGoalAmount();
}

/**
* @dev Processes a pledge: transfers tokens, mints NFT, and updates state.
* @param backer Recipient of the pledge NFT.
* @param pledgeToken Token used for the pledge.
* @param reward First reward tier (ZERO_BYTES for non-reward pledges).
* @param pledgeAmount Pledge amount in the token's native decimals (must be denormalized by caller).
* @param shippingFee Shipping fee in the token's native decimals (must be denormalized by caller; use 0 for non-reward).
* @param rewards Full reward selection (for event).
*/
function _pledge(
address backer,
address pledgeToken,
Expand All @@ -402,39 +413,24 @@ contract AllOrNothing is IReward, BaseTreasury, TimestampChecker, ReentrancyGuar
uint256 shippingFee,
bytes32[] memory rewards
) private {
// Validate token is accepted
if (!INFO.isTokenAccepted(pledgeToken)) {
revert AllOrNothingTokenNotAccepted(pledgeToken);
}

// If this is for a reward, pledgeAmount and shippingFee are in 18 decimals
// If not for a reward, amounts are already in token decimals
uint256 pledgeAmountInTokenDecimals;
uint256 shippingFeeInTokenDecimals;

if (reward != ZERO_BYTES) {
// Reward pledge: denormalize from 18 decimals to token decimals
pledgeAmountInTokenDecimals = _denormalizeAmount(pledgeToken, pledgeAmount);
shippingFeeInTokenDecimals = _denormalizeAmount(pledgeToken, shippingFee);
} else {
// Non-reward pledge: already in token decimals
pledgeAmountInTokenDecimals = pledgeAmount;
shippingFeeInTokenDecimals = shippingFee;
}

uint256 totalAmount = pledgeAmountInTokenDecimals + shippingFeeInTokenDecimals;
// pledgeAmount and shippingFee are always in pledgeToken's native decimals (callers must denormalize)
uint256 totalAmount = pledgeAmount + shippingFee;

IERC20(pledgeToken).safeTransferFrom(backer, address(this), totalAmount);

uint256 tokenId = INFO.mintNFTForPledge(
backer, reward, pledgeToken, pledgeAmountInTokenDecimals, shippingFeeInTokenDecimals, 0
backer, reward, pledgeToken, pledgeAmount, shippingFee, 0
);

s_tokenToPledgedAmount[tokenId] = pledgeAmountInTokenDecimals;
s_tokenToPledgedAmount[tokenId] = pledgeAmount;
s_tokenToTotalCollectedAmount[tokenId] = totalAmount;
s_tokenIdToPledgeToken[tokenId] = pledgeToken;
s_tokenRaisedAmounts[pledgeToken] += pledgeAmountInTokenDecimals;
s_tokenLifetimeRaisedAmounts[pledgeToken] += pledgeAmountInTokenDecimals;
s_tokenRaisedAmounts[pledgeToken] += pledgeAmount;
s_tokenLifetimeRaisedAmounts[pledgeToken] += pledgeAmount;

emit Receipt(backer, pledgeToken, reward, pledgeAmount, shippingFee, tokenId, rewards);
}
Expand Down
40 changes: 20 additions & 20 deletions src/treasuries/KeepWhatsRaised.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -765,7 +765,8 @@ contract KeepWhatsRaised is IReward, BaseTreasury, TimestampChecker, ICampaignDa
}
pledgeAmount += tempReward.rewardValue;
}
_pledge(pledgeId, backer, pledgeToken, reward[0], pledgeAmount, tip, reward, tokenSource);
uint256 pledgeAmountInTokenDecimals = _denormalizeAmount(pledgeToken, pledgeAmount);
Copy link

@chatgpt-codex-connector chatgpt-codex-connector bot Mar 3, 2026

Choose a reason for hiding this comment

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

P2 Badge 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 👍 / 👎.

_pledge(pledgeId, backer, pledgeToken, reward[0], pledgeAmountInTokenDecimals, tip, reward, tokenSource);
}

/**
Expand Down Expand Up @@ -1107,6 +1108,17 @@ contract KeepWhatsRaised is IReward, BaseTreasury, TimestampChecker, ICampaignDa
return true;
}

/**
* @dev Processes a pledge: transfers tokens, mints NFT, and updates state.
* @param pledgeId Unique identifier for the pledge.
* @param backer Recipient of the pledge NFT.
* @param pledgeToken Token used for the pledge.
* @param reward First reward tier (ZERO_BYTES for non-reward pledges).
* @param pledgeAmount Pledge amount in the token's native decimals (must be denormalized by caller).
* @param tip Tip amount in the token's native decimals.
* @param rewards Full reward selection (for event).
* @param tokenSource Address from which tokens are transferred.
*/
function _pledge(
bytes32 pledgeId,
address backer,
Expand All @@ -1117,37 +1129,25 @@ contract KeepWhatsRaised is IReward, BaseTreasury, TimestampChecker, ICampaignDa
bytes32[] memory rewards,
address tokenSource
) private {
// Validate token is accepted
if (!INFO.isTokenAccepted(pledgeToken)) {
revert KeepWhatsRaisedTokenNotAccepted(pledgeToken);
}

// If this is for a reward, pledgeAmount is in 18 decimals and needs to be denormalized
// If not for a reward (pledgeWithoutAReward), pledgeAmount is already in token decimals
// Tip is always in the pledgeToken's decimals (same token used for payment)
uint256 pledgeAmountInTokenDecimals;
if (reward != ZERO_BYTES) {
// Reward pledge: denormalize from 18 decimals to token decimals
pledgeAmountInTokenDecimals = _denormalizeAmount(pledgeToken, pledgeAmount);
} else {
// Non-reward pledge: already in token decimals
pledgeAmountInTokenDecimals = pledgeAmount;
}

uint256 totalAmount = pledgeAmountInTokenDecimals + tip;
// pledgeAmount and tip are always in pledgeToken's native decimals (callers must denormalize)
uint256 totalAmount = pledgeAmount + tip;

IERC20(pledgeToken).safeTransferFrom(tokenSource, address(this), totalAmount);

uint256 tokenId = INFO.mintNFTForPledge(backer, reward, pledgeToken, pledgeAmountInTokenDecimals, 0, tip);
uint256 tokenId = INFO.mintNFTForPledge(backer, reward, pledgeToken, pledgeAmount, 0, tip);

s_tokenToPledgedAmount[tokenId] = pledgeAmountInTokenDecimals;
s_tokenToPledgedAmount[tokenId] = pledgeAmount;
s_tokenToTippedAmount[tokenId] = tip;
s_tokenIdToPledgeToken[tokenId] = pledgeToken;
s_tipPerToken[pledgeToken] += tip;
s_tokenRaisedAmounts[pledgeToken] += pledgeAmountInTokenDecimals;
s_tokenLifetimeRaisedAmounts[pledgeToken] += pledgeAmountInTokenDecimals;
s_tokenRaisedAmounts[pledgeToken] += pledgeAmount;
s_tokenLifetimeRaisedAmounts[pledgeToken] += pledgeAmount;

uint256 netAvailable = _calculateNetAvailable(pledgeId, pledgeToken, tokenId, pledgeAmountInTokenDecimals);
uint256 netAvailable = _calculateNetAvailable(pledgeId, pledgeToken, tokenId, pledgeAmount);
s_availablePerToken[pledgeToken] += netAvailable;

emit Receipt(backer, pledgeToken, reward, pledgeAmount, tip, tokenId, rewards);
Expand Down