Skip to content

GenericSchemeMultiCall : Simplification. + Schemeconstraint #795

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.

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

Merged
merged 18 commits into from
Oct 14, 2020
149 changes: 149 additions & 0 deletions contracts/schemes/DxDaoSchemeConstraints.sol
Original file line number Diff line number Diff line change
@@ -0,0 +1,149 @@
pragma solidity 0.5.17;
pragma experimental ABIEncoderV2;

import "./SchemeConstraints.sol";


contract DxDaoSchemeConstraints is SchemeConstraints {
using SafeMath for uint256;

uint256 public initialTimestamp;
uint256 public periodSize;
uint256 public periodLimitWei;

mapping(uint256=>uint256) public periodSpendingTokens;
Copy link
Contributor

Choose a reason for hiding this comment

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

Unused mapping can be removed

mapping(address=>uint256) public periodLimitToken;
mapping (uint256 => mapping(address => uint256)) public periodSpendingToken;
mapping(uint256=>uint256) public periodSpendingWei;
mapping(address=>bool) public contractsWhiteListMap;
bytes4 private constant APPROVE_SIGNATURE = 0x095ea7b3;//approve(address,uint256)

/* @dev initialize
* @param _periodSize the time period to limit the tokens and eth spending
* @param _periodLimitWei the limit of eth which can be sent per period
* @param _periodLimitTokensAddresses tokens to limit
* @param _periodLimitTokensAmounts the limit of token which can be sent per period
* @param _contractsWhiteList the contracts the scheme is allowed to interact with
*/
function initialize(
uint256 _periodSize,
uint256 _periodLimitWei,
address[] calldata _periodLimitTokensAddresses,
uint256[] calldata _periodLimitTokensAmounts,
address[] calldata _contractsWhiteList
)
external {
require(initialTimestamp == 0, "cannot initialize twice");
require(_periodSize > 0, "preriod size should be greater than 0");
require(_periodLimitTokensAddresses.length == _periodLimitTokensAmounts.length,
"invalid length _periodLimitTokensAddresses");
periodSize = _periodSize;
periodLimitWei = _periodLimitWei;
// solhint-disable-next-line not-rely-on-time
initialTimestamp = block.timestamp;
for (uint i = 0; i < _contractsWhiteList.length; i++) {
contractsWhiteListMap[_contractsWhiteList[i]] = true;
}
for (uint i = 0; i < _periodLimitTokensAmounts.length; i++) {
periodLimitToken[_periodLimitTokensAddresses[i]] = _periodLimitTokensAmounts[i];
}
contractsWhiteList = _contractsWhiteList;
}

/*
* @dev isAllowedToCall should be called upon a proposal execution.
* - check that the total spending of tokens within a 'periodSize' does not exceed the periodLimit per token
* - check that the total sending of eth within a 'periodSize' does not exceed the periodLimit
* @param _contractsToCall the contracts to be called
* @param _callsData - The abi encode data for the calls
* @param _values value(ETH) to transfer with the calls
* @param _avatar avatar
* @return bool value true-allowed false not allowed
*/
function isAllowedToCall(
address[] calldata _contractsToCall,
bytes[] calldata _callsData,
uint256[] calldata _values,
Avatar
)
external
returns(bool)
{

uint256 observervationIndex = observationIndex();
uint256 totalPeriodSpendingInWei;
for (uint i = 0; i < _contractsToCall.length; i++) {
// constraint eth transfer
totalPeriodSpendingInWei = totalPeriodSpendingInWei.add(_values[i]);
bytes memory callData = _callsData[i];
// constraint approve calls
if (callData[0] == APPROVE_SIGNATURE[0] &&
callData[1] == APPROVE_SIGNATURE[1] &&
callData[2] == APPROVE_SIGNATURE[2] &&
callData[3] == APPROVE_SIGNATURE[3]) {
uint256 amount;
address contractToCall = _contractsToCall[i];
// solhint-disable-next-line no-inline-assembly
assembly {
amount := mload(add(callData, 68))
}
periodSpendingToken[observervationIndex][contractToCall] =
periodSpendingToken[observervationIndex][contractToCall].add(amount);
require(
periodSpendingToken[observervationIndex][contractToCall] <= periodLimitToken[contractToCall],
"periodSpendingTokensExceeded");
}

}
periodSpendingWei[observervationIndex] =
periodSpendingWei[observervationIndex].add(totalPeriodSpendingInWei);
require(periodSpendingWei[observervationIndex] <= periodLimitWei, "periodSpendingWeiExceeded");
return true;
}

/*
* @dev isAllowedToPropose should be called upon a proposal submition.
* allow only whitelisted target contracts or 'approve' calls which the 'spender' is whitelisted
* @param _contractsToCall the contracts to be called
* @param _callsData - The abi encode data for the calls
* @param _values value(ETH) to transfer with the calls
* @param _avatar avatar
* @return bool value true-allowed false not allowed
*/
function isAllowedToPropose(
address[] calldata _contractsToCall,
bytes[] calldata _callsData,
uint256[] calldata,
Copy link
Contributor

Choose a reason for hiding this comment

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

uint256[] calldata > uint256[] calldata _values?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes, because the interface defined that function and there is no use for values at this function

Avatar)
external
returns(bool)
{
for (uint i = 0; i < _contractsToCall.length; i++) {
if (!contractsWhiteListMap[_contractsToCall[i]]) {
address spender;
bytes memory callData = _callsData[i];
require(
callData[0] == APPROVE_SIGNATURE[0] &&
callData[1] == APPROVE_SIGNATURE[1] &&
callData[2] == APPROVE_SIGNATURE[2] &&
callData[3] == APPROVE_SIGNATURE[3],
"allow only approve call for none whitelistedContracts");
//in solidity > 6 this can be replaced by:
//(spender,) = abi.descode(callData[4:], (address, uint));
// see https://github.com/ethereum/solidity/issues/9439
// solhint-disable-next-line no-inline-assembly
assembly {
spender := mload(add(callData, 36))
}
require(contractsWhiteListMap[spender], "spender contract not whitelisted");
}
}
return true;
}

function observationIndex() public view returns (uint256) {
Copy link
Contributor

Choose a reason for hiding this comment

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

@orenyodfat this could become vulnerable to overflows if the period is set to a super low level, I would propose to add a require in the initialize() to set the period to a min limit as of 7days.

// solhint-disable-next-line not-rely-on-time
return uint8((block.timestamp - initialTimestamp) / periodSize);
}

}
72 changes: 25 additions & 47 deletions contracts/schemes/GenericSchemeMultiCall.sol
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ pragma experimental ABIEncoderV2;
import "@daostack/infra/contracts/votingMachines/IntVoteInterface.sol";
import "@daostack/infra/contracts/votingMachines/ProposalExecuteInterface.sol";
import "../votingMachines/VotingMachineCallbacks.sol";
import "./SchemeConstraints.sol";


