Skip to content

Commit

Permalink
fix: contributor fee mitigations (#301)
Browse files Browse the repository at this point in the history 
* fix: mitigations contributor router

* gatekeepers mitigation

* initial work on fixing

* fix tests

* add invalid message value tests

* test gatekeeper

* fix gatekeep revert

* add docs

* Update comment contracts/crowdfund/ContributionRouter.sol

Co-authored-by: Brian Le <[email protected]>

* add delegate note

---------

Co-authored-by: Brian Le <[email protected]>
  • Loading branch information
@arr00 @0xble
arr00 and 0xble authored Sep 18, 2023
1 parent 9b5f379 commit 155b275
Show file tree
Hide file tree
Showing 16 changed files with 233 additions and 106 deletions.
38 changes: 30 additions & 8 deletions contracts/crowdfund/ContributionRouter.sol
View file
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,21 +53,34 @@ contract ContributionRouter {
emit ClaimedFees(msg.sender, recipient, balance);
}

/// @notice View function to get the `feePerMint` value.
function feePerMint() external view returns (uint96) {
return _storage.feePerMint;
}

/// @notice View function to get the most recent caller to the contract.
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.
/// @dev Only initial contributions per address allow for setting the delegate.
/// Use the `delegate` function to set afterwards.
fallback() external payable {
uint256 feeAmount = feePerMint;
uint256 feeAmount = _storage.feePerMint;
_storage.caller = msg.sender;
address target;
assembly {
target := shr(96, calldataload(sub(calldatasize(), 20)))
}
if (msg.sig == InitialETHCrowdfund.batchContributeFor.selector) {
uint256 numOfMints;
assembly {
// 228 is the offset of the length of `tokenIds` in the
// 196 is the offset of the length of `tokenIds` in the
// calldata.
numOfMints := calldataload(228)
numOfMints := calldataload(196)
}
feeAmount *= numOfMints;
}
Expand All @@ -72,4 +89,9 @@ contract ContributionRouter {

emit ReceivedFees(msg.sender, feeAmount);
}

// Revert for any vanilla ETH transfers.
receive() external payable {
revert();
}
}
28 changes: 16 additions & 12 deletions contracts/crowdfund/Crowdfund.sol
View file
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
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
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
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
12 changes: 11 additions & 1 deletion contracts/gatekeepers/AllowListGateKeeper.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,19 +3,29 @@ 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 {
/// @notice The address of the canonical contribution router.
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
10 changes: 10 additions & 0 deletions contracts/gatekeepers/TokenGateKeeper.sol
View file
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,15 @@ interface Token {
* @notice a contract that implements an token gatekeeper
*/
contract TokenGateKeeper is IGateKeeper {
/// @notice The address of the canonical contribution router.
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 +40,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
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
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
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

0 comments on commit 155b275

Please sign in to comment.