diff --git a/contracts/TokenAEur.sol b/contracts/TokenAEur.sol index 409baa70..c790cb8c 100644 --- a/contracts/TokenAEur.sol +++ b/contracts/TokenAEur.sol @@ -5,8 +5,8 @@ import "./generic/AugmintToken.sol"; contract TokenAEur is AugmintToken { - constructor(TransferFeeInterface _feeAccount) - public AugmintToken("Augmint Crypto Euro", "AEUR", "EUR", 2, _feeAccount) + constructor(address _txDelegator, TransferFeeInterface _feeAccount) + public AugmintToken("Augmint Crypto Euro", "AEUR", "EUR", 2, _txDelegator, _feeAccount) {} // solhint-disable-line no-empty-blocks } diff --git a/contracts/TxDelegator.sol b/contracts/TxDelegator.sol new file mode 100644 index 00000000..ed4e4130 --- /dev/null +++ b/contracts/TxDelegator.sol @@ -0,0 +1,83 @@ +/* + WIP, first pass proof of concept. Implementation will change a lot. + + TODO: + - No point to have this as a separate contract unless: + - we make it generic, i.e. any arbitary calldata can be signed + - make it changeable on AugmintToken + - Maybe reorg some parts to interfaces/abstract contract/lib and use it directly on AugmintToken? + - Double check if we don't need to add network id to signed data: + In addition to being implicitly stored in the augmintTokenaddress (ie. deployment address is unique), + the chain id is also explicitly stored in the v parameter (chain_id = (v - 35) / 2). + - test signing with trezor signature: + https://github.com/0xProject/0x-monorepo/blob/095388ffe05ca51e92db87ba81d6e4f29b1ab087/packages/contracts/src/contracts/current/protocol/Exchange/MixinSignatureValidator.sol + + - EIP712 & ERC191 signature schemes? +*/ +pragma solidity 0.4.23; + +import "./generic/SafeMath.sol"; +import "./interfaces/AugmintTokenInterface.sol"; + +contract TxDelegator { + using SafeMath for uint256; + mapping(bytes32 => bool) public noncesUsed; + + function delegatedTransfer(AugmintTokenInterface augmintToken, 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 { + 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, augmintToken, from, to, amount, narrative, minGasPrice, maxExecutorFee, nonce); + txHash = keccak256("\x19Ethereum Signed Message:\n32", txHash); + + address recovered = recover(txHash, signature); + + require(recovered == from, "invalid signature"); + + require(augmintToken.delegatedTransferExecution(from, to, amount, narrative, requestedExecutorFee), + "delegatedTransferExecution failed"); + + } + + /* from: https://github.com/OpenZeppelin/openzeppelin-solidity/blob/master/contracts/ECRecovery.sol */ + function recover(bytes32 hash, bytes sig) internal pure returns (address) { + bytes32 r; + bytes32 s; + uint8 v; + + //Check the signature length + if (sig.length != 65) { + return (address(0)); + } + + // Divide the signature in r, s and v variables + assembly { // solhint-disable-line no-inline-assembly + r := mload(add(sig, 32)) + s := mload(add(sig, 64)) + v := byte(0, mload(add(sig, 96))) + } + + // Version of signature should be 27 or 28, but 0 and 1 are also possible versions + if (v < 27) { + v += 27; + } + + // If the version is correct return the signer address + if (v != 27 && v != 28) { + return (address(0)); + } else { + return ecrecover(hash, v, r, s); + } + } + +} diff --git a/contracts/generic/AugmintToken.sol b/contracts/generic/AugmintToken.sol index d5e6d7ef..3ef75a26 100644 --- a/contracts/generic/AugmintToken.sol +++ b/contracts/generic/AugmintToken.sol @@ -4,6 +4,8 @@ * Issues/burns tokens TODO: + - reconsider delegatedTransfer and how to structure it + - shall we allow change of txDelegator? - consider generic bytes arg instead of uint for transferAndNotify - consider separate transfer fee params and calculation to separate contract (to feeAccount?) */ @@ -16,7 +18,8 @@ contract AugmintToken is AugmintTokenInterface { event FeeAccountChanged(TransferFeeInterface newFeeAccount); - constructor(string _name, string _symbol, bytes32 _peggedSymbol, uint8 _decimals, TransferFeeInterface _feeAccount) + constructor(string _name, string _symbol, bytes32 _peggedSymbol, uint8 _decimals, address _txDelegator, + TransferFeeInterface _feeAccount) public { require(_feeAccount != address(0), "feeAccount must be set"); require(bytes(_name).length > 0, "name must be set"); @@ -28,14 +31,26 @@ 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) + external returns(bool) { + require(msg.sender == txDelegator); + + _transfer(from, msg.sender, executorFee, "Delegated execution fee"); + _transfer(from, to, amount, narrative); + + return true; + } + function approve(address _spender, uint256 amount) external returns (bool) { require(_spender != 0x0, "spender must be set"); allowed[msg.sender][_spender] = amount; diff --git a/contracts/interfaces/AugmintTokenInterface.sol b/contracts/interfaces/AugmintTokenInterface.sol index 9adb69fb..7bea7f47 100644 --- a/contracts/interfaces/AugmintTokenInterface.sol +++ b/contracts/interfaces/AugmintTokenInterface.sol @@ -26,10 +26,11 @@ contract AugmintTokenInterface is Restricted, ERC20Interface { mapping(address => mapping (address => uint256)) public allowed; // allowances added with approve() TransferFeeInterface public feeAccount; + address public txDelegator; event TransferFeesChanged(uint transferFeePt, uint transferFeeMin, uint transferFeeMax); event Transfer(address indexed from, address indexed to, uint amount); - event AugmintTransfer(address indexed from, address indexed to, uint amount, string narrative, uint fee); + event AugmintTransfer(address from, address to, uint amount, string narrative, uint fee); event TokenIssued(uint amount); event TokenBurned(uint amount); event Approval(address indexed _owner, address indexed _spender, uint256 _value); @@ -37,6 +38,10 @@ contract AugmintTokenInterface is Restricted, ERC20Interface { function transfer(address to, uint value) external returns (bool); // solhint-disable-line no-simple-event-func-name 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 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 new file mode 100644 index 00000000..974ff0d7 --- /dev/null +++ b/migrations/5_deploy_TxDelegator.js @@ -0,0 +1,10 @@ +const TxDelegator = artifacts.require("./TxDelegator.sol"); +const FeeAccount = artifacts.require("./FeeAccount.sol"); + +module.exports = function(deployer) { + deployer.deploy(TxDelegator); + deployer.then(async () => { + const feeAccount = FeeAccount.at(FeeAccount.address); + await feeAccount.grantPermission(TxDelegator.address, "NoFeeTransferContracts"); + }); +}; diff --git a/migrations/6_deploy_TokenAEur.js b/migrations/6_deploy_TokenAEur.js index 87b92baf..bf01cb16 100644 --- a/migrations/6_deploy_TokenAEur.js +++ b/migrations/6_deploy_TokenAEur.js @@ -1,9 +1,10 @@ const SafeMath = artifacts.require("./SafeMath.sol"); const TokenAEur = artifacts.require("./TokenAEur.sol"); const FeeAccount = artifacts.require("./FeeAccount.sol"); +const TxDelegator = artifacts.require("./TxDelegator.sol"); module.exports = function(deployer) { deployer.link(SafeMath, TokenAEur); - deployer.deploy(TokenAEur, FeeAccount.address); + deployer.deploy(TokenAEur, TxDelegator.address, FeeAccount.address); }; diff --git a/migrations/98_add_legacyTokens.js b/migrations/98_add_legacyTokens.js index 5047dabd..fcc0f6c8 100644 --- a/migrations/98_add_legacyTokens.js +++ b/migrations/98_add_legacyTokens.js @@ -1,12 +1,13 @@ 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(FeeAccount.address); + const oldToken = await TokenAEur.new(TxDelegator.address, 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 new file mode 100644 index 00000000..aef5125c --- /dev/null +++ b/test/delegatedTransfer.js @@ -0,0 +1,89 @@ +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 => { + before(async () => { + from = accounts[1]; + tokenAEur = tokenTestHelpers.augmintToken; + txDelegator = new global.web3v1.eth.Contract(TxDelegator.abi, TxDelegator.address); + // txDelegator = TxDelegator.at(TxDelegator.address); + }); + + it("should delegatedTransfer function signed", async function() { + await tokenTestHelpers.issueToReserve(1000000000); + await tokenTestHelpers.withdrawFromReserve(from, 500000000); + + // params sent and signed by client + const to = accounts[2]; + const amount = 1000; + const narrative = "here we go"; + const minGasPrice = 1; + const maxExecutorFee = 200; + const nonce = "0x0000000000000000000000000000000000000000000000000000000000000001"; // to be a random hash with proper entrophy + + // executor params + const txSender = accounts[3]; + const actualGasPrice = minGasPrice; + const requestedExecutorFee = maxExecutorFee; + + let txHash; + + if (narrative === "") { + // workaround b/c solidity keccak256 results different txHAsh with empty string than web3 + txHash = global.web3v1.utils.soliditySha3( + TxDelegator.address, + tokenAEur.address, + from, + to, + amount, + minGasPrice, + maxExecutorFee, + nonce + ); + } else { + txHash = global.web3v1.utils.soliditySha3( + TxDelegator.address, + tokenAEur.address, + from, + to, + amount, + narrative, + minGasPrice, + maxExecutorFee, + nonce + ); + } + + const signature = await global.web3v1.eth.sign(txHash, from); + + const tx = await txDelegator.methods + .delegatedTransfer( + tokenAEur.address, + from, + to, + amount, + narrative, + minGasPrice, + maxExecutorFee, + nonce, + signature, + requestedExecutorFee + ) + .send({ from: txSender, gas: 1200000, gasPrice: actualGasPrice }); + testHelpers.logGasUse(this, tx, "delegatedTransfer"); + + // TODO: assert events & balances + }); + + it("should not execute with the same nonce twice"); + + it("should not execute with higher requestedExecutorFee than signed"); + + it("should not execute with lower gasPrice than signed"); +}); diff --git a/test/helpers/testHelpers.js b/test/helpers/testHelpers.js index 9293d8f7..9b4414a4 100644 --- a/test/helpers/testHelpers.js +++ b/test/helpers/testHelpers.js @@ -195,7 +195,12 @@ function revertSnapshot(snapshotId) { function logGasUse(testObj, tx, txName) { if (!gasUseLogDisabled) { - gasUseLog.push([testObj.test.parent.title, testObj.test.fullTitle(), txName || "", tx.receipt.gasUsed]); + gasUseLog.push([ + testObj.test.parent.title, + testObj.test.fullTitle(), + txName || "", + tx.receipt ? tx.receipt.gasUsed : tx.gasUsed /* web3v0 w/ receipt, v1 w/o */ + ]); } }