From 6337e3ccab6e8a6e7f8ad6978c0d376be0705162 Mon Sep 17 00:00:00 2001 From: Jeeyong Um Date: Sat, 2 Feb 2019 15:29:12 +0900 Subject: [PATCH 1/3] fix to compatible with solidity 5 --- contracts/HashedTimelock.sol | 14 ++++++-------- contracts/HashedTimelockERC20.sol | 12 +++++------- contracts/Migrations.sol | 2 +- package-lock.json | 6 +++--- package.json | 2 +- 5 files changed, 16 insertions(+), 20 deletions(-) diff --git a/contracts/HashedTimelock.sol b/contracts/HashedTimelock.sol index d13e8f1..1bd1396 100644 --- a/contracts/HashedTimelock.sol +++ b/contracts/HashedTimelock.sol @@ -1,5 +1,4 @@ -pragma solidity ^0.4.24; -pragma experimental "v0.5.0"; +pragma solidity ^0.5.0; /** * @title Hashed Timelock Contracts (HTLCs) on Ethereum ETH. @@ -20,7 +19,7 @@ pragma experimental "v0.5.0"; * back with this function. */ contract HashedTimelock { - + event LogHTLCNew( bytes32 indexed contractId, address indexed sender, @@ -33,8 +32,8 @@ contract HashedTimelock { event LogHTLCRefund(bytes32 indexed contractId); struct LockContract { - address sender; - address receiver; + address payable sender; + address payable receiver; uint amount; bytes32 hashlock; // sha-2 sha256 hash uint timelock; // UNIX timestamp seconds - locked UNTIL this time @@ -92,7 +91,7 @@ contract HashedTimelock { * @return contractId Id of the new HTLC. This is needed for subsequent * calls. */ - function newContract(address _receiver, bytes32 _hashlock, uint _timelock) + function newContract(address payable _receiver, bytes32 _hashlock, uint _timelock) external payable fundsSent @@ -187,6 +186,7 @@ contract HashedTimelock { function getContract(bytes32 _contractId) public view + contractExists(_contractId) returns ( address sender, address receiver, @@ -198,8 +198,6 @@ contract HashedTimelock { bytes32 preimage ) { - if (haveContract(_contractId) == false) - return; LockContract storage c = contracts[_contractId]; return (c.sender, c.receiver, c.amount, c.hashlock, c.timelock, c.withdrawn, c.refunded, c.preimage); diff --git a/contracts/HashedTimelockERC20.sol b/contracts/HashedTimelockERC20.sol index 64b9492..c4ce6fe 100644 --- a/contracts/HashedTimelockERC20.sol +++ b/contracts/HashedTimelockERC20.sol @@ -1,5 +1,4 @@ -pragma solidity ^0.4.24; -pragma experimental "v0.5.0"; +pragma solidity ^0.5.0; import "openzeppelin-solidity/contracts/token/ERC20/ERC20.sol"; @@ -51,7 +50,7 @@ contract HashedTimelockERC20 { modifier tokensTransferable(address _token, address _sender, uint _amount) { require(_amount > 0, "token amount must be > 0"); require( - ERC20(_token).allowance(_sender, this) >= _amount, + ERC20(_token).allowance(_sender, address(this)) >= _amount, "token allowance must be >= amount" ); _; @@ -136,9 +135,9 @@ contract HashedTimelockERC20 { revert(); // This contract becomes the temporary owner of the tokens - if (!ERC20(_tokenContract).transferFrom(msg.sender, this, _amount)) + if (!ERC20(_tokenContract).transferFrom(msg.sender, address(this), _amount)) revert(); - + contracts[contractId] = LockContract( msg.sender, _receiver, @@ -213,6 +212,7 @@ contract HashedTimelockERC20 { function getContract(bytes32 _contractId) public view + contractExists(_contractId) returns ( address sender, address receiver, @@ -225,8 +225,6 @@ contract HashedTimelockERC20 { bytes32 preimage ) { - if (haveContract(_contractId) == false) - return; LockContract storage c = contracts[_contractId]; return ( c.sender, diff --git a/contracts/Migrations.sol b/contracts/Migrations.sol index c4efb65..4ca7a41 100644 --- a/contracts/Migrations.sol +++ b/contracts/Migrations.sol @@ -1,4 +1,4 @@ -pragma solidity ^0.4.23; +pragma solidity ^0.5.0; contract Migrations { address public owner; diff --git a/package-lock.json b/package-lock.json index 8e15bcb..3d36032 100644 --- a/package-lock.json +++ b/package-lock.json @@ -783,9 +783,9 @@ "integrity": "sha1-CXtgK1NCKlIsGvuHkDGDNpQaAR0=" }, "openzeppelin-solidity": { - "version": "1.9.0", - "resolved": "https://registry.npmjs.org/openzeppelin-solidity/-/openzeppelin-solidity-1.9.0.tgz", - "integrity": "sha512-MGI8clDbjrfWUg90AM82O+CHOaabtE2u9HyaUhBMKfWdIaO1urRUIgXIrEuloLvFEBHb5rtcgTARb5DkhUB4KQ==" + "version": "2.1.2", + "resolved": "https://registry.npmjs.org/openzeppelin-solidity/-/openzeppelin-solidity-2.1.2.tgz", + "integrity": "sha512-1ggh+AZFpMAgGfgnVMQ8dwYawjD2QN4xuWkQS4FUbeUz1fnCKJpguUl2cyadyfDYjBq1XJ6MA6VkzYpTZtJMqw==" }, "os-homedir": { "version": "1.0.2", diff --git a/package.json b/package.json index 5eecc76..f667fbf 100644 --- a/package.json +++ b/package.json @@ -31,7 +31,7 @@ "babel-polyfill": "^6.26.0", "babel-preset-env": "^1.7.0", "babel-register": "^6.26.0", - "openzeppelin-solidity": "^1.9.0" + "openzeppelin-solidity": "^2.1.2" }, "devDependencies": { "bluebird": "^3.5.1" From b2323bc4faf0e126a97bd9073ca30342af09fb87 Mon Sep 17 00:00:00 2001 From: Jeeyong Um Date: Sun, 3 Feb 2019 00:03:06 +0900 Subject: [PATCH 2/3] fix getContract to return 0 when it fails to find contract ethereum/solidity#4740 --- contracts/HashedTimelock.sol | 3 ++- contracts/HashedTimelockERC20.sol | 3 ++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/contracts/HashedTimelock.sol b/contracts/HashedTimelock.sol index 1bd1396..9cdf587 100644 --- a/contracts/HashedTimelock.sol +++ b/contracts/HashedTimelock.sol @@ -186,7 +186,6 @@ contract HashedTimelock { function getContract(bytes32 _contractId) public view - contractExists(_contractId) returns ( address sender, address receiver, @@ -198,6 +197,8 @@ contract HashedTimelock { bytes32 preimage ) { + if (haveContract(_contractId) == false) + return (address(0), address(0), 0, 0, 0, false, false, 0); LockContract storage c = contracts[_contractId]; return (c.sender, c.receiver, c.amount, c.hashlock, c.timelock, c.withdrawn, c.refunded, c.preimage); diff --git a/contracts/HashedTimelockERC20.sol b/contracts/HashedTimelockERC20.sol index c4ce6fe..22a8eeb 100644 --- a/contracts/HashedTimelockERC20.sol +++ b/contracts/HashedTimelockERC20.sol @@ -212,7 +212,6 @@ contract HashedTimelockERC20 { function getContract(bytes32 _contractId) public view - contractExists(_contractId) returns ( address sender, address receiver, @@ -225,6 +224,8 @@ contract HashedTimelockERC20 { bytes32 preimage ) { + if (haveContract(_contractId) == false) + return (address(0), address(0), address(0), 0, 0, 0, false, false, 0); LockContract storage c = contracts[_contractId]; return ( c.sender, From 281d878407ef8afb6a1f294f66363dfdd7295a33 Mon Sep 17 00:00:00 2001 From: Jeeyong Um Date: Sun, 3 Feb 2019 16:03:22 +0900 Subject: [PATCH 3/3] fix tests to be compatible with solidity 5 --- test/helper/ASEANToken.sol | 12 +++++------ test/helper/assert.js | 6 +++++- test/helper/utils.js | 7 +++++-- test/htlc.js | 41 ++++++++++++++++++++------------------ test/htlcERC20.js | 26 ++++++++++++------------ 5 files changed, 51 insertions(+), 41 deletions(-) diff --git a/test/helper/ASEANToken.sol b/test/helper/ASEANToken.sol index 123bdb0..8cbb974 100644 --- a/test/helper/ASEANToken.sol +++ b/test/helper/ASEANToken.sol @@ -1,16 +1,16 @@ -pragma solidity ^0.4.23; +pragma solidity ^0.5.0; -import "openzeppelin-solidity/contracts/token/ERC20/StandardToken.sol"; +import "openzeppelin-solidity/contracts/token/ERC20/ERC20.sol"; /** * A basic token for testing the HashedTimelockERC20. */ -contract ASEANToken is StandardToken { +contract ASEANToken is ERC20 { string public constant name = "ASEAN Token"; string public constant symbol = "ASEAN"; uint8 public constant decimals = 18; - - constructor(uint _initialBalance) public { - balances[msg.sender] = _initialBalance; + + constructor(uint256 _initialBalance) public { + _mint(msg.sender, _initialBalance); } } diff --git a/test/helper/assert.js b/test/helper/assert.js index efab923..44202ba 100644 --- a/test/helper/assert.js +++ b/test/helper/assert.js @@ -1,8 +1,12 @@ if (!global.assert) global.assert = require('chai').assert const assertEqualBN = (actual, expected, msg = 'numbers not equal') => { + if (!web3.utils.isBN(actual)) + actual = web3.utils.toBN(actual) + if (!web3.utils.isBN(expected)) + expected = web3.utils.toBN(expected) assert.isTrue( - actual.equals(expected), + actual.eq(expected), ` \tmsg: ${msg} \tactual: ${actual.toString()} diff --git a/test/helper/utils.js b/test/helper/utils.js index 6143146..d395e10 100644 --- a/test/helper/utils.js +++ b/test/helper/utils.js @@ -26,8 +26,8 @@ const newSecretHashPair = () => { const nowSeconds = () => Math.floor(Date.now() / 1000) -const gasPrice = 100000000000 // truffle fixed gas price -const txGas = txReceipt => txReceipt.receipt.gasUsed * gasPrice +const defaultGasPrice = 100000000000 // truffle fixed gas price +const txGas = (txReceipt, gasPrice = defaultGasPrice) => web3.utils.toBN(txReceipt.receipt.gasUsed * gasPrice) const txLoggedArgs = txReceipt => txReceipt.logs[0].args const txContractId = txReceipt => txLoggedArgs(txReceipt).contractId @@ -58,8 +58,11 @@ const htlcERC20ArrayToObj = c => { } } +const getBalance = async (address) => web3.utils.toBN(await web3.eth.getBalance(address)) + export { bufToStr, + getBalance, htlcArrayToObj, htlcERC20ArrayToObj, isSha256Hash, diff --git a/test/htlc.js b/test/htlc.js index 95d9eb9..d8421be 100644 --- a/test/htlc.js +++ b/test/htlc.js @@ -3,6 +3,7 @@ import Promise from 'bluebird' import {assertEqualBN} from './helper/assert' import { bufToStr, + getBalance, htlcArrayToObj, isSha256Hash, newSecretHashPair, @@ -15,11 +16,11 @@ import { const HashedTimelock = artifacts.require('./HashedTimelock.sol') -const REQUIRE_FAILED_MSG = 'VM Exception while processing transaction: revert' +const REQUIRE_FAILED_MSG = 'Returned error: VM Exception while processing transaction: revert' const hourSeconds = 3600 const timeLock1Hour = nowSeconds() + hourSeconds -const oneFinney = web3.toWei(1, 'finney') +const oneFinney = web3.utils.toWei(web3.utils.toBN(1), 'finney') contract('HashedTimelock', accounts => { const sender = accounts[1] @@ -44,7 +45,7 @@ contract('HashedTimelock', accounts => { assert.equal(logArgs.sender, sender) assert.equal(logArgs.receiver, receiver) - assert.equal(logArgs.amount, oneFinney) + assertEqualBN(logArgs.amount, oneFinney) assert.equal(logArgs.hashlock, hashPair.hash) assert.equal(logArgs.timelock, timeLock1Hour) @@ -52,7 +53,7 @@ contract('HashedTimelock', accounts => { const contract = htlcArrayToObj(contractArr) assert.equal(contract.sender, sender) assert.equal(contract.receiver, receiver) - assert.equal(contract.amount, oneFinney) + assertEqualBN(contract.amount, oneFinney) assert.equal(contract.hashlock, hashPair.hash) assert.equal(contract.timelock.toNumber(), timeLock1Hour) assert.isFalse(contract.withdrawn) @@ -73,7 +74,7 @@ contract('HashedTimelock', accounts => { }) assert.fail('expected failure due to 0 value transferred') } catch (err) { - assert.equal(err.message, REQUIRE_FAILED_MSG) + assert.isTrue(err.message.startsWith(REQUIRE_FAILED_MSG)) } }) @@ -89,7 +90,7 @@ contract('HashedTimelock', accounts => { assert.fail('expected failure due past timelock') } catch (err) { - assert.equal(err.message, REQUIRE_FAILED_MSG) + assert.isTrue(err.message.startsWith(REQUIRE_FAILED_MSG)) } }) @@ -109,7 +110,7 @@ contract('HashedTimelock', accounts => { }) assert.fail('expected failure due to duplicate request') } catch (err) { - assert.equal(err.message, REQUIRE_FAILED_MSG) + assert.isTrue(err.message.startsWith(REQUIRE_FAILED_MSG)) } }) @@ -127,19 +128,20 @@ contract('HashedTimelock', accounts => { ) const contractId = txContractId(newContractTx) - const receiverBalBefore = web3.eth.getBalance(receiver) + const receiverBalBefore = await getBalance(receiver) // receiver calls withdraw with the secret to get the funds const withdrawTx = await htlc.withdraw(contractId, hashPair.secret, { from: receiver, }) + const tx = await web3.eth.getTransaction(withdrawTx.tx) // Check contract funds are now at the receiver address const expectedBal = receiverBalBefore - .plus(oneFinney) - .minus(txGas(withdrawTx)) + .add(oneFinney) + .sub(txGas(withdrawTx, tx.gasPrice)) assertEqualBN( - web3.eth.getBalance(receiver), + await getBalance(receiver), expectedBal, "receiver balance doesn't match" ) @@ -170,7 +172,7 @@ contract('HashedTimelock', accounts => { await htlc.withdraw(contractId, wrongSecret, {from: receiver}) assert.fail('expected failure due to 0 value transferred') } catch (err) { - assert.equal(err.message, REQUIRE_FAILED_MSG) + assert.isTrue(err.message.startsWith(REQUIRE_FAILED_MSG)) } }) @@ -192,7 +194,7 @@ contract('HashedTimelock', accounts => { await htlc.withdraw(contractId, hashPair.secret, {from: someGuy}) assert.fail('expected failure due to wrong receiver') } catch (err) { - assert.equal(err.message, REQUIRE_FAILED_MSG) + assert.isTrue(err.message.startsWith(REQUIRE_FAILED_MSG)) } }) @@ -222,7 +224,7 @@ contract('HashedTimelock', accounts => { new Error('expected failure due to withdraw after timelock expired') ) } catch (err) { - assert.equal(err.message, REQUIRE_FAILED_MSG) + assert.isTrue(err.message.startsWith(REQUIRE_FAILED_MSG)) resolve() } }, 1000) @@ -249,12 +251,13 @@ contract('HashedTimelock', accounts => { return new Promise((resolve, reject) => setTimeout(async () => { try { - const balBefore = web3.eth.getBalance(sender) - const tx = await htlc.refund(contractId, {from: sender}) + const balBefore = await getBalance(sender) + const refundTx = await htlc.refund(contractId, {from: sender}) + const tx = await web3.eth.getTransaction(refundTx.tx) // Check contract funds are now at the senders address - const expectedBal = balBefore.plus(oneFinney).minus(txGas(tx)) + const expectedBal = balBefore.add(oneFinney).sub(txGas(refundTx, tx.gasPrice)) assertEqualBN( - web3.eth.getBalance(sender), + await getBalance(sender), expectedBal, "sender balance doesn't match" ) @@ -286,7 +289,7 @@ contract('HashedTimelock', accounts => { await htlc.refund(contractId, {from: sender}) assert.fail('expected failure due to timelock') } catch (err) { - assert.equal(err.message, REQUIRE_FAILED_MSG) + assert.isTrue(err.message.startsWith(REQUIRE_FAILED_MSG)) } }) diff --git a/test/htlcERC20.js b/test/htlcERC20.js index 8cb7160..5152ac7 100644 --- a/test/htlcERC20.js +++ b/test/htlcERC20.js @@ -13,7 +13,7 @@ import { const HashedTimelockERC20 = artifacts.require('./HashedTimelockERC20.sol') const ASEANToken = artifacts.require('./helper/ASEANToken.sol') -const REQUIRE_FAILED_MSG = 'VM Exception while processing transaction: revert' +const REQUIRE_FAILED_MSG = 'Returned error: VM Exception while processing transaction: revert' // some testing data const hourSeconds = 3600 @@ -119,12 +119,12 @@ contract('HashedTimelockERC20', accounts => { it('newContract() should reject a duplicate contract request', async () => { const hashlock = newSecretHashPair().hash const timelock = timeLock1Hour + 5 - const balBefore = await token.balanceOf(htlc.address) + const balBefore = web3.utils.toBN(await token.balanceOf(htlc.address)) await newContract({hashlock: hashlock, timelock: timelock}) await assertTokenBal( htlc.address, - balBefore.plus(tokenAmount), + balBefore.add(web3.utils.toBN(tokenAmount)), 'tokens not transfered to htlc contract' ) @@ -172,7 +172,7 @@ contract('HashedTimelockERC20', accounts => { await htlc.withdraw(contractId, wrongSecret, {from: receiver}) assert.fail('expected failure due to 0 value transferred') } catch (err) { - assert.equal(err.message, REQUIRE_FAILED_MSG) + assert.isTrue(err.message.startsWith(REQUIRE_FAILED_MSG)) } }) @@ -188,14 +188,14 @@ contract('HashedTimelockERC20', accounts => { await htlc.withdraw(contractId, hashPair.secret, {from: someGuy}) assert.fail('expected failure due to wrong receiver') } catch (err) { - assert.equal(err.message, REQUIRE_FAILED_MSG) + assert.isTrue(err.message.startsWith(REQUIRE_FAILED_MSG)) } }) it('withdraw() should fail after timelock expiry', async () => { const hashPair = newSecretHashPair() - const curBlkTime = web3.eth.getBlock('latest').timestamp - const timelock2Seconds = curBlkTime + 2 + const curBlock = await web3.eth.getBlock('latest') + const timelock2Seconds = curBlock.timestamp + 2 const newContractTx = await newContract({ hashlock: hashPair.hash, @@ -213,7 +213,7 @@ contract('HashedTimelockERC20', accounts => { new Error('expected failure due to withdraw after timelock expired') ) } catch (err) { - assert.equal(err.message, REQUIRE_FAILED_MSG) + assert.isTrue(err.message.startsWith(REQUIRE_FAILED_MSG)) resolve({message: 'success'}) } }, 2000) @@ -222,8 +222,8 @@ contract('HashedTimelockERC20', accounts => { it('refund() should pass after timelock expiry', async () => { const hashPair = newSecretHashPair() - const curBlkTime = web3.eth.getBlock('latest').timestamp - const timelock2Seconds = curBlkTime + 2 + const curBlock = await web3.eth.getBlock('latest') + const timelock2Seconds = curBlock.timestamp + 2 await token.approve(htlc.address, tokenAmount, {from: sender}) const newContractTx = await newContract({ @@ -243,7 +243,7 @@ contract('HashedTimelockERC20', accounts => { // Check tokens returned to the sender await assertTokenBal( sender, - balBefore.plus(tokenAmount), + balBefore.add(web3.utils.toBN(tokenAmount)), `sender balance unexpected` ) @@ -266,7 +266,7 @@ contract('HashedTimelockERC20', accounts => { await htlc.refund(contractId, {from: sender}) assert.fail('expected failure due to timelock') } catch (err) { - assert.equal(err.message, REQUIRE_FAILED_MSG) + assert.isTrue(err.message.startsWith(REQUIRE_FAILED_MSG)) } }) @@ -321,7 +321,7 @@ contract('HashedTimelockERC20', accounts => { ) assert.fail(shouldFailMsg) } catch (err) { - assert.equal(err.message, REQUIRE_FAILED_MSG) + assert.isTrue(err.message.startsWith(REQUIRE_FAILED_MSG)) } } })