Payroll: Fix tokens transfer formula

aragon · Apr 26, 2019 · a9f3a07 · a9f3a07
1 parent 065139c
commit a9f3a07
Show file tree

Hide file tree

Showing 16 changed files with 211 additions and 198 deletions.
diff --git a/future-apps/payroll/contracts/Payroll.sol b/future-apps/payroll/contracts/Payroll.sol
@@ -692,12 +692,10 @@ contract Payroll is EtherTokenConstant, IForwarder, IsContract, AragonApp {
             address token = allowedTokensArray[i];
             uint256 tokenAllocation = employee.allocation[token];
             if (tokenAllocation != uint256(0)) {
-                uint128 exchangeRate = _getExchangeRate(token);
+                uint256 exchangeRate = uint256(_getExchangeRate(token));
                 require(exchangeRate > 0, ERROR_EXCHANGE_RATE_ZERO);
+                uint256 tokenAmount = _totalAmount.mul(tokenAllocation).div(exchangeRate).mul(ONE / 100);
                 // Salary converted to token and applied allocation percentage
-                uint256 tokenAmount = _totalAmount.mul(exchangeRate).mul(tokenAllocation);
-                // Divide by 100 for the allocation and by ONE for the exchange rate
-                tokenAmount = tokenAmount / (100 * ONE);
 
                 finance.newImmediatePayment(token, employeeAddress, tokenAmount, paymentReference);
                 emit SendPayment(employeeAddress, token, tokenAmount, paymentReference);

diff --git a/future-apps/payroll/test/contracts/Payroll_add_employee.test.js b/future-apps/payroll/test/contracts/Payroll_add_employee.test.js
@@ -1,8 +1,8 @@
 const { assertRevert } = require('@aragon/test-helpers/assertThrow')
 const { getEvents, getEventArgument } = require('../helpers/events')
-const { MAX_UINT64, annualSalaryPerSecond } = require('../helpers/numbers')(web3)
-const { USD, deployDAI } = require('../helpers/tokens.js')(artifacts, web3)
+const { USD, deployDAI } = require('../helpers/tokens')(artifacts, web3)
 const { NOW, TWO_MONTHS, RATE_EXPIRATION_TIME } = require('../helpers/time')
+const { MAX_UINT64, annualSalaryPerSecond } = require('../helpers/numbers')(web3)
 const { deployContracts, createPayrollAndPriceFeed } = require('../helpers/deploy')(artifacts, web3)
 
 const ZERO_ADDRESS = '0x0000000000000000000000000000000000000000'

diff --git a/future-apps/payroll/test/contracts/Payroll_allowed_tokens.test.js b/future-apps/payroll/test/contracts/Payroll_allowed_tokens.test.js
@@ -1,9 +1,10 @@
 const PAYMENT_TYPES = require('../helpers/payment_types')
 const { getEvent } = require('../helpers/events')
 const { assertRevert } = require('@aragon/test-helpers/assertThrow')
+const { annualSalaryPerSecond } = require('../helpers/numbers')(web3)
 const { NOW, ONE_MONTH, RATE_EXPIRATION_TIME } = require('../helpers/time')
 const { deployContracts, createPayrollAndPriceFeed } = require('../helpers/deploy')(artifacts, web3)
-const { USD, deployDAI, deployTokenAndDeposit, setTokenRates } = require('../helpers/tokens.js')(artifacts, web3)
+const { USD, deployDAI, deployTokenAndDeposit, setTokenRates, formatRate } = require('../helpers/tokens')(artifacts, web3)
 
 const MAX_GAS_USED = 6.5e6
 const ZERO_ADDRESS = '0x0000000000000000000000000000000000000000'
@@ -80,10 +81,10 @@ contract('Payroll allowed tokens,', ([owner, employee, anyone]) => {
             await Promise.all(tokenAddresses.map(address => payroll.addAllowedToken(address, { from: owner })))
             assert.equal(await payroll.getAllowedTokensArrayLength(), MAX_ALLOWED_TOKENS, 'amount of allowed tokens does not match')
 
-            const rates = tokenAddresses.map(() => 5)
+            const rates = tokenAddresses.map(() => formatRate(5))
             await setTokenRates(priceFeed, USD, tokenAddresses, rates)
 
-            await payroll.addEmployee(employee, 100000, 'Boss', NOW - ONE_MONTH, { from: owner })
+            await payroll.addEmployee(employee, annualSalaryPerSecond(100000), 'Boss', NOW - ONE_MONTH, { from: owner })
           })
 
           it('can not add one more token', async () => {

diff --git a/future-apps/payroll/test/contracts/Payroll_bonuses.test.js b/future-apps/payroll/test/contracts/Payroll_bonuses.test.js
@@ -1,10 +1,10 @@
 const PAYMENT_TYPES = require('../helpers/payment_types')
-const { MAX_UINT256 } = require('../helpers/numbers')(web3)
 const { assertRevert } = require('@aragon/test-helpers/assertThrow')
 const { getEvents, getEventArgument } = require('../helpers/events')
 const { NOW, ONE_MONTH, RATE_EXPIRATION_TIME } = require('../helpers/time')
+const { bn, bigExp, annualSalaryPerSecond, ONE, MAX_UINT256 } = require('../helpers/numbers')(web3)
 const { deployContracts, createPayrollAndPriceFeed } = require('../helpers/deploy')(artifacts, web3)
-const { USD, deployDAI, deployANT, DAI_RATE, ANT_RATE, setTokenRates } = require('../helpers/tokens.js')(artifacts, web3)
+const { USD, deployDAI, deployANT, DAI_RATE, ANT_RATE, setTokenRates } = require('../helpers/tokens')(artifacts, web3)
 
 contract('Payroll bonuses', ([owner, employee, anyone]) => {
   let dao, payroll, payrollBase, finance, vault, priceFeed, DAI, ANT
@@ -37,7 +37,7 @@ contract('Payroll bonuses', ([owner, employee, anyone]) => {
           let employeeId
 
           beforeEach('add employee', async () => {
-            const receipt = await payroll.addEmployee(employee, 1000, 'Boss', await payroll.getTimestampPublic())
+            const receipt = await payroll.addEmployee(employee, annualSalaryPerSecond(100000), 'Boss', await payroll.getTimestampPublic())
             employeeId = getEventArgument(receipt, 'AddEmployee', 'employeeId')
           })
 
@@ -63,7 +63,7 @@ contract('Payroll bonuses', ([owner, employee, anyone]) => {
             }
 
             context('when the given bonus greater than zero', () => {
-              const amount = 1000
+              const amount = bigExp(1000, 18)
 
               context('when there was no previous bonus', () => {
                 itAddsBonusesSuccessfully(amount)
@@ -79,7 +79,7 @@ contract('Payroll bonuses', ([owner, employee, anyone]) => {
             })
 
             context('when the given bonus is zero', () => {
-              const amount = 0
+              const amount = bn(0)
 
               itAddsBonusesSuccessfully(amount)
             })
@@ -102,7 +102,7 @@ contract('Payroll bonuses', ([owner, employee, anyone]) => {
             })
 
             it('reverts', async () => {
-              await assertRevert(payroll.addBonus(employeeId, 1000, { from }), 'PAYROLL_NON_ACTIVE_EMPLOYEE')
+              await assertRevert(payroll.addBonus(employeeId, bigExp(1000, 18), { from }), 'PAYROLL_NON_ACTIVE_EMPLOYEE')
             })
           })
         })
@@ -111,14 +111,14 @@ contract('Payroll bonuses', ([owner, employee, anyone]) => {
           const employeeId = 0
 
           it('reverts', async () => {
-            await assertRevert(payroll.addBonus(employeeId, 1000, { from }), 'PAYROLL_NON_ACTIVE_EMPLOYEE')
+            await assertRevert(payroll.addBonus(employeeId, bigExp(1000, 18), { from }), 'PAYROLL_NON_ACTIVE_EMPLOYEE')
           })
         })
       })
 
       context('when the sender does not have permissions', () => {
         const from = anyone
-        const amount = 1000
+        const amount = bigExp(1000, 18)
         const employeeId = 0
 
         it('reverts', async () => {
@@ -128,7 +128,7 @@ contract('Payroll bonuses', ([owner, employee, anyone]) => {
     })
 
     context('when it has not been initialized yet', function () {
-      const amount = 10000
+      const amount = bigExp(1000, 18)
       const employeeId = 0
 
       it('reverts', async () => {
@@ -149,7 +149,7 @@ contract('Payroll bonuses', ([owner, employee, anyone]) => {
 
       context('when the sender is an employee', () => {
         const from = employee
-        let employeeId, salary = 1000
+        let employeeId, salary = annualSalaryPerSecond(100000)
 
         beforeEach('add employee and accumulate some salary', async () => {
           const receipt = await payroll.addEmployee(employee, salary, 'Boss', await payroll.getTimestampPublic())
@@ -169,16 +169,16 @@ contract('Payroll bonuses', ([owner, employee, anyone]) => {
           })
 
           context('when the employee has a pending bonus', () => {
-            const bonusAmount = 100
+            const bonusAmount = bigExp(100, 18)
 
             beforeEach('add bonus', async () => {
-              await payroll.addBonus(employeeId, bonusAmount / 2, { from: owner })
-              await payroll.addBonus(employeeId, bonusAmount / 2, { from: owner })
+              await payroll.addBonus(employeeId, bonusAmount.div(2), { from: owner })
+              await payroll.addBonus(employeeId, bonusAmount.div(2), { from: owner })
             })
 
             const assertTransferredAmounts = (requestedAmount, expectedRequestedAmount = requestedAmount) => {
-              const requestedDAI = DAI_RATE.mul(expectedRequestedAmount * allocationDAI / 100).trunc()
-              const requestedANT = ANT_RATE.mul(expectedRequestedAmount * allocationANT / 100).trunc()
+              const requestedDAI = expectedRequestedAmount.div(DAI_RATE).mul(allocationDAI).mul(ONE.div(100)).trunc()
+              const requestedANT = expectedRequestedAmount.div(ANT_RATE).mul(allocationANT).mul(ONE.div(100)).trunc()
 
               it('transfers all the pending bonus', async () => {
                 const previousDAI = await DAI.balanceOf(employee)
@@ -223,7 +223,7 @@ contract('Payroll bonuses', ([owner, employee, anyone]) => {
                 const [address, employeeSalary, bonus] = await payroll.getEmployee(employeeId)
 
                 assert.equal(address, employee, 'employee address does not match')
-                assert.equal(employeeSalary, salary, 'employee salary does not match')
+                assert.equal(employeeSalary.toString(), salary.toString(), 'employee salary does not match')
                 assert.equal(previousBonus.minus(expectedRequestedAmount).toString(), bonus.toString(), 'employee bonus does not match')
               })
             }
@@ -247,7 +247,7 @@ contract('Payroll bonuses', ([owner, employee, anyone]) => {
             }
 
             context('when the requested amount is zero', () => {
-              const requestedAmount = 0
+              const requestedAmount = bn(0)
 
               context('when the employee has some pending salary', () => {
                 context('when the employee is not terminated', () => {
@@ -302,7 +302,7 @@ contract('Payroll bonuses', ([owner, employee, anyone]) => {
             })
 
             context('when the requested amount is less than the total bonus amount', () => {
-              const requestedAmount = bonusAmount - 1
+              const requestedAmount = bonusAmount.div(2)
 
               context('when the employee has some pending salary', () => {
                 context('when the employee is not terminated', () => {
@@ -393,7 +393,7 @@ contract('Payroll bonuses', ([owner, employee, anyone]) => {
             })
 
             context('when the requested amount is greater than the total bonus amount', () => {
-              const requestedAmount = bonusAmount + 1
+              const requestedAmount = bonusAmount.plus(1)
 
               it('reverts', async () => {
                 await assertRevert(payroll.payday(PAYMENT_TYPES.BONUS, requestedAmount, { from }), 'PAYROLL_INVALID_REQUESTED_AMT')
@@ -403,15 +403,15 @@ contract('Payroll bonuses', ([owner, employee, anyone]) => {
 
           context('when the employee does not have pending reimbursements', () => {
             context('when the requested amount is greater than zero', () => {
-              const requestedAmount = 100
+              const requestedAmount = bigExp(100, 18)
 
               it('reverts', async () => {
                 await assertRevert(payroll.payday(PAYMENT_TYPES.BONUS, requestedAmount, { from }), 'PAYROLL_NOTHING_PAID')
               })
             })
 
             context('when the requested amount is zero', () => {
-              const requestedAmount = 0
+              const requestedAmount = bn(0)
 
               it('reverts', async () => {
                 await assertRevert(payroll.payday(PAYMENT_TYPES.BONUS, requestedAmount, { from }), 'PAYROLL_NOTHING_PAID')
@@ -422,23 +422,23 @@ contract('Payroll bonuses', ([owner, employee, anyone]) => {
 
         context('when the employee did not set any token allocations yet', () => {
           context('when the employee has a pending bonus', () => {
-            const bonusAmount = 100
+            const bonusAmount = bigExp(100, 18)
 
             beforeEach('add bonus', async () => {
-              await payroll.addBonus(employeeId, bonusAmount / 2, { from: owner })
-              await payroll.addBonus(employeeId, bonusAmount / 2, { from: owner })
+              await payroll.addBonus(employeeId, bonusAmount.div(2), { from: owner })
+              await payroll.addBonus(employeeId, bonusAmount.div(2), { from: owner })
             })
 
             context('when the requested amount is zero', () => {
-              const requestedAmount = 0
+              const requestedAmount = bn(0)
 
               it('reverts', async () => {
                 await assertRevert(payroll.payday(PAYMENT_TYPES.BONUS, requestedAmount, { from }), 'PAYROLL_NOTHING_PAID')
               })
             })
 
             context('when the requested amount is less than the total bonus amount', () => {
-              const requestedAmount = bonusAmount - 1
+              const requestedAmount = bonusAmount.minus(1)
 
               it('reverts', async () => {
                 await assertRevert(payroll.payday(PAYMENT_TYPES.BONUS, requestedAmount, { from }), 'PAYROLL_NOTHING_PAID')
@@ -454,7 +454,7 @@ contract('Payroll bonuses', ([owner, employee, anyone]) => {
             })
 
             context('when the requested amount is greater than the total bonus amount', () => {
-              const requestedAmount = bonusAmount + 1
+              const requestedAmount = bonusAmount.plus(1)
 
               it('reverts', async () => {
                 await assertRevert(payroll.payday(PAYMENT_TYPES.BONUS, requestedAmount, { from }), 'PAYROLL_INVALID_REQUESTED_AMT')
@@ -464,15 +464,15 @@ contract('Payroll bonuses', ([owner, employee, anyone]) => {
 
           context('when the employee does not have pending reimbursements', () => {
             context('when the requested amount is greater than zero', () => {
-              const requestedAmount = 100
+              const requestedAmount = bigExp(100, 18)
 
               it('reverts', async () => {
                 await assertRevert(payroll.payday(PAYMENT_TYPES.BONUS, requestedAmount, { from }), 'PAYROLL_NOTHING_PAID')
               })
             })
 
             context('when the requested amount is zero', () => {
-              const requestedAmount = 0
+              const requestedAmount = bn(0)
 
               it('reverts', async () => {
                 await assertRevert(payroll.payday(PAYMENT_TYPES.BONUS, requestedAmount, { from }), 'PAYROLL_NOTHING_PAID')
@@ -486,15 +486,15 @@ contract('Payroll bonuses', ([owner, employee, anyone]) => {
         const from = anyone
 
         context('when the requested amount is greater than zero', () => {
-          const requestedAmount = 100
+          const requestedAmount = bigExp(100, 18)
 
           it('reverts', async () => {
             await assertRevert(payroll.payday(PAYMENT_TYPES.BONUS, requestedAmount, { from }), 'PAYROLL_EMPLOYEE_DOES_NOT_MATCH')
           })
         })
 
         context('when the requested amount is zero', () => {
-          const requestedAmount = 0
+          const requestedAmount = bn(0)
 
           it('reverts', async () => {
             await assertRevert(payroll.payday(PAYMENT_TYPES.BONUS, requestedAmount, { from }), 'PAYROLL_EMPLOYEE_DOES_NOT_MATCH')
@@ -504,7 +504,7 @@ contract('Payroll bonuses', ([owner, employee, anyone]) => {
     })
 
     context('when it has not been initialized yet', function () {
-      const requestedAmount = 0
+      const requestedAmount = bn(0)
 
       it('reverts', async () => {
         await assertRevert(payroll.payday(PAYMENT_TYPES.BONUS, requestedAmount, { from: employee }), 'PAYROLL_EMPLOYEE_DOES_NOT_MATCH')

diff --git a/future-apps/payroll/test/contracts/Payroll_forwarding.test.js b/future-apps/payroll/test/contracts/Payroll_forwarding.test.js
@@ -1,7 +1,8 @@
 const { assertRevert } = require('@aragon/test-helpers/assertThrow')
 const { getEventArgument } = require('../helpers/events')
 const { encodeCallScript } = require('@aragon/test-helpers/evmScript')
-const { USD, deployDAI } = require('../helpers/tokens.js')(artifacts, web3)
+const { annualSalaryPerSecond } = require('../helpers/numbers')(web3)
+const { USD, deployDAI } = require('../helpers/tokens')(artifacts, web3)
 const { NOW, ONE_MONTH, RATE_EXPIRATION_TIME } = require('../helpers/time')
 const { deployContracts, createPayrollAndPriceFeed } = require('../helpers/deploy')(artifacts, web3)
 
@@ -48,7 +49,7 @@ contract('Payroll forwarding,', ([owner, employee, anyone]) => {
         const sender = employee
 
         beforeEach('add employee', async () => {
-          const receipt = await payroll.addEmployee(employee, 100000, 'Boss', await payroll.getTimestampPublic(), { from: owner })
+          const receipt = await payroll.addEmployee(employee, annualSalaryPerSecond(100000), 'Boss', await payroll.getTimestampPublic(), { from: owner })
           employeeId = getEventArgument(receipt, 'AddEmployee', 'employeeId').toString()
         })
 
@@ -105,7 +106,7 @@ contract('Payroll forwarding,', ([owner, employee, anyone]) => {
         const from = employee
 
         beforeEach('add employee', async () => {
-          const receipt = await payroll.addEmployee(employee, 100000, 'Boss', await payroll.getTimestampPublic(), { from: owner })
+          const receipt = await payroll.addEmployee(employee, annualSalaryPerSecond(100000), 'Boss', await payroll.getTimestampPublic(), { from: owner })
           employeeId = getEventArgument(receipt, 'AddEmployee', 'employeeId').toString()
         })
 

diff --git a/future-apps/payroll/test/contracts/Payroll_gas_costs.test.js b/future-apps/payroll/test/contracts/Payroll_gas_costs.test.js
@@ -2,7 +2,7 @@ const PAYMENT_TYPES = require('../helpers/payment_types')
 const { annualSalaryPerSecond } = require('../helpers/numbers')(web3)
 const { NOW, ONE_MONTH, RATE_EXPIRATION_TIME } = require('../helpers/time')
 const { deployContracts, createPayrollAndPriceFeed } = require('../helpers/deploy')(artifacts, web3)
-const { USD, deployDAI, deployANT, DAI_RATE, ANT_RATE, setTokenRates } = require('../helpers/tokens.js')(artifacts, web3)
+const { USD, deployDAI, deployANT, DAI_RATE, ANT_RATE, setTokenRates } = require('../helpers/tokens')(artifacts, web3)
 
 contract('Payroll gas costs', ([owner, employee, anotherEmployee]) => {
   let dao, payroll, payrollBase, finance, vault, priceFeed, DAI, ANT