Payroll: revert to old way of obtaining exchange rate and add more cl…

…arity (#821)

future-apps/payroll/contracts/Payroll.sol

@@ -655,17 +655,23 @@ contract Payroll is EtherTokenConstant, IForwarder, IsContract, AragonApp {
     }
 
     /**
-     * @dev Get token exchange rate for a token based on the denomination token
-     * @param _token Token
+     * @dev Get token exchange rate for a token based on the denomination token.
+     *      If the denomination token was USD and ETH's price was 100USD,
+     *      this would return 0.01 for ETH.
+     * @param _token Token to get price of in denomination tokens
      * @return ONE if _token is denominationToken or 0 if the exchange rate isn't recent enough
      */
-    function _getExchangeRate(address _token) internal view returns (uint128) {
+    function _getExchangeRateInDenominationToken(address _token) internal view returns (uint128) {
         // Denomination token has always exchange rate of 1
         if (_token == denominationToken) {
             return ONE;
         }
 
-        (uint128 xrt, uint64 when) = feed.get(_token, denominationToken);
+        // xrt is the number of `_token` that can be exchanged for one `denominationToken`
+        (uint128 xrt, uint64 when) = feed.get(
+            denominationToken,  // Base (e.g. USD)
+            _token              // Quote (e.g. ETH)
+        );
 
         // Check the price feed is recent enough
         if (getTimestamp64().sub(when) >= rateExpiryTime) {
@@ -692,10 +698,16 @@ contract Payroll is EtherTokenConstant, IForwarder, IsContract, AragonApp {
             address token = allowedTokensArray[i];
             uint256 tokenAllocation = employee.allocation[token];
             if (tokenAllocation != uint256(0)) {
-                uint256 exchangeRate = uint256(_getExchangeRate(token));
+                // Get the exchange rate for the token in denomination token,
+                // as we do accounting in denomination tokens
+                uint128 exchangeRate = _getExchangeRateInDenominationToken(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
+
+                // Salary (in denomination tokens) converted to payout 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 precision
+                tokenAmount = tokenAmount / (100 * ONE);
 
                 finance.newImmediatePayment(token, employeeAddress, tokenAmount, paymentReference);
                 emit SendPayment(employeeAddress, token, tokenAmount, paymentReference);

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

@@ -4,7 +4,7 @@ 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')(artifacts, web3)
+const { USD, DAI_RATE, ANT_RATE, exchangedAmount, deployDAI, deployANT, setTokenRates } = require('../helpers/tokens')(artifacts, web3)
 
 contract('Payroll bonuses', ([owner, employee, anyone]) => {
   let dao, payroll, payrollBase, finance, vault, priceFeed, DAI, ANT
@@ -177,8 +177,8 @@ contract('Payroll bonuses', ([owner, employee, anyone]) => {
             })
 
             const assertTransferredAmounts = (requestedAmount, expectedRequestedAmount = requestedAmount) => {
-              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()
+              const requestedDAI = exchangedAmount(expectedRequestedAmount, DAI_RATE, allocationDAI)
+              const requestedANT = exchangedAmount(expectedRequestedAmount, ANT_RATE, allocationANT)
 
               it('transfers all the pending bonus', async () => {
                 const previousDAI = await DAI.balanceOf(employee)

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

@@ -4,7 +4,7 @@ const { getEventArgument } = require('../helpers/events')
 const { bn, ONE, MAX_UINT256, bigExp } = require('../helpers/numbers')(web3)
 const { NOW, ONE_MONTH, TWO_MONTHS, 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')(artifacts, web3)
+const { USD, DAI_RATE, ANT_RATE, exchangedAmount, deployDAI, deployANT, setTokenRates } = require('../helpers/tokens')(artifacts, web3)
 
 contract('Payroll payday', ([owner, employee, anyone]) => {
   let dao, payroll, payrollBase, finance, vault, priceFeed, DAI, ANT
@@ -57,8 +57,8 @@ contract('Payroll payday', ([owner, employee, anyone]) => {
             })
 
             const assertTransferredAmounts = (requestedAmount, expectedRequestedAmount = requestedAmount) => {
-              const requestedDAI = expectedRequestedAmount.div(DAI_RATE).mul(allocationDAI).trunc().mul(ONE.div(100))
-              const requestedANT = expectedRequestedAmount.div(ANT_RATE).mul(allocationANT).trunc().mul(ONE.div(100))
+              const requestedDAI = exchangedAmount(expectedRequestedAmount, DAI_RATE, allocationDAI)
+              const requestedANT = exchangedAmount(expectedRequestedAmount, ANT_RATE, allocationANT)
 
               it('transfers the requested salary amount', async () => {
                 const previousDAI = await DAI.balanceOf(employee)
@@ -104,8 +104,8 @@ contract('Payroll payday', ([owner, employee, anyone]) => {
                 await payroll.payday(PAYMENT_TYPES.PAYROLL, requestedAmount, { from })
 
                 const newOwedAmount = salary.mul(ONE_MONTH)
-                const newDAIAmount = newOwedAmount.div(DAI_RATE).mul(allocationDAI).trunc().mul(ONE.div(100))
-                const newANTAmount = newOwedAmount.div(ANT_RATE).mul(allocationANT).trunc().mul(ONE.div(100))
+                const newDAIAmount = exchangedAmount(newOwedAmount, DAI_RATE, allocationDAI)
+                const newANTAmount = exchangedAmount(newOwedAmount, ANT_RATE, allocationANT)
 
                 await increaseTime(ONE_MONTH)
                 await setTokenRates(priceFeed, USD, [DAI, ANT], [DAI_RATE, ANT_RATE])
@@ -615,8 +615,8 @@ contract('Payroll payday', ([owner, employee, anyone]) => {
                   const requestedAmount = bigExp(1000, 18)
 
                   const assertTransferredAmounts = requestedAmount => {
-                    const requestedDAI = requestedAmount.div(DAI_RATE).mul(allocationDAI).mul(ONE.div(100)).trunc()
-                    const requestedANT = requestedAmount.div(ANT_RATE).mul(allocationANT).mul(ONE.div(100)).trunc()
+                    const requestedDAI = exchangedAmount(requestedAmount, DAI_RATE, allocationDAI)
+                    const requestedANT = exchangedAmount(requestedAmount, ANT_RATE, allocationANT)
 
                     it('transfers the requested salary amount', async () => {
                       const previousDAI = await DAI.balanceOf(employee)

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

@@ -4,7 +4,7 @@ const { getEvents, getEventArgument } = require('../helpers/events')
 const { NOW, ONE_MONTH, RATE_EXPIRATION_TIME } = require('../helpers/time')
 const { deployContracts, createPayrollAndPriceFeed } = require('../helpers/deploy')(artifacts, web3)
 const { ONE, bn, bigExp, annualSalaryPerSecond, MAX_UINT256 } = require('../helpers/numbers')(web3)
-const { USD, deployDAI, deployANT, DAI_RATE, ANT_RATE, setTokenRates } = require('../helpers/tokens')(artifacts, web3)
+const { USD, DAI_RATE, ANT_RATE, exchangedAmount, deployDAI, deployANT, setTokenRates } = require('../helpers/tokens')(artifacts, web3)
 
 contract('Payroll reimbursements', ([owner, employee, anyone]) => {
   let dao, payroll, payrollBase, finance, vault, priceFeed, DAI, ANT
@@ -166,8 +166,8 @@ contract('Payroll reimbursements', ([owner, employee, anyone]) => {
             })
 
             const assertTransferredAmounts = (requestedAmount, expectedRequestedAmount = requestedAmount) => {
-              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()
+              const requestedDAI = exchangedAmount(expectedRequestedAmount, DAI_RATE, allocationDAI)
+              const requestedANT = exchangedAmount(expectedRequestedAmount, ANT_RATE, allocationANT)
 
               it('transfers all the pending reimbursements', async () => {
                 const previousDAI = await DAI.balanceOf(employee)

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

@@ -2,7 +2,7 @@ const PAYMENT_TYPES = require('../helpers/payment_types')
 const { assertRevert } = require('@aragon/test-helpers/assertThrow')
 const { getEvents, getEventArgument } = require('../helpers/events')
 const { NOW, ONE_MONTH, RATE_EXPIRATION_TIME } = require('../helpers/time')
-const { USD, DAI_RATE, deployDAI, setTokenRate } = require('../helpers/tokens')(artifacts, web3)
+const { USD, DAI_RATE, exchangedAmount, deployDAI, setTokenRate } = require('../helpers/tokens')(artifacts, web3)
 const { ONE, bn, bigExp, MAX_UINT64, annualSalaryPerSecond } = require('../helpers/numbers')(web3)
 const { deployContracts, createPayrollAndPriceFeed } = require('../helpers/deploy')(artifacts, web3)
 
@@ -81,8 +81,6 @@ contract('Payroll employees termination', ([owner, employee, anyone]) => {
 
                 // Accrue some salary and extras
                 await increaseTime(ONE_MONTH)
-                // Mimic EVM truncation when calculating token amount to transfer
-                const owedSalary = salary.times(ONE_MONTH).div(ONE.div(100)).trunc().mul(ONE.div(100))
                 const reimbursement = bigExp(100000, 18)
                 await payroll.addReimbursement(employeeId, reimbursement, { from: owner })
 
@@ -95,8 +93,11 @@ contract('Payroll employees termination', ([owner, employee, anyone]) => {
                 await payroll.payday(PAYMENT_TYPES.REIMBURSEMENT, 0, { from: employee })
                 await assertRevert(payroll.getEmployee(employeeId), 'PAYROLL_EMPLOYEE_DOESNT_EXIST')
 
+                const owedSalaryInDai = exchangedAmount(salary.times(ONE_MONTH), DAI_RATE, 100)
+                const reimbursementInDai = exchangedAmount(reimbursement, DAI_RATE, 100)
+
                 const currentDAI = await DAI.balanceOf(employee)
-                const expectedDAI = previousDAI.plus(owedSalary).plus(reimbursement)
+                const expectedDAI = previousDAI.plus(owedSalaryInDai).plus(reimbursementInDai)
                 assert.equal(currentDAI.toString(), expectedDAI.toString(), 'current balance does not match')
               })
 

future-apps/payroll/test/helpers/tokens.js

@@ -12,6 +12,14 @@ module.exports = (artifacts, web3) => {
   const DAI_RATE = formatRate(1)    // 1 DAI = 1 USD
   const ANT_RATE = formatRate(0.5)  // 1 ANT = 0.5 USD
 
+  function exchangedAmount(amount, rate, tokenAllocation) {
+    // Mimic PPF inversion truncation, as we set the denomination token always
+    // as the price feed's quote token
+    const inverseRate = ONE.pow(2).div(rate).trunc()
+    // Mimic EVM calculation and truncation for token conversion
+    return amount.mul(inverseRate).mul(tokenAllocation).div(ONE.mul(100)).trunc()
+  }
+
   const deployANT = async (sender, finance) => deployTokenAndDeposit(sender, finance, 'ANT')
   const deployDAI = async (sender, finance) => deployTokenAndDeposit(sender, finance, 'DAI')
 
@@ -52,6 +60,7 @@ module.exports = (artifacts, web3) => {
     DAI_RATE,
     ANT_RATE,
     formatRate,
+    exchangedAmount,
     deployANT,
     deployDAI,
     deployTokenAndDeposit,