Skip to content

Commit

Permalink
do not allow arbitrary calls to Zora.cancelAuction() unless it is t…
Browse files Browse the repository at this point in the history
…he last call in sequence (#154)
  • Loading branch information
merklejerk committed Oct 25, 2022
1 parent 83b2b70 commit f3af0ed
Show file tree
Hide file tree
Showing 4 changed files with 75 additions and 6 deletions.
29 changes: 26 additions & 3 deletions contracts/proposals/ArbitraryCallsProposal.sol
Expand Up @@ -7,6 +7,7 @@ import "../tokens/IERC721Receiver.sol";
import "../tokens/ERC1155Receiver.sol";
import "../utils/LibSafeERC721.sol";
import "../utils/LibAddress.sol";
import "../vendor/markets/IZoraAuctionHouse.sol";
import "./vendor/IOpenseaExchange.sol";

import "./LibProposal.sol";
Expand Down Expand Up @@ -39,6 +40,12 @@ contract ArbitraryCallsProposal {

event ArbitraryCallExecuted(uint256 proposalId, uint256 idx, uint256 count);

IZoraAuctionHouse private immutable _ZORA;

constructor(IZoraAuctionHouse zora) {
_ZORA = zora;
}

function _executeArbitraryCalls(
IProposalExecutionEngine.ExecuteProposalParams memory params
)
Expand Down Expand Up @@ -67,7 +74,7 @@ contract ArbitraryCallsProposal {
// Execute an arbitrary call.
_executeSingleArbitraryCall(
i,
calls[i],
calls,
params.preciousTokens,
params.preciousTokenIds,
isUnanimous,
Expand Down Expand Up @@ -101,16 +108,24 @@ contract ArbitraryCallsProposal {

function _executeSingleArbitraryCall(
uint256 idx,
ArbitraryCall memory call,
ArbitraryCall[] memory calls,
IERC721[] memory preciousTokens,
uint256[] memory preciousTokenIds,
bool isUnanimous,
uint256 ethAvailable
)
private
{
ArbitraryCall memory call = calls[idx];
// Check that the call is not prohibited.
if (!_isCallAllowed(call, isUnanimous, preciousTokens, preciousTokenIds)) {
if (!_isCallAllowed(
call,
isUnanimous,
idx,
calls.length,
preciousTokens,
preciousTokenIds))
{
revert CallProhibitedError(call.target, call.data);
}
// Check that we have enough ETH to execute the call.
Expand Down Expand Up @@ -151,6 +166,8 @@ contract ArbitraryCallsProposal {
function _isCallAllowed(
ArbitraryCall memory call,
bool isUnanimous,
uint256 callIndex,
uint256 callsCount,
IERC721[] memory preciousTokens,
uint256[] memory preciousTokenIds
)
Expand Down Expand Up @@ -197,6 +214,12 @@ contract ArbitraryCallsProposal {
if (isApproved) {
return !LibProposal.isTokenPrecious(IERC721(call.target), preciousTokens);
}
// Can only call cancelAuction on the zora AH if it's the last call
// in the sequence.
} else if (selector == IZoraAuctionHouse.cancelAuction.selector) {
if (call.target == address(_ZORA)) {
return callIndex + 1 == callsCount;
}
}
}
// Can never call receive hooks on any target.
Expand Down
1 change: 1 addition & 0 deletions contracts/proposals/ProposalExecutionEngine.sol
Expand Up @@ -88,6 +88,7 @@ contract ProposalExecutionEngine is
ListOnOpenseaProposal(globals, seaport, seaportConduitController)
ListOnZoraProposal(globals, zoraAuctionHouse)
FractionalizeProposal(fractionalVaultFactory)
ArbitraryCallsProposal(zoraAuctionHouse)
{
_GLOBALS = globals;
}
Expand Down
2 changes: 0 additions & 2 deletions sol-tests/crowdfund/Crowdfund.t.sol
Expand Up @@ -1022,7 +1022,6 @@ contract CrowdfundTest is Test, TestUtils {
function test_canReuseContributionEntry() external {
TestableCrowdfund cf = _createCrowdfund(0);
address contributor = _randomAddress();
address payable badERC721Receiver = payable(new BadERC721Receiver());
// Contributor contributes twice back-to-back.
vm.deal(contributor, 3);
vm.prank(contributor);
Expand All @@ -1039,7 +1038,6 @@ contract CrowdfundTest is Test, TestUtils {
TestableCrowdfund cf = _createCrowdfund(0);
address contributor1 = _randomAddress();
address contributor2 = _randomAddress();
address payable badERC721Receiver = payable(new BadERC721Receiver());
// contributor1 sandwiches contributor2.
vm.deal(contributor1, 3);
vm.deal(contributor2, 10);
Expand Down
49 changes: 48 additions & 1 deletion sol-tests/proposals/ArbitraryCallsProposal.t.sol
Expand Up @@ -11,6 +11,9 @@ import "../TestUtils.sol";
import "../DummyERC721.sol";

contract TestableArbitraryCallsProposal is ArbitraryCallsProposal {

constructor(IZoraAuctionHouse zora) ArbitraryCallsProposal(zora) {}

function execute(
IProposalExecutionEngine.ExecuteProposalParams calldata params
)
Expand Down Expand Up @@ -66,6 +69,10 @@ contract ArbitraryCallTarget {
}
}

contract FakeZora {
function cancelAuction(uint256 auctionId) external {}
}

contract ArbitraryCallsProposalTest is
Test,
TestUtils,
Expand All @@ -80,7 +87,9 @@ contract ArbitraryCallsProposalTest is
event ArbitraryCallExecuted(uint256 proposalId, uint256 idx, uint256 count);

ArbitraryCallTarget target = new ArbitraryCallTarget();
TestableArbitraryCallsProposal testContract = new TestableArbitraryCallsProposal();
FakeZora zora = new FakeZora();
TestableArbitraryCallsProposal testContract =
new TestableArbitraryCallsProposal(IZoraAuctionHouse(address(zora)));
IERC721[] preciousTokens;
uint256[] preciousTokenIds;

Expand Down Expand Up @@ -615,6 +624,44 @@ contract ArbitraryCallsProposalTest is
testContract.execute(prop);
}

function test_cannotCallCancelAuctionOnZoraIfNotLastCall() external {
(
ArbitraryCallsProposal.ArbitraryCall[] memory calls,
) = _createSimpleCalls(2, false);
calls[0].target = payable(address(zora));
calls[0].data = abi.encodeCall(
IZoraAuctionHouse.cancelAuction,
(_randomUint256()) // params don't matter
);
IProposalExecutionEngine.ExecuteProposalParams memory prop =
_createTestProposal(calls);
vm.expectRevert(abi.encodeWithSelector(
ArbitraryCallsProposal.CallProhibitedError.selector,
calls[0].target,
calls[0].data
));
testContract.execute(prop);
}

function test_canCallCancelAuctionOnZoraIfLastCall() external {
(
ArbitraryCallsProposal.ArbitraryCall[] memory calls,
) = _createSimpleCalls(2, false);
calls[1].target = payable(address(zora));
calls[1].data = abi.encodeCall(
IZoraAuctionHouse.cancelAuction,
(_randomUint256()) // params don't matter
);
IProposalExecutionEngine.ExecuteProposalParams memory prop =
_createTestProposal(calls);
_expectEmit0();
emit ArbitraryCallExecuted(prop.proposalId, 0, 2);
_expectEmit0();
emit ArbitraryCallExecuted(prop.proposalId, 1, 2);
testContract.execute(prop);
}


function test_cannotExecuteShortApproveCallData() external {
(
ArbitraryCallsProposal.ArbitraryCall[] memory calls,
Expand Down

0 comments on commit f3af0ed

Please sign in to comment.