From b5f912330763aa71139a723d4f31cec7fe898822 Mon Sep 17 00:00:00 2001 From: Jakub Nowakowski Date: Thu, 16 Jan 2020 17:41:43 +0100 Subject: [PATCH 1/6] Changed bond ID to a hash We changed the way we calculate bond ID to end with a 32 byte hash to save storage. --- solidity/contracts/KeepBonding.sol | 9 +++++---- solidity/test/contracts/KeepBondingStub.sol | 2 +- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/solidity/contracts/KeepBonding.sol b/solidity/contracts/KeepBonding.sol index 3d583a69b..6a1459467 100644 --- a/solidity/contracts/KeepBonding.sol +++ b/solidity/contracts/KeepBonding.sol @@ -5,8 +5,9 @@ pragma solidity ^0.5.4; contract KeepBonding { // Unassigned ether values deposited by operators. mapping(address => uint256) internal unbondedValue; - // References to created bonds. - mapping(bytes => uint256) internal lockedBonds; + // References to created bonds. Bond identifier is built from operator's + // address, holder's address and reference assigned on bond creation. + mapping(bytes32 => uint256) internal lockedBonds; /// @notice Returns value of ether available for bonding for the operator. /// @param operator Address of the operator. @@ -24,7 +25,7 @@ contract KeepBonding { /// @notice Draw amount from sender's value available for bonding. /// @param amount Value to withdraw. /// @param destination Address to send the amount to. - function withdraw(uint256 amount, address payable destination) external { + function withdraw(uint256 amount, address payable destination) public { require(availableBondingValue(msg.sender) >= amount, "Insufficient unbonded value"); unbondedValue[msg.sender] -= amount; @@ -41,7 +42,7 @@ contract KeepBonding { require(availableBondingValue(operator) >= amount, "Insufficient unbonded value"); address holder = msg.sender; - bytes memory bondID = abi.encodePacked(operator, holder, referenceID); + bytes32 bondID = keccak256(abi.encodePacked(operator, holder, referenceID)); require(lockedBonds[bondID] == 0, "Reference ID not unique for holder and operator"); diff --git a/solidity/test/contracts/KeepBondingStub.sol b/solidity/test/contracts/KeepBondingStub.sol index 3db6c7d89..3f0d1203e 100644 --- a/solidity/test/contracts/KeepBondingStub.sol +++ b/solidity/test/contracts/KeepBondingStub.sol @@ -10,7 +10,7 @@ contract KeepBondingStub is KeepBonding { /// @dev This is a stub implementation to validate bonds mapping. /// @return Value assigned in the locked bond mapping. function getLockedBonds(address holder, address operator, uint256 referenceID) public view returns (uint256) { - bytes memory bondID = abi.encodePacked(operator, holder, referenceID); + bytes32 bondID = keccak256(abi.encodePacked(operator, holder, referenceID)); return lockedBonds[bondID]; } } From eedc7a840482d28aed9b85909faddc6fac3f1783 Mon Sep 17 00:00:00 2001 From: Jakub Nowakowski Date: Fri, 17 Jan 2020 14:14:01 +0100 Subject: [PATCH 2/6] Implemented seizeBond function Function is used to seize a bond and transfer amount to holder's account. --- solidity/contracts/KeepBonding.sol | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/solidity/contracts/KeepBonding.sol b/solidity/contracts/KeepBonding.sol index 6a1459467..b5ba9e3a1 100644 --- a/solidity/contracts/KeepBonding.sol +++ b/solidity/contracts/KeepBonding.sol @@ -50,6 +50,22 @@ contract KeepBonding { lockedBonds[bondID] += amount; } + /// @notice Transfers a bond to the holder's account. + /// @dev Function requires that a caller is the holder of the bond which is + /// being seized. + /// @param operator Address of the bonded operator. + /// @param referenceID Reference ID of the bond. + /// @param amount Amount to be seized. + function seizeBond(address operator, uint256 referenceID, uint256 amount) public { + address payable holder = msg.sender; + bytes32 bondID = keccak256(abi.encodePacked(operator, holder, referenceID)); + + require(lockedBonds[bondID] >= amount, "Requested amount is greater than the bond"); + + lockedBonds[bondID] -= amount; + holder.transfer(amount); + } + /// @notice Checks if the caller is an authorized contract. /// @dev Throws an error if called by any account other than one of the authorized /// contracts. From 37d5492a154da55b5ae37e975b0a84900729e4a4 Mon Sep 17 00:00:00 2001 From: Jakub Nowakowski Date: Fri, 17 Jan 2020 14:15:24 +0100 Subject: [PATCH 3/6] Added tests for seize bond function --- solidity/test/KeepBondingTest.js | 71 ++++++++++++++++++++++++++++++++ 1 file changed, 71 insertions(+) diff --git a/solidity/test/KeepBondingTest.js b/solidity/test/KeepBondingTest.js index 7d9867b74..ba6f43d90 100644 --- a/solidity/test/KeepBondingTest.js +++ b/solidity/test/KeepBondingTest.js @@ -167,4 +167,75 @@ contract('KeepBonding', (accounts) => { ) }) }) + describe('seizeBond', async () => { + const operator = accounts[1] + const holder = accounts[2] + const bondValue = new BN(100) + const reference = 777 + + beforeEach(async () => { + await keepBonding.deposit(operator, { value: bondValue }) + await keepBonding.createBond(operator, reference, bondValue, { from: holder }) + }) + + it('transfers whole bond amount to holder\'s account', async () => { + const amount = bondValue + let expectedBalance = web3.utils.toBN(await web3.eth.getBalance(holder)).add(amount) + + const tx = await keepBonding.seizeBond(operator, reference, amount, { from: holder }) + + const gasPrice = web3.utils.toBN(await web3.eth.getGasPrice()) + const txCost = gasPrice.mul(web3.utils.toBN(tx.receipt.gasUsed)) + expectedBalance = expectedBalance.sub(txCost) + + const actualBalance = await web3.eth.getBalance(holder) + expect(actualBalance).to.eq.BN(expectedBalance, 'invalid holder\'s account balance') + + const lockedBonds = await keepBonding.getLockedBonds(holder, operator, reference) + expect(lockedBonds).to.eq.BN(0, 'invalid locked bonds') + }) + + it('transfers less than bond amount to holder\'s account', async () => { + const remainingBond = new BN(1) + const amount = bondValue.sub(remainingBond) + let expectedBalance = web3.utils.toBN(await web3.eth.getBalance(holder)).add(amount) + + const tx = await keepBonding.seizeBond(operator, reference, amount, { from: holder }) + + const gasPrice = web3.utils.toBN(await web3.eth.getGasPrice()) + const txCost = gasPrice.mul(web3.utils.toBN(tx.receipt.gasUsed)) + expectedBalance = expectedBalance.sub(txCost) + + const actualBalance = await web3.eth.getBalance(holder) + expect(actualBalance).to.eq.BN(expectedBalance, 'invalid holder\'s account balance') + + const lockedBonds = await keepBonding.getLockedBonds(holder, operator, reference) + expect(lockedBonds).to.eq.BN(remainingBond, 'invalid locked bonds') + }) + + it('accepts seized amount equal zero', async () => { + const amount = new BN(0) + let expectedBalance = web3.utils.toBN(await web3.eth.getBalance(holder)) + + const tx = await keepBonding.seizeBond(operator, reference, amount, { from: holder }) + + const gasPrice = web3.utils.toBN(await web3.eth.getGasPrice()) + const txCost = gasPrice.mul(web3.utils.toBN(tx.receipt.gasUsed)) + expectedBalance = expectedBalance.sub(txCost) + + const actualBalance = await web3.eth.getBalance(holder) + expect(actualBalance).to.eq.BN(expectedBalance, 'invalid holder\'s account balance') + + const lockedBonds = await keepBonding.getLockedBonds(holder, operator, reference) + expect(lockedBonds).to.eq.BN(bondValue, 'invalid locked bonds') + }) + + it('fails if seized amount is greater than bond value', async () => { + const amount = bondValue.add(new BN(1)) + await expectRevert( + keepBonding.seizeBond(operator, reference, amount, { from: holder }), + "Requested amount is greater than the bond" + ) + }) + }) }) From b7d41acb5bd9d437714283040aea337efd9d05f3 Mon Sep 17 00:00:00 2001 From: Jakub Nowakowski Date: Wed, 22 Jan 2020 09:46:55 +0100 Subject: [PATCH 4/6] Updated docs in bonding contract --- solidity/contracts/KeepBonding.sol | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/solidity/contracts/KeepBonding.sol b/solidity/contracts/KeepBonding.sol index b5ba9e3a1..eae4e90eb 100644 --- a/solidity/contracts/KeepBonding.sol +++ b/solidity/contracts/KeepBonding.sol @@ -6,7 +6,7 @@ contract KeepBonding { // Unassigned ether values deposited by operators. mapping(address => uint256) internal unbondedValue; // References to created bonds. Bond identifier is built from operator's - // address, holder's address and reference assigned on bond creation. + // address, holder's address and reference ID assigned on bond creation. mapping(bytes32 => uint256) internal lockedBonds; /// @notice Returns value of ether available for bonding for the operator. @@ -50,7 +50,8 @@ contract KeepBonding { lockedBonds[bondID] += amount; } - /// @notice Transfers a bond to the holder's account. + /// @notice Seizes the bond by moving some or all of a locked bond to holder's + /// account. /// @dev Function requires that a caller is the holder of the bond which is /// being seized. /// @param operator Address of the bonded operator. From faa53fa906f6793170d214f7f5a708a247df231b Mon Sep 17 00:00:00 2001 From: Jakub Nowakowski Date: Wed, 22 Jan 2020 09:50:33 +0100 Subject: [PATCH 5/6] Updated test error messages --- solidity/test/KeepBondingTest.js | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/solidity/test/KeepBondingTest.js b/solidity/test/KeepBondingTest.js index ba6f43d90..deff0ccca 100644 --- a/solidity/test/KeepBondingTest.js +++ b/solidity/test/KeepBondingTest.js @@ -118,7 +118,7 @@ contract('KeepBonding', (accounts) => { expect(unbonded).to.eq.BN(expectedUnbonded, 'invalid unbonded value') const lockedBonds = await keepBonding.getLockedBonds(holder, operator, reference) - expect(lockedBonds).to.eq.BN(value, 'invalid locked bonds') + expect(lockedBonds).to.eq.BN(value, 'unexpected bond value') }) it('creates two bonds with the same reference for different operators', async () => { @@ -140,10 +140,10 @@ contract('KeepBonding', (accounts) => { expect(unbonded2).to.eq.BN(expectedUnbonded, 'invalid unbonded value 2') const lockedBonds1 = await keepBonding.getLockedBonds(holder, operator, reference) - expect(lockedBonds1).to.eq.BN(bondValue, 'invalid locked bonds') + expect(lockedBonds1).to.eq.BN(bondValue, 'unexpected bond value 1') const lockedBonds2 = await keepBonding.getLockedBonds(holder, operator2, reference) - expect(lockedBonds2).to.eq.BN(bondValue, 'invalid locked bonds') + expect(lockedBonds2).to.eq.BN(bondValue, 'unexpected bond value 2') }) it('fails to create two bonds with the same reference for the same operator', async () => { @@ -192,7 +192,7 @@ contract('KeepBonding', (accounts) => { expect(actualBalance).to.eq.BN(expectedBalance, 'invalid holder\'s account balance') const lockedBonds = await keepBonding.getLockedBonds(holder, operator, reference) - expect(lockedBonds).to.eq.BN(0, 'invalid locked bonds') + expect(lockedBonds).to.eq.BN(0, 'unexpected remaining bond value') }) it('transfers less than bond amount to holder\'s account', async () => { @@ -210,7 +210,7 @@ contract('KeepBonding', (accounts) => { expect(actualBalance).to.eq.BN(expectedBalance, 'invalid holder\'s account balance') const lockedBonds = await keepBonding.getLockedBonds(holder, operator, reference) - expect(lockedBonds).to.eq.BN(remainingBond, 'invalid locked bonds') + expect(lockedBonds).to.eq.BN(remainingBond, 'unexpected remaining bond value') }) it('accepts seized amount equal zero', async () => { @@ -227,7 +227,7 @@ contract('KeepBonding', (accounts) => { expect(actualBalance).to.eq.BN(expectedBalance, 'invalid holder\'s account balance') const lockedBonds = await keepBonding.getLockedBonds(holder, operator, reference) - expect(lockedBonds).to.eq.BN(bondValue, 'invalid locked bonds') + expect(lockedBonds).to.eq.BN(bondValue, 'unexpected remaining bond value') }) it('fails if seized amount is greater than bond value', async () => { From fc1c72c6a0b751b22a9e679aad97635b6448793f Mon Sep 17 00:00:00 2001 From: Jakub Nowakowski Date: Wed, 22 Jan 2020 10:05:35 +0100 Subject: [PATCH 6/6] Require seized bond > 0 We require that requested value of bond to seize is greater than zero. Accepting zero doesn't make sense. --- solidity/contracts/KeepBonding.sol | 4 +++- solidity/test/KeepBondingTest.js | 19 +++++-------------- 2 files changed, 8 insertions(+), 15 deletions(-) diff --git a/solidity/contracts/KeepBonding.sol b/solidity/contracts/KeepBonding.sol index bbd752c35..594c10a85 100644 --- a/solidity/contracts/KeepBonding.sol +++ b/solidity/contracts/KeepBonding.sol @@ -76,7 +76,7 @@ contract KeepBonding { lockedBonds[bondID] = 0; } - /// @notice Seizes the bond by moving some or all of a locked bond to holder's + /// @notice Seizes the bond by moving some or all of a locked bond to holder's /// account. /// @dev Function requires that a caller is the holder of the bond which is /// being seized. @@ -84,6 +84,8 @@ contract KeepBonding { /// @param referenceID Reference ID of the bond. /// @param amount Amount to be seized. function seizeBond(address operator, uint256 referenceID, uint256 amount) public { + require(amount > 0, "Requested amount should be greater than zero"); + address payable holder = msg.sender; bytes32 bondID = keccak256(abi.encodePacked(operator, holder, referenceID)); diff --git a/solidity/test/KeepBondingTest.js b/solidity/test/KeepBondingTest.js index 81ef2cd3c..93ecf5ba1 100644 --- a/solidity/test/KeepBondingTest.js +++ b/solidity/test/KeepBondingTest.js @@ -281,21 +281,12 @@ contract('KeepBonding', (accounts) => { expect(lockedBonds).to.eq.BN(remainingBond, 'unexpected remaining bond value') }) - it('accepts seized amount equal zero', async () => { + it('fails if seized amount equals zero', async () => { const amount = new BN(0) - let expectedBalance = web3.utils.toBN(await web3.eth.getBalance(holder)) - - const tx = await keepBonding.seizeBond(operator, reference, amount, { from: holder }) - - const gasPrice = web3.utils.toBN(await web3.eth.getGasPrice()) - const txCost = gasPrice.mul(web3.utils.toBN(tx.receipt.gasUsed)) - expectedBalance = expectedBalance.sub(txCost) - - const actualBalance = await web3.eth.getBalance(holder) - expect(actualBalance).to.eq.BN(expectedBalance, 'invalid holder\'s account balance') - - const lockedBonds = await keepBonding.getLockedBonds(holder, operator, reference) - expect(lockedBonds).to.eq.BN(bondValue, 'unexpected remaining bond value') + await expectRevert( + keepBonding.seizeBond(operator, reference, amount, { from: holder }), + "Requested amount should be greater than zero" + ) }) it('fails if seized amount is greater than bond value', async () => {