Payroll: Support employee's min acceptable exchange rates

future-apps/payroll/arapp.json

@@ -67,14 +67,9 @@
       "params": ["Token", "Allowed"]
     },
     {
-      "name": "Modify Price Feed",
-      "id": "MODIFY_PRICE_FEED_ROLE",
-      "params": ["New price feed", "Old price feed"]
-    },
-    {
-      "name": "Modify Rate Expiry Time",
-      "id": "MODIFY_RATE_EXPIRY_ROLE",
-      "params": ["New rate expiry time", "Old rate expiry time"]
+      "name": "Modify Price Feed settings",
+      "id": "MODIFY_PRICE_FEED_SETTINGS_ROLE",
+      "params": ["New price feed", "Old price feed", "New rate expiry time", "Old rate expiry time"]
     }
   ],
   "path": "contracts/Payroll.sol"

future-apps/payroll/contracts/examples/PayrollKit.sol

@@ -93,7 +93,7 @@ contract PayrollKit is KitBase {
 
         // Payroll prerequisites
         PPFMock priceFeed = new PPFMock();
-        MiniMeToken denominationToken = newToken("USD Dolar", "USD");
+        MiniMeToken denominationToken = newToken("USD Dollar", "USD");
 
         // Allow this contract to install new apps for now
         acl.createPermission(this, dao, dao.APP_MANAGER_ROLE(), this);
@@ -212,13 +212,11 @@ contract PayrollKit is KitBase {
         address account2 = 0x8401Eb5ff34cc943f096A32EF3d5113FEbE8D4Eb;
         address account3 = 0x306469457266CBBe7c0505e8Aad358622235e768;
         address account4 = 0xd873F6DC68e3057e4B7da74c6b304d0eF0B484C7;
-        address account5 = 0xDcC5dD922fb1D0fd0c450a0636a8cE827521f0eD;
 
         uint256 salary1 = 2535047025122316; // 80000
         uint256 salary2 = 2851927903262605; // 90000
         uint256 salary3 = 3168808781402895; // 100000
         uint256 salary4 = 2218166146982026; // 70000
-        uint256 salary5 = 1901285268841737; // 60000
 
         // Set up first user; use this contract as the account so we can set up the initial distribution
         payroll.addEmployee(this, salary1, uint64(now - 86400), "CEO");
@@ -238,6 +236,5 @@ contract PayrollKit is KitBase {
         payroll.addEmployee(account2, salary2, uint64(now - 86400), "Project Manager");
         payroll.addEmployee(account3, salary3, uint64(now - 172800), "Developer");
         payroll.addEmployee(account4, salary4, uint64(now - 172800), "Developer");
-        payroll.addEmployee(account5, salary5, uint64(now - 172800), "Developer");
     }
 }

future-apps/payroll/contracts/test/mocks/MaliciousEmployee.sol

@@ -21,7 +21,7 @@ contract MaliciousEmployee {
     }
 
     function payday() public {
-        payroll.payday(Payroll.PaymentType.Payroll, 0);
+        payroll.payday(Payroll.PaymentType.Payroll, 0, new uint256[](0));
     }
 
     function determineAllocation(address[] _tokens, uint256[] _distribution) public {
@@ -36,15 +36,17 @@ contract MaliciousEmployee {
         counter++;
 
         if (action == Action.Payday) {
-            payroll.payday(Payroll.PaymentType.Payroll, 0);
+            payroll.payday(Payroll.PaymentType.Payroll, 0, new uint256[](0));
         } else if (action == Action.ChangeAddress) {
             payroll.changeAddressByEmployee(msg.sender);
         } else if (action == Action.SetAllocation) {
             address[] memory tokens = new address[](1);
             tokens[0] = address(0);
-            uint256[] memory allocations = new uint256[](1);
-            allocations[0] = 100;
-            payroll.determineAllocation(tokens, allocations);
+            uint256[] memory distribution = new uint256[](1);
+            distribution[0] = 100;
+            uint256[] memory minRates = new uint256[](1);
+            minRates[0] = 1e18;
+            payroll.determineAllocation(tokens, distribution);
         }
     }
 }

future-apps/payroll/contracts/test/mocks/PayrollMock.sol

@@ -6,5 +6,4 @@ import "@aragon/test-helpers/contracts/TimeHelpersMock.sol";
 
 contract PayrollMock is Payroll, TimeHelpersMock {
     function getMaxAllowedTokens() public pure returns (uint256) { return MAX_ALLOWED_TOKENS; }
-    function getAllowedTokensArrayLength() public view returns (uint256) { return allowedTokensArray.length; }
 }

future-apps/payroll/test/contracts/Payroll_allowed_tokens.test.js

@@ -35,15 +35,13 @@ contract('Payroll allowed tokens,', ([owner, employee, anyone]) => {
             const receipt = await payroll.setAllowedToken(DAI.address, true, { from })
             assertEvent(receipt, 'SetAllowedToken', { token: DAI.address, allowed: true })
 
-            assert.equal(await payroll.getAllowedTokensArrayLength(), 1, 'allowed tokens length does not match')
             assert(await payroll.isTokenAllowed(DAI.address), 'denomination token should be allowed')
           })
 
           it('can allow a the zero address', async () => {
             const receipt = await payroll.setAllowedToken(ZERO_ADDRESS, true, { from })
             assertEvent(receipt, 'SetAllowedToken', { token: ZERO_ADDRESS, allowed: true })
 
-            assert.equal(await payroll.getAllowedTokensArrayLength(), 1, 'allowed tokens length does not match')
             assert(await payroll.isTokenAllowed(ZERO_ADDRESS), 'zero address token should be allowed')
           })
 
@@ -55,7 +53,6 @@ contract('Payroll allowed tokens,', ([owner, employee, anyone]) => {
             await payroll.setAllowedToken(erc20Token1.address, true, { from })
             await payroll.setAllowedToken(erc20Token2.address, true, { from })
 
-            assert.equal(await payroll.getAllowedTokensArrayLength(), 3, 'allowed tokens length does not match')
             assert(await payroll.isTokenAllowed(DAI.address), 'denomination token should be allowed')
             assert(await payroll.isTokenAllowed(erc20Token1.address), 'ERC20 token 1 should be allowed')
             assert(await payroll.isTokenAllowed(erc20Token2.address), 'ERC20 token 2 should be allowed')
@@ -75,29 +72,32 @@ contract('Payroll allowed tokens,', ([owner, employee, anyone]) => {
 
           beforeEach('allow tokens, set rates, and add employee', async () => {
             await Promise.all(tokenAddresses.map(address => payroll.setAllowedToken(address, true, { from: owner })))
-            assert.equal(await payroll.getAllowedTokensArrayLength(), MAX_ALLOWED_TOKENS, 'amount of allowed tokens does not match')
 
             const rates = tokenAddresses.map(() => formatRate(5))
             await setTokenRates(priceFeed, USD, tokenAddresses, rates)
 
             await payroll.addEmployee(employee, annualSalaryPerSecond(100000), NOW - ONE_MONTH, 'Boss', { from: owner })
           })
 
-          it('can not add one more token', async () => {
-            const erc20Token = await deployTokenAndDeposit(owner, finance, 'Extra token', 18)
-
-            await assertRevert(payroll.setAllowedToken(erc20Token.address, true), 'PAYROLL_MAX_ALLOWED_TOKENS')
-          })
-
-          it('does not run out of gas to payout salary', async () => {
+          it('does not run out of gas to payout salary using 20 tokens', async () => {
             const allocations = tokenAddresses.map(() => 100 / MAX_ALLOWED_TOKENS)
 
             const allocationTx = await payroll.determineAllocation(tokenAddresses, allocations, { from: employee })
             assert.isBelow(allocationTx.receipt.cumulativeGasUsed, MAX_GAS_USED, 'too much gas consumed for allocation')
 
-            const paydayTx = await payroll.payday(PAYMENT_TYPES.PAYROLL, 0, { from: employee })
+            const paydayTx = await payroll.payday(PAYMENT_TYPES.PAYROLL, 0, [], { from: employee })
             assert.isBelow(paydayTx.receipt.cumulativeGasUsed, MAX_GAS_USED, 'too much gas consumed for payday')
           })
+
+          it('can not add one more token', async () => {
+            const extraToken = await deployTokenAndDeposit(owner, finance, 'Extra token', 18)
+            await payroll.setAllowedToken(extraToken.address, true)
+
+            const exceedingTokenAddresses = tokenAddresses.concat(extraToken.address)
+            const allocations = exceedingTokenAddresses.map(() => 100 / exceedingTokenAddresses.length)
+
+            await assertRevert(payroll.determineAllocation(exceedingTokenAddresses, allocations, { from: employee }), 'PAYROLL_MAX_ALLOWED_TOKENS')
+          })
         })
       })
 
@@ -132,12 +132,9 @@ contract('Payroll allowed tokens,', ([owner, employee, anyone]) => {
           })
 
           it('can remove an allowed token', async () => {
-            assert.equal(await payroll.getAllowedTokensArrayLength(), 1, 'allowed tokens length does not match')
-
             const receipt = await payroll.setAllowedToken(DAI.address, false, { from })
             assertEvent(receipt, 'SetAllowedToken', { token: DAI.address, allowed: false })
 
-            assert.equal(await payroll.getAllowedTokensArrayLength(), 0, 'allowed tokens length does not match')
             assert.isFalse(await payroll.isTokenAllowed(DAI.address), 'token should not be allowed')
           })
 
@@ -146,12 +143,10 @@ contract('Payroll allowed tokens,', ([owner, employee, anyone]) => {
             await payroll.setAllowedToken(erc20Token1.address, true, { from })
             const erc20Token2 = await deployTokenAndDeposit(owner, finance, 'Token 2', 16)
             await payroll.setAllowedToken(erc20Token2.address, true, { from })
-            assert.equal(await payroll.getAllowedTokensArrayLength(), 3, 'allowed tokens length does not match')
 
             await payroll.setAllowedToken(DAI.address, false, { from })
             await payroll.setAllowedToken(erc20Token1.address, false, { from })
 
-            assert.equal(await payroll.getAllowedTokensArrayLength(), 1, 'allowed tokens length does not match')
             assert.isFalse(await payroll.isTokenAllowed(DAI.address), 'token should not be allowed')
             assert.isFalse(await payroll.isTokenAllowed(erc20Token1.address), 'token should be allowed')
             assert.isTrue(await payroll.isTokenAllowed(erc20Token2.address), 'token should not be allowed')
@@ -161,7 +156,7 @@ contract('Payroll allowed tokens,', ([owner, employee, anyone]) => {
         context('when the given token is not allowed', () => {
           it('reverts', async () => {
             const erc20Token = await deployTokenAndDeposit(owner, finance, 'Some Token', 18)
-            await assertRevert(payroll.setAllowedToken(erc20Token.address, false), 'PAYROLL_TOKEN_NOT_ALLOWED')
+            await assertRevert(payroll.setAllowedToken(erc20Token.address, false), 'PAYROLL_TOKEN_ALREADY_SET')
           })
         })
       })