Add TokenTrade scheme #774
Conversation
contracts/schemes/TokenTrade.sol
Outdated
| * - A member can donate to a dao. | ||
| */ | ||
| contract TokenTrade is VotingMachineCallbacks, ProposalExecuteInterface { | ||
| using SafeMath for uint; |
| * @param _voteParamsHash voting machine parameters. | ||
| */ | ||
| function initialize( | ||
| Avatar _avatar, |
There was a problem hiding this comment.
I think it should get also the inTradingToken
There was a problem hiding this comment.
Why not allow any token? What's the reason to limit it?
There was a problem hiding this comment.
- easy in the UI.
- simple code
- more secure
| "Token address must not be null" | ||
| ); | ||
| require(_sendTokenAmount > 0 && _receiveTokenAmount > 0, "Token amount must be greater than 0"); | ||
|
|
There was a problem hiding this comment.
I think we need a way to lock the recieve token in the scheme.. so there will not be a case where the proposal execute and the dao cannot pay .
so maybe upon proposal the scheme will ask the receive token amount from the avatar.. ,if not enough exist will revert.
if proposal is accepted the user will redeem (from the scheme) .
if proposal is rejected the allocated token will be sent back to the avatar
There was a problem hiding this comment.
This is bad because an attacker can open dummy proposal and lock the avatar's balance. The way I handled that case is that if upon execution the DAO doesn't have enough funds, the proposal will be canceled as if it was rejected, and the user will get his tokens back.
contracts/schemes/TokenTrade.sol
Outdated
| uint256 sendTokenAmount; | ||
| IERC20 receiveToken; | ||
| uint256 receiveTokenAmount; | ||
| bool exist; |
There was a problem hiding this comment.
does the exist var is really needed ?
does checking other filed like beneficiary or sendToken is not enough ?
contracts/schemes/TokenTrade.sol
Outdated
| require(_sendTokenAmount > 0 && _receiveTokenAmount > 0, "Token amount must be greater than 0"); | ||
|
|
||
| _sendToken.safeTransferFrom(msg.sender, address(this), _sendTokenAmount); | ||
| bytes32 proposalId = votingMachine.propose(2, voteParamsHash, msg.sender, address(avatar)); |
There was a problem hiding this comment.
proposalId can be defined as return value at https://github.com/daostack/arc/pull/774/files#diff-0568445d702e1cdbb17b4e3cdc934e5dR142
contracts/schemes/TokenTrade.sol
Outdated
|
|
||
| /** | ||
| * @title A scheme for join in a dao. | ||
| * - A member can be proposed to join in by sending a min amount of fee. |
There was a problem hiding this comment.
this comment should be updated
| function execute(bytes32 _proposalId) public { | ||
| Proposal storage proposal = proposals[_proposalId]; | ||
| require(address(_sendToken) != address(0), "must be a live proposal"); | ||
| require(proposal.decided, "must be a decided proposal"); |
There was a problem hiding this comment.
does this condition is not enough ? why need the previous one ?
No description provided.