From 15ada12379bbf69b972f2cb8d725649dceb1db26 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Nicol=C3=A1s=20Venturo?= Date: Fri, 2 Aug 2019 15:33:53 -0300 Subject: [PATCH] Fix GSNBouncerERC20Fee, add tests --- contracts/gsn/bouncers/GSNBouncerBase.sol | 13 ++++ contracts/gsn/bouncers/GSNBouncerERC20Fee.sol | 17 +++-- contracts/mocks/GSNBouncerERC20FeeMock.sol | 20 ++++++ test/gsn/GSNBouncerERC20Fee.test.js | 69 +++++++++++++++++++ 4 files changed, 114 insertions(+), 5 deletions(-) create mode 100644 contracts/mocks/GSNBouncerERC20FeeMock.sol create mode 100644 test/gsn/GSNBouncerERC20Fee.test.js diff --git a/contracts/gsn/bouncers/GSNBouncerBase.sol b/contracts/gsn/bouncers/GSNBouncerBase.sol index 2f83e859b50..49cc6ffb656 100644 --- a/contracts/gsn/bouncers/GSNBouncerBase.sol +++ b/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; + } } diff --git a/contracts/gsn/bouncers/GSNBouncerERC20Fee.sol b/contracts/gsn/bouncers/GSNBouncerERC20Fee.sol index eb433b3e88f..f8c1662c3ca 100644 --- a/contracts/gsn/bouncers/GSNBouncerERC20Fee.sol +++ b/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)); diff --git a/contracts/mocks/GSNBouncerERC20FeeMock.sol b/contracts/mocks/GSNBouncerERC20FeeMock.sol new file mode 100644 index 00000000000..6d38327b2f6 --- /dev/null +++ b/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())); + } +} diff --git a/test/gsn/GSNBouncerERC20Fee.test.js b/test/gsn/GSNBouncerERC20Fee.test.js new file mode 100644 index 00000000000..df4af21e30d --- /dev/null +++ b/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)); + }); + }); +});