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 7 commits
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
31 changes: 24 additions & 7 deletions contracts/crowdfund/ContributionRouter.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,12 +18,16 @@ contract ContributionRouter {
/// @notice The address allowed to claim fees from the contract.
address public immutable OWNER;

/// @notice The amount of fees to pay to the DAO per mint.
uint96 public feePerMint;
struct Storage {
uint96 feePerMint;
address caller;
}

Storage private _storage;

constructor(address owner, uint96 initialFeePerMint) {
OWNER = owner;
feePerMint = initialFeePerMint;
_storage.feePerMint = initialFeePerMint;
}

modifier onlyOwner() {
Expand All @@ -34,9 +38,9 @@ contract ContributionRouter {
/// @notice Set the fee per mint. Only the owner can call.
/// @param newFeePerMint The new amount to set fee per mint to.
function setFeePerMint(uint96 newFeePerMint) external onlyOwner {
emit FeePerMintUpdated(feePerMint, newFeePerMint);
emit FeePerMintUpdated(_storage.feePerMint, newFeePerMint);

feePerMint = newFeePerMint;
_storage.feePerMint = newFeePerMint;
}

/// @notice Claim fees from the contract. Only the owner can call.
Expand All @@ -49,11 +53,20 @@ contract ContributionRouter {
emit ClaimedFees(msg.sender, recipient, balance);
}

function feePerMint() external view returns (uint96) {
return _storage.feePerMint;
}

function caller() external view returns (address) {
return _storage.caller;
}

/// @notice Fallback function that forwards the call to the target contract
/// and keeps the fee amount. The target contract is expected to
/// be appended to the calldata.
fallback() external payable {
uint256 feeAmount = feePerMint;
uint256 feeAmount = _storage.feePerMint;
_storage.caller = msg.sender;
address target;
assembly {
target := shr(96, calldataload(sub(calldatasize(), 20)))
Expand All @@ -63,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 All @@ -72,4 +85,8 @@ contract ContributionRouter {

emit ReceivedFees(msg.sender, feeAmount);
}

receive() external payable {
revert();
}
}
28 changes: 16 additions & 12 deletions 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 @@ -366,24 +367,27 @@ abstract contract Crowdfund is Implementation, ERC721Receiver, CrowdfundNFT {
/// @param initialDelegates The addresses to delegate to for each recipient.
/// @param values The ETH to contribute for each recipient.
/// @param gateDatas Data to pass to the gatekeeper to prove eligibility.
/// @param revertOnFailure If true, revert if any contribution fails.
function batchContributeFor(
address[] memory recipients,
address[] memory initialDelegates,
uint256[] memory values,
bytes[] memory gateDatas,
bool revertOnFailure
Copy link
Collaborator

Choose a reason for hiding this comment

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

I don't think it does, but note to self that we'll have to check with FE this doesn't break anything.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

good callout. Agree that it's probably fine.

uint96[] memory values,
bytes[] memory gateDatas
) external payable {
uint256 valuesSum;
for (uint256 i; i < recipients.length; ++i) {
(bool s, bytes memory r) = address(this).call{ value: values[i] }(
abi.encodeCall(
this.contributeFor,
(recipients[i], initialDelegates[i], gateDatas[i])
)
_setDelegate(recipients[i], initialDelegates[i]);

_contribute(
recipients[i],
initialDelegates[i],
values[i],
totalContributions,
gateDatas[i]
);
if (revertOnFailure && !s) {
r.rawRevert();
}
valuesSum += values[i];
}
if (msg.value != valuesSum) {
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
49 changes: 14 additions & 35 deletions contracts/crowdfund/InitialETHCrowdfund.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -89,8 +89,6 @@ contract InitialETHCrowdfund is ETHCrowdfundBase {
// The data required to be validated by the `gatekeeper`, if set. If no
// `gatekeeper` is set, this can be empty.
bytes[] gateDatas;
// Whether to revert if any individual contribution fails or continue.
bool revertOnFailure;
}

event Refunded(address indexed contributor, uint256 indexed tokenId, uint256 amount);
Expand Down Expand Up @@ -255,35 +253,21 @@ contract InitialETHCrowdfund is ETHCrowdfundBase {
function batchContributeFor(
BatchContributeForArgs calldata args
) external payable onlyDelegateCall returns (uint96[] memory votingPowers) {
uint256 numContributions = args.recipients.length;
votingPowers = new uint96[](numContributions);

uint256 ethAvailable = msg.value;
for (uint256 i; i < numContributions; ++i) {
(bool s, bytes memory r) = address(this).call{ value: args.values[i] }(
abi.encodeCall(
this.contributeFor,
(
args.tokenIds[i],
args.recipients[i],
args.initialDelegates[i],
args.gateDatas[i]
)
)
votingPowers = new uint96[](args.recipients.length);
uint256 valuesSum;
for (uint256 i; i < args.recipients.length; ++i) {
votingPowers[i] = _contribute(
args.recipients[i],
args.initialDelegates[i],
args.values[i],
args.tokenIds[i],
args.gateDatas[i]
);

if (!s) {
if (args.revertOnFailure) {
r.rawRevert();
}
} else {
votingPowers[i] = abi.decode(r, (uint96));
ethAvailable -= args.values[i];
}
valuesSum += args.values[i];
}
if (msg.value != valuesSum) {
revert InvalidMessageValue();
}

// Refund any unused ETH.
if (ethAvailable > 0) payable(msg.sender).transfer(ethAvailable);
}

function _contribute(
Expand All @@ -302,12 +286,7 @@ contract InitialETHCrowdfund is ETHCrowdfundBase {
IGateKeeper _gateKeeper = gateKeeper;
if (_gateKeeper != IGateKeeper(address(0))) {
if (!_gateKeeper.isAllowed(msg.sender, gateKeeperId, gateData)) {
revert NotAllowedByGateKeeperError(
contributor,
_gateKeeper,
gateKeeperId,
gateData
);
revert NotAllowedByGateKeeperError(msg.sender, _gateKeeper, gateKeeperId, gateData);
}
}

Expand Down
11 changes: 10 additions & 1 deletion contracts/gatekeepers/AllowListGateKeeper.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,19 +3,28 @@ pragma solidity 0.8.20;

import "./IGateKeeper.sol";
import "openzeppelin/contracts/utils/cryptography/MerkleProof.sol";
import { ContributionRouter } from "../crowdfund/ContributionRouter.sol";

/// @notice A gateKeeper that implements a simple allow list per gate.
contract AllowListGateKeeper is IGateKeeper {
address public immutable CONTRIBUTION_ROUTER;
uint96 private _lastId;
/// @notice Get the merkle root used by a gate identifyied by it's `id`.
/// @notice Get the merkle root used by a gate identified by it's `id`.
mapping(uint96 => bytes32) public merkleRoots;

constructor(address contributionRouter) {
CONTRIBUTION_ROUTER = contributionRouter;
}

/// @inheritdoc IGateKeeper
function isAllowed(
address participant,
bytes12 id,
bytes memory userData
) external view returns (bool) {
if (participant == CONTRIBUTION_ROUTER) {
participant = ContributionRouter(payable(CONTRIBUTION_ROUTER)).caller();
}
bytes32[] memory proof = abi.decode(userData, (bytes32[]));
bytes32 leaf;
assembly {
Expand Down
9 changes: 9 additions & 0 deletions contracts/gatekeepers/TokenGateKeeper.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
pragma solidity 0.8.20;

import { IGateKeeper } from "./IGateKeeper.sol";
import { ContributionRouter } from "../crowdfund/ContributionRouter.sol";

/**
* @notice Compatible with both ER20s and ERC721s.
Expand All @@ -14,9 +15,14 @@ interface Token {
* @notice a contract that implements an token gatekeeper
*/
contract TokenGateKeeper is IGateKeeper {
address public immutable CONTRIBUTION_ROUTER;
// last gate id
uint96 private _lastId;

constructor(address contributionRouter) {
CONTRIBUTION_ROUTER = contributionRouter;
}

struct TokenGate {
Token token;
uint256 minimumBalance;
Expand All @@ -33,6 +39,9 @@ contract TokenGateKeeper is IGateKeeper {
bytes12 id,
bytes memory /* userData */
) external view returns (bool) {
if (participant == CONTRIBUTION_ROUTER) {
participant = ContributionRouter(payable(CONTRIBUTION_ROUTER)).caller();
}
TokenGate memory _gate = gateInfo[uint96(id)];
return _gate.token.balanceOf(participant) >= _gate.minimumBalance;
}
Expand Down
4 changes: 2 additions & 2 deletions deploy/Deploy.s.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -360,7 +360,7 @@ abstract contract Deploy {
console.log("### GateKeepers");
console.log(" Deploying - AllowListGateKeeper");
_trackDeployerGasBefore();
allowListGateKeeper = new AllowListGateKeeper();
allowListGateKeeper = new AllowListGateKeeper(address(0));
_trackDeployerGasAfter();
console.log(" Deployed - AllowListGateKeeper", address(allowListGateKeeper));

Expand Down Expand Up @@ -391,7 +391,7 @@ abstract contract Deploy {
console.log("");
console.log(" Deploying - TokenGateKeeper");
_trackDeployerGasBefore();
tokenGateKeeper = new TokenGateKeeper();
tokenGateKeeper = new TokenGateKeeper(address(0));
_trackDeployerGasAfter();
console.log(" Deployed - TokenGateKeeper", address(tokenGateKeeper));

Expand Down
4 changes: 2 additions & 2 deletions test/crowdfund/AuctionCrowdfund.t.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -663,7 +663,7 @@ contract AuctionCrowdfundTest is Test, TestUtils {
address contributor = _randomAddress();

// Create a AuctionCrowdfund instance with onlyHostCanBid and gatekeeper enabled.
AllowListGateKeeper gateKeeper = new AllowListGateKeeper();
AllowListGateKeeper gateKeeper = new AllowListGateKeeper(address(0));
bytes32 contributorHash = keccak256(abi.encodePacked(contributor));
bytes12 gateKeeperId = gateKeeper.createGate(contributorHash);
(uint256 auctionId, uint256 tokenId) = market.createAuction(0);
Expand Down Expand Up @@ -710,7 +710,7 @@ contract AuctionCrowdfundTest is Test, TestUtils {
address contributor = _randomAddress();

// Create a AuctionCrowdfund instance with a gatekeeper enabled.
AllowListGateKeeper gateKeeper = new AllowListGateKeeper();
AllowListGateKeeper gateKeeper = new AllowListGateKeeper(address(0));
bytes32 contributorHash = keccak256(abi.encodePacked(contributor));
bytes12 gateKeeperId = gateKeeper.createGate(contributorHash);
(uint256 auctionId, uint256 tokenId) = market.createAuction(0);
Expand Down
6 changes: 3 additions & 3 deletions test/crowdfund/BuyCrowdfund.t.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -288,7 +288,7 @@ contract BuyCrowdfundTest is Test, TestUtils {
address contributor = _randomAddress();

// Create a BuyCrowdfund instance with a gatekeeper enabled.
AllowListGateKeeper gateKeeper = new AllowListGateKeeper();
AllowListGateKeeper gateKeeper = new AllowListGateKeeper(address(0));
bytes32 contributorHash = keccak256(abi.encodePacked(contributor));
bytes12 gateKeeperId = gateKeeper.createGate(contributorHash);
BuyCrowdfund cf = _createCrowdfund(
Expand Down Expand Up @@ -328,7 +328,7 @@ contract BuyCrowdfundTest is Test, TestUtils {
address contributor = _randomAddress();

// Create a BuyCrowdfund instance with onlyHostCanBuy and a gatekeeper enabled.
AllowListGateKeeper gateKeeper = new AllowListGateKeeper();
AllowListGateKeeper gateKeeper = new AllowListGateKeeper(address(0));
bytes32 contributorHash = keccak256(abi.encodePacked(contributor));
bytes12 gateKeeperId = gateKeeper.createGate(contributorHash);
BuyCrowdfund cf = _createCrowdfund(
Expand Down Expand Up @@ -368,7 +368,7 @@ contract BuyCrowdfundTest is Test, TestUtils {
address host = _randomAddress();

// Create a BuyCrowdfund instance with onlyHostCanBuy and a gatekeeper enabled.
AllowListGateKeeper gateKeeper = new AllowListGateKeeper();
AllowListGateKeeper gateKeeper = new AllowListGateKeeper(address(0));
bytes12 gateKeeperId = gateKeeper.createGate(0);
BuyCrowdfund cf = _createCrowdfund(
0,
Expand Down
Loading