From d9fd6ca5c92772fa6381fb18bb861be98f4e8600 Mon Sep 17 00:00:00 2001 From: NicholasDotSol Date: Tue, 4 Feb 2020 20:21:35 -0600 Subject: [PATCH 01/17] Forward value to keep factory requestNewKeep is now payable, and takes the keep creation cost as value --- implementation/contracts/system/TBTCSystem.sol | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/implementation/contracts/system/TBTCSystem.sol b/implementation/contracts/system/TBTCSystem.sol index acc568854..cfa79583e 100644 --- a/implementation/contracts/system/TBTCSystem.sol +++ b/implementation/contracts/system/TBTCSystem.sol @@ -228,6 +228,6 @@ contract TBTCSystem is Ownable, ITBTCSystem, DepositLog { { IBondedECDSAKeepVendor _keepVendor = IBondedECDSAKeepVendor(keepVendor); IBondedECDSAKeepFactory _keepFactory = IBondedECDSAKeepFactory(_keepVendor.selectFactory()); - return _keepFactory.openKeep(_n, _m, msg.sender, _bond); + return _keepFactory.openKeep.value(msg.value)(_n, _m, msg.sender, _bond); } } From 2e5e971d3122fb6254b9df3bcf5e184bce329ec2 Mon Sep 17 00:00:00 2001 From: NicholasDotSol Date: Tue, 4 Feb 2020 20:22:38 -0600 Subject: [PATCH 02/17] add fee estimate getter external function returning the fee needed to create a new deposit --- implementation/contracts/system/TBTCSystem.sol | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/implementation/contracts/system/TBTCSystem.sol b/implementation/contracts/system/TBTCSystem.sol index cfa79583e..f046cc065 100644 --- a/implementation/contracts/system/TBTCSystem.sol +++ b/implementation/contracts/system/TBTCSystem.sol @@ -217,6 +217,18 @@ contract TBTCSystem is Ownable, ITBTCSystem, DepositLog { return IRelay(relay).getPrevEpochDifficulty(); } + /// @notice Gets a fee estimate for creating a new Deposit. + /// @return Uint256 estimate. + function createNewDepositFeeEstimate() + external + payable + returns (uint256) + { + IBondedECDSAKeepVendor _keepVendor = IBondedECDSAKeepVendor(keepVendor); + IBondedECDSAKeepFactory _keepFactory = IBondedECDSAKeepFactory(_keepVendor.selectFactory()); + return _keepFactory.openKeepFeeEstimate(); + } + /// @notice Request a new keep opening. /// @param _m Minimum number of honest keep members required to sign. /// @param _n Number of members in the keep. From 79092b550f7ae75fd0570e79cb34f9078e663c95 Mon Sep 17 00:00:00 2001 From: NicholasDotSol Date: Tue, 4 Feb 2020 20:36:04 -0600 Subject: [PATCH 03/17] Bump keep-ecdsa version dependency --- implementation/package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/implementation/package.json b/implementation/package.json index 177143699..c251e4566 100644 --- a/implementation/package.json +++ b/implementation/package.json @@ -20,7 +20,7 @@ "author": "James Prestwich", "license": "UNLICENSED", "dependencies": { - "@keep-network/keep-ecdsa": "^0.7.5", + "@keep-network/keep-ecdsa": "^0.7.7", "@summa-tx/bitcoin-spv-sol": "^2.2.0", "@summa-tx/relay-sol": "^1.1.0", "@truffle/hdwallet-provider": "^1.0.26", From c73bb350d79705e136658fed063c64a0662aaff2 Mon Sep 17 00:00:00 2001 From: NicholasDotSol Date: Tue, 4 Feb 2020 21:19:16 -0600 Subject: [PATCH 04/17] Remove funder bond requirement --- implementation/contracts/deposit/DepositFunding.sol | 1 - 1 file changed, 1 deletion(-) diff --git a/implementation/contracts/deposit/DepositFunding.sol b/implementation/contracts/deposit/DepositFunding.sol index 235e1eecc..8f13c12ab 100644 --- a/implementation/contracts/deposit/DepositFunding.sol +++ b/implementation/contracts/deposit/DepositFunding.sol @@ -57,7 +57,6 @@ library DepositFunding { require(_system.getAllowNewDeposits(), "Opening new deposits is currently disabled."); require(_d.inStart(), "Deposit setup already requested"); /* solium-disable-next-line value-in-payable */ - require(msg.value == TBTCConstants.getFunderBondAmount(), "incorrect funder bond amount"); require(_system.isAllowedLotSize(_lotSize), "provided lot size not supported"); // TODO: Whole value is stored as funder bond in the deposit, but part // of it should be transferred to keep: https://github.com/keep-network/tbtc/issues/297 From a4aaccf7d8cb1fe19d02a9da5ffc9d8870471e6f Mon Sep 17 00:00:00 2001 From: NicholasDotSol Date: Tue, 4 Feb 2020 21:31:01 -0600 Subject: [PATCH 05/17] Remove keepAddress param These functions don't take an address parameter anymore --- implementation/contracts/deposit/DepositLiquidation.sol | 2 +- implementation/contracts/deposit/DepositUtils.sol | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/implementation/contracts/deposit/DepositLiquidation.sol b/implementation/contracts/deposit/DepositLiquidation.sol index 0bce8d1a8..4de9f4fa1 100644 --- a/implementation/contracts/deposit/DepositLiquidation.sol +++ b/implementation/contracts/deposit/DepositLiquidation.sol @@ -39,7 +39,7 @@ library DepositLiquidation { bytes memory _preimage ) public returns (bool _isFraud) { IBondedECDSAKeep _keep = IBondedECDSAKeep(_d.keepAddress); - return _keep.submitSignatureFraud(_d.keepAddress, _v, _r, _s, _signedDigest, _preimage); + return _keep.submitSignatureFraud(_v, _r, _s, _signedDigest, _preimage); } /// @notice Determines the collateralization percentage of the signing group diff --git a/implementation/contracts/deposit/DepositUtils.sol b/implementation/contracts/deposit/DepositUtils.sol index 656f98f81..03dbd6b14 100644 --- a/implementation/contracts/deposit/DepositUtils.sol +++ b/implementation/contracts/deposit/DepositUtils.sol @@ -317,7 +317,7 @@ library DepositUtils { /// @return The amount of bonded ETH in wei function fetchBondAmount(Deposit storage _d) public view returns (uint256) { IBondedECDSAKeep _keep = IBondedECDSAKeep(_d.keepAddress); - return _keep.checkBondAmount(_d.keepAddress); + return _keep.checkBondAmount(); } /// @notice Convert a LE bytes8 to a uint256 @@ -372,7 +372,7 @@ library DepositUtils { function seizeSignerBonds(Deposit storage _d) internal returns (uint256) { uint256 _preCallBalance = address(this).balance; IBondedECDSAKeep _keep = IBondedECDSAKeep(_d.keepAddress); - _keep.seizeSignerBonds(_d.keepAddress); + _keep.seizeSignerBonds(); uint256 _postCallBalance = address(this).balance; require(_postCallBalance > _preCallBalance, "No funds received, unexpected"); return _postCallBalance.sub(_preCallBalance); From c2bcca12791d061cf332ec0b62d33a339ad0539f Mon Sep 17 00:00:00 2001 From: NicholasDotSol Date: Wed, 5 Feb 2020 01:33:53 -0600 Subject: [PATCH 06/17] Keep dependency version bump stub updates --- implementation/test/contracts/keep/ECDSAKeepFactoryStub.sol | 4 ++++ implementation/test/contracts/keep/ECDSAKeepStub.sol | 5 ++--- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/implementation/test/contracts/keep/ECDSAKeepFactoryStub.sol b/implementation/test/contracts/keep/ECDSAKeepFactoryStub.sol index b3b8b73fd..3d9e16e9f 100644 --- a/implementation/test/contracts/keep/ECDSAKeepFactoryStub.sol +++ b/implementation/test/contracts/keep/ECDSAKeepFactoryStub.sol @@ -15,4 +15,8 @@ contract ECDSAKeepFactoryStub is IBondedECDSAKeepFactory { keepOwner = _owner; return keepAddress; } + function openKeepFeeEstimate() external returns (uint256){ + return 0; + } + } \ No newline at end of file diff --git a/implementation/test/contracts/keep/ECDSAKeepStub.sol b/implementation/test/contracts/keep/ECDSAKeepStub.sol index d171c5805..5d45cc0ff 100644 --- a/implementation/test/contracts/keep/ECDSAKeepStub.sol +++ b/implementation/test/contracts/keep/ECDSAKeepStub.sol @@ -55,7 +55,6 @@ contract ECDSAKeepStub is IBondedECDSAKeep { // Functions implemented for IBondedECDSAKeep interface. function submitSignatureFraud( - address, uint8, bytes32, bytes32, @@ -65,11 +64,11 @@ contract ECDSAKeepStub is IBondedECDSAKeep { return success; } - function checkBondAmount(address) external view returns (uint256){ + function checkBondAmount() external view returns (uint256){ return bondAmount; } - function seizeSignerBonds(address) external returns (bool){ + function seizeSignerBonds() external returns (bool){ if (address(this).balance > 0) { msg.sender.transfer(address(this).balance); } From 0f89ca025f98e4c3eed25c013c8ab71f37cf4096 Mon Sep 17 00:00:00 2001 From: NicholasDotSol Date: Wed, 5 Feb 2020 01:35:30 -0600 Subject: [PATCH 07/17] forward value to requestNewKeep --- implementation/contracts/deposit/DepositFunding.sol | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/implementation/contracts/deposit/DepositFunding.sol b/implementation/contracts/deposit/DepositFunding.sol index 8f13c12ab..e5db08fd5 100644 --- a/implementation/contracts/deposit/DepositFunding.sol +++ b/implementation/contracts/deposit/DepositFunding.sol @@ -62,7 +62,7 @@ library DepositFunding { // of it should be transferred to keep: https://github.com/keep-network/tbtc/issues/297 _d.lotSizeSatoshis = _lotSize; uint256 _bondRequirement = _lotSize.mul(_system.getInitialCollateralizedPercent()).div(100); - _d.keepAddress = _system.requestNewKeep(_m, _n, _bondRequirement); + _d.keepAddress = _system.requestNewKeep.value(msg.value)(_m, _n, _bondRequirement); _d.signerFeeDivisor = _system.getSignerFeeDivisor(); _d.undercollateralizedThresholdPercent = _system.getUndercollateralizedThresholdPercent(); _d.severelyUndercollateralizedThresholdPercent = _system.getSeverelyUndercollateralizedThresholdPercent(); From 58bd191cb511502481eb060e0f581612cb2c09f7 Mon Sep 17 00:00:00 2001 From: NicholasDotSol Date: Wed, 5 Feb 2020 01:39:46 -0600 Subject: [PATCH 08/17] Update deposit factory tests - Check that value is forwarded properly - update variable naming --- implementation/test/DepositFactoryTest.js | 37 +++++++++++------------ 1 file changed, 17 insertions(+), 20 deletions(-) diff --git a/implementation/test/DepositFactoryTest.js b/implementation/test/DepositFactoryTest.js index 99e7e011d..ea82089c2 100644 --- a/implementation/test/DepositFactoryTest.js +++ b/implementation/test/DepositFactoryTest.js @@ -11,16 +11,17 @@ const Deposit = artifacts.require('Deposit') const TestDeposit = artifacts.require('TestDeposit') contract('DepositFactory', () => { - const funderBondAmount = new BN('10').pow(new BN('5')) + const openKeepFee = new BN('10').pow(new BN('5')) const fullBtc = 100000000 describe('createDeposit()', async () => { let depositFactory - + let tbtcSystemStub before(async () => { // To properly test createDeposit, we deploy the real Deposit contract and // make sure we don't get hit by the ACL hammer. ({ + tbtcSystemStub, depositFactory, } = await deployTestDeposit([], { 'TestDeposit': Deposit })) }) @@ -30,12 +31,12 @@ contract('DepositFactory', () => { await depositFactory.createDeposit( fullBtc, - { value: funderBondAmount } + { value: openKeepFee } ) await depositFactory.createDeposit( fullBtc, - { value: funderBondAmount } + { value: openKeepFee } ) const eventList = await depositFactory.getPastEvents('DepositCloneCreated', { fromBlock: blockNumber, toBlock: 'latest' }) @@ -46,24 +47,20 @@ contract('DepositFactory', () => { assert.notEqual(eventList[0].returnValues.depositCloneAddress, eventList[1].returnValues.depositCloneAddress, 'clone addresses should not be equal') }) - it('correctly forwards value to Deposit', async () => { - const blockNumber = await web3.eth.getBlockNumber() + it('correctly forwards value to Keep', async () => { + // Check for tbtcSystemStub receiving value since it does not + // implement a value forward to keep. + const initialBalance = await web3.eth.getBalance(tbtcSystemStub.address) await depositFactory.createDeposit( fullBtc, - { value: funderBondAmount } + { value: openKeepFee } ) - const eventList = await depositFactory.getPastEvents( - 'DepositCloneCreated', - { - fromBlock: blockNumber, - toBlock: 'latest', - }) - const depositAddress = eventList[eventList.length - 1].returnValues.depositCloneAddress + const finalBalance = await web3.eth.getBalance(tbtcSystemStub.address) + const balanceCheck = new BN(finalBalance).sub(new BN(initialBalance)) - const balance = await web3.eth.getBalance(depositAddress) - assert.equal(balance, funderBondAmount, 'Factory did not correctly forward value on Deposit creation') + expect(balanceCheck, 'Factory did not correctly forward value on Deposit creation').to.eq.BN(openKeepFee) }) }) @@ -95,12 +92,12 @@ contract('DepositFactory', () => { await depositFactory.createDeposit( fullBtc, - { value: funderBondAmount } + { value: openKeepFee } ) await depositFactory.createDeposit( fullBtc, - { value: funderBondAmount } + { value: openKeepFee } ) const eventList = await depositFactory.getPastEvents('DepositCloneCreated', { fromBlock: blockNumber, toBlock: 'latest' }) @@ -155,7 +152,7 @@ contract('DepositFactory', () => { 1, 1, fullBtc, - { value: funderBondAmount } + { value: openKeepFee } ) await testDeposit.setKeepAddress(keep.address) @@ -171,7 +168,7 @@ contract('DepositFactory', () => { await depositFactory.createDeposit( fullBtc, - { value: funderBondAmount } + { value: openKeepFee } ) const eventList = await depositFactory.getPastEvents('DepositCloneCreated', { fromBlock: blockNumber, toBlock: 'latest' }) From 4bcb12210cd78e7f4a58ff31599c37c9891e4790 Mon Sep 17 00:00:00 2001 From: NicholasDotSol Date: Wed, 5 Feb 2020 01:44:16 -0600 Subject: [PATCH 09/17] sol:lint fix solium-disable value-in-payable --- implementation/contracts/deposit/DepositFunding.sol | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/implementation/contracts/deposit/DepositFunding.sol b/implementation/contracts/deposit/DepositFunding.sol index e5db08fd5..b5bd24a4c 100644 --- a/implementation/contracts/deposit/DepositFunding.sol +++ b/implementation/contracts/deposit/DepositFunding.sol @@ -56,12 +56,12 @@ library DepositFunding { require(_system.getAllowNewDeposits(), "Opening new deposits is currently disabled."); require(_d.inStart(), "Deposit setup already requested"); - /* solium-disable-next-line value-in-payable */ require(_system.isAllowedLotSize(_lotSize), "provided lot size not supported"); // TODO: Whole value is stored as funder bond in the deposit, but part // of it should be transferred to keep: https://github.com/keep-network/tbtc/issues/297 _d.lotSizeSatoshis = _lotSize; uint256 _bondRequirement = _lotSize.mul(_system.getInitialCollateralizedPercent()).div(100); + /* solium-disable-next-line value-in-payable */ _d.keepAddress = _system.requestNewKeep.value(msg.value)(_m, _n, _bondRequirement); _d.signerFeeDivisor = _system.getSignerFeeDivisor(); _d.undercollateralizedThresholdPercent = _system.getUndercollateralizedThresholdPercent(); From 027e50720d720957b9a19378b4ab5e4e7cea9555 Mon Sep 17 00:00:00 2001 From: Jakub Nowakowski Date: Wed, 5 Feb 2020 16:02:06 +0100 Subject: [PATCH 10/17] Updated tests for value forwarding --- implementation/test/DepositFactoryTest.js | 24 +++++++++++++++-------- implementation/test/TBTCSystemTest.js | 9 +++++++++ 2 files changed, 25 insertions(+), 8 deletions(-) diff --git a/implementation/test/DepositFactoryTest.js b/implementation/test/DepositFactoryTest.js index ea82089c2..6a79bf661 100644 --- a/implementation/test/DepositFactoryTest.js +++ b/implementation/test/DepositFactoryTest.js @@ -10,6 +10,8 @@ const ECDSAKeepStub = artifacts.require('ECDSAKeepStub') const Deposit = artifacts.require('Deposit') const TestDeposit = artifacts.require('TestDeposit') +const TBTCSystem = artifacts.require('TBTCSystem') + contract('DepositFactory', () => { const openKeepFee = new BN('10').pow(new BN('5')) const fullBtc = 100000000 @@ -47,20 +49,26 @@ contract('DepositFactory', () => { assert.notEqual(eventList[0].returnValues.depositCloneAddress, eventList[1].returnValues.depositCloneAddress, 'clone addresses should not be equal') }) - it('correctly forwards value to Keep', async () => { - // Check for tbtcSystemStub receiving value since it does not - // implement a value forward to keep. - const initialBalance = await web3.eth.getBalance(tbtcSystemStub.address) + it('correctly forwards value to keep factory', async () => { + let depositFactory + let ecdsaKeepFactoryStub + + // Use real TBTCSystem contract to validate value forwarding: + // DepositFactory -> Deposit -> TBTCSystem -> ECDSAKeepFactory + ({ + depositFactory, + ecdsaKeepFactoryStub, + } = await deployTestDeposit([], { 'TestDeposit': Deposit, 'TBTCSystemStub': TBTCSystem })) await depositFactory.createDeposit( fullBtc, { value: openKeepFee } ) - const finalBalance = await web3.eth.getBalance(tbtcSystemStub.address) - const balanceCheck = new BN(finalBalance).sub(new BN(initialBalance)) - - expect(balanceCheck, 'Factory did not correctly forward value on Deposit creation').to.eq.BN(openKeepFee) + expect( + await web3.eth.getBalance(ecdsaKeepFactoryStub.address), + 'Factory did not correctly forward value on Deposit creation' + ).to.eq.BN(openKeepFee) }) }) diff --git a/implementation/test/TBTCSystemTest.js b/implementation/test/TBTCSystemTest.js index 92c2d73e1..743fdf693 100644 --- a/implementation/test/TBTCSystemTest.js +++ b/implementation/test/TBTCSystemTest.js @@ -50,6 +50,15 @@ contract('TBTCSystem', (accounts) => { assert.equal(expectedKeepAddress, result, 'incorrect keep address') }) + + it('forwards value to keep factory', async () => { + const openKeepFee = new BN(99) + + await tbtcSystem.requestNewKeep(5, 10, 0, { value: openKeepFee }) + + const finalBalance = await web3.eth.getBalance(ecdsaKeepFactory.address) + expect(finalBalance, 'TBTCSystem did not correctly forward value to keep factory').to.eq.BN(openKeepFee) + }) }) describe('setSignerFeeDivisor', async () => { From aaebd8140f464f82c5031c99bea3261db217964d Mon Sep 17 00:00:00 2001 From: Jakub Nowakowski Date: Wed, 5 Feb 2020 16:04:16 +0100 Subject: [PATCH 11/17] Added updated package-lock.json --- implementation/package-lock.json | 26 ++++++++++++++++++++------ 1 file changed, 20 insertions(+), 6 deletions(-) diff --git a/implementation/package-lock.json b/implementation/package-lock.json index 500fc1357..a4f0ee82a 100644 --- a/implementation/package-lock.json +++ b/implementation/package-lock.json @@ -53,9 +53,9 @@ } }, "@keep-network/keep-ecdsa": { - "version": "0.7.5", - "resolved": "https://npm.pkg.github.com/download/@keep-network/keep-ecdsa/0.7.5/cddc7b12c93d137580f6b207ea5f70750464ec5db8062dc3845127cb2cd57b72", - "integrity": "sha512-La2nTtNlLFzHsZ+GjofakqfCoxnM2fPK9tsuBNBg5/mlgwicJDfNVCinThbOvo8vELY8cPzL80drACYbEemAuQ==", + "version": "0.7.7", + "resolved": "https://npm.pkg.github.com/download/@keep-network/keep-ecdsa/0.7.7/862f4948210eb9a6111234bfbf16d341765c30e1c8cf790245063e61b4558198", + "integrity": "sha512-mrmg66eLcBJZp42SiBdqPMaKGd8f1g9ILCM4JJQd+UFsqYQmWSQdl/yhQPOd+i+m48f9xWmTO8kXR3snl7CB9A==", "requires": { "@keep-network/sortition-pools": "git+https://github.com/keep-network/sortition-pools.git#915ac518ee31fc3c53a5dec50d5d84262553046e", "openzeppelin-solidity": "2.3.0", @@ -71,9 +71,9 @@ } }, "@openzeppelin/contracts": { - "version": "2.4.0", - "resolved": "https://registry.npmjs.org/@openzeppelin/contracts/-/contracts-2.4.0.tgz", - "integrity": "sha512-xeKP59REgow5TPBJh3S9BRIm7DDG+Rz3Nt4ANWGUkjk4305DHpyUD5CyMJ6nd2JMmZuFyx4mjvvlCtSJLRnN6w==" + "version": "2.5.0", + "resolved": "https://registry.npmjs.org/@openzeppelin/contracts/-/contracts-2.5.0.tgz", + "integrity": "sha512-t3jm8FrhL9tkkJTofkznTqo/XXdHi21w5yXwalEnaMOp22ZwZ0f/mmKdlgMMLPFa6bSVHbY88mKESwJT/7m5Lg==" }, "@sindresorhus/is": { "version": "0.14.0", @@ -85,6 +85,15 @@ "resolved": "https://registry.npmjs.org/@summa-tx/bitcoin-spv-sol/-/bitcoin-spv-sol-2.2.0.tgz", "integrity": "sha512-HSfxfNldNS4pvZUjLLYqw/wgVrxmIzp67WoRVTxY5SEV7TbOBcn8aYlPCnyLSUX6TeHjMVjAv3inDoIq0zwfCQ==" }, + "@summa-tx/relay-sol": { + "version": "1.1.0", + "resolved": "https://registry.npmjs.org/@summa-tx/relay-sol/-/relay-sol-1.1.0.tgz", + "integrity": "sha512-NXWvDRfH0YbEanY8oSK8sEShKI7QO0b3VkJyKRiGJfeGrjenBj13l59gbi+A2/27Q+UD0X+sBQ+BxDjUcInHBw==", + "requires": { + "@summa-tx/bitcoin-spv-sol": "^2.1.0", + "dotenv": "^8.1.0" + } + }, "@szmarczak/http-timer": { "version": "1.1.2", "resolved": "https://registry.npmjs.org/@szmarczak/http-timer/-/http-timer-1.1.2.tgz", @@ -2395,6 +2404,11 @@ "resolved": "https://registry.npmjs.org/dom-walk/-/dom-walk-0.1.1.tgz", "integrity": "sha1-ZyIm3HTI95mtNTB9+TaroRrNYBg=" }, + "dotenv": { + "version": "8.2.0", + "resolved": "https://registry.npmjs.org/dotenv/-/dotenv-8.2.0.tgz", + "integrity": "sha512-8sJ78ElpbDJBHNeBzUbUVLsqKdccaa/BXF1uPTw3GrvQTBgrQrtObr2mUrE38vzYd8cEv+m/JBfDLioYcfXoaw==" + }, "drbg.js": { "version": "1.0.1", "resolved": "https://registry.npmjs.org/drbg.js/-/drbg.js-1.0.1.tgz", From f51b89ca3aacf835e92a79cc93fa5a7ef3f6f007 Mon Sep 17 00:00:00 2001 From: Jakub Nowakowski Date: Wed, 5 Feb 2020 16:08:50 +0100 Subject: [PATCH 12/17] Fixed linting - variables in DepositFactoryTest --- implementation/test/DepositFactoryTest.js | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/implementation/test/DepositFactoryTest.js b/implementation/test/DepositFactoryTest.js index 6a79bf661..65cd65674 100644 --- a/implementation/test/DepositFactoryTest.js +++ b/implementation/test/DepositFactoryTest.js @@ -18,7 +18,7 @@ contract('DepositFactory', () => { describe('createDeposit()', async () => { let depositFactory - let tbtcSystemStub + before(async () => { // To properly test createDeposit, we deploy the real Deposit contract and // make sure we don't get hit by the ACL hammer. @@ -50,9 +50,6 @@ contract('DepositFactory', () => { }) it('correctly forwards value to keep factory', async () => { - let depositFactory - let ecdsaKeepFactoryStub - // Use real TBTCSystem contract to validate value forwarding: // DepositFactory -> Deposit -> TBTCSystem -> ECDSAKeepFactory ({ From adf65d00a5aa39edfc790f57b2914464fbd2b07a Mon Sep 17 00:00:00 2001 From: NicholasDotSol Date: Wed, 5 Feb 2020 10:29:20 -0600 Subject: [PATCH 13/17] Fix keep value forwarding test --- implementation/test/DepositFactoryTest.js | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/implementation/test/DepositFactoryTest.js b/implementation/test/DepositFactoryTest.js index 65cd65674..9781760d4 100644 --- a/implementation/test/DepositFactoryTest.js +++ b/implementation/test/DepositFactoryTest.js @@ -23,7 +23,6 @@ contract('DepositFactory', () => { // To properly test createDeposit, we deploy the real Deposit contract and // make sure we don't get hit by the ACL hammer. ({ - tbtcSystemStub, depositFactory, } = await deployTestDeposit([], { 'TestDeposit': Deposit })) }) @@ -51,17 +50,18 @@ contract('DepositFactory', () => { it('correctly forwards value to keep factory', async () => { // Use real TBTCSystem contract to validate value forwarding: - // DepositFactory -> Deposit -> TBTCSystem -> ECDSAKeepFactory + // DepositFactory -> Deposit -> TBTCSystem -> ECDSAKeepFactory + let ecdsaKeepFactoryStub; ({ - depositFactory, ecdsaKeepFactoryStub, + depositFactory, } = await deployTestDeposit([], { 'TestDeposit': Deposit, 'TBTCSystemStub': TBTCSystem })) await depositFactory.createDeposit( fullBtc, { value: openKeepFee } ) - + // const factory = await ECDSAKeepVendorStub.selectFactory() expect( await web3.eth.getBalance(ecdsaKeepFactoryStub.address), 'Factory did not correctly forward value on Deposit creation' From a6c21533c0fbbc6fbb26b733ff770f63f77e436b Mon Sep 17 00:00:00 2001 From: NicholasDotSol Date: Wed, 5 Feb 2020 10:32:43 -0600 Subject: [PATCH 14/17] Remove payable from createNewDepositFeeEstimate --- implementation/contracts/system/TBTCSystem.sol | 1 - 1 file changed, 1 deletion(-) diff --git a/implementation/contracts/system/TBTCSystem.sol b/implementation/contracts/system/TBTCSystem.sol index 27452c481..877e3b3c0 100644 --- a/implementation/contracts/system/TBTCSystem.sol +++ b/implementation/contracts/system/TBTCSystem.sol @@ -221,7 +221,6 @@ contract TBTCSystem is Ownable, ITBTCSystem, DepositLog { /// @return Uint256 estimate. function createNewDepositFeeEstimate() external - payable returns (uint256) { IBondedECDSAKeepVendor _keepVendor = IBondedECDSAKeepVendor(keepVendor); From a1ab0f97926390014a7490c9ce123dc7eac34c3c Mon Sep 17 00:00:00 2001 From: NicholasDotSol Date: Wed, 5 Feb 2020 10:48:28 -0600 Subject: [PATCH 15/17] remove old comment --- implementation/test/DepositFactoryTest.js | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/implementation/test/DepositFactoryTest.js b/implementation/test/DepositFactoryTest.js index 9781760d4..97dff12df 100644 --- a/implementation/test/DepositFactoryTest.js +++ b/implementation/test/DepositFactoryTest.js @@ -50,7 +50,7 @@ contract('DepositFactory', () => { it('correctly forwards value to keep factory', async () => { // Use real TBTCSystem contract to validate value forwarding: - // DepositFactory -> Deposit -> TBTCSystem -> ECDSAKeepFactory + // DepositFactory -> Deposit -> TBTCSystem -> ECDSAKeepFactory let ecdsaKeepFactoryStub; ({ ecdsaKeepFactoryStub, @@ -61,7 +61,6 @@ contract('DepositFactory', () => { fullBtc, { value: openKeepFee } ) - // const factory = await ECDSAKeepVendorStub.selectFactory() expect( await web3.eth.getBalance(ecdsaKeepFactoryStub.address), 'Factory did not correctly forward value on Deposit creation' From da063be4026e73eedf47f0ae7a5bffbfeb5a5fc3 Mon Sep 17 00:00:00 2001 From: NicholasDotSol Date: Wed, 5 Feb 2020 14:10:09 -0600 Subject: [PATCH 16/17] Update openKeepFeeEstimate with non-zero value --- .../test/contracts/keep/ECDSAKeepFactoryStub.sol | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/implementation/test/contracts/keep/ECDSAKeepFactoryStub.sol b/implementation/test/contracts/keep/ECDSAKeepFactoryStub.sol index 3d9e16e9f..6cc81ea5a 100644 --- a/implementation/test/contracts/keep/ECDSAKeepFactoryStub.sol +++ b/implementation/test/contracts/keep/ECDSAKeepFactoryStub.sol @@ -5,6 +5,7 @@ import {IBondedECDSAKeepFactory} from "@keep-network/keep-ecdsa/contracts/api/IB contract ECDSAKeepFactoryStub is IBondedECDSAKeepFactory { address public keepOwner; address public keepAddress = address(888); + uint256 feeEstimate = 123456; function openKeep( uint256, @@ -12,11 +13,17 @@ contract ECDSAKeepFactoryStub is IBondedECDSAKeepFactory { address _owner, uint256 ) external payable returns (address) { + require(msg.value >= feeEstimate, "Insufficient value for new keep creation"); keepOwner = _owner; return keepAddress; } + function openKeepFeeEstimate() external returns (uint256){ - return 0; + return feeEstimate; + } + + function setOpenKeepFeeEstimate(uint256 _fee) external { + feeEstimate = _fee; } } \ No newline at end of file From 6d92d5f3e662f48faa8aade80e67a03b9357dcd1 Mon Sep 17 00:00:00 2001 From: NicholasDotSol Date: Wed, 5 Feb 2020 14:11:08 -0600 Subject: [PATCH 17/17] Update tests Fix tests that rely on keep value logic --- implementation/test/DepositFactoryTest.js | 16 ++++++++++++++-- implementation/test/TBTCSystemTest.js | 13 +++++++++---- 2 files changed, 23 insertions(+), 6 deletions(-) diff --git a/implementation/test/DepositFactoryTest.js b/implementation/test/DepositFactoryTest.js index 97dff12df..a1c38aa5b 100644 --- a/implementation/test/DepositFactoryTest.js +++ b/implementation/test/DepositFactoryTest.js @@ -1,4 +1,5 @@ import deployTestDeposit from './helpers/deployTestDeposit' +import expectThrow from './helpers/expectThrow' import BN from 'bn.js' import utils from './utils' @@ -13,11 +14,12 @@ const TestDeposit = artifacts.require('TestDeposit') const TBTCSystem = artifacts.require('TBTCSystem') contract('DepositFactory', () => { - const openKeepFee = new BN('10').pow(new BN('5')) + const openKeepFee = new BN('123456') // set in ECDAKeepFactory const fullBtc = 100000000 describe('createDeposit()', async () => { let depositFactory + let ecdsaKeepFactoryStub before(async () => { // To properly test createDeposit, we deploy the real Deposit contract and @@ -51,7 +53,6 @@ contract('DepositFactory', () => { it('correctly forwards value to keep factory', async () => { // Use real TBTCSystem contract to validate value forwarding: // DepositFactory -> Deposit -> TBTCSystem -> ECDSAKeepFactory - let ecdsaKeepFactoryStub; ({ ecdsaKeepFactoryStub, depositFactory, @@ -66,6 +67,17 @@ contract('DepositFactory', () => { 'Factory did not correctly forward value on Deposit creation' ).to.eq.BN(openKeepFee) }) + + it('reverts if insufficient fee is provided', async () => { + const badOpenKeepFee = openKeepFee.sub(new BN(1)) + await expectThrow( + depositFactory.createDeposit( + fullBtc, + { value: badOpenKeepFee } + ), + 'Insufficient value for new keep creation' + ) + }) }) describe('clone state', async () => { diff --git a/implementation/test/TBTCSystemTest.js b/implementation/test/TBTCSystemTest.js index 743fdf693..3ef4a6ba0 100644 --- a/implementation/test/TBTCSystemTest.js +++ b/implementation/test/TBTCSystemTest.js @@ -34,10 +34,14 @@ contract('TBTCSystem', (accounts) => { }) describe('requestNewKeep()', async () => { + let openKeepFee + before(async () => { + openKeepFee = await ecdsaKeepFactory.openKeepFeeEstimate.call() + }) it('sends caller as owner to open new keep', async () => { const expectedKeepOwner = accounts[2] - await tbtcSystem.requestNewKeep(5, 10, 0, { from: expectedKeepOwner }) + await tbtcSystem.requestNewKeep(5, 10, 0, { from: expectedKeepOwner, value: openKeepFee }) const keepOwner = await ecdsaKeepFactory.keepOwner.call() assert.equal(expectedKeepOwner, keepOwner, 'incorrect keep owner address') @@ -46,18 +50,19 @@ contract('TBTCSystem', (accounts) => { it('returns keep address', async () => { const expectedKeepAddress = await ecdsaKeepFactory.keepAddress.call() - const result = await tbtcSystem.requestNewKeep.call(5, 10, 0) + const result = await tbtcSystem.requestNewKeep.call(5, 10, 0, { value: openKeepFee }) assert.equal(expectedKeepAddress, result, 'incorrect keep address') }) it('forwards value to keep factory', async () => { - const openKeepFee = new BN(99) + const initialBalance = await web3.eth.getBalance(ecdsaKeepFactory.address) await tbtcSystem.requestNewKeep(5, 10, 0, { value: openKeepFee }) const finalBalance = await web3.eth.getBalance(ecdsaKeepFactory.address) - expect(finalBalance, 'TBTCSystem did not correctly forward value to keep factory').to.eq.BN(openKeepFee) + const balanceCheck = new BN(finalBalance).sub(new BN(initialBalance)) + expect(balanceCheck, 'TBTCSystem did not correctly forward value to keep factory').to.eq.BN(openKeepFee) }) })