payroll: fix accrued salary maths

aragon · Jul 17, 2019 · 24c4e0f · 24c4e0f
1 parent 7988a73
commit 24c4e0f
Show file tree

Hide file tree

Showing 2 changed files with 11 additions and 20 deletions.
diff --git a/future-apps/payroll/contracts/Payroll.sol b/future-apps/payroll/contracts/Payroll.sol
@@ -664,22 +664,20 @@ contract Payroll is EtherTokenConstant, IForwarder, IsContract, AragonApp {
             // first use up their accrued salary
             // No need to use SafeMath here as we already know _paymentAmount > accruedSalary
             currentSalaryPaid = _paymentAmount - accruedSalary;
+            // We finally need to clear their accrued salary
+            _employee.accruedSalary = 0;
         }
+
         uint256 salary = _employee.denominationTokenSalary;
         uint256 timeDiff = currentSalaryPaid.div(salary);
 
         // If they're being paid an amount that doesn't match perfectly with the adjusted time
         // (up to a seconds' worth of salary), add the second and put the extra remaining salary
         // into their accrued salary
-        // The extra check is to handle the case where someone requested less than one second of their salary
-        uint256 extraSalary = currentSalaryPaid < salary ? salary - currentSalaryPaid : currentSalaryPaid % salary;
+        uint256 extraSalary = currentSalaryPaid % salary;
         if (extraSalary > 0) {
             timeDiff = timeDiff.add(1);
-            _employee.accruedSalary = extraSalary;
-        } else if (accruedSalary > 0) {
-            // We finally need to clear their accrued salary, but as an optimization, we only do
-            // this if they had a non-zero value before
-            _employee.accruedSalary = 0;
+            _employee.accruedSalary = salary - extraSalary;
         }
 
         uint256 lastPayrollDate = uint256(_employee.lastPayroll).add(timeDiff);

diff --git a/future-apps/payroll/test/contracts/Payroll_payday.test.js b/future-apps/payroll/test/contracts/Payroll_payday.test.js
@@ -127,22 +127,15 @@ contract('Payroll payday', ([owner, employee, anyone]) => {
             }
 
             const assertEmployeeIsUpdatedCorrectly = (requestedAmount, expectedRequestedAmount, minRates = []) => {
-              it('updates the accrued salary and the last payroll date', async () => {
+              it('updates the employee accounting', async () => {
                 let expectedLastPayrollDate, expectedAccruedSalary
-                const [employeeSalary, previousAccruedSalary, , , previousPayrollDate] = (await payroll.getEmployee(employeeId)).slice(1, 6)
+                const [, previousAccruedSalary, , , previousPayrollDate] = (await payroll.getEmployee(employeeId)).slice(1, 6)
 
                 if (expectedRequestedAmount.gte(previousAccruedSalary)) {
-                  const remainder = expectedRequestedAmount.minus(previousAccruedSalary)
-                  if (remainder.eq(0)) {
-                    expectedAccruedSalary = bn(0)
-                  } else {
-                    // Have remaining salary that needs to be put back into the accrued salary
-                    expectedAccruedSalary =
-                      remainder.lt(employeeSalary)
-                        ? salary.minus(remainder)
-                        : expectedAccruedSalary = remainder.mod(employeeSalary)
-                  }
-                  expectedLastPayrollDate = previousPayrollDate.plus(remainder.div(employeeSalary).ceil())
+                  const currentSalaryPaid = expectedRequestedAmount.minus(previousAccruedSalary)
+                  const extraSalary = currentSalaryPaid.mod(salary)
+                  expectedAccruedSalary = extraSalary.gt(0) ? salary.minus(extraSalary) : 0
+                  expectedLastPayrollDate = previousPayrollDate.plus(currentSalaryPaid.div(salary).ceil())
                 } else {
                   expectedAccruedSalary = previousAccruedSalary.minus(expectedRequestedAmount).toString()
                   expectedLastPayrollDate = previousPayrollDate