/**
Expand All @@ -24,12 +25,10 @@ contract GenericSchemeMultiCall is VotingMachineCallbacks, ProposalExecuteInterf
}

mapping(bytes32=>MultiCallProposal) public proposals;

IntVoteInterface public votingMachine;
bytes32 public voteParams;
mapping(address=>bool) internal contractWhitelist;
address[] public whitelistedContracts;
Avatar public avatar;
SchemeConstraints public schemeConstraints;

event NewMultiCallProposal(
address indexed _avatar,
Expand Down Expand Up @@ -61,37 +60,26 @@ contract GenericSchemeMultiCall is VotingMachineCallbacks, ProposalExecuteInterf

event ProposalDeleted(address indexed _avatar, bytes32 indexed _proposalId);

/**
* @dev initialize
/* @dev initialize
* @param _avatar the avatar to mint reputation from
* @param _votingMachine the voting machines address to
* @param _voteParams voting machine parameters.
* @param _contractWhitelist the contracts the scheme is allowed to interact with
*
* @param _schemeConstraints the schemeConstraints contracts.
*/
function initialize(
Avatar _avatar,
IntVoteInterface _votingMachine,
bytes32 _voteParams,
address[] calldata _contractWhitelist
SchemeConstraints _schemeConstraints
)
external
{
require(avatar == Avatar(0), "can be called only one time");
require(_avatar != Avatar(0), "avatar cannot be zero");
require(_contractWhitelist.length > 0, "contractWhitelist cannot be empty");
avatar = _avatar;
votingMachine = _votingMachine;
voteParams = _voteParams;
/* Whitelist controller by default*/
Controller controller = Controller(_avatar.owner());
whitelistedContracts.push(address(controller));
contractWhitelist[address(controller)] = true;

for (uint i = 0; i < _contractWhitelist.length; i++) {
contractWhitelist[_contractWhitelist[i]] = true;
whitelistedContracts.push(_contractWhitelist[i]);
}
schemeConstraints = _schemeConstraints;
}

/**
Expand Down Expand Up @@ -127,28 +115,23 @@ contract GenericSchemeMultiCall is VotingMachineCallbacks, ProposalExecuteInterf
MultiCallProposal storage proposal = proposals[_proposalId];
require(proposal.exist, "must be a live proposal");
require(proposal.passed, "proposal must passed by voting machine");
if (schemeConstraints != SchemeConstraints(0)) {
require(
schemeConstraints.isAllowedToCall(
proposal.contractsToCall,
proposal.callsData,
proposal.values,
avatar),
"call is not allowed");
}
proposal.exist = false;
bytes memory genericCallReturnValue;
bool success;
Controller controller = Controller(whitelistedContracts[0]);

Controller controller = Controller(avatar.owner());
for (uint i = 0; i < proposal.contractsToCall.length; i++) {
bytes memory callData = proposal.callsData[i];
if (proposal.contractsToCall[i] == address(controller)) {
(IERC20 extToken,
address spender,
uint256 valueToSpend
) =
abi.decode(
callData,
(IERC20, address, uint256)
);
success = controller.externalTokenApproval(extToken, spender, valueToSpend, avatar);
} else {
(success, genericCallReturnValue) =
controller.genericCall(proposal.contractsToCall[i], callData, avatar, proposal.values[i]);
}

(success, genericCallReturnValue) =
controller.genericCall(proposal.contractsToCall[i], callData, avatar, proposal.values[i]);
/* Whole transaction will be reverted if at least one call fails*/
require(success, "Proposal call failed");
emit ProposalCallExecuted(
Expand Down Expand Up @@ -190,19 +173,14 @@ contract GenericSchemeMultiCall is VotingMachineCallbacks, ProposalExecuteInterf
(_contractsToCall.length == _callsData.length) && (_contractsToCall.length == _values.length),
"Wrong length of _contractsToCall, _callsDataLens or _values arrays"
);
for (uint i = 0; i < _contractsToCall.length; i++) {
if (schemeConstraints != SchemeConstraints(0)) {
require(
contractWhitelist[_contractsToCall[i]], "contractToCall is not whitelisted"
);
if (_contractsToCall[i] == whitelistedContracts[0]) {

(, address spender,) =
abi.decode(
_callsData[i],
(IERC20, address, uint256)
);
require(contractWhitelist[spender], "spender contract not whitelisted");
}
schemeConstraints.isAllowedToPropose(
_contractsToCall,
_callsData,
_values,
avatar),
"propose is not allowed");
}

proposalId = votingMachine.propose(2, voteParams, msg.sender, address(avatar));
Expand Down
44 changes: 44 additions & 0 deletions contracts/schemes/SchemeConstraints.sol
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
pragma solidity 0.5.17;
pragma experimental ABIEncoderV2;
import "../controller/Avatar.sol";


contract SchemeConstraints {

address[] public contractsWhiteList;

/*
* @dev isAllowedToCall should be called upon a proposal execution.
* @param _contractsToCall the contracts to be called
* @param _callsData - The abi encode data for the calls
* @param _values value(ETH) to transfer with the calls
* @param _avatar avatar
* @return bool value true-allowed false not allowed
*/
function isAllowedToCall(
address[] calldata _contractsToCall,
bytes[] calldata _callsData,
uint256[] calldata _values,
Avatar _avatar)
external returns(bool);

/*
* @dev isAllowedToPropose should be called upon a proposal submition.
* @param _contractsToCall the contracts to be called
* @param _callsData - The abi encode data for the calls
* @param _values value(ETH) to transfer with the calls
* @param _avatar avatar
* @return bool value true-allowed false not allowed
*/
function isAllowedToPropose(
address[] calldata _contractsToCall,
bytes[] calldata _callsData,
uint256[] calldata _values,
Avatar _avatar)
external returns(bool);

function getContractsWhiteList() external view returns(address[] memory) {
return contractsWhiteList;
}

}
Loading