From 4f4d305784b042a4e5d314e29edc8508ed5e20cd Mon Sep 17 00:00:00 2001 From: Francisco Giordano Date: Mon, 7 Oct 2019 19:21:28 -0300 Subject: [PATCH] Fix the GSNBouncerERC20Fee token decimals to 18 (#1929) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix erc20 fee token decimals to 18 * lint * Update contracts/GSN/bouncers/GSNBouncerERC20Fee.sol Co-Authored-By: Nicolás Venturo * change location of hardcoded decimals for clarity * remove mention of decimals from docs * remove trailing whitespace from guide (cherry picked from commit b8ccf8e0f185685fb9da427888565ec41c2875ae) --- contracts/GSN/bouncers/GSNBouncerERC20Fee.sol | 7 +++---- contracts/mocks/GSNBouncerERC20FeeMock.sol | 2 +- docs/modules/ROOT/pages/gsn-bouncers.adoc | 20 +++++++++---------- test/GSN/GSNBouncerERC20Fee.test.js | 9 ++++----- 4 files changed, 17 insertions(+), 21 deletions(-) diff --git a/contracts/GSN/bouncers/GSNBouncerERC20Fee.sol b/contracts/GSN/bouncers/GSNBouncerERC20Fee.sol index 4bf042169..563ff3f39 100644 --- a/contracts/GSN/bouncers/GSNBouncerERC20Fee.sol +++ b/contracts/GSN/bouncers/GSNBouncerERC20Fee.sol @@ -27,11 +27,10 @@ contract GSNBouncerERC20Fee is GSNBouncerBase { __unstable__ERC20PrimaryAdmin private _token; /** - * @dev The arguments to the constructor are the details that the gas payment token will have: `name`, `symbol`, and - * `decimals`. + * @dev The arguments to the constructor are the details that the gas payment token will have: `name` and `symbol`. `decimals` is hard-coded to 18. */ - constructor(string memory name, string memory symbol, uint8 decimals) public { - _token = new __unstable__ERC20PrimaryAdmin(name, symbol, decimals); + constructor(string memory name, string memory symbol) public { + _token = new __unstable__ERC20PrimaryAdmin(name, symbol, 18); } /** diff --git a/contracts/mocks/GSNBouncerERC20FeeMock.sol b/contracts/mocks/GSNBouncerERC20FeeMock.sol index d58e673b8..9d2898ddb 100644 --- a/contracts/mocks/GSNBouncerERC20FeeMock.sol +++ b/contracts/mocks/GSNBouncerERC20FeeMock.sol @@ -4,7 +4,7 @@ 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) { + constructor(string memory name, string memory symbol) public GSNBouncerERC20Fee(name, symbol) { // solhint-disable-previous-line no-empty-blocks } diff --git a/docs/modules/ROOT/pages/gsn-bouncers.adoc b/docs/modules/ROOT/pages/gsn-bouncers.adoc index d43808f17..f0fb1eae9 100644 --- a/docs/modules/ROOT/pages/gsn-bouncers.adoc +++ b/docs/modules/ROOT/pages/gsn-bouncers.adoc @@ -45,7 +45,7 @@ On the other hand, you need to have a backend server, microservice, or lambda fu === How does GSNBouncerSignature work? -`GSNBouncerSignature` decides whether or not to accept the relayed call based on the included signature. +`GSNBouncerSignature` decides whether or not to accept the relayed call based on the included signature. The `acceptRelayedCall` implementation recovers the address from the signature of the relayed call parameters in `approvalData` and compares to the trusted signer. If the included signature matches the trusted signer, the relayed call is approved. @@ -62,7 +62,7 @@ Your GSN recipient contract needs to inherit from `GSNRecipient` and `GSNBouncer contract MyContract is GSNRecipient, GSNBouncerSignature { constructor(address trustedSigner) public GSNBouncerSignature(trustedSigner) { } -} +} ---- == GSNBouncerERC20Fee @@ -80,7 +80,7 @@ NOTE: *Users do not need to call approve* on their tokens for your recipient con === How does GSNBouncerERC20Fee work? -`GSNBouncerERC20Fee` decides to approve or reject relayed calls based on the balance of the users tokens. +`GSNBouncerERC20Fee` decides to approve or reject relayed calls based on the balance of the users tokens. The `acceptRelayedCall` function implementation checks the users token balance. If the user doesn't have enough tokens the relayed call gets rejected with an error of `INSUFFICIENT_BALANCE`. @@ -94,18 +94,16 @@ The maximum amount of tokens required is transferred in `_preRelayedCall` to pro NOTE: The gas cost estimation is not 100% accurate, we may tweak it further down the road. -NOTE: Always use `_preRelayedCall` and `_postRelayedCall` functions. Internal `_preRelayedCall` and `_postRelayedCall` functions are used instead of public `preRelayedCall` and `postRelayedCall` functions, as the public functions are prevented from being called by non-RelayHub contracts. +NOTE: Always use `_preRelayedCall` and `_postRelayedCall` functions. Internal `_preRelayedCall` and `_postRelayedCall` functions are used instead of public `preRelayedCall` and `postRelayedCall` functions, as the public functions are prevented from being called by non-RelayHub contracts. === How to use GSNBouncerERC20Fee Your GSN recipient contract needs to inherit from `GSNRecipient` and `GSNBouncerERC20Fee` along with appropriate xref:access-control.adoc[access control] (for token minting), set the token details in the constructor of `GSNBouncerERC20Fee` and create a public `mint` function suitably protected by your chosen access control as per the following sample code (which uses the xref:api:access.adoc#MinterRole[MinterRole]): -NOTE: The token must have decimals of 18 to match that of ether, due to the baked-in exchange rate of 1:1. - [source,solidity] ---- contract MyContract is GSNRecipient, GSNBouncerERC20Fee, MinterRole { - constructor() public GSNBouncerERC20Fee("FeeToken", "FEE", 18) { + constructor() public GSNBouncerERC20Fee("FeeToken", "FEE") { } function mint(address account, uint256 amount) public onlyMinter { @@ -120,13 +118,13 @@ You can create your own Custom Bouncer! For example, your Custom Bouncer could Your Custom Bouncer can inherit from `GSNBouncerBase` and must implement the `acceptRelayedCall` function. -Your `acceptRelayedCall` implementation decides whether or not to accept the relayed call. -If your logic accepts the relayed call then you should return `_approveRelayedCall`. +Your `acceptRelayedCall` implementation decides whether or not to accept the relayed call. +If your logic accepts the relayed call then you should return `_approveRelayedCall`. If your logic rejects the relayed call then you should return `_rejectRelayedCall` with an error code. See https://github.com/OpenZeppelin/openzeppelin-contracts/blob/master/contracts/GSN/bouncers/GSNBouncerSignature.sol[GSNBouncerSignature.sol] as an example implementation. -For Custom Bouncers charging end users, `_postRelayedCall` and `_preRelayedCall` should be implemented to handle the charging. -Your `_preRelayedCall` implementation should take the maximum possible charge, with your `_postRelayedCall` implementation refunding any difference from the actual charge once the relayed call has been made. +For Custom Bouncers charging end users, `_postRelayedCall` and `_preRelayedCall` should be implemented to handle the charging. +Your `_preRelayedCall` implementation should take the maximum possible charge, with your `_postRelayedCall` implementation refunding any difference from the actual charge once the relayed call has been made. When returning `_approveRelayedCall` to approve the relayed call, the end users address, `maxPossibleCharge`, `transactionFee` and `gasPrice` data can also be returned so that the data can be used in `_preRelayedCall` and `_postRelayedCall`. See https://github.com/OpenZeppelin/openzeppelin-contracts/blob/release-v2.4.0/contracts/GSN/bouncers/GSNBouncerERC20Fee.sol[GSNBouncerERC20Fee.sol] as an example implementation. diff --git a/test/GSN/GSNBouncerERC20Fee.test.js b/test/GSN/GSNBouncerERC20Fee.test.js index c92c94f5d..71dd423cd 100644 --- a/test/GSN/GSNBouncerERC20Fee.test.js +++ b/test/GSN/GSNBouncerERC20Fee.test.js @@ -1,4 +1,4 @@ -const { BN, ether, expectEvent } = require('openzeppelin-test-helpers'); +const { ether, expectEvent } = require('openzeppelin-test-helpers'); const gsn = require('@openzeppelin/gsn-helpers'); const { expect } = require('chai'); @@ -10,10 +10,9 @@ 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.recipient = await GSNBouncerERC20FeeMock.new(name, symbol); this.token = await ERC20Detailed.at(await this.recipient.token()); }); @@ -26,8 +25,8 @@ contract('GSNBouncerERC20Fee', function ([_, sender, other]) { expect(await this.token.symbol()).to.equal(symbol); }); - it('has decimals', async function () { - expect(await this.token.decimals()).to.be.bignumber.equal(decimals); + it('has 18 decimals', async function () { + expect(await this.token.decimals()).to.be.bignumber.equal('18'); }); });