Payroll: Add payroll remainders as accrued salary (#903)

* payroll: add payday remainders as accrued salary * payroll: optimize current payday remainders (#906) * payroll: fix payroll maths
aragon · Jul 7, 2019 · bc799c4 · bc799c4
1 parent d28394c
commit bc799c4
Show file tree

Hide file tree

Showing 2 changed files with 169 additions and 150 deletions.
diff --git a/future-apps/payroll/contracts/Payroll.sol b/future-apps/payroll/contracts/Payroll.sol
@@ -651,11 +651,30 @@ contract Payroll is EtherTokenConstant, IForwarder, IsContract, AragonApp {
         if (accruedSalary > 0) {
             // Employee is cashing out a mixed amount between previous and current owed salaries;
             // first use up their accrued salary
-            employee.accruedSalary = uint256(0);
             // No need to use SafeMath here as we already know _paymentAmount > accruedSalary
             currentSalaryPaid = _paymentAmount - accruedSalary;
         }
-        employee.lastPayroll = _getLastPayrollDate(_employeeId, currentSalaryPaid);
+        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
+        uint256 extraSalary = currentSalaryPaid % salary;
+        if (extraSalary > 0) {
+            timeDiff = timeDiff.add(1);
+            employee.accruedSalary = salary - currentSalaryPaid;
+        } else if (accruedSalary > 0) {
+            employee.accruedSalary = 0;
+        }
+
+        uint256 lastPayrollDate = uint256(employee.lastPayroll).add(timeDiff);
+        // Even though this function should never receive a currentSalaryPaid value that would
+        // result in the lastPayrollDate being higher than the current time,
+        // let's double check to be safe
+        require(lastPayrollDate <= uint256(getTimestamp64()), ERROR_LAST_PAYROLL_DATE_TOO_BIG);
+        // Already know lastPayrollDate must fit in uint64 from above
+        employee.lastPayroll = uint64(lastPayrollDate);
     }
 
     /**
@@ -726,34 +745,6 @@ contract Payroll is EtherTokenConstant, IForwarder, IsContract, AragonApp {
         return uint256(xrt);
     }
 
-    /**
-     * @dev Calculate the new last payroll date for an employee based on the requested payment amount
-     * @param _employeeId Employee's identifier
-     * @param _paidAmount Requested amount to be paid to the employee
-     * @return The new last payroll timestamp in seconds based on the requested payment amount
-     */
-    function _getLastPayrollDate(uint256 _employeeId, uint256 _paidAmount) internal view returns (uint64) {
-        Employee storage employee = employees[_employeeId];
-
-        uint256 timeDiff = _paidAmount.div(employee.denominationTokenSalary);
-
-        // We check if the division was perfect, and if not, take its ceiling to avoid giving away
-        // tiny amounts of salary.
-        // Employees may lose up to a second's worth of payroll if they use the "request partial
-        // amount" feature or reach salary amounts that get capped by uint max.
-        if (timeDiff.mul(employee.denominationTokenSalary) != _paidAmount) {
-            timeDiff = timeDiff.add(1);
-        }
-
-        uint256 lastPayrollDate = uint256(employee.lastPayroll).add(timeDiff);
-
-        // Even though this function should never receive a _paidAmount value that would result in
-        // the lastPayrollDate being higher than the current time, let's double check to be safe
-        require(lastPayrollDate <= uint256(getTimestamp64()), ERROR_LAST_PAYROLL_DATE_TOO_BIG);
-        // Already know lastPayrollDate must fit in uint64 from above
-        return uint64(lastPayrollDate);
-    }
-
     /**
      * @dev Get owed salary since last payroll for an employee
      * @param _employeeId Employee's identifier