-
Notifications
You must be signed in to change notification settings - Fork 21
etherGC.sol +reputationGC.sol #756
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
base: master
Are you sure you want to change the base?
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,84 @@ | ||
pragma solidity 0.5.17; | ||
|
||
import "openzeppelin-solidity/contracts/token/ERC20/IERC20.sol"; | ||
import "./GlobalConstraintInterface.sol"; | ||
import "openzeppelin-solidity/contracts/math/SafeMath.sol"; | ||
|
||
|
||
/** | ||
* @title EtherGC ether constraint per period | ||
*/ | ||
contract EtherGC { | ||
using SafeMath for uint256; | ||
|
||
uint256 public constraintPeriodWindow; //a static window | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. maybe call it "periodLength"? and add in the comment that the unit is blocks |
||
uint256 public amountAllowedPerPeriod; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Not sure if this should be percentage-wise, or absolute values. Probably better to keep it absolute. |
||
address public avatar; | ||
uint256 public latestEthBalance; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Not sure what this is fore, this is just set in the initliaze call... if you need it, give it a more meaningful name perhaps? |
||
mapping(uint256=>uint256) public totalAmountSentForPeriods; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
uint256 public startBlock; | ||
address public controller; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this needs some explanation, I think. So the constraint holds for what? it is a limit on how much the funds from the avatar the controller is allowed to spend per period. |
||
uint256 public avatarBalanceBefore; | ||
|
||
/** | ||
* @dev initialize | ||
* @param _avatar the avatar to mint reputation from | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this is a bit a confusing comment |
||
* @param _constraintPeriodWindow the constraintPeriodWindow in blocks units | ||
* @param _amountAllowedPerPeriod the amount of eth to constraint for each period | ||
*/ | ||
function initialize( | ||
address _avatar, | ||
uint256 _constraintPeriodWindow, | ||
uint256 _amountAllowedPerPeriod, | ||
address _controller | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. why do you need the controller as a separate argument, can't you get that from the avatar_? |
||
) | ||
external | ||
{ | ||
require(avatar == address(0), "can be called only one time"); | ||
require(_avatar != address(0), "avatar cannot be zero"); | ||
avatar = _avatar; | ||
constraintPeriodWindow = _constraintPeriodWindow; | ||
amountAllowedPerPeriod = _amountAllowedPerPeriod; | ||
latestEthBalance = avatar.balance; | ||
startBlock = block.number; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Start time/block is actually not really required, as you can pick any point in time to count from. But it makes the mapping more readable (start from index 0 instead of a random shift). Guessing it is worth the one-time 20k gas. |
||
controller = _controller; | ||
} | ||
|
||
/** | ||
* @dev check the constraint before the action. | ||
* @return true | ||
*/ | ||
function pre(address, bytes32, bytes32) public returns(bool) { | ||
require(msg.sender == controller, "only controller is authorize to call"); | ||
avatarBalanceBefore = avatar.balance; | ||
return true; | ||
} | ||
|
||
/** | ||
* @dev check the allowance of ether sent per period. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. "and throws an error if the constraint is violated" |
||
* @return bool which represents a success | ||
*/ | ||
function post(address, bytes32, bytes32) public returns(bool) { | ||
require(msg.sender == controller, "only controller is authorize to call"); | ||
uint256 currentPeriodIndex = (block.number - startBlock)/constraintPeriodWindow; | ||
if ((block.number - startBlock) % constraintPeriodWindow > 0) { | ||
currentPeriodIndex = currentPeriodIndex.add(1); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. why do we need this? Was line 53 not enough? |
||
} | ||
if (avatarBalanceBefore >= avatar.balance) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. so you are checking the netto balance, not how much ether the avatar actually did send (i.e. if the constraint is 1 ETH/day, then it would be ok if the avatar receives 100.000 and sends 99.999) |
||
uint256 ethSentAmount = avatarBalanceBefore.sub(avatar.balance); | ||
totalAmountSentForPeriods[currentPeriodIndex] = | ||
totalAmountSentForPeriods[currentPeriodIndex].add(ethSentAmount); | ||
require(totalAmountSentForPeriods[currentPeriodIndex] <= amountAllowedPerPeriod, | ||
"exceed the amount allowed"); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. maybe at a reference to where this error message comes from, something like `Violation of Global constraint EtherGC: amount sent exceed in current period° |
||
} | ||
return true; | ||
} | ||
|
||
/** | ||
* @dev when return if this globalConstraints is pre, post or both. | ||
* @return CallPhase enum indication Pre, Post or PreAndPost. | ||
*/ | ||
function when() public pure returns(GlobalConstraintInterface.CallPhase) { | ||
return GlobalConstraintInterface.CallPhase.PreAndPost; | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,75 @@ | ||
import * as helpers from './helpers'; | ||
const DAOToken = artifacts.require("./DAOToken.sol"); | ||
const EtherGC = artifacts.require('./globalConstraints/EtherGC.sol'); | ||
const Controller = artifacts.require("./Controller.sol"); | ||
const Reputation = artifacts.require("./Reputation.sol"); | ||
const Avatar = artifacts.require("./Avatar.sol"); | ||
var constants = require('../test/constants'); | ||
|
||
|
||
let reputation, avatar,token,controller,etherGC; | ||
const setup = async function (permission='0') { | ||
var _controller; | ||
token = await DAOToken.new("TEST","TST",0); | ||
// set up a reputation system | ||
reputation = await Reputation.new(); | ||
avatar = await Avatar.new('name', token.address, reputation.address); | ||
if (permission !== '0'){ | ||
_controller = await Controller.new(avatar.address,{from:accounts[1],gas: constants.ARC_GAS_LIMIT}); | ||
await _controller.registerScheme(accounts[0],0,permission,0,{from:accounts[1]}); | ||
await _controller.unregisterSelf(0,{from:accounts[1]}); | ||
} | ||
else { | ||
_controller = await Controller.new(avatar.address,{gas: constants.ARC_GAS_LIMIT}); | ||
} | ||
controller = _controller; | ||
await avatar.transferOwnership(controller.address); | ||
etherGC = await EtherGC.new(); | ||
await etherGC.initialize(avatar.address,10,web3.utils.toWei('5', "ether"),controller.address); //10 blocks ,5 eth | ||
return _controller; | ||
}; | ||
|
||
contract('EtherGC', accounts => { | ||
it("initialize", async () => { | ||
await setup(); | ||
assert.equal(await etherGC.avatar(),avatar.address); | ||
assert.equal(await etherGC.amountAllowedPerPeriod(),web3.utils.toWei('5', "ether")); | ||
assert.equal(await etherGC.constraintPeriodWindow(),10); | ||
}); | ||
|
||
it("send ether check", async () => { | ||
|
||
await setup(); | ||
try { | ||
await etherGC.initialize(avatar.address,20,web3.utils.toWei('5', "ether"),controller.address); //10 blocks ,5 eth | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. comment says 10 blocks, code says 20 |
||
assert(false,"cannpt init twice "); | ||
} catch(ex){ | ||
helpers.assertVMException(ex); | ||
} | ||
await controller.addGlobalConstraint(etherGC.address,helpers.NULL_HASH,avatar.address); | ||
//move 10 ether to avatar | ||
await web3.eth.sendTransaction({from:accounts[0],to:avatar.address, value: web3.utils.toWei('10', "ether")}); | ||
await controller.sendEther(web3.utils.toWei('1', "ether"), accounts[2],avatar.address); | ||
await controller.sendEther(web3.utils.toWei('4', "ether"), accounts[2],avatar.address); | ||
|
||
try { | ||
await controller.sendEther(web3.utils.toWei('1', "ether"), accounts[2],avatar.address); | ||
assert(false,"sendEther should fail due to the etherGC global constraint "); | ||
} | ||
catch(ex){ | ||
helpers.assertVMException(ex); | ||
} | ||
for (var i = 0 ;i< 21;i++) { | ||
//increment 21 blocks in ganache | ||
await Reputation.new(); | ||
|
||
} | ||
|
||
await controller.sendEther(web3.utils.toWei('1', "ether"), accounts[2],avatar.address); | ||
await controller.sendEther(web3.utils.toWei('4', "ether"), accounts[2],avatar.address); | ||
//send more than 10 eth | ||
await web3.eth.sendTransaction({from:accounts[0],to:avatar.address, value: web3.utils.toWei('10', "ether")}); | ||
await controller.sendEther(web3.utils.toWei('4', "ether"), accounts[2],avatar.address); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think you need to explain in a comment why this statement is not failing. Afaics, if this and the previous lines fall within the same period, you have sent 9 ether and violated the constraint. So by some coincidence, this last transfer happens in the next period? |
||
|
||
}); | ||
}); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you have a GlobalConstraint interface to inherit from?