Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Rename setPaymentDisabled to setPaymentStatus #474

Merged
merged 5 commits into from Oct 16, 2018
Merged

Rename setPaymentDisabled to setPaymentStatus #474

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
c6bfa8d
Renames setPaymentDisabled to setPaymentStatus
Oct 9, 2018
221f4f8
Rename bytes32 and add extra test for status
Oct 11, 2018
1db7805
Merge branch 'master' of https://github.com/aragon/aragon-apps into h…
Oct 11, 2018
271b89f
Revert MiniMe to previous state
Oct 11, 2018
51539ca
Add active boolean as a param to MANAGE_PAYMENTS_ROLE
izqui Oct 16, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
16 changes: 4 additions & 12 deletions apps/finance/arapp.json
View file
Open in desktop
Expand Up @@ -30,10 +30,7 @@
{
"name": "Change period duration",
"id": "CHANGE_PERIOD_ROLE",
"params": [
"New period duration",
"Old period duration"
]
"params": ["New period duration", "Old period duration"]
},
{
"name": "Change budgets",
Expand All @@ -48,17 +45,12 @@
{
"name": "Execute payments",
"id": "EXECUTE_PAYMENTS_ROLE",
"params": [
"Payment ID",
"Payment amount"
]
"params": ["Payment ID"]
},
{
"name": "Disable payments",
"id": "DISABLE_PAYMENTS_ROLE",
"params": [
"Payment ID"
]
"id": "MANAGE_PAYMENTS_ROLE",
"params": ["Payment ID", "Payment active"]
}
],
"path": "contracts/Finance.sol"
Expand Down
12 changes: 6 additions & 6 deletions apps/finance/artifact.json
View file
Open in desktop
Expand Up @@ -206,7 +206,7 @@
"type": "string"
},
{
"name": "disabled",
"name": "inactive",
"type": "bool"
},
{
Expand Down Expand Up @@ -683,11 +683,11 @@
"type": "uint256"
},
{
"name": "_disabled",
"name": "_active",
"type": "bool"
}
],
"name": "setPaymentDisabled",
"name": "setPaymentStatus",
"outputs": [],
"payable": false,
"stateMutability": "nonpayable",
Expand Down Expand Up @@ -890,7 +890,7 @@
},
{
"indexed": false,
"name": "disabled",
"name": "inactive",
"type": "bool"
}
],
Expand Down Expand Up @@ -984,11 +984,11 @@
"notice": "Execute pending payment #`_paymentId`"
},
{
"sig": "setPaymentDisabled(uint256,bool)",
"sig": "setPaymentStatus(uint256,bool)",
"roles": [
"DISABLE_PAYMENTS_ROLE"
],
"notice": "`_disabled ? 'Disable' : 'Enable'` payment `_paymentId`"
"notice": "`_active ? 'Active' : 'Inactive'` payment `_paymentId`"
0xKiwi marked this conversation as resolved.
Show resolved Hide resolved
},
{
"sig": "depositToVault(address)",
Expand Down
26 changes: 13 additions & 13 deletions apps/finance/contracts/Finance.sol
View file
Open in desktop
Expand Up @@ -23,7 +23,7 @@ contract Finance is EtherTokenConstant, IsContract, AragonApp {
bytes32 public constant CHANGE_PERIOD_ROLE = keccak256("CHANGE_PERIOD_ROLE");
bytes32 public constant CHANGE_BUDGETS_ROLE = keccak256("CHANGE_BUDGETS_ROLE");
bytes32 public constant EXECUTE_PAYMENTS_ROLE = keccak256("EXECUTE_PAYMENTS_ROLE");
bytes32 public constant DISABLE_PAYMENTS_ROLE = keccak256("DISABLE_PAYMENTS_ROLE");
bytes32 public constant MANAGE_PAYMENTS_ROLE = keccak256("MANAGE_PAYMENTS_ROLE");

uint64 public constant MAX_PAYMENTS_PER_TX = 20;

Expand All @@ -37,7 +37,7 @@ contract Finance is EtherTokenConstant, IsContract, AragonApp {
address token;
address receiver;
address createdBy;
bool disabled;
bool inactive;
uint256 amount;
uint64 initialPaymentTime;
uint64 interval;
Expand Down Expand Up @@ -98,7 +98,7 @@ contract Finance is EtherTokenConstant, IsContract, AragonApp {
event SetBudget(address indexed token, uint256 amount, bool hasBudget);
event NewPayment(uint256 indexed paymentId, address indexed recipient, uint64 maxRepeats);
event NewTransaction(uint256 indexed transactionId, bool incoming, address indexed entity, uint256 amount);
event ChangePaymentState(uint256 indexed paymentId, bool disabled);
event ChangePaymentState(uint256 indexed paymentId, bool inactive);
event ChangePeriodDuration(uint64 newDuration);
event PaymentFailure(uint256 paymentId);

Expand Down Expand Up @@ -155,7 +155,7 @@ contract Finance is EtherTokenConstant, IsContract, AragonApp {
settings.periodDuration = _periodDuration;

// Reserve the first recurring payment index as an unused index for transactions not linked to a payment
payments[0].disabled = true;
payments[0].inactive = true;
paymentsNextIndex = 1;

// Reserve the first transaction index as an unused index for periods with no transactions
Expand Down Expand Up @@ -319,17 +319,17 @@ contract Finance is EtherTokenConstant, IsContract, AragonApp {
}

/**
* @notice `_disabled ? 'Disable' : 'Enable'` payment `_paymentId`
* @notice `_active ? 'Active' : 'Inactive'` payment `_paymentId`
* @param _paymentId Identifier for payment
* @param _disabled Whether it will be disabled or enabled
* @param _active Whether it will be active or inactive
*/
function setPaymentDisabled(uint256 _paymentId, bool _disabled)
function setPaymentStatus(uint256 _paymentId, bool _active)
external
authP(DISABLE_PAYMENTS_ROLE, arr(_paymentId))
authP(MANAGE_PAYMENTS_ROLE, arr(_paymentId, uint256(_active)))
paymentExists(_paymentId)
{
payments[_paymentId].disabled = _disabled;
emit ChangePaymentState(_paymentId, _disabled);
payments[_paymentId].inactive = !_active;
emit ChangePaymentState(_paymentId, _active);
}

/**
Expand Down Expand Up @@ -388,7 +388,7 @@ contract Finance is EtherTokenConstant, IsContract, AragonApp {
uint64 interval,
uint64 maxRepeats,
string reference,
bool disabled,
bool inactive,
uint64 repeats,
address createdBy
)
Expand All @@ -402,7 +402,7 @@ contract Finance is EtherTokenConstant, IsContract, AragonApp {
interval = payment.interval;
maxRepeats = payment.maxRepeats;
repeats = payment.repeats;
disabled = payment.disabled;
inactive = payment.inactive;
reference = payment.reference;
createdBy = payment.createdBy;
}
Expand Down Expand Up @@ -560,7 +560,7 @@ contract Finance is EtherTokenConstant, IsContract, AragonApp {

function _executePayment(uint256 _paymentId) internal {
Payment storage payment = payments[_paymentId];
require(!payment.disabled);
require(!payment.inactive);

uint64 payed = 0;
while (nextPaymentTime(_paymentId) <= getTimestamp64() && payed < MAX_PAYMENTS_PER_TX) {
Expand Down
17 changes: 12 additions & 5 deletions apps/finance/test/finance.js
View file
Open in desktop
Expand Up @@ -11,7 +11,7 @@ contract('Finance App', accounts => {
let daoFact, financeBase, finance, vaultBase, vault, token1, token2, executionTarget, etherToken = {}

let ETH, MAX_UINT64, ANY_ENTITY, APP_MANAGER_ROLE
let CREATE_PAYMENTS_ROLE, CHANGE_PERIOD_ROLE, CHANGE_BUDGETS_ROLE, EXECUTE_PAYMENTS_ROLE, DISABLE_PAYMENTS_ROLE
let CREATE_PAYMENTS_ROLE, CHANGE_PERIOD_ROLE, CHANGE_BUDGETS_ROLE, EXECUTE_PAYMENTS_ROLE, MANAGE_PAYMENTS_ROLE
let TRANSFER_ROLE

const root = accounts[0]
Expand Down Expand Up @@ -39,7 +39,7 @@ contract('Finance App', accounts => {
CHANGE_PERIOD_ROLE = await financeBase.CHANGE_PERIOD_ROLE()
CHANGE_BUDGETS_ROLE = await financeBase.CHANGE_BUDGETS_ROLE()
EXECUTE_PAYMENTS_ROLE = await financeBase.EXECUTE_PAYMENTS_ROLE()
DISABLE_PAYMENTS_ROLE = await financeBase.DISABLE_PAYMENTS_ROLE()
MANAGE_PAYMENTS_ROLE = await financeBase.MANAGE_PAYMENTS_ROLE()
TRANSFER_ROLE = await vaultBase.TRANSFER_ROLE()
})

Expand Down Expand Up @@ -70,7 +70,7 @@ contract('Finance App', accounts => {
await acl.createPermission(ANY_ENTITY, financeApp.address, CHANGE_PERIOD_ROLE, root, { from: root })
await acl.createPermission(ANY_ENTITY, financeApp.address, CHANGE_BUDGETS_ROLE, root, { from: root })
await acl.createPermission(ANY_ENTITY, financeApp.address, EXECUTE_PAYMENTS_ROLE, root, { from: root })
await acl.createPermission(ANY_ENTITY, financeApp.address, DISABLE_PAYMENTS_ROLE, root, { from: root })
await acl.createPermission(ANY_ENTITY, financeApp.address, MANAGE_PAYMENTS_ROLE, root, { from: root })

const recoveryVault = await setupRecoveryVault(dao)

Expand Down Expand Up @@ -667,14 +667,21 @@ contract('Finance App', accounts => {
})
})

it('fails executing disabled payment', async () => {
await finance.setPaymentDisabled(1, true)
it('fails executing inactive payment', async () => {
await finance.setPaymentStatus(1, false)
0xKiwi marked this conversation as resolved.
Show resolved Hide resolved
await finance.mock_setTimestamp(time + 1)

return assertRevert(async () => {
await finance.executePayment(1, { from: recipient })
})
})

it('succeeds payment after setting payment status to active', async () => {
await finance.setPaymentStatus(1, true)
await finance.mock_setTimestamp(time + 1)

await finance.executePayment(1, { from: recipient })
})
})

const assertPaymentFailure = receipt => {
Expand Down
2 changes: 1 addition & 1 deletion apps/survey/contracts/test/mocks/BadToken.sol
View file
Open in desktop
Expand Up @@ -20,7 +20,7 @@ contract BadToken is MiniMeToken {
}

// should be changed to view when MiniMe is updated
function totalSupplyAt(uint) public constant returns(uint) {
function totalSupplyAt(uint) public view returns(uint) {
return 1;
}
}
2 changes: 1 addition & 1 deletion shared/minime/contracts/MiniMeToken.sol
View file
Open in desktop
Expand Up @@ -572,4 +572,4 @@ contract MiniMeTokenFactory {
newToken.changeController(msg.sender);
return newToken;
}
}
}