diff --git a/implementation/contracts/deposit/DepositFunding.sol b/implementation/contracts/deposit/DepositFunding.sol index 235e1eecc..b5bd24a4c 100644 --- a/implementation/contracts/deposit/DepositFunding.sol +++ b/implementation/contracts/deposit/DepositFunding.sol @@ -56,14 +56,13 @@ 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 _d.lotSizeSatoshis = _lotSize; uint256 _bondRequirement = _lotSize.mul(_system.getInitialCollateralizedPercent()).div(100); - _d.keepAddress = _system.requestNewKeep(_m, _n, _bondRequirement); + /* 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(); _d.severelyUndercollateralizedThresholdPercent = _system.getSeverelyUndercollateralizedThresholdPercent(); 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); diff --git a/implementation/contracts/system/TBTCSystem.sol b/implementation/contracts/system/TBTCSystem.sol index 80c17a590..877e3b3c0 100644 --- a/implementation/contracts/system/TBTCSystem.sol +++ b/implementation/contracts/system/TBTCSystem.sol @@ -217,6 +217,17 @@ 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 + 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. @@ -228,6 +239,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); } } 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", 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", diff --git a/implementation/test/DepositFactoryTest.js b/implementation/test/DepositFactoryTest.js index 99e7e011d..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' @@ -10,12 +11,15 @@ const ECDSAKeepStub = artifacts.require('ECDSAKeepStub') const Deposit = artifacts.require('Deposit') const TestDeposit = artifacts.require('TestDeposit') +const TBTCSystem = artifacts.require('TBTCSystem') + contract('DepositFactory', () => { - const funderBondAmount = 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 @@ -30,12 +34,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 +50,33 @@ 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 factory', async () => { + // Use real TBTCSystem contract to validate value forwarding: + // DepositFactory -> Deposit -> TBTCSystem -> ECDSAKeepFactory + ({ + ecdsaKeepFactoryStub, + depositFactory, + } = await deployTestDeposit([], { 'TestDeposit': Deposit, 'TBTCSystemStub': TBTCSystem })) 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 + expect( + await web3.eth.getBalance(ecdsaKeepFactoryStub.address), + 'Factory did not correctly forward value on Deposit creation' + ).to.eq.BN(openKeepFee) + }) - const balance = await web3.eth.getBalance(depositAddress) - assert.equal(balance, funderBondAmount, 'Factory did not correctly forward value on Deposit creation') + 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' + ) }) }) @@ -95,12 +108,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 +168,7 @@ contract('DepositFactory', () => { 1, 1, fullBtc, - { value: funderBondAmount } + { value: openKeepFee } ) await testDeposit.setKeepAddress(keep.address) @@ -171,7 +184,7 @@ contract('DepositFactory', () => { await depositFactory.createDeposit( fullBtc, - { value: funderBondAmount } + { value: openKeepFee } ) const eventList = await depositFactory.getPastEvents('DepositCloneCreated', { fromBlock: blockNumber, toBlock: 'latest' }) diff --git a/implementation/test/TBTCSystemTest.js b/implementation/test/TBTCSystemTest.js index 92c2d73e1..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,10 +50,20 @@ 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 initialBalance = await web3.eth.getBalance(ecdsaKeepFactory.address) + + await tbtcSystem.requestNewKeep(5, 10, 0, { value: openKeepFee }) + + const finalBalance = await web3.eth.getBalance(ecdsaKeepFactory.address) + const balanceCheck = new BN(finalBalance).sub(new BN(initialBalance)) + expect(balanceCheck, 'TBTCSystem did not correctly forward value to keep factory').to.eq.BN(openKeepFee) + }) }) describe('setSignerFeeDivisor', async () => { diff --git a/implementation/test/contracts/keep/ECDSAKeepFactoryStub.sol b/implementation/test/contracts/keep/ECDSAKeepFactoryStub.sol index b3b8b73fd..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,7 +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 feeEstimate; + } + + function setOpenKeepFeeEstimate(uint256 _fee) external { + feeEstimate = _fee; + } + } \ 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); }