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.

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

Jump to bottom

fix: contributor fee mitigations #301

Merged
merged 10 commits into from
Sep 18, 2023
Merged

fix: contributor fee mitigations #301

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
4d43a5a
fix: mitigations contributor router
arr00 Sep 15, 2023
20ba15e
gatekeepers mitigation
arr00 Sep 15, 2023
ec0a21d
initial work on fixing
arr00 Sep 18, 2023
66f6e19
fix tests
arr00 Sep 18, 2023
bb02d22
add invalid message value tests
arr00 Sep 18, 2023
1fa487a
test gatekeeper
arr00 Sep 18, 2023
16b7d4b
fix gatekeep revert
arr00 Sep 18, 2023
f7c5190
add docs
arr00 Sep 18, 2023
4435d9c
Update comment contracts/crowdfund/ContributionRouter.sol
arr00 Sep 18, 2023
40b8089
add delegate note
arr00 Sep 18, 2023
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
2 changes: 1 addition & 1 deletion contracts/crowdfund/ContributionRouter.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ contract ContributionRouter {
assembly {
// 228 is the offset of the length of `tokenIds` in the
arr00 marked this conversation as resolved.
Show resolved Hide resolved
// calldata.
numOfMints := calldataload(228)
numOfMints := calldataload(196)
}
feeAmount *= numOfMints;
}
Expand Down
3 changes: 2 additions & 1 deletion contracts/crowdfund/Crowdfund.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -110,6 +110,7 @@ abstract contract Crowdfund is Implementation, ERC721Receiver, CrowdfundNFT {
error OnlyWhenEmergencyActionsAllowedError();
error BelowMinimumContributionsError(uint96 contributions, uint96 minContributions);
error AboveMaximumContributionsError(uint96 contributions, uint96 maxContributions);
error InvalidMessageValue();

event Burned(address contributor, uint256 ethUsed, uint256 ethOwed, uint256 votingPower);
event Contributed(
Expand Down Expand Up @@ -386,7 +387,7 @@ abstract contract Crowdfund is Implementation, ERC721Receiver, CrowdfundNFT {
valuesSum += values[i];
}
if (msg.value != valuesSum) {
revert();
revert InvalidMessageValue();
}
}

Expand Down
1 change: 1 addition & 0 deletions contracts/crowdfund/ETHCrowdfundBase.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,7 @@ contract ETHCrowdfundBase is Implementation {
error ZeroVotingPowerError();
error FundingSplitAlreadyPaidError();
error FundingSplitNotConfiguredError();
error InvalidMessageValue();

event Contributed(
address indexed sender,
Expand Down
5 changes: 3 additions & 2 deletions contracts/crowdfund/InitialETHCrowdfund.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -253,9 +253,10 @@ contract InitialETHCrowdfund is ETHCrowdfundBase {
function batchContributeFor(
BatchContributeForArgs calldata args
) external payable onlyDelegateCall returns (uint96[] memory votingPowers) {
votingPowers = new uint96[](args.recipients.length);
uint256 valuesSum;
for (uint256 i; i < args.recipients.length; ++i) {
_contribute(
votingPowers[i] = _contribute(
args.recipients[i],
args.initialDelegates[i],
args.values[i],
Expand All @@ -265,7 +266,7 @@ contract InitialETHCrowdfund is ETHCrowdfundBase {
valuesSum += args.values[i];
}
if (msg.value != valuesSum) {
revert();
revert InvalidMessageValue();
}
}

Expand Down
3 changes: 1 addition & 2 deletions test/crowdfund/ContributionRouterIntegration.t.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -141,8 +141,7 @@ contract ContributionRouterIntegrationTest is TestUtils {
recipients: recipients,
initialDelegates: delegates,
values: values,
gateDatas: gateDatas,
revertOnFailure: true
gateDatas: gateDatas
})
)
);
Expand Down
18 changes: 9 additions & 9 deletions test/crowdfund/InitialETHCrowdfund.t.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -974,17 +974,17 @@ contract InitialETHCrowdfundTest is LintJSON, TestUtils, ERC721Receiver {

// Batch contribute for
vm.prank(sender);
uint256[] memory tokenIds = new uint256[](4);
address payable[] memory recipients = new address payable[](4);
address[] memory delegates = new address[](4);
uint96[] memory values = new uint96[](4);
bytes[] memory gateDatas = new bytes[](4);
for (uint256 i; i < 4; ++i) {
uint256[] memory tokenIds = new uint256[](3);
address payable[] memory recipients = new address payable[](3);
address[] memory delegates = new address[](3);
uint96[] memory values = new uint96[](3);
bytes[] memory gateDatas = new bytes[](3);
for (uint256 i; i < 3; ++i) {
recipients[i] = _randomAddress();
delegates[i] = _randomAddress();
values[i] = i != 3 ? 1 ether : 0.5 ether; // Last contribution is below min contribution
values[i] = 1 ether;
}
uint96[] memory votingPowers = crowdfund.batchContributeFor{ value: 4 ether }(
uint96[] memory votingPowers = crowdfund.batchContributeFor{ value: 3 ether }(
InitialETHCrowdfund.BatchContributeForArgs({
tokenIds: tokenIds,
recipients: recipients,
Expand All @@ -994,7 +994,7 @@ contract InitialETHCrowdfundTest is LintJSON, TestUtils, ERC721Receiver {
})
);

assertEq(address(sender).balance, 1 ether); // Should be refunded 1 ETH
assertEq(address(sender).balance, 1 ether);
for (uint256 i; i < 3; ++i) {
assertEq(votingPowers[i], 1 ether);
assertEq(crowdfund.delegationsByContributor(recipients[i]), delegates[i]);
Expand Down