get rid of sep. TxDelegator, fx in AugmintToken + ECRecovery lib

Augmint · May 8, 2018 · eaf2554 · eaf2554
1 parent 168e75b
commit eaf2554
Show file tree

Hide file tree

Showing 10 changed files with 43 additions and 96 deletions.
diff --git a/contracts/TokenAEur.sol b/contracts/TokenAEur.sol
@@ -5,8 +5,8 @@ import "./generic/AugmintToken.sol";
 
 
 contract TokenAEur is AugmintToken {
-    constructor(address _txDelegator, TransferFeeInterface _feeAccount)
-    public AugmintToken("Augmint Crypto Euro", "AEUR", "EUR", 2, _txDelegator, _feeAccount)
+    constructor(TransferFeeInterface _feeAccount)
+    public AugmintToken("Augmint Crypto Euro", "AEUR", "EUR", 2, _feeAccount)
     {} // solhint-disable-line no-empty-blocks
 
 }
diff --git a/contracts/TxDelegator.sol b/contracts/TxDelegator.sol
diff --git a/contracts/generic/AugmintToken.sol b/contracts/generic/AugmintToken.sol
@@ -11,15 +11,15 @@
 */
 pragma solidity ^0.4.23;
 import "../interfaces/AugmintTokenInterface.sol";
+import "./ECRecovery.sol";
 import "../interfaces/TransferFeeInterface.sol";
 
 
 contract AugmintToken is AugmintTokenInterface {
 
     event FeeAccountChanged(TransferFeeInterface newFeeAccount);
 
-    constructor(string _name, string _symbol, bytes32 _peggedSymbol, uint8 _decimals, address _txDelegator,
-        TransferFeeInterface _feeAccount)
+    constructor(string _name, string _symbol, bytes32 _peggedSymbol, uint8 _decimals, TransferFeeInterface _feeAccount)
     public {
         require(_feeAccount != address(0), "feeAccount must be set");
         require(bytes(_name).length > 0, "name must be set");
@@ -31,21 +31,36 @@ contract AugmintToken is AugmintTokenInterface {
         decimals = _decimals;
 
         feeAccount = _feeAccount;
-        txDelegator = _txDelegator;
 
     }
     function transfer(address to, uint256 amount) external returns (bool) {
         _transfer(msg.sender, to, amount, "");
         return true;
     }
 
-    /* Transfers based on an offline signed transfer instruction.
-        It trusts txDelegator checked the signature of from account and the executorFee */
-    function delegatedTransferExecution(address from, address to, uint amount, string narrative, uint executorFee)
+    /* Transfers based on an offline signed transfer instruction. */
+    function delegatedTransfer(address from, address to, uint amount, string narrative,
+                                     uint minGasPrice,  /* client provided gasPrice on which she expects tx to be exec. */
+                                     uint maxExecutorFee, /* client provided max fee for executing the tx */
+                                     bytes32 nonce, /* random nonce generated by client */
+                                     /* ^^^^ end of signed data ^^^^ */
+                                     bytes signature,
+                                     uint requestedExecutorFee /* the executor can decide to request lower fee */
+                                     )
     external returns(bool) {
-        require(msg.sender == txDelegator);
 
-        _transfer(from, msg.sender, executorFee, "Delegated execution fee");
+        require(!noncesUsed[nonce], "nonce already used");
+        require(tx.gasprice >= minGasPrice, "tx.gasprice must be >= minGasPrice");
+        require(requestedExecutorFee <= maxExecutorFee, "requestedExecutorFee must be <= maxExecutorFee");
+        noncesUsed[nonce] = true;
+
+        bytes32 txHash = keccak256(this, from, to, amount, narrative, minGasPrice, maxExecutorFee, nonce);
+
+        address recovered = ECRecovery.recover(ECRecovery.toEthSignedMessageHash(txHash), signature);
+
+        require(recovered == from, "invalid signature");
+
+        _transfer(from, msg.sender, requestedExecutorFee, "Delegated execution fee");
         _transfer(from, to, amount, narrative);
 
         return true;

diff --git a/contracts/interfaces/AugmintTokenInterface.sol b/contracts/interfaces/AugmintTokenInterface.sol
@@ -26,7 +26,7 @@ contract AugmintTokenInterface is Restricted, ERC20Interface {
     mapping(address => mapping (address => uint256)) public allowed; // allowances added with approve()
 
     TransferFeeInterface public feeAccount;
-    address public txDelegator;
+    mapping(bytes32 => bool) public noncesUsed; // record nonces used by delegatedTransfer
 
     event TransferFeesChanged(uint transferFeePt, uint transferFeeMin, uint transferFeeMax);
     event Transfer(address indexed from, address indexed to, uint amount);
@@ -39,8 +39,14 @@ contract AugmintTokenInterface is Restricted, ERC20Interface {
     function transferFrom(address from, address to, uint value) external returns (bool);
     function approve(address spender, uint value) external returns (bool);
 
-    function delegatedTransferExecution(address from, address to, uint amount, string narrative, uint executorFee)
-    external returns(bool);
+    function delegatedTransfer(address from, address to, uint amount, string narrative,
+                                     uint minGasPrice,  /* client provided gasPrice on which she expects tx to be exec. */
+                                     uint maxExecutorFee, /* client provided max fee for executing the tx */
+                                     bytes32 nonce, /* random nonce generated by client */
+                                     /* ^^^^ end of signed data ^^^^ */
+                                     bytes signature,
+                                     uint requestedExecutorFee /* the executor can decide to request lower fee */
+                                ) external returns(bool);
 
     function increaseApproval(address spender, uint addedValue) external returns (bool);
     function decreaseApproval(address spender, uint subtractedValue) external returns (bool);

diff --git a/migrations/5_deploy_TxDelegator.js b/migrations/5_deploy_TxDelegator.js
diff --git a/migrations/7_deploy_TokenAEur.js b/migrations/7_deploy_TokenAEur.js
@@ -1,7 +1,6 @@
 const TokenAEur = artifacts.require("./TokenAEur.sol");
 const FeeAccount = artifacts.require("./FeeAccount.sol");
-const TxDelegator = artifacts.require("./TxDelegator.sol");
 
 module.exports = function(deployer) {
-    deployer.deploy(TokenAEur, TxDelegator.address, FeeAccount.address);
+    deployer.deploy(TokenAEur, FeeAccount.address);
 };
diff --git a/migrations/98_add_legacyTokens.js b/migrations/98_add_legacyTokens.js
@@ -1,13 +1,12 @@
 const FeeAccount = artifacts.require("./FeeAccount.sol");
 const TokenAEur = artifacts.require("./TokenAEur.sol");
 const MonetarySupervisor = artifacts.require("./MonetarySupervisor.sol");
-const TxDelegator = artifacts.require("./TxDelegator.sol");
 
 module.exports = async function(deployer, network, accounts) {
     deployer.then(async () => {
         const monetarySupervisor = MonetarySupervisor.at(MonetarySupervisor.address);
         const feeAccount = FeeAccount.at(FeeAccount.address);
-        const oldToken = await TokenAEur.new(TxDelegator.address, FeeAccount.address);
+        const oldToken = await TokenAEur.new(FeeAccount.address);
 
         await Promise.all([
             oldToken.grantPermission(accounts[0], "MonetarySupervisorContract"), // "hack" for test to issue

diff --git a/test/delegatedTransfer.js b/test/delegatedTransfer.js
@@ -1,24 +1,21 @@
 const tokenTestHelpers = require("./helpers/tokenTestHelpers.js");
 const testHelpers = require("./helpers/testHelpers.js");
-const TxDelegator = artifacts.require("TxDelegator.sol");
 const TokenAEur = artifacts.require("TokenAEur.sol");
 
 let txDelegator;
 let tokenAEur;
 let from;
 
-contract("TxDelegator", accounts => {
+contract("Delegated Transfers", accounts => {
     before(async () => {
         from = accounts[1];
-        tokenAEur = tokenTestHelpers.augmintToken;
-        txDelegator = new global.web3v1.eth.Contract(TxDelegator.abi, TxDelegator.address);
+        tokenAEur = new global.web3v1.eth.Contract(TokenAEur.abi, TokenAEur.address);
         // txDelegator = TxDelegator.at(TxDelegator.address);
         await tokenTestHelpers.issueToReserve(1000000000);
         await tokenTestHelpers.withdrawFromReserve(from, 1000000000);
     });
 
-    it("should delegatedTransfer function signed", async function() {
-
+    it("should transfer when delegatedTransfer is signed", async function() {
         // params sent and signed by client
         const to = accounts[2];
         const amount = 1000;
@@ -37,8 +34,7 @@ contract("TxDelegator", accounts => {
         if (narrative === "") {
             // workaround b/c solidity keccak256 results different txHAsh with empty string than web3
             txHash = global.web3v1.utils.soliditySha3(
-                TxDelegator.address,
-                tokenAEur.address,
+                TokenAEur.address,
                 from,
                 to,
                 amount,
@@ -48,8 +44,7 @@ contract("TxDelegator", accounts => {
             );
         } else {
             txHash = global.web3v1.utils.soliditySha3(
-                TxDelegator.address,
-                tokenAEur.address,
+                TokenAEur.address,
                 from,
                 to,
                 amount,
@@ -62,9 +57,8 @@ contract("TxDelegator", accounts => {
 
         const signature = await global.web3v1.eth.sign(txHash, from);
 
-        const tx = await txDelegator.methods
+        const tx = await tokenAEur.methods
             .delegatedTransfer(
-                tokenAEur.address,
                 from,
                 to,
                 amount,

diff --git a/test/token.js b/test/token.js
@@ -17,7 +17,6 @@ contract("AugmintToken tests", accounts => {
                 "AEUR", // symbol
                 "EUR", // peggedSymbol
                 2, // decimals
-                TxDelegator.address,
                 0 // feeaccount
             )
         );
@@ -30,7 +29,6 @@ contract("AugmintToken tests", accounts => {
                 "AEUR", // symbol
                 "EUR", // peggedSymbol
                 2, // decimals
-                TxDelegator.address,
                 tokenTestHelpers.feeAccount.address
             )
         );
@@ -43,7 +41,6 @@ contract("AugmintToken tests", accounts => {
                 "", // symbol
                 "EUR", // peggedSymbol
                 2, // decimals
-                TxDelegator.address,
                 tokenTestHelpers.feeAccount.address
             )
         );

diff --git a/test/tokenConversion.js b/test/tokenConversion.js
@@ -2,7 +2,6 @@ const tokenTestHelpers = require("./helpers/tokenTestHelpers.js");
 const testHelpers = require("./helpers/testHelpers.js");
 const MonetarySupervisor = artifacts.require("./MonetarySupervisor.sol");
 const AugmintToken = artifacts.require("./TokenAEur.sol");
-const TxDelegator = artifacts.require("TxDelegator.sol");
 
 let augmintToken = null;
 let monetarySupervisor = null;
@@ -15,7 +14,7 @@ contract("token conversion tests", accounts => {
         monetarySupervisor = tokenTestHelpers.monetarySupervisor;
 
         [newToken] = await Promise.all([
-            AugmintToken.new(TxDelegator.address, tokenTestHelpers.feeAccount.address),
+            AugmintToken.new(tokenTestHelpers.feeAccount.address),
             tokenTestHelpers.issueToReserve(10000000)
         ]);