From fabb7f0933982c8b973201198c51b883fb55225d Mon Sep 17 00:00:00 2001 From: Alex Forshtat Date: Thu, 28 May 2020 00:55:19 +0300 Subject: [PATCH] Versioning contracts (#383) Create 'IPeanalizer' interface & add 'versionX' fields to all GSN interfaces --- .idea/dictionaries/alexf.xml | 9 ++++++ contracts/BaseRelayRecipient.sol | 2 +- contracts/Penalizer.sol | 24 +++++++------- contracts/RelayHub.sol | 11 ++----- contracts/StakeManager.sol | 2 ++ contracts/TrustedBatchForwarder.sol | 6 ++++ contracts/TrustedForwarder.sol | 4 +++ contracts/interfaces/IPaymaster.sol | 1 + contracts/interfaces/IPenalizer.sol | 32 +++++++++++++++++++ contracts/interfaces/IRelayHub.sol | 2 +- contracts/interfaces/IRelayRecipient.sol | 2 ++ contracts/interfaces/IStakeManager.sol | 2 ++ contracts/interfaces/ITrustedForwarder.sol | 2 ++ contracts/paymaster/TokenPaymaster.sol | 1 + contracts/test/PayableWithEmit.sol | 2 ++ .../test/TestPaymasterEverythingAccepted.sol | 4 +++ .../test/TestPaymasterVariableGasLimits.sol | 2 ++ contracts/test/TestProxy.sol | 2 ++ contracts/test/TestRecipient.sol | 2 ++ src/cli/CommandsLogic.ts | 2 +- src/common/VersionsManager.ts | 6 ++++ src/relayserver/RelayServer.ts | 7 ++-- test/RelayHub.test.ts | 4 +-- test/RelayHubGasCalculations.test.ts | 4 +-- 24 files changed, 105 insertions(+), 30 deletions(-) create mode 100644 contracts/interfaces/IPenalizer.sol create mode 100644 src/common/VersionsManager.ts diff --git a/.idea/dictionaries/alexf.xml b/.idea/dictionaries/alexf.xml index 6c648fd95..e37dc56c5 100644 --- a/.idea/dictionaries/alexf.xml +++ b/.idea/dictionaries/alexf.xml @@ -17,7 +17,13 @@ gtransaction gtxdatanonzero ierc + iforwarder interactor + ipaymaster + ipenalizer + irelayhub + irelayrecipient + istakemanager jsonrpc keccak kovan @@ -28,6 +34,7 @@ nonces offchain openeth + opengsn openzeppelin ownable penalizable @@ -43,7 +50,9 @@ ropsten solc solhint + stakemanager subprovider + testproxy typechain typehash unauthorize diff --git a/contracts/BaseRelayRecipient.sol b/contracts/BaseRelayRecipient.sol index a0d898879..b2fe12728 100644 --- a/contracts/BaseRelayRecipient.sol +++ b/contracts/BaseRelayRecipient.sol @@ -9,7 +9,7 @@ import "./interfaces/IRelayRecipient.sol"; * A base contract to be inherited by any contract that want to receive relayed transactions * A subclass must use "_msgSender()" instead of "msg.sender" */ -contract BaseRelayRecipient is IRelayRecipient { +abstract contract BaseRelayRecipient is IRelayRecipient { /// the TrustedForwarder singleton we accept calls from. // we trust it to verify the caller's signature, and pass the caller's address as last 20 bytes diff --git a/contracts/Penalizer.sol b/contracts/Penalizer.sol index 84fe7ce29..78ef24785 100644 --- a/contracts/Penalizer.sol +++ b/contracts/Penalizer.sol @@ -7,18 +7,13 @@ import "@openzeppelin/contracts/cryptography/ECDSA.sol"; import "./utils/RLPReader.sol"; import "./utils/GsnUtils.sol"; import "./interfaces/IRelayHub.sol"; +import "./interfaces/IPenalizer.sol"; -contract Penalizer { - using ECDSA for bytes32; +contract Penalizer is IPenalizer{ - struct Transaction { - uint256 nonce; - uint256 gasPrice; - uint256 gasLimit; - address to; - uint256 value; - bytes data; - } + string public override versionPenalizer = "2.0.0-alpha.1+opengsn.penalizer.ipenalizer"; + + using ECDSA for bytes32; function decodeTransaction(bytes memory rawTransaction) private pure returns (Transaction memory transaction) { (transaction.nonce, @@ -37,7 +32,9 @@ contract Penalizer { bytes memory unsignedTx2, bytes memory signature2, IRelayHub hub - ) public + ) + public + override { // Can be called by anyone. // If a relay attacked the system by signing multiple transactions with the same nonce @@ -79,7 +76,10 @@ contract Penalizer { bytes memory unsignedTx, bytes memory signature, IRelayHub hub - ) public { + ) + public + override + { Transaction memory decodedTx = decodeTransaction(unsignedTx); if (decodedTx.to == address(hub)) { bytes4 selector = GsnUtils.getMethodSig(decodedTx.data); diff --git a/contracts/RelayHub.sol b/contracts/RelayHub.sol index 0b1a7bf50..566641e42 100644 --- a/contracts/RelayHub.sol +++ b/contracts/RelayHub.sol @@ -21,7 +21,7 @@ import "./Penalizer.sol"; contract RelayHub is IRelayHub { - string constant public COMMIT_ID = "$Id$"; + string public override versionHub = "2.0.0-alpha.1+opengsn.hub.irelayhub"; // Minimum stake a relay can have. An attack to the network will never cost less than half this value. uint256 constant private MINIMUM_STAKE = 1 ether; @@ -42,7 +42,7 @@ contract RelayHub is IRelayHub { */ // Gas cost of all relayCall() instructions after actual 'calculateCharge()' - uint256 constant private GAS_OVERHEAD = 36834; + uint256 constant private GAS_OVERHEAD = 36800; function getHubOverhead() external override view returns (uint256) { return GAS_OVERHEAD; @@ -63,13 +63,6 @@ contract RelayHub is IRelayHub { mapping(address => uint256) private balances; - string public version = "1.0.0"; - // TODO: remove with 0.6 solc - function getVersion() external override view returns (string memory) { - return version; - } - - EIP712Sig public eip712sig; StakeManager public stakeManager; Penalizer public penalizer; diff --git a/contracts/StakeManager.sol b/contracts/StakeManager.sol index d8b4117e1..bd5313502 100644 --- a/contracts/StakeManager.sol +++ b/contracts/StakeManager.sol @@ -8,6 +8,8 @@ import "./interfaces/IStakeManager.sol"; contract StakeManager is IStakeManager { + string public override versionSM = "2.0.0-alpha.1+opengsn.stakemanager.istakemanager"; + /// maps relay managers to their stakes mapping(address => StakeInfo) public stakes; function getStakeInfo(address relayManager) external override view returns (StakeInfo memory stakeInfo) { diff --git a/contracts/TrustedBatchForwarder.sol b/contracts/TrustedBatchForwarder.sol index 29375c242..b6e335373 100644 --- a/contracts/TrustedBatchForwarder.sol +++ b/contracts/TrustedBatchForwarder.sol @@ -12,6 +12,12 @@ import "./utils/GsnUtils.sol"; */ contract TrustedBatchForwarder is TrustedForwarder, BaseRelayRecipient { + function versionForwarder() external view override returns (string memory){ + return "2.0.0-alpha.1+opengsn.batched.iforwarder"; + } + + string public override versionRecipient = "2.0.0-alpha.1+opengsn.batched.irelayrecipient"; + constructor() public { //needed for sendBatch trustedForwarder = address(this); diff --git a/contracts/TrustedForwarder.sol b/contracts/TrustedForwarder.sol index e3581dde8..4b03fe4b7 100644 --- a/contracts/TrustedForwarder.sol +++ b/contracts/TrustedForwarder.sol @@ -9,6 +9,10 @@ import "./interfaces/ITrustedForwarder.sol"; contract TrustedForwarder is ITrustedForwarder { + function versionForwarder() external view virtual override returns (string memory){ + return "2.0.0-alpha.1+opengsn.forwarder.iforwarder"; + } + EIP712Sig private eip712sig; // Nonces of senders, used to prevent replay attacks diff --git a/contracts/interfaces/IPaymaster.sol b/contracts/interfaces/IPaymaster.sol index 1ce4980be..1629e57b5 100644 --- a/contracts/interfaces/IPaymaster.sol +++ b/contracts/interfaces/IPaymaster.sol @@ -91,4 +91,5 @@ interface IPaymaster { GSNTypes.GasData calldata gasData ) external; + function versionPaymaster() external view returns (string memory); } diff --git a/contracts/interfaces/IPenalizer.sol b/contracts/interfaces/IPenalizer.sol new file mode 100644 index 000000000..52cf38077 --- /dev/null +++ b/contracts/interfaces/IPenalizer.sol @@ -0,0 +1,32 @@ +// SPDX-License-Identifier:MIT +pragma solidity ^0.6.2; + +import "./IRelayHub.sol"; + +interface IPenalizer { + + struct Transaction { + uint256 nonce; + uint256 gasPrice; + uint256 gasLimit; + address to; + uint256 value; + bytes data; + } + + function penalizeRepeatedNonce( + bytes calldata unsignedTx1, + bytes calldata signature1, + bytes calldata unsignedTx2, + bytes calldata signature2, + IRelayHub hub + ) external; + + function penalizeIllegalTransaction( + bytes calldata unsignedTx, + bytes calldata signature, + IRelayHub hub + ) external; + + function versionPenalizer() external view returns (string memory); +} diff --git a/contracts/interfaces/IRelayHub.sol b/contracts/interfaces/IRelayHub.sol index 1eda52a9a..d751d6384 100644 --- a/contracts/interfaces/IRelayHub.sol +++ b/contracts/interfaces/IRelayHub.sol @@ -145,7 +145,7 @@ interface IRelayHub { /// charged for 1.4 times the spent amount. function calculateCharge(uint256 gasUsed, GSNTypes.GasData calldata gasData) external view returns (uint256); - function getVersion() external view returns (string memory); + function versionHub() external view returns (string memory); } diff --git a/contracts/interfaces/IRelayRecipient.sol b/contracts/interfaces/IRelayRecipient.sol index 000172b4c..00cc8923c 100644 --- a/contracts/interfaces/IRelayRecipient.sol +++ b/contracts/interfaces/IRelayRecipient.sol @@ -22,4 +22,6 @@ abstract contract IRelayRecipient { * should be used in the contract anywhere instead of msg.sender */ function _msgSender() internal virtual view returns (address payable); + + function versionRecipient() external virtual view returns (string memory); } diff --git a/contracts/interfaces/IStakeManager.sol b/contracts/interfaces/IStakeManager.sol index d6ee30623..b1fd62781 100644 --- a/contracts/interfaces/IStakeManager.sol +++ b/contracts/interfaces/IStakeManager.sol @@ -88,4 +88,6 @@ interface IStakeManager { function penalizeRelayManager(address relayManager, address payable beneficiary, uint256 amount) external; function getStakeInfo(address relayManager) external view returns (StakeInfo memory stakeInfo); + + function versionSM() external view returns (string memory); } diff --git a/contracts/interfaces/ITrustedForwarder.sol b/contracts/interfaces/ITrustedForwarder.sol index 411d10ef7..f227ebd67 100644 --- a/contracts/interfaces/ITrustedForwarder.sol +++ b/contracts/interfaces/ITrustedForwarder.sol @@ -14,4 +14,6 @@ interface ITrustedForwarder { function verifyAndCall(GSNTypes.RelayRequest calldata req, bytes calldata sig) external; function getNonce(address from) external view returns (uint256); + + function versionForwarder() external view returns (string memory); } diff --git a/contracts/paymaster/TokenPaymaster.sol b/contracts/paymaster/TokenPaymaster.sol index 48a89cd86..6cf41b782 100644 --- a/contracts/paymaster/TokenPaymaster.sol +++ b/contracts/paymaster/TokenPaymaster.sol @@ -18,6 +18,7 @@ import "./IUniswap.sol"; * - postRelayedCall - refund the caller for the unused gas */ contract TokenPaymaster is BasePaymaster { + string public override versionPaymaster = "2.0.0-alpha.1+opengsn.token.ipaymaster"; IUniswap public uniswap; IERC20 public token; diff --git a/contracts/test/PayableWithEmit.sol b/contracts/test/PayableWithEmit.sol index 9b4515bfc..4848aa1fe 100644 --- a/contracts/test/PayableWithEmit.sol +++ b/contracts/test/PayableWithEmit.sol @@ -9,6 +9,8 @@ import "../0x/LibBytesV06.sol"; // it should work) contract PayableWithEmit is BaseRelayRecipient { + string public override versionRecipient = "2.0.0-alpha.1+opengsn.payablewithemit.irelayrecipient"; + event Received(address sender, uint value, uint gasleft); receive () external payable { diff --git a/contracts/test/TestPaymasterEverythingAccepted.sol b/contracts/test/TestPaymasterEverythingAccepted.sol index 9f4141abd..6e6b1f272 100644 --- a/contracts/test/TestPaymasterEverythingAccepted.sol +++ b/contracts/test/TestPaymasterEverythingAccepted.sol @@ -7,6 +7,10 @@ import "../BasePaymaster.sol"; contract TestPaymasterEverythingAccepted is BasePaymaster { + function versionPaymaster() external view override virtual returns (string memory){ + return "2.0.0-alpha.1+opengsn.test_pea.ipaymaster"; + } + event SampleRecipientPreCall(); event SampleRecipientPostCall(bool success, uint actualCharge, bytes32 preRetVal); diff --git a/contracts/test/TestPaymasterVariableGasLimits.sol b/contracts/test/TestPaymasterVariableGasLimits.sol index 1f977ca1c..754ad0fbd 100644 --- a/contracts/test/TestPaymasterVariableGasLimits.sol +++ b/contracts/test/TestPaymasterVariableGasLimits.sol @@ -6,6 +6,8 @@ import "./TestPaymasterEverythingAccepted.sol"; contract TestPaymasterVariableGasLimits is TestPaymasterEverythingAccepted { + string public override versionPaymaster = "2.0.0-alpha.1+opengsn.test-vgl.ipaymaster"; + event SampleRecipientPreCallWithValues( uint256 gasleft, uint256 arcGasleft, diff --git a/contracts/test/TestProxy.sol b/contracts/test/TestProxy.sol index afe0f0d56..cbc0990f0 100644 --- a/contracts/test/TestProxy.sol +++ b/contracts/test/TestProxy.sol @@ -8,6 +8,8 @@ import "../BaseRelayRecipient.sol"; contract TestProxy is BaseRelayRecipient, Ownable { + string public override versionRecipient = "2.0.0-alpha.1+opengsn.testproxy.irelayrecipient"; + constructor(address forwarder) public { trustedForwarder = forwarder; } diff --git a/contracts/test/TestRecipient.sol b/contracts/test/TestRecipient.sol index 40db054dc..3431f14e9 100644 --- a/contracts/test/TestRecipient.sol +++ b/contracts/test/TestRecipient.sol @@ -10,6 +10,8 @@ import "../interfaces/IKnowForwarderAddress.sol"; contract TestRecipient is BaseRelayRecipient, IKnowForwarderAddress { + string public override versionRecipient = "2.0.0-alpha.1+opengsn.test.irelayrecipient"; + constructor() public { //should be a singleton, since Paymaster should (eventually) trust it. trustedForwarder = address(new TrustedForwarder()); diff --git a/src/cli/CommandsLogic.ts b/src/cli/CommandsLogic.ts index eff01ad5e..991ca678c 100644 --- a/src/cli/CommandsLogic.ts +++ b/src/cli/CommandsLogic.ts @@ -206,7 +206,7 @@ export default class CommandsLogic { async deployGsnContracts (from: Address, gasPrice?: string, paymaster?: any): Promise { const options = { from, - gas: 1e6, + gas: 3e6, gasPrice: gasPrice ?? (1e9).toString() } diff --git a/src/common/VersionsManager.ts b/src/common/VersionsManager.ts new file mode 100644 index 000000000..27541ea15 --- /dev/null +++ b/src/common/VersionsManager.ts @@ -0,0 +1,6 @@ +// TODO: this is just a stub of what would be fully-featured version manager +export default class VersionsManager { + isHubVersionSupported (version: string): boolean { + return version === '2.0.0-alpha.1+opengsn.hub.irelayhub' + } +} diff --git a/src/relayserver/RelayServer.ts b/src/relayserver/RelayServer.ts index d2af42326..a40784555 100644 --- a/src/relayserver/RelayServer.ts +++ b/src/relayserver/RelayServer.ts @@ -25,6 +25,7 @@ import { TransactionReceipt } from 'web3-core' import { toBN, toHex } from 'web3-utils' import { configureGSN } from '../relayclient/GSNConfigurator' import { defaultEnvironment } from '../relayclient/types/Environments' +import VersionsManager from '../common/VersionsManager' abiDecoder.addABI(RelayHubABI) abiDecoder.addABI(PayMasterABI) @@ -135,6 +136,7 @@ export class RelayServer extends EventEmitter { private readonly web3provider: provider readonly keyManager: KeyManager private readonly contractInteractor: ContractInteractor + private readonly versionManager: VersionsManager readonly hubAddress: PrefixedHexString readonly baseRelayFee: number readonly pctRelayFee: number @@ -147,6 +149,7 @@ export class RelayServer extends EventEmitter { constructor (params: RelayServerParams) { super() + this.versionManager = new VersionsManager() this.txStoreManager = params.txStoreManager this.web3provider = params.web3provider this.keyManager = params.keyManager @@ -407,8 +410,8 @@ export class RelayServer extends EventEmitter { } else { debug('code length', code.length) } - const version = await this.relayHubContract.getVersion().catch(_ => 'no getVersion() method') - if (version !== '1.0.0') { + const version = await this.relayHubContract.versionHub().catch(_ => 'no getVersion() method') + if (!this.versionManager.isHubVersionSupported(version)) { this.fatal(`Not a valid RelayHub at ${relayHubAddress}: version: ${version}`) } const stakeManagerAddress = await this.relayHubContract.getStakeManager() diff --git a/test/RelayHub.test.ts b/test/RelayHub.test.ts index 65d67a01d..25536fb73 100644 --- a/test/RelayHub.test.ts +++ b/test/RelayHub.test.ts @@ -62,8 +62,8 @@ contract('RelayHub', function ([_, relayOwner, relayManager, relayWorker, sender }) it('should retrieve version number', async function () { - const version = await relayHubInstance.version() - assert.equal(version, '1.0.0') + const version = await relayHubInstance.versionHub() + assert.match(version, /2\.\d*\.\d*-?.*\+opengsn\.hub\.irelayhub/) }) describe.skip('balances', function () { async function testDeposit (sender: string, paymaster: string, amount: BN): Promise { diff --git a/test/RelayHubGasCalculations.test.ts b/test/RelayHubGasCalculations.test.ts index b1da02ffe..00970fe6b 100644 --- a/test/RelayHubGasCalculations.test.ts +++ b/test/RelayHubGasCalculations.test.ts @@ -47,8 +47,8 @@ contract('RelayHub gas calculations', function ([_, relayOwner, relayWorker, rel const gasLimit = new BN('1000000') const senderNonce = new BN('0') const magicNumbers = { - arc: 844, - pre: 1508, + arc: 867, + pre: 1486, post: 1583 }