GSN renaming (#1963)

* Merge GSNBouncerBase into GSNRecipient

* Remove emtpy implementations for _pre and _post

* Rename bouncers to recipients

* Rename bouncers documentation to strategies

* Rewrite guides and docstrings to use the strategy naming scheme

* Address review comments

* Apply suggestions from code review

Co-Authored-By: Francisco Giordano <frangio.1@gmail.com>

* change wording of docs

contracts/GSN/GSNRecipient.sol

@@ -3,18 +3,28 @@ pragma solidity ^0.5.0;
 import "./IRelayRecipient.sol";
 import "./IRelayHub.sol";
 import "./Context.sol";
-import "./bouncers/GSNBouncerBase.sol";
 
 /**
- * @dev Base GSN recipient contract: includes the {IRelayRecipient} interface and enables GSN support on all contracts
- * in the inheritance tree.
+ * @dev Base GSN recipient contract: includes the {IRelayRecipient} interface
+ * and enables GSN support on all contracts in the inheritance tree.
  *
- * Not all interface methods are implemented (e.g. {acceptRelayedCall}, derived contracts must provide one themselves.
+ * TIP: This contract is abstract. The functions {acceptRelayedCall},
+ *  {_preRelayedCall}, and {_postRelayedCall} are not implemented and must be
+ * provided by derived contracts. See the
+ * xref:ROOT:gsn-strategies.adoc#gsn-strategies[GSN strategies] for more
+ * information on how to use the pre-built {GSNRecipientSignature} and
+ * {GSNRecipientERC20Fee}, or how to write your own.
  */
-contract GSNRecipient is IRelayRecipient, Context, GSNBouncerBase {
+contract GSNRecipient is IRelayRecipient, Context {
     // Default RelayHub address, deployed on mainnet and all testnets at the same address
     address private _relayHub = 0xD216153c06E857cD7f72665E0aF1d7D82172F494;
 
+    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;
+
     /**
      * @dev Emitted when a contract changes its {IRelayHub} contract to a new one.
      */
@@ -97,6 +107,89 @@ contract GSNRecipient is IRelayRecipient, Context, GSNBouncerBase {
         }
     }
 
+    // Base implementations for pre and post relayedCall: only RelayHub can invoke them, and data is forwarded to the
+    // internal hook.
+
+    /**
+     * @dev See `IRelayRecipient.preRelayedCall`.
+     *
+     * This function should not be overriden directly, use `_preRelayedCall` instead.
+     *
+     * * Requirements:
+     *
+     * - the caller must be the `RelayHub` contract.
+     */
+    function preRelayedCall(bytes calldata context) external returns (bytes32) {
+        require(msg.sender == getHubAddr(), "GSNRecipient: caller is not RelayHub");
+        return _preRelayedCall(context);
+    }
+
+    /**
+     * @dev See `IRelayRecipient.preRelayedCall`.
+     *
+     * Called by `GSNRecipient.preRelayedCall`, which asserts the caller is the `RelayHub` contract. Derived contracts
+     * must implement this function with any relayed-call preprocessing they may wish to do.
+     *
+     */
+    function _preRelayedCall(bytes memory context) internal returns (bytes32);
+
+    /**
+     * @dev See `IRelayRecipient.postRelayedCall`.
+     *
+     * This function should not be overriden directly, use `_postRelayedCall` instead.
+     *
+     * * Requirements:
+     *
+     * - the caller must be the `RelayHub` contract.
+     */
+    function postRelayedCall(bytes calldata context, bool success, uint256 actualCharge, bytes32 preRetVal) external {
+        require(msg.sender == getHubAddr(), "GSNRecipient: caller is not RelayHub");
+        _postRelayedCall(context, success, actualCharge, preRetVal);
+    }
+
+    /**
+     * @dev See `IRelayRecipient.postRelayedCall`.
+     *
+     * Called by `GSNRecipient.postRelayedCall`, which asserts the caller is the `RelayHub` contract. Derived contracts
+     * must implement this function with any relayed-call postprocessing they may wish to do.
+     *
+     */
+    function _postRelayedCall(bytes memory context, bool success, uint256 actualCharge, bytes32 preRetVal) internal;
+
+    /**
+     * @dev Return this in acceptRelayedCall to proceed with the execution of a relayed call. Note that this contract
+     * will be charged a fee by RelayHub
+     */
+    function _approveRelayedCall() internal pure returns (uint256, bytes memory) {
+        return _approveRelayedCall("");
+    }
+
+    /**
+     * @dev See `GSNRecipient._approveRelayedCall`.
+     *
+     * This overload forwards `context` to _preRelayedCall and _postRelayedCall.
+     */
+    function _approveRelayedCall(bytes memory context) internal pure returns (uint256, bytes memory) {
+        return (RELAYED_CALL_ACCEPTED, context);
+    }
+
+    /**
+     * @dev Return this in acceptRelayedCall to impede execution of a relayed call. No fees will be charged.
+     */
+    function _rejectRelayedCall(uint256 errorCode) internal pure returns (uint256, bytes memory) {
+        return (RELAYED_CALL_REJECTED + errorCode, "");
+    }
+
+    /*
+     * @dev Calculates how much RelayHub 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;
+    }
+
     function _getRelayedCallSender() private pure returns (address payable result) {
         // We need to read 20 bytes (an address) located at array index msg.data.length - 20. In memory, the array
         // is prefixed with a 32-byte length value, so we first add 32 to get the memory read index. However, doing

contracts/GSN/bouncers/GSNBouncerERC20Fee.sol → contracts/GSN/GSNRecipientERC20Fee.sol

 pragma solidity ^0.5.0;
 
-import "./GSNBouncerBase.sol";
-import "../../math/SafeMath.sol";
-import "../../ownership/Secondary.sol";
-import "../../token/ERC20/SafeERC20.sol";
-import "../../token/ERC20/ERC20.sol";
-import "../../token/ERC20/ERC20Detailed.sol";
+import "./GSNRecipient.sol";
+import "../math/SafeMath.sol";
+import "../ownership/Secondary.sol";
+import "../token/ERC20/SafeERC20.sol";
+import "../token/ERC20/ERC20.sol";
+import "../token/ERC20/ERC20Detailed.sol";
 
 /**
- * @dev A xref:ROOT:gsn-bouncers.adoc#gsn-bouncers[GSN Bouncer] that charges transaction fees in a special purpose ERC20
+ * @dev A xref:ROOT:gsn-strategies.adoc#gsn-strategies[GSN strategy] that charges transaction fees in a special purpose ERC20
  * token, which we refer to as the gas payment token. The amount charged is exactly the amount of Ether charged to the
  * recipient. This means that the token is essentially pegged to the value of Ether.
  *
@@ -16,11 +16,11 @@ import "../../token/ERC20/ERC20Detailed.sol";
  * whose only minter is the recipient, so the strategy must be implemented in a derived contract, making use of the
  * internal {_mint} function.
  */
-contract GSNBouncerERC20Fee is GSNBouncerBase {
+contract GSNRecipientERC20Fee is GSNRecipient {
     using SafeERC20 for __unstable__ERC20PrimaryAdmin;
     using SafeMath for uint256;
 
-    enum GSNBouncerERC20FeeErrorCodes {
+    enum GSNRecipientERC20FeeErrorCodes {
         INSUFFICIENT_BALANCE
     }
 
@@ -66,7 +66,7 @@ contract GSNBouncerERC20Fee is GSNBouncerBase {
         returns (uint256, bytes memory)
     {
         if (_token.balanceOf(from) < maxPossibleCharge) {
-            return _rejectRelayedCall(uint256(GSNBouncerERC20FeeErrorCodes.INSUFFICIENT_BALANCE));
+            return _rejectRelayedCall(uint256(GSNRecipientERC20FeeErrorCodes.INSUFFICIENT_BALANCE));
         }
 
         return _approveRelayedCall(abi.encode(from, maxPossibleCharge, transactionFee, gasPrice));

contracts/GSN/bouncers/GSNBouncerSignature.sol → contracts/GSN/GSNRecipientSignature.sol

 pragma solidity ^0.5.0;
 
-import "./GSNBouncerBase.sol";
-import "../../cryptography/ECDSA.sol";
+import "./GSNRecipient.sol";
+import "../cryptography/ECDSA.sol";
 
 /**
- * @dev A xref:ROOT:gsn-bouncers.adoc#gsn-bouncers[GSN Bouncer] that allows relayed transactions through when they are
+ * @dev A xref:ROOT:gsn-strategies.adoc#gsn-strategies[GSN strategy] that allows relayed transactions through when they are
  * accompanied by the signature of a trusted signer. The intent is for this signature to be generated by a server that
  * performs validations off-chain. Note that nothing is charged to the user in this scheme. Thus, the server should make
  * sure to account for this in their economic and threat model.
  */
-contract GSNBouncerSignature is GSNBouncerBase {
+contract GSNRecipientSignature is GSNRecipient {
     using ECDSA for bytes32;
 
     address private _trustedSigner;
 
-    enum GSNBouncerSignatureErrorCodes {
+    enum GSNRecipientSignatureErrorCodes {
         INVALID_SIGNER
     }
 
@@ -22,7 +22,7 @@ contract GSNBouncerSignature is GSNBouncerBase {
      * @dev Sets the trusted signer that is going to be producing signatures to approve relayed calls.
      */
     constructor(address trustedSigner) public {
-        require(trustedSigner != address(0), "GSNBouncerSignature: trusted signer is the zero address");
+        require(trustedSigner != address(0), "GSNRecipientSignature: trusted signer is the zero address");
         _trustedSigner = trustedSigner;
     }
 
@@ -58,7 +58,15 @@ contract GSNBouncerSignature is GSNBouncerBase {
         if (keccak256(blob).toEthSignedMessageHash().recover(approvalData) == _trustedSigner) {
             return _approveRelayedCall();
         } else {
-            return _rejectRelayedCall(uint256(GSNBouncerSignatureErrorCodes.INVALID_SIGNER));
+            return _rejectRelayedCall(uint256(GSNRecipientSignatureErrorCodes.INVALID_SIGNER));
         }
     }
+
+    function _preRelayedCall(bytes memory) internal returns (bytes32) {
+        // solhint-disable-previous-line no-empty-blocks
+    }
+
+    function _postRelayedCall(bytes memory, bool, uint256, bytes32) internal {
+        // solhint-disable-previous-line no-empty-blocks
+    }
 }

contracts/GSN/README.adoc

@@ -6,10 +6,10 @@ TIP: If you're new to the GSN, head over to our xref:openzeppelin::gsn/what-is-t
 
 The core contract a recipient must inherit from is {GSNRecipient}: it includes all necessary interfaces, as well as some helper methods to make interacting with the GSN easier.
 
-Utilities to make writing xref:ROOT:gsn-bouncers.adoc[GSN Bouncers] easy are available in {GSNBouncerBase}, or you can simply use one of our pre-made bouncers:
+Utilities to make writing xref:ROOT:gsn-strategies.adoc[GSN strategies] easy are available in {GSNRecipient}, or you can simply use one of our pre-made strategies:
 
-* {GSNBouncerERC20Fee} charges the end user for gas costs in an application-specific xref:ROOT:tokens.adoc#ERC20[ERC20 token]
-* {GSNBouncerSignature} accepts all relayed calls that have been signed by a trusted third party (e.g. a private key in a backend)
+* {GSNRecipientERC20Fee} charges the end user for gas costs in an application-specific xref:ROOT:tokens.adoc#ERC20[ERC20 token]
+* {GSNRecipientSignature} accepts all relayed calls that have been signed by a trusted third party (e.g. a private key in a backend)
 
 You can also take a look at the two contract interfaces that make up the GSN protocol: {IRelayRecipient} and {IRelayHub}, but you won't need to use those directly.
 
@@ -19,11 +19,10 @@ NOTE: This feature is being released in the next version of OpenZeppelin Contrac
 
 {{GSNRecipient}}
 
-== Bouncers
+== Strategies
 
-{{GSNBouncerBase}}
-{{GSNBouncerERC20Fee}}
-{{GSNBouncerSignature}}
+{{GSNRecipientSignature}}
+{{GSNRecipientERC20Fee}}
 
 == Protocol
 

contracts/GSN/bouncers/GSNBouncerBase.sol

-pragma solidity ^0.5.0;
-
-import "../IRelayRecipient.sol";
-
-/**
- * @dev Base contract used to implement GSNBouncers.
- *
- * > This contract does not perform all required tasks to implement a GSN
- * recipient contract: end users should use `GSNRecipient` instead.
- */
-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;
-
-    // Base implementations for pre and post relayedCall: only RelayHub can invoke them, and data is forwarded to the
-    // internal hook.
-
-    /**
-     * @dev See `IRelayRecipient.preRelayedCall`.
-     *
-     * This function should not be overriden directly, use `_preRelayedCall` instead.
-     *
-     * * Requirements:
-     *
-     * - the caller must be the `RelayHub` contract.
-     */
-    function preRelayedCall(bytes calldata context) external returns (bytes32) {
-        require(msg.sender == getHubAddr(), "GSNBouncerBase: caller is not RelayHub");
-        return _preRelayedCall(context);
-    }
-
-    /**
-     * @dev See `IRelayRecipient.postRelayedCall`.
-     *
-     * This function should not be overriden directly, use `_postRelayedCall` instead.
-     *
-     * * Requirements:
-     *
-     * - the caller must be the `RelayHub` contract.
-     */
-    function postRelayedCall(bytes calldata context, bool success, uint256 actualCharge, bytes32 preRetVal) external {
-        require(msg.sender == getHubAddr(), "GSNBouncerBase: caller is not RelayHub");
-        _postRelayedCall(context, success, actualCharge, preRetVal);
-    }
-
-    /**
-     * @dev Return this in acceptRelayedCall to proceed with the execution of a relayed call. Note that this contract
-     * will be charged a fee by RelayHub
-     */
-    function _approveRelayedCall() internal pure returns (uint256, bytes memory) {
-        return _approveRelayedCall("");
-    }
-
-    /**
-     * @dev See `GSNBouncerBase._approveRelayedCall`.
-     *
-     * This overload forwards `context` to _preRelayedCall and _postRelayedCall.
-     */
-    function _approveRelayedCall(bytes memory context) internal pure returns (uint256, bytes memory) {
-        return (RELAYED_CALL_ACCEPTED, context);
-    }
-
-    /**
-     * @dev Return this in acceptRelayedCall to impede execution of a relayed call. No fees will be charged.
-     */
-    function _rejectRelayedCall(uint256 errorCode) internal pure returns (uint256, bytes memory) {
-        return (RELAYED_CALL_REJECTED + errorCode, "");
-    }
-
-    // Empty hooks for pre and post relayed call: users only have to define these if they actually use them.
-
-    function _preRelayedCall(bytes memory) internal returns (bytes32) {
-        // solhint-disable-previous-line no-empty-blocks
-    }
-
-    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/mocks/ERC721GSNRecipientMock.sol

@@ -2,14 +2,14 @@ pragma solidity ^0.5.0;
 
 import "../token/ERC721/ERC721.sol";
 import "../GSN/GSNRecipient.sol";
-import "../GSN/bouncers/GSNBouncerSignature.sol";
+import "../GSN/GSNRecipientSignature.sol";
 
 /**
  * @title ERC721GSNRecipientMock
  * A simple ERC721 mock that has GSN support enabled
  */
-contract ERC721GSNRecipientMock is ERC721, GSNRecipient, GSNBouncerSignature {
-    constructor(address trustedSigner) public GSNBouncerSignature(trustedSigner) { }
+contract ERC721GSNRecipientMock is ERC721, GSNRecipient, GSNRecipientSignature {
+    constructor(address trustedSigner) public GSNRecipientSignature(trustedSigner) { }
     // solhint-disable-previous-line no-empty-blocks
 
     function mint(uint256 tokenId) public {

contracts/mocks/GSNBouncerERC20FeeMock.sol → contracts/mocks/GSNRecipientERC20FeeMock.sol

 pragma solidity ^0.5.0;
 
 import "../GSN/GSNRecipient.sol";
-import "../GSN/bouncers/GSNBouncerERC20Fee.sol";
+import "../GSN/GSNRecipientERC20Fee.sol";
 
-contract GSNBouncerERC20FeeMock is GSNRecipient, GSNBouncerERC20Fee {
-    constructor(string memory name, string memory symbol) public GSNBouncerERC20Fee(name, symbol) {
+contract GSNRecipientERC20FeeMock is GSNRecipient, GSNRecipientERC20Fee {
+    constructor(string memory name, string memory symbol) public GSNRecipientERC20Fee(name, symbol) {
         // solhint-disable-previous-line no-empty-blocks
     }
 

contracts/mocks/GSNRecipientMock.sol

@@ -17,11 +17,11 @@ contract GSNRecipientMock is ContextMock, GSNRecipient {
         return (0, "");
     }
 
-    function preRelayedCall(bytes calldata) external returns (bytes32) {
+    function _preRelayedCall(bytes memory) internal returns (bytes32) {
         // solhint-disable-previous-line no-empty-blocks
     }
 
-    function postRelayedCall(bytes calldata, bool, uint256, bytes32) external {
+    function _postRelayedCall(bytes memory, bool, uint256, bytes32) internal {
         // solhint-disable-previous-line no-empty-blocks
     }
 

contracts/mocks/GSNBouncerSignatureMock.sol → contracts/mocks/GSNRecipientSignatureMock.sol

 pragma solidity ^0.5.0;
 
 import "../GSN/GSNRecipient.sol";
-import "../GSN/bouncers/GSNBouncerSignature.sol";
+import "../GSN/GSNRecipientSignature.sol";
 
-contract GSNBouncerSignatureMock is GSNRecipient, GSNBouncerSignature {
-    constructor(address trustedSigner) public GSNBouncerSignature(trustedSigner) {
+contract GSNRecipientSignatureMock is GSNRecipient, GSNRecipientSignature {
+    constructor(address trustedSigner) public GSNRecipientSignature(trustedSigner) {
         // solhint-disable-previous-line no-empty-blocks
     }
 

docs/modules/ROOT/nav.adoc

@@ -10,7 +10,7 @@
 
 .In Depth
 * xref:erc20-supply.adoc[ERC20 Supply]
-* xref:gsn-bouncers.adoc[GSN Bouncers]
+* xref:gsn-strategies.adoc[GSN Strategies]
 
 .FAQ
 * xref:api-stability.adoc[API Stability]

test/GSN/GSNBouncerERC20Fee.test.js → test/GSN/GSNRecipientERC20Fee.test.js

@@ -3,16 +3,16 @@ const gsn = require('@openzeppelin/gsn-helpers');
 
 const { expect } = require('chai');
 
-const GSNBouncerERC20FeeMock = artifacts.require('GSNBouncerERC20FeeMock');
+const GSNRecipientERC20FeeMock = artifacts.require('GSNRecipientERC20FeeMock');
 const ERC20Detailed = artifacts.require('ERC20Detailed');
 const IRelayHub = artifacts.require('IRelayHub');
 
-contract('GSNBouncerERC20Fee', function ([_, sender, other]) {
+contract('GSNRecipientERC20Fee', function ([_, sender, other]) {
   const name = 'FeeToken';
   const symbol = 'FTKN';
 
   beforeEach(async function () {
-    this.recipient = await GSNBouncerERC20FeeMock.new(name, symbol);
+    this.recipient = await GSNRecipientERC20FeeMock.new(name, symbol);
     this.token = await ERC20Detailed.at(await this.recipient.token());
   });
 

test/GSN/GSNBouncerSignature.test.js → test/GSN/GSNRecipientSignature.test.js

@@ -4,11 +4,11 @@ const { fixSignature } = require('../helpers/sign');
 const { utils: { toBN } } = require('web3');
 const { ZERO_ADDRESS } = constants;
 
-const GSNBouncerSignatureMock = artifacts.require('GSNBouncerSignatureMock');
+const GSNRecipientSignatureMock = artifacts.require('GSNRecipientSignatureMock');
 
-contract('GSNBouncerSignature', function ([_, signer, other]) {
+contract('GSNRecipientSignature', function ([_, signer, other]) {
   beforeEach(async function () {
-    this.recipient = await GSNBouncerSignatureMock.new(signer);
+    this.recipient = await GSNRecipientSignatureMock.new(signer);
   });
 
   context('when called directly', function () {
@@ -21,10 +21,10 @@ contract('GSNBouncerSignature', function ([_, signer, other]) {
   context('when constructor is called with a zero address', function () {
     it('fails when constructor called with a zero address', async function () {
       await expectRevert(
-        GSNBouncerSignatureMock.new(
+        GSNRecipientSignatureMock.new(
           ZERO_ADDRESS
         ),
-        'GSNBouncerSignature: trusted signer is the zero address'
+        'GSNRecipientSignature: trusted signer is the zero address'
       );
     });
   });
@@ -66,7 +66,7 @@ contract('GSNBouncerSignature', function ([_, signer, other]) {
 
       const { tx } = await this.recipient.mockFunction({ value: 0, useGSN: true, approveFunction });
 
-      await expectEvent.inTransaction(tx, GSNBouncerSignatureMock, 'MockFunctionCalled');
+      await expectEvent.inTransaction(tx, GSNRecipientSignatureMock, 'MockFunctionCalled');
     });
 
     it('rejects relay requests where all parameters are signed by an invalid signer', async function () {