Fix GSNBouncerERC20Fee, add tests

contracts/gsn/bouncers/GSNBouncerBase.sol

@@ -12,6 +12,9 @@ contract GSNBouncerBase is IRelayRecipient {
     uint256 constant private RELAYED_CALL_ACCEPTED = 0;
     uint256 constant private RELAYED_CALL_REJECTED = 11;
 
+    // How much gas is forwarded to postRelayedCall
+    uint256 constant internal POST_RELAYED_CALL_MAX_GAS = 100000;
+
     modifier onlyRelayHub() {
         require(msg.sender == getHubAddr(), "GSNBouncerBase: caller is not RelayHub");
         _;
@@ -79,4 +82,14 @@ contract GSNBouncerBase is IRelayRecipient {
     function _postRelayedCall(bytes memory, bool, uint256, bytes32) internal {
         // solhint-disable-previous-line no-empty-blocks
     }
+
+    /*
+     * @dev Calculates how much RelaHub will charge a recipient for using `gas` at a `gasPrice`, given a relayer's
+     * `serviceFee`.
+     */
+    function _computeCharge(uint256 gas, uint256 gasPrice, uint256 serviceFee) internal pure returns (uint256) {
+        // The fee is expressed as a percentage. E.g. a value of 40 stands for a 40% fee, so the recipient will be
+        // charged for 1.4 times the spent amount.
+        return (gas * gasPrice * (100 + serviceFee)) / 100;
+    }
 }

contracts/gsn/bouncers/GSNBouncerERC20Fee.sol

@@ -26,7 +26,7 @@ contract GSNBouncerERC20Fee is GSNBouncerBase {
         return IERC20(_token);
     }
 
-    function _mintBuiltIn(address account, uint256 amount) internal {
+    function _mint(address account, uint256 amount) internal {
         _token.mint(account, amount);
     }
 
@@ -34,8 +34,8 @@ contract GSNBouncerERC20Fee is GSNBouncerBase {
         address,
         address from,
         bytes calldata,
-        uint256,
-        uint256,
+        uint256 transactionFee,
+        uint256 gasPrice,
         uint256,
         uint256,
         bytes calldata,
@@ -51,7 +51,7 @@ contract GSNBouncerERC20Fee is GSNBouncerBase {
             return _declineRelayedCall(uint256(GSNRecipientERC20ChargeErrorCodes.INSUFFICIENT_ALLOWANCE));
         }
 
-        return _confirmRelayedCall(abi.encode(from, maxPossibleCharge));
+        return _confirmRelayedCall(abi.encode(from, maxPossibleCharge, transactionFee, gasPrice));
     }
 
     function _preRelayedCall(bytes memory context) internal returns (bytes32) {
@@ -62,7 +62,14 @@ contract GSNBouncerERC20Fee is GSNBouncerBase {
     }
 
     function _postRelayedCall(bytes memory context, bool, uint256 actualCharge, bytes32) internal {
-        (address from, uint256 maxPossibleCharge) = abi.decode(context, (address, uint256));
+        (address from, uint256 maxPossibleCharge, uint256 transactionFee, uint256 gasPrice) =
+            abi.decode(context, (address, uint256, uint256, uint256));
+
+        // actualCharge is an _estimated_ charge, which assumes postRelayedCall will use all available gas.
+        // This implementation's gas cost can be roughly estimated as 10k gas, for the two SSTORE operations in an
+        // ERC20 transfer.
+        uint256 overestimation = _computeCharge(POST_RELAYED_CALL_MAX_GAS.sub(10000), gasPrice, transactionFee);
+        actualCharge = actualCharge.sub(overestimation);
 
         // After the relayed call has been executed and the actual charge estimated, the excess pre-charge is returned
         _token.safeTransfer(from, maxPossibleCharge.sub(actualCharge));

contracts/mocks/GSNBouncerERC20FeeMock.sol

@@ -0,0 +1,20 @@
+pragma solidity ^0.5.0;
+
+import "../gsn/GSNRecipient.sol";
+import "../gsn/bouncers/GSNBouncerERC20Fee.sol";
+
+contract GSNBouncerERC20FeeMock is GSNRecipient, GSNBouncerERC20Fee {
+    constructor(string memory name, string memory symbol, uint8 decimals) public GSNBouncerERC20Fee(name, symbol, decimals) {
+        // solhint-disable-previous-line no-empty-blocks
+    }
+
+    function mint(address account, uint256 amount) public {
+        _mint(account, amount);
+    }
+
+    event MockFunctionCalled(uint256 senderBalance);
+
+    function mockFunction() public {
+        emit MockFunctionCalled(token().balanceOf(_msgSender()));
+    }
+}

test/gsn/GSNBouncerERC20Fee.test.js

@@ -0,0 +1,69 @@
+const { BN, ether, expectEvent } = require('openzeppelin-test-helpers');
+const gsn = require('@openzeppelin/gsn-helpers');
+
+const { expect } = require('chai');
+
+const GSNBouncerERC20FeeMock = artifacts.require('GSNBouncerERC20FeeMock');
+const ERC20Detailed = artifacts.require('ERC20Detailed');
+const IRelayHub = artifacts.require('IRelayHub');
+
+contract('GSNBouncerERC20Fee', function ([_, sender, other]) {
+  const name = 'FeeToken';
+  const symbol = 'FTKN';
+  const decimals = new BN('18');
+
+  beforeEach(async function () {
+    this.recipient = await GSNBouncerERC20FeeMock.new(name, symbol, decimals);
+    this.token = await ERC20Detailed.at(await this.recipient.token());
+  });
+
+  describe('token', function () {
+    it('has a name', async function () {
+      expect(await this.token.name()).to.equal(name);
+    });
+
+    it('has a symbol', async function () {
+      expect(await this.token.symbol()).to.equal(symbol);
+    });
+
+    it('has decimals', async function () {
+      expect(await this.token.decimals()).to.be.bignumber.equal(decimals);
+    });
+  });
+
+  context('when called directly', function () {
+    it('mock function can be called', async function () {
+      const { logs } = await this.recipient.mockFunction();
+      expectEvent.inLogs(logs, 'MockFunctionCalled');
+    });
+  });
+
+  context('when relay-called', function () {
+    beforeEach(async function () {
+      await gsn.fundRecipient(web3, { recipient: this.recipient.address });
+      this.relayHub = await IRelayHub.at('0x537F27a04470242ff6b2c3ad247A05248d0d27CE');
+    });
+
+    it('charges the sender for GSN fees in tokens', async function () {
+      // The recipient will be charged from its RelayHub balance, and in turn charge the sender from its sender balance.
+      // Both amounts should be roughly equal.
+
+      // The sender has a balance in tokens, not ether, but since the exchange rate is 1:1, this works fine.
+      const senderPreBalance = ether('2');
+      await this.recipient.mint(sender, senderPreBalance);
+
+      const recipientPreBalance = await this.relayHub.balanceOf(this.recipient.address);
+
+      const { tx } = await this.recipient.mockFunction({ from: sender, useGSN: true });
+      await expectEvent.inTransaction(tx, IRelayHub, 'TransactionRelayed', { status: '0' });
+
+      const senderPostBalance = await this.token.balanceOf(sender);
+      const recipientPostBalance = await this.relayHub.balanceOf(this.recipient.address);
+
+      const senderCharge = senderPreBalance.sub(senderPostBalance);
+      const recipientCharge = recipientPreBalance.sub(recipientPostBalance);
+
+      expect(senderCharge).to.be.bignumber.closeTo(recipientCharge, recipientCharge.divn(10));
+    });
+  });
+});