From 05f62df2427a81d6a5837df8c1e77227f188556f Mon Sep 17 00:00:00 2001 From: noiach Date: Wed, 3 Jul 2019 15:57:43 +1000 Subject: [PATCH 1/4] Added Shifter.transferTokenOwnerShip --- contracts/Shifter/Shifter.sol | 17 ++++++++---- contracts/Shifter/ShifterRegistry.sol | 6 ++--- contracts/Shifter/examples/BasicAdapter.sol | 29 +++++++++++++++++++++ 3 files changed, 44 insertions(+), 8 deletions(-) create mode 100644 contracts/Shifter/examples/BasicAdapter.sol diff --git a/contracts/Shifter/Shifter.sol b/contracts/Shifter/Shifter.sol index ad551df4..69068505 100644 --- a/contracts/Shifter/Shifter.sol +++ b/contracts/Shifter/Shifter.sol @@ -58,20 +58,27 @@ contract Shifter is Ownable { // Public functions //////////////////////////////////////////////////////// /// @notice Claims ownership of the token passed in to the constructor. - /// `transferStoreOwnership` must have previously been called. - /// Anyone can call this function. + /// `transferStoreOwnership` must have previously been called. + /// Anyone can call this function. function claimTokenOwnership() public { token.claimOwnership(); } - /// @notice Allow the mint authority to update the fee recipient. + /// @notice Allow the owner to update the owner of the ERC20Shifted token. + /// The next token owner should then call `claimOwnership` on the token. + function transferTokenOwnership(Shifter _nextTokenOwner) public onlyOwner { + token.transferOwnership(address(_nextTokenOwner)); + _nextTokenOwner.claimTokenOwnership(); + } + + /// @notice Allow the owner to update the fee recipient. /// /// @param _nextMintAuthority The address to start paying fees to. function updateMintAuthority(address _nextMintAuthority) public onlyOwner { mintAuthority = _nextMintAuthority; } - /// @notice Allow the mint authority to update the fee recipient. + /// @notice Allow the owner to update the fee recipient. /// /// @param _nextFeeRecipient The address to start paying fees to. function updateFeeRecipient(address _nextFeeRecipient) public onlyOwner { @@ -81,7 +88,7 @@ contract Shifter is Ownable { feeRecipient = _nextFeeRecipient; } - /// @notice Allow the mint authority to update the fee. + /// @notice Allow the owner to update the fee. /// /// @param _nextFee The new fee for minting and burning. function updateFee(uint16 _nextFee) public onlyOwner { diff --git a/contracts/Shifter/ShifterRegistry.sol b/contracts/Shifter/ShifterRegistry.sol index 3703ee90..0315099c 100644 --- a/contracts/Shifter/ShifterRegistry.sol +++ b/contracts/Shifter/ShifterRegistry.sol @@ -44,7 +44,7 @@ contract ShifterRegistry is Claimable { // Add to list of shifters LinkedList.append(shifterList, _shifterAddress); - + // Add to list of shifted tokens LinkedList.append(shiftedTokenList, _tokenAddress); @@ -67,7 +67,7 @@ contract ShifterRegistry is Claimable { // Remove to list of shifters LinkedList.remove(shifterList, currentShifter); - + // Add to list of shifted tokens LinkedList.append(shifterList, _newShifterAddress); @@ -106,7 +106,7 @@ contract ShifterRegistry is Claimable { } else { count = _count; } - + address[] memory shifters = new address[](count); // Begin with the first node in the list diff --git a/contracts/Shifter/examples/BasicAdapter.sol b/contracts/Shifter/examples/BasicAdapter.sol new file mode 100644 index 00000000..0ef1ab6d --- /dev/null +++ b/contracts/Shifter/examples/BasicAdapter.sol @@ -0,0 +1,29 @@ +pragma solidity ^0.5.8; + +import "../Shifter.sol"; + +contract BasicAdapter { + + function shiftIn( + // Payload + Shifter _shifter, + address _address, + // Required + uint256 _amount, + bytes32 _nonce, + bytes calldata _sig + ) external { + bytes32 payloadHash = keccak256(abi.encode(_shifter, _address)); + uint256 amount = _shifter.shiftIn(payloadHash, _amount, _nonce, _sig); + _shifter.token().transfer(_address, amount); + } + + function shiftOut( + Shifter _shifter, + bytes calldata _to, + uint256 _amount + ) external { + require(_shifter.token().transferFrom(msg.sender, address(this), _amount), "token transfer failed"); + _shifter.shiftOut(_to, _amount); + } +} From e0ab5b8da50b5a0af5212931df713cd59b16cfa3 Mon Sep 17 00:00:00 2001 From: noiach Date: Wed, 3 Jul 2019 15:57:55 +1000 Subject: [PATCH 2/4] Added Shifter upgrade test, fixed lint warnings --- test/CompatibleERC20.ts | 4 +- test/DarknodePayment.ts | 131 +++++++++++++++++++++------------------ test/DarknodeRegistry.ts | 8 ++- test/Shifter.ts | 87 ++++++++++++++++++++++---- test/String.ts | 10 +-- test/Vesting.ts | 29 +++++---- test/helper/logs.ts | 20 +++--- test/helper/testUtils.ts | 28 ++++----- tslint.json | 16 ++++- 9 files changed, 214 insertions(+), 119 deletions(-) diff --git a/test/CompatibleERC20.ts b/test/CompatibleERC20.ts index 8d5eb804..ccba7fe1 100644 --- a/test/CompatibleERC20.ts +++ b/test/CompatibleERC20.ts @@ -1,7 +1,7 @@ import BN from "bn.js"; -import "./helper/testUtils"; import { CompatibleERC20TestInstance, ReturnsFalseTokenInstance } from "../types/truffle-contracts"; +import "./helper/testUtils"; const CompatibleERC20Test = artifacts.require("CompatibleERC20Test"); const NormalToken = artifacts.require("NormalToken"); @@ -32,7 +32,7 @@ contract("CompatibleERC20", (accounts) => { const FEE = VALUE.mul(new BN(testCase.fees)).div(new BN(1000)); before(async () => { - token = await testCase.contract.new(); + token = await testCase.contract.new() as ReturnsFalseTokenInstance; }); it("approve and transferFrom", async () => { diff --git a/test/DarknodePayment.ts b/test/DarknodePayment.ts index fbe3b429..465a71eb 100644 --- a/test/DarknodePayment.ts +++ b/test/DarknodePayment.ts @@ -82,7 +82,7 @@ contract("DarknodePayment", (accounts: string[]) => { } } return i; - } + }; const printTokens = async () => { console.log(`Registered tokens: [`); @@ -97,7 +97,7 @@ contract("DarknodePayment", (accounts: string[]) => { } } console.log(`]`); - } + }; const checkTokenIndexes = async () => { let i = 0; @@ -114,7 +114,7 @@ contract("DarknodePayment", (accounts: string[]) => { throw error; } } - } + }; it("cannot register token if not owner", async () => { await dnp.registerToken(dai.address, { from: accounts[1] }).should.be.rejected; @@ -125,7 +125,7 @@ contract("DarknodePayment", (accounts: string[]) => { await dnp.registerToken(dai.address); await dnp.registerToken(dai.address).should.be.rejectedWith(/token already pending registration/); - await dnp.registerToken(erc20Token.address); // .should.not.be.rejectedWith(/token already pending registration/); + await dnp.registerToken(erc20Token.address); // complete token registration await waitForCycle(); (await dnp.registeredTokens(lengthBefore)).should.equal(dai.address); @@ -161,7 +161,7 @@ contract("DarknodePayment", (accounts: string[]) => { it("can deregister tokens", async () => { await dnp.deregisterToken(ETHEREUM_TOKEN_ADDRESS); await dnp.deregisterToken(ETHEREUM_TOKEN_ADDRESS).should.be.rejectedWith(/token not registered/); - await dnp.deregisterToken(erc20Token.address).should.not.be.rejectedWith(/token not registered/); + await dnp.deregisterToken(erc20Token.address); // check token deregistration (await dnp.registeredTokenIndex(ETHEREUM_TOKEN_ADDRESS)).should.bignumber.equal(0); (await dnp.registeredTokenIndex(erc20Token.address)).should.bignumber.equal(0); @@ -213,7 +213,8 @@ contract("DarknodePayment", (accounts: string[]) => { await dnp.deposit(amount, ETHEREUM_TOKEN_ADDRESS, { value: amount.toString(), from: accounts[0] }); new BN(await store.totalBalance(ETHEREUM_TOKEN_ADDRESS)).should.bignumber.equal(oldETHBalance.add(amount)); // We should have increased the reward pool - (new BN(await dnp.currentCycleRewardPool(ETHEREUM_TOKEN_ADDRESS))).should.bignumber.equal(previousReward.add(amount)); + (new BN(await dnp.currentCycleRewardPool(ETHEREUM_TOKEN_ADDRESS))) + .should.bignumber.equal(previousReward.add(amount)); }); it("can deposit ETH via direct payment to DarknodePayment contract", async () => { @@ -227,7 +228,8 @@ contract("DarknodePayment", (accounts: string[]) => { await web3.eth.sendTransaction({ to: dnp.address, from: owner, value: amount.toString() }); new BN(await store.totalBalance(ETHEREUM_TOKEN_ADDRESS)).should.bignumber.equal(oldETHBalance.add(amount)); // We should have increased the reward pool - (new BN(await dnp.currentCycleRewardPool(ETHEREUM_TOKEN_ADDRESS))).should.bignumber.equal(previousReward.add(amount)); + (new BN(await dnp.currentCycleRewardPool(ETHEREUM_TOKEN_ADDRESS))) + .should.bignumber.equal(previousReward.add(amount)); }); it("can deposit ETH via direct payment to DarknodePaymentStore contract", async () => { @@ -238,24 +240,25 @@ contract("DarknodePayment", (accounts: string[]) => { await web3.eth.sendTransaction({ to: store.address, from: owner, value: amount.toString() }); new BN(await store.totalBalance(ETHEREUM_TOKEN_ADDRESS)).should.bignumber.equal(oldETHBalance.add(amount)); // We should have increased the reward pool - (new BN(await dnp.currentCycleRewardPool(ETHEREUM_TOKEN_ADDRESS))).should.bignumber.equal(previousReward.add(amount)); + (new BN(await dnp.currentCycleRewardPool(ETHEREUM_TOKEN_ADDRESS))) + .should.bignumber.equal(previousReward.add(amount)); }); it("cannot deposit ERC20 with ETH attached", async () => { const amount = new BN("100000000000000000"); - await dnp.deposit(amount, dai.address, { value: "1", from: accounts[0] }).should.be.rejectedWith(/unexpected ether transfer/); + await dnp.deposit(amount, dai.address, { value: "1", from: accounts[0] }) + .should.be.rejectedWith(/unexpected ether transfer/); }); }); - describe("Claiming rewards", async () => { it("cannot tick if not registered", async () => { await dnp.claim(accounts[0]).should.be.rejectedWith(/darknode is not registered/); - }) + }); it("cannot withdraw if there is no balance", async () => { await dnp.withdraw(darknode1, dai.address).should.be.rejectedWith(/nothing to withdraw/); - }) + }); it("can whitelist darknodes", async () => { await waitForCycle(); @@ -266,8 +269,9 @@ contract("DarknodePayment", (accounts: string[]) => { await dnp.claim(darknode1).should.be.rejectedWith(/cannot claim for this cycle/); await store.isWhitelisted(darknode1).should.eventually.be.true; await waitForCycle(); - new BN(await store.darknodeWhitelistLength()).should.bignumber.equal(new BN(whitelistLength).add(new BN(1))); - }) + new BN(await store.darknodeWhitelistLength()) + .should.bignumber.equal(new BN(whitelistLength).add(new BN(1))); + }); it("can be paid DAI from a payee", async () => { // darknode1 is whitelisted and can participate in rewards @@ -354,7 +358,7 @@ contract("DarknodePayment", (accounts: string[]) => { postWithdrawRewards.should.bignumber.equal(new BN(0)); // Deregister ETH - await dnp.deregisterToken(ETHEREUM_TOKEN_ADDRESS); // .should.not.be.rejectedWith(/token already pending deregistration/); + await dnp.deregisterToken(ETHEREUM_TOKEN_ADDRESS); await waitForCycle(); (await dnp.registeredTokenIndex(ETHEREUM_TOKEN_ADDRESS)).should.bignumber.equal(0); }); @@ -363,18 +367,18 @@ contract("DarknodePayment", (accounts: string[]) => { const darknode1Balance = new BN(await dnp.darknodeBalances(darknode1, dai.address)); darknode1Balance.gt(new BN(0)).should.be.true; await withdraw(darknode1); - }) + }); it("cannot call tick twice in the same cycle", async () => { await dnp.claim(darknode1); await dnp.claim(darknode1).should.be.rejectedWith(/reward already claimed/); - }) + }); it("can tick again after a cycle has passed", async () => { await dnp.claim(darknode1); await waitForCycle(); - await dnp.claim(darknode1); // .should.not.be.rejectedWith(/reward already claimed/); - }) + await dnp.claim(darknode1); + }); it("should evenly split reward pool between ticked darknodes", async () => { const numDarknodes = 3; @@ -399,7 +403,8 @@ contract("DarknodePayment", (accounts: string[]) => { await multiTick(startDarknode, numDarknodes); for (let i = startDarknode; i < startDarknode + numDarknodes; i++) { - (new BN(await dnp.darknodeBalances(accounts[i], dai.address))).should.bignumber.equal(rewards.div(new BN(await dnp.shareCount()))); + (new BN(await dnp.darknodeBalances(accounts[i], dai.address))) + .should.bignumber.equal(rewards.div(new BN(await dnp.shareCount()))); } // Withdraw for each darknode @@ -437,7 +442,7 @@ contract("DarknodePayment", (accounts: string[]) => { await dnp.withdraw(NULL, dai.address).should.eventually.be.rejectedWith(/invalid darknode owner/); // accounts[0] is not a registered darknode await dnp.withdraw(accounts[0], dai.address).should.eventually.be.rejectedWith(/invalid darknode owner/); - }) + }); it("cannot withdraw more than once in a cycle", async () => { const numDarknodes = 4; @@ -453,7 +458,7 @@ contract("DarknodePayment", (accounts: string[]) => { await multiTick(1, numDarknodes); // First withdraw should pass - await withdraw(darknode1); // .should.not.be.rejectedWith(/nothing to withdraw/); + await withdraw(darknode1); // Rest should fail await dnp.withdraw(darknode1, dai.address).should.be.rejectedWith(/nothing to withdraw/); @@ -509,25 +514,25 @@ contract("DarknodePayment", (accounts: string[]) => { await store.isBlacklisted(darknode1).should.eventually.be.false; await dnp.blacklist(darknode1, { from: accounts[2] }).should.be.rejectedWith(/not Blacklister/); await store.isBlacklisted(darknode1).should.eventually.be.false; - }) + }); it("should disallow unauthorized updates to blacklister address", async () => { await dnp.updateBlacklister(accounts[2], { from: accounts[2] }).should.be.rejected; - }) + }); it("can update the blacklister address", async () => { await store.isBlacklisted(darknode4).should.eventually.be.false; await dnp.updateBlacklister(accounts[2]).should.be.not.rejectedWith(/invalid contract address/); await waitForCycle(); - await dnp.blacklist(darknode4, { from: accounts[2] }); // .should.not.be.rejectedWith(/not Blacklister/); + await dnp.blacklist(darknode4, { from: accounts[2] }); await store.isBlacklisted(darknode4).should.eventually.be.true; await dnp.updateBlacklister(owner).should.be.not.rejectedWith(/invalid contract address/); await waitForCycle(); - }) + }); it("cannot update the blacklister address to an invalid address", async () => { await dnp.updateBlacklister(NULL).should.be.rejectedWith(/invalid contract address/); - }) + }); it("cannot blacklist invalid addresses", async () => { const invalidAddress = NULL; @@ -535,7 +540,7 @@ contract("DarknodePayment", (accounts: string[]) => { await dnp.blacklist(invalidAddress).should.be.rejectedWith(/darknode is not registered/); await store.isBlacklisted(owner).should.eventually.be.false; await dnp.blacklist(owner).should.be.rejectedWith(/darknode is not registered/); - }) + }); it("should reject white/blacklist attempts from non-store contract", async () => { await store.isBlacklisted(darknode1).should.eventually.be.false; @@ -544,19 +549,19 @@ contract("DarknodePayment", (accounts: string[]) => { await store.isWhitelisted(darknode5).should.eventually.be.false; await store.whitelist(darknode5, { from: darknode1 }).should.be.rejected; await store.isWhitelisted(darknode5).should.eventually.be.false; - }) + }); it("can blacklist darknodes", async () => { await store.isBlacklisted(darknode5).should.eventually.be.false; await dnp.blacklist(darknode5); await store.isBlacklisted(darknode5).should.eventually.be.true; - }) + }); it("cannot blacklist already blacklisted darknodes", async () => { await store.isBlacklisted(darknode5).should.eventually.be.true; await dnp.blacklist(darknode5).should.be.rejectedWith(/darknode already blacklisted/); await store.isBlacklisted(darknode5).should.eventually.be.true; - }) + }); it("cannot whitelist blacklisted darknodes", async () => { await store.isBlacklisted(darknode5).should.eventually.be.true; @@ -569,7 +574,8 @@ contract("DarknodePayment", (accounts: string[]) => { describe("Changing cycles", async () => { it("cannot change cycle if insufficient time has passed", async () => { - await waitForCycle(DARKNODE_PAYMENT_CYCLE_DURATION_SECONDS / 4).should.eventually.be.rejectedWith(/cannot cycle yet: too early/); + await waitForCycle(DARKNODE_PAYMENT_CYCLE_DURATION_SECONDS / 4) + .should.eventually.be.rejectedWith(/cannot cycle yet: too early/); }); it("should disallow unauthorized changes to cycle duration", async () => { @@ -646,26 +652,31 @@ contract("DarknodePayment", (accounts: string[]) => { it("cannot whitelist already whitelisted darknodes", async () => { await store.isWhitelisted(darknode1).should.eventually.be.true; await store.whitelist(darknode1).should.eventually.be.rejectedWith(/darknode already whitelisted/); - }) + }); it("cannot increment balances by an invalid amounts", async () => { - await store.incrementDarknodeBalance(darknode1, dai.address, 0).should.eventually.be.rejectedWith(/invalid amount/); + await store.incrementDarknodeBalance(darknode1, dai.address, 0) + .should.eventually.be.rejectedWith(/invalid amount/); const invalidAmount = new BN(await store.availableBalance(dai.address)).add(new BN(1)); - await store.incrementDarknodeBalance(darknode1, dai.address, invalidAmount).should.eventually.be.rejectedWith(/insufficient contract balance/); - }) + await store.incrementDarknodeBalance(darknode1, dai.address, invalidAmount) + .should.eventually.be.rejectedWith(/insufficient contract balance/); + }); it("cannot transfer more than is in the balance", async () => { const invalidAmount = new BN(await dnp.darknodeBalances(darknode1, dai.address)).add(new BN(1)); - await store.transfer(darknode1, dai.address, invalidAmount, darknode1).should.eventually.be.rejectedWith(/insufficient darknode balance/); - }) + await store.transfer(darknode1, dai.address, invalidAmount, darknode1) + .should.eventually.be.rejectedWith(/insufficient darknode balance/); + }); it("cannot call functions from non-owner", async () => { await store.blacklist(darknode1, { from: accounts[2] }).should.eventually.be.rejected; await store.whitelist(darknode1, { from: accounts[2] }).should.eventually.be.rejected; - await store.incrementDarknodeBalance(darknode1, dai.address, new BN(0), { from: accounts[2] }).should.eventually.be.rejected; - await store.transfer(darknode1, dai.address, new BN(0), darknode1, { from: accounts[2] }).should.eventually.be.rejected; + await store.incrementDarknodeBalance(darknode1, dai.address, new BN(0), { from: accounts[2] }) + .should.eventually.be.rejected; + await store.transfer(darknode1, dai.address, new BN(0), darknode1, { from: accounts[2] }) + .should.eventually.be.rejected; await store.transferOwnership(dnp.address, { from: accounts[2] }).should.eventually.be.rejected; - }) + }); // Transfer the ownership back to DNP after(async () => { @@ -678,17 +689,17 @@ contract("DarknodePayment", (accounts: string[]) => { }); }); - const tick = async (address) => { + const tick = async (address: string) => { return dnp.claim(address); - } + }; const multiTick = async (start = 1, numberOfDarknodes = 1) => { for (let i = start; i < start + numberOfDarknodes; i++) { await tick(accounts[i]); } - } + }; - const withdraw = async (address) => { + const withdraw = async (address: string) => { // Our claimed amount should be positive const earnedDAIRewards = new BN(await dnp.darknodeBalances(address, dai.address)); earnedDAIRewards.gt(new BN(0)).should.be.true; @@ -704,15 +715,15 @@ contract("DarknodePayment", (accounts: string[]) => { // We should have nothing left to withdraw const postWithdrawRewards = new BN(await dnp.darknodeBalances(address, dai.address)); postWithdrawRewards.should.bignumber.equal(new BN(0)); - } + }; const multiWithdraw = async (start = 1, numberOfDarknodes = 1) => { for (let i = start; i < start + numberOfDarknodes; i++) { await withdraw(accounts[i]); } - } + }; - const depositDai = async (amount) => { + const depositDai = async (amount: number | BN | string) => { const amountBN = new BN(amount); const previousBalance = new BN(await dnp.currentCycleRewardPool(dai.address)); // Approve the contract to use DAI @@ -720,7 +731,7 @@ contract("DarknodePayment", (accounts: string[]) => { await dnp.deposit(amountBN, dai.address); // We should expect the DAI balance to have increased by what we deposited (await dnp.currentCycleRewardPool(dai.address)).should.bignumber.equal(previousBalance.add(amountBN)); - } + }; const changeCycleDuration = async (timeInSeconds: number) => { const currentCycleDurationInSeconds = new BN(await dnp.cycleDuration()).toNumber(); @@ -730,9 +741,9 @@ contract("DarknodePayment", (accounts: string[]) => { // put into effect the new cycle duration await increaseTime(currentCycleDurationInSeconds); - await dnp.changeCycle().should.not.eventually.be.rejectedWith(/cannot cycle yet: too early/); - if (timeInSeconds == 0) { - await dnp.changeCycle().should.not.eventually.be.rejectedWith(/cannot cycle yet: too early/); + await dnp.changeCycle(); + if (timeInSeconds === 0) { + await dnp.changeCycle(); return; } @@ -740,25 +751,23 @@ contract("DarknodePayment", (accounts: string[]) => { if (timeInSeconds < currentCycleDurationInSeconds) { await increaseTime(timeInSeconds); - await dnp.changeCycle().should.not.eventually.be.rejected; // With(null, /cannot cycle yet: too early/); + await dnp.changeCycle(); } else { await increaseTime(currentCycleDurationInSeconds); - await dnp.changeCycle().should.eventually.be.rejected; //With(null, /cannot cycle yet: too early/); + await dnp.changeCycle().should.eventually.be.rejectedWith(null, /cannot cycle yet: too early/); await increaseTime(timeInSeconds - currentCycleDurationInSeconds); - await dnp.changeCycle().should.not.eventually.be.rejected; // With(null, /cannot cycle yet: too early/); + await dnp.changeCycle(); } - } - - const waitForCycle = async (seconds?) => { - let retry = seconds === undefined; + }; + const waitForCycle = async (seconds?: number) => { const timeout = new BN(await dnp.cycleTimeout()); const now = new BN(await cc.time()); - if (retry) { + if (seconds === undefined) { // seconds = (new BN(await dnp.cycleDuration()).toNumber()); seconds = Math.max(1, (timeout).sub(now).toNumber()); } await increaseTime(seconds); await dnp.changeCycle(); - } + }; }); diff --git a/test/DarknodeRegistry.ts b/test/DarknodeRegistry.ts index d1545f68..46a71f2c 100644 --- a/test/DarknodeRegistry.ts +++ b/test/DarknodeRegistry.ts @@ -1,10 +1,12 @@ import BN from "bn.js"; import { - ID, MINIMUM_BOND, MINIMUM_EPOCH_INTERVAL, MINIMUM_POD_SIZE, - NULL, PUBK, waitForEpoch, + DarknodeRegistryInstance, DarknodeRegistryStoreInstance, DarknodeSlasherInstance, + RenTokenInstance, +} from "../types/truffle-contracts"; +import { + ID, MINIMUM_BOND, MINIMUM_EPOCH_INTERVAL, MINIMUM_POD_SIZE, NULL, PUBK, waitForEpoch, } from "./helper/testUtils"; -import { RenTokenInstance, DarknodeRegistryStoreInstance, DarknodeRegistryInstance, DarknodeSlasherInstance } from "../types/truffle-contracts"; const RenToken = artifacts.require("RenToken"); const DarknodeRegistryStore = artifacts.require("DarknodeRegistryStore"); diff --git a/test/Shifter.ts b/test/Shifter.ts index d9867853..4308e650 100644 --- a/test/Shifter.ts +++ b/test/Shifter.ts @@ -1,5 +1,6 @@ import BN from "bn.js"; import { ecrecover, ecsign, pubToAddress } from "ethereumjs-util"; +import { Account } from "web3-eth-accounts"; import { keccak256 } from "web3-utils"; import { @@ -18,15 +19,15 @@ contract("Shifter", ([owner, feeRecipient, user, malicious]) => { // We generate a new account so that we have access to its private key for // `ecsign`. Web3's sign functions all prefix the message being signed. - let mintAuthority; - let privKey; + let mintAuthority: Account; + let privKey: Buffer; const feeInBips = new BN(10); before(async () => { zbtc = await zBTC.new(); mintAuthority = web3.eth.accounts.create(); - privKey = Buffer.from(mintAuthority.privateKey.slice(2), "hex") + privKey = Buffer.from(mintAuthority.privateKey.slice(2), "hex"); btcShifter = await BTCShifter.new( zbtc.address, @@ -39,7 +40,8 @@ contract("Shifter", ([owner, feeRecipient, user, malicious]) => { await btcShifter.claimTokenOwnership(); }); - const removeFee = (value: number | BN, bips: number | BN) => new BN(value).sub(new BN(value).mul(new BN(bips)).div(new BN(10000))); + const removeFee = (value: number | BN, bips: number | BN) => + new BN(value).sub(new BN(value).mul(new BN(bips)).div(new BN(10000))); const mintTest = async (shifter: ShifterInstance, value: number | BN, shiftID = undefined) => { const nHash = randomBytes(32); @@ -61,12 +63,15 @@ contract("Shifter", ([owner, feeRecipient, user, malicious]) => { const _shiftID = await shifter.nextShiftID(); (await shifter.shiftIn(pHash, value, nHash, sigString, { from: user }) as any) .should.emit.logs([ - log("LogShiftIn", { _to: user, _amount: removeFee(value, 10), _shiftID: shiftID !== undefined ? shiftID : _shiftID }), + log( + "LogShiftIn", + { _to: user, _amount: removeFee(value, 10), _shiftID: shiftID !== undefined ? shiftID : _shiftID }, + ), ]); (await zbtc.balanceOf(user)).should.bignumber.equal(balanceBefore.add(removeFee(value, 10))); return [pHash, nHash]; - } + }; const burnTest = async (shifter: ShifterInstance, value: number | BN, btcAddress?: string, shiftID = undefined) => { // Note: we don't use `||` because we want to pass in `""` @@ -76,10 +81,18 @@ contract("Shifter", ([owner, feeRecipient, user, malicious]) => { const _shiftID = await shifter.nextShiftID(); (await shifter.shiftOut(btcAddress, value, { from: user }) as any) .should.emit.logs([ - log("LogShiftOut", { _to: btcAddress, _amount: removeFee(value, 10), _shiftID: shiftID !== undefined ? shiftID : _shiftID, _indexedTo: keccak256(btcAddress) }), + log( + "LogShiftOut", + { + _to: btcAddress, + _amount: removeFee(value, 10), + _shiftID: shiftID !== undefined ? shiftID : _shiftID, + _indexedTo: keccak256(btcAddress), + }, + ), ]); (await zbtc.balanceOf(user)).should.bignumber.equal(balanceBefore.sub(new BN(value))); - } + }; describe("can mint and burn", () => { const value = new BN(20000); @@ -144,12 +157,13 @@ contract("Shifter", ([owner, feeRecipient, user, malicious]) => { const hash = await btcShifter.hashForSignature(pHash, value.toNumber(), user, nHash); - let sig = ecsign(Buffer.from(hash.slice(2), "hex"), privKey); + const sig = ecsign(Buffer.from(hash.slice(2), "hex"), privKey); // Invalid signature const altSig = { ...sig, - s: new BN("FFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFEBAAEDCE6AF48A03BBFD25E8CD0364141", "hex").sub(new BN(sig.s)).toArrayLike(Buffer, "be", 32), + s: new BN("FFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFEBAAEDCE6AF48A03BBFD25E8CD0364141", "hex") + .sub(new BN(sig.s)).toArrayLike(Buffer, "be", 32), v: sig.v === 27 ? 28 : 27, }; const altSigString = Ox(`${altSig.r.toString("hex")}${altSig.s.toString("hex")}${(altSig.v).toString(16)}`); @@ -194,6 +208,57 @@ contract("Shifter", ([owner, feeRecipient, user, malicious]) => { }); }); + describe("upgrading shifter", () => { + let newShifter: ShifterInstance; + + it("can upgrade the shifter", async () => { + newShifter = await BTCShifter.new( + zbtc.address, + feeRecipient, + mintAuthority.address, + feeInBips, + ); + + // Fund and unlock the mintAuthority - not used currently but + // may be needed in the future. + /* await web3.eth.sendTransaction({ to: mintAuthority.address, from: owner, value: web3.utils.toWei("1") }); + * await web3.eth.personal.importRawKey(mintAuthority.privateKey, ""); + * await web3.eth.personal.unlockAccount(mintAuthority.address, "", 6000); + */ + + await (btcShifter.transferTokenOwnership(newShifter.address, { from: malicious })) + .should.be.rejectedWith(/caller is not the owner/); + + await btcShifter.transferTokenOwnership(newShifter.address, { from: owner }); + (await zbtc.owner()).should.equal(newShifter.address); + }); + + it("can mint and burn using new shifter", async () => { + const value = new BN(200000); + await mintTest(newShifter, value); + await burnTest(newShifter, removeFee(value, 10)); + }); + + it("can't upgrade to an invalid shifter", async () => { + await (newShifter.transferTokenOwnership(malicious, { from: owner })) + .should.be.rejectedWith(/revert/); + + await zbtc.claimOwnership({ from: malicious }) + .should.be.rejectedWith(/caller is not the pending owner/); + }); + + it("can reset the upgrade", async () => { + // Trying to reset upgrade in btcShifter without owning the token + await (btcShifter.transferTokenOwnership(NULL, { from: owner })) + .should.be.rejectedWith(/caller is not the owner/); + + // Upgrade newShifter to point to btcShifter + await newShifter.transferTokenOwnership(btcShifter.address, { from: owner }); + + (await zbtc.owner()).should.equal(btcShifter.address); + }); + }); + describe("shifter registry", () => { let registry: ShifterRegistryInstance; @@ -211,7 +276,7 @@ contract("Shifter", ([owner, feeRecipient, user, malicious]) => { it("can retrieve shifters", async () => { { // Try to register token with an existing symbol - const altZbtc = await zBTC.new() + const altZbtc = await zBTC.new(); await registry.setShifter(altZbtc.address, NULL) .should.be.rejectedWith(/symbol already registered/); } diff --git a/test/String.ts b/test/String.ts index 52172034..d1acb4bb 100644 --- a/test/String.ts +++ b/test/String.ts @@ -5,28 +5,28 @@ const StringTest = artifacts.require("StringTest"); contract("String", (accounts) => { - let String: StringTestInstance; + let StringInstance: StringTestInstance; before(async () => { - String = await StringTest.new(); + StringInstance = await StringTest.new(); }); // Skipped for now due to an issue with the coverage tool. // The tests pass when run without coverage. it.skip("can add strings", async () => { - (await String.add4("1", "2", "3", "4")) + (await StringInstance.add4("1", "2", "3", "4")) .should.equal("1234"); }); it.skip("can convert addresses to hex strings", async () => { - (await String.fromAddress(accounts[0])) + (await StringInstance.fromAddress(accounts[0])) .should.equal(accounts[0].toLowerCase()); }); it.skip("can convert bytes32 to hex strings", async () => { const bytes32 = randomBytes(32); - (await String.fromBytes32(bytes32)) + (await StringInstance.fromBytes32(bytes32)) .should.equal(bytes32.toLowerCase()); }); }); diff --git a/test/Vesting.ts b/test/Vesting.ts index 41e6ed0c..2a839986 100644 --- a/test/Vesting.ts +++ b/test/Vesting.ts @@ -1,10 +1,12 @@ -import BN from "bn.js"; -import { ecsign, keccak256 } from "ethereumjs-util"; import BigNumber from "bignumber.js"; +import BN from "bn.js"; import { rawEncode } from "ethereumjs-abi"; +import { ecsign, keccak256 } from "ethereumjs-util"; -import { BTCShifterInstance, VestingInstance, zBTCInstance, ShifterRegistryInstance } from "../types/truffle-contracts"; -import { increaseTime, NULL, Ox, randomBytes } from "./helper/testUtils"; +import { + BTCShifterInstance, ShifterRegistryInstance, VestingInstance, zBTCInstance, +} from "../types/truffle-contracts"; +import { increaseTime, Ox, randomBytes } from "./helper/testUtils"; const BTCShifter = artifacts.require("BTCShifter"); const ShifterRegistry = artifacts.require("ShifterRegistry"); @@ -34,7 +36,7 @@ contract("Vesting", (accounts) => { mintAuthority.address, feeInBips, ); - + await zbtc.transferOwnership(btcShifter.address); await btcShifter.claimTokenOwnership(); @@ -57,15 +59,20 @@ contract("Vesting", (accounts) => { const startTime = 0; const pHash = keccak256(rawEncode( ["address", "uint256", "uint16"], - [beneficiary, startTime, duration] + [beneficiary, startTime, duration], )).toString("hex"); - const hashForSignature = await btcShifter.hashForSignature(Ox(pHash), amount.toNumber(), vesting.address, nonce); + const hashForSignature = await btcShifter.hashForSignature( + Ox(pHash), + amount.toNumber(), + vesting.address, + nonce, + ); const sig = ecsign(Buffer.from(hashForSignature.slice(2), "hex"), privKey); const sigString = Ox(`${sig.r.toString("hex")}${sig.s.toString("hex")}${(sig.v).toString(16)}`); // User should have no schedules prior to adding. - let schedule = await vesting.schedules(beneficiary); + const schedule = await vesting.schedules(beneficiary); (schedule as any).startTime.should.bignumber.equal(new BN(0)); await vesting.addVestingSchedule( @@ -74,7 +81,7 @@ contract("Vesting", (accounts) => { // Required amount, nonce, sigString, ); - } + }; it("can add a vesting schedule", async () => { await addVestingSchedule(); @@ -90,7 +97,7 @@ contract("Vesting", (accounts) => { const amountBN = new BigNumber(amountAfterFee.toString()); const resultBN = amountBN.times(new BigNumber(elapsedMonths)).dividedToIntegerBy(new BigNumber(duration)); return new BN(resultBN.toString()); - } + }; it("can check claimable amount", async () => { let claimable = await vesting.calculateClaimable(beneficiary); @@ -149,4 +156,4 @@ contract("Vesting", (accounts) => { claimable[1].should.bignumber.equal(amountAfterFee); }); }); -}); \ No newline at end of file +}); diff --git a/test/helper/logs.ts b/test/helper/logs.ts index 820e8490..3d96f16b 100644 --- a/test/helper/logs.ts +++ b/test/helper/logs.ts @@ -83,7 +83,7 @@ chai.use(function (newChai: any, utils: any): void { const expectedArg = expectedLog.args[arg]; const actualArg = actualLog.args[arg]; - let sameValues; + let sameValues: boolean; if (BN.isBN(expectedArg) || expectedArg.isBigNumber) { sameValues = (new BigNumber(expectedArg).eq(new BigNumber(actualArg))); } else { @@ -105,11 +105,11 @@ chai.use(function (newChai: any, utils: any): void { // Pretty-print logs const logsToString = (logs: Log[]): string => { - return `[${logs.map((log: Log) => log.event).join(", ")}]`; -}; -const logToString = (log: Log): string => { - return `${log.event} ${JSON.stringify(log.args)}`; + return `[${logs.map((logItem: Log) => logItem.event).join(", ")}]`; }; +// const logToString = (logItem: Log): string => { +// return `${logItem.event} ${JSON.stringify(logItem.args)}`; +// }; // Compare logs const compareArrayOfLogs = (expected: Log[], actual: Log[]): boolean => { @@ -127,18 +127,18 @@ const compareArrayOfLogs = (expected: Log[], actual: Log[]): boolean => { // Extract logs from transaction receipt in correct format.s export const getLogsFromTx = (tx: TransactionReceipt): Log[] => { - return tx.logs.map((log) => { + return tx.logs.map((logItem) => { const args = {}; - for (const arg in log.args) { + for (const arg in logItem.args) { // skip if the property is from prototype - if (!log.args.hasOwnProperty(arg)) { continue; } + if (!logItem.args.hasOwnProperty(arg)) { continue; } if (isNaN(parseInt(arg, 10)) && arg !== "__length__") { - args[arg] = log.args[arg]; + args[arg] = logItem.args[arg]; } } return { - event: log.event, + event: logItem.event, args, }; }); diff --git a/test/helper/testUtils.ts b/test/helper/testUtils.ts index 42105c17..47c6ba9f 100644 --- a/test/helper/testUtils.ts +++ b/test/helper/testUtils.ts @@ -1,12 +1,10 @@ import * as chai from "chai"; import * as crypto from "crypto"; -// @ts-ignore -import chaiAsPromised from "chai-as-promised"; -// @ts-ignore -import chaiBigNumber from "chai-bignumber"; import BigNumber from "bignumber.js"; import BN from "bn.js"; +import chaiAsPromised from "chai-as-promised"; +import chaiBigNumber from "chai-bignumber"; import { keccak256, toChecksumAddress, toHex } from "web3-utils"; import { DarknodeRegistryInstance } from "../../types/truffle-contracts"; @@ -56,27 +54,28 @@ const increaseTimeHelper = async (seconds: number) => { await new Promise((resolve, reject) => { web3.currentProvider.send( { jsonrpc: "2.0", method: "evm_increaseTime", params: [seconds], id: 0 } as any, - ((err, _) => { + ((err: Error) => { if (err) { reject(err); } web3.currentProvider.send({ - jsonrpc: '2.0', - method: 'evm_mine', + jsonrpc: "2.0", + method: "evm_mine", params: [], - id: new Date().getSeconds() - } as any, ((err, _) => { - if (err) { + id: new Date().getSeconds(), + } as any, ((innerErr: Error) => { + if (innerErr) { reject(); } resolve(); }) as any); - }) as any - ) + }) as any, + ); }); -} +}; -const getCurrentTimestamp = async (): Promise => parseInt((await web3.eth.getBlock(await web3.eth.getBlockNumber())).timestamp.toString(), 10); +const getCurrentTimestamp = async (): Promise => + parseInt((await web3.eth.getBlock(await web3.eth.getBlockNumber())).timestamp.toString(), 10); export const increaseTime = async (seconds: number) => { let currentTimestamp = await getCurrentTimestamp(); @@ -85,7 +84,6 @@ export const increaseTime = async (seconds: number) => { const increase = Math.ceil(target - currentTimestamp + 1); await increaseTimeHelper(increase); currentTimestamp = await getCurrentTimestamp(); - // console.log(`Increased by ${increase} to ${currentTimestamp}. Target is ${target}. Reached: ${currentTimestamp >= target}`); } while (currentTimestamp < target); }; diff --git a/tslint.json b/tslint.json index 0ffc61f2..79a43732 100644 --- a/tslint.json +++ b/tslint.json @@ -4,7 +4,7 @@ ], "linterOptions": { "exclude": [ - "node_modules", + "node_modules" ] }, "rules": { @@ -12,6 +12,20 @@ true, "double" ], + "variable-name": [ + true, + "ban-keywords", + "check-format", + "allow-leading-underscore", + "allow-trailing-underscore", + "allow-pascal-case" + ], + "no-shadowed-variable": [ + true, + { + "underscore": false + } + ], "interface-name": false, "max-classes-per-file": false, "no-console": false, From 1e375ea1f5272ad3911ab814ff6ac6f58760bff8 Mon Sep 17 00:00:00 2001 From: noiach Date: Wed, 3 Jul 2019 16:05:16 +1000 Subject: [PATCH 3/4] Removed BasicAdapter --- contracts/Shifter/examples/BasicAdapter.sol | 29 --------------------- 1 file changed, 29 deletions(-) delete mode 100644 contracts/Shifter/examples/BasicAdapter.sol diff --git a/contracts/Shifter/examples/BasicAdapter.sol b/contracts/Shifter/examples/BasicAdapter.sol deleted file mode 100644 index 0ef1ab6d..00000000 --- a/contracts/Shifter/examples/BasicAdapter.sol +++ /dev/null @@ -1,29 +0,0 @@ -pragma solidity ^0.5.8; - -import "../Shifter.sol"; - -contract BasicAdapter { - - function shiftIn( - // Payload - Shifter _shifter, - address _address, - // Required - uint256 _amount, - bytes32 _nonce, - bytes calldata _sig - ) external { - bytes32 payloadHash = keccak256(abi.encode(_shifter, _address)); - uint256 amount = _shifter.shiftIn(payloadHash, _amount, _nonce, _sig); - _shifter.token().transfer(_address, amount); - } - - function shiftOut( - Shifter _shifter, - bytes calldata _to, - uint256 _amount - ) external { - require(_shifter.token().transferFrom(msg.sender, address(this), _amount), "token transfer failed"); - _shifter.shiftOut(_to, _amount); - } -} From 2b7f65f68fd0fb1b484ba1f26958db1fe1d125e7 Mon Sep 17 00:00:00 2001 From: noiach Date: Wed, 3 Jul 2019 16:14:59 +1000 Subject: [PATCH 4/4] Fixed transferTokenOwnership comment --- contracts/Shifter/Shifter.sol | 1 - 1 file changed, 1 deletion(-) diff --git a/contracts/Shifter/Shifter.sol b/contracts/Shifter/Shifter.sol index 69068505..4bd507ce 100644 --- a/contracts/Shifter/Shifter.sol +++ b/contracts/Shifter/Shifter.sol @@ -65,7 +65,6 @@ contract Shifter is Ownable { } /// @notice Allow the owner to update the owner of the ERC20Shifted token. - /// The next token owner should then call `claimOwnership` on the token. function transferTokenOwnership(Shifter _nextTokenOwner) public onlyOwner { token.transferOwnership(address(_nextTokenOwner)); _nextTokenOwner.claimTokenOwnership();