From cee1be7730601a8445a23927e31ae668e3c08222 Mon Sep 17 00:00:00 2001 From: boolafish Date: Wed, 2 Oct 2019 14:14:37 +0900 Subject: [PATCH] feat: take care of BondSize boundary number feat: block ability of setting lowerBoundDivisor to 0 feat: BondSize fail the call when trying setting to 0 Bond size would be stuck at 0 and become non-upgradeable if it ever goes to 0. Also, it makes no sense to have bond size 0 logically. test: BondSize boundary limits add tests with uint128 limit and 0 limit --- .../contracts/mocks/utils/BondSizeMock.sol | 4 +- .../contracts/src/exits/utils/BondSize.sol | 44 +-- .../test/src/utils/BondSize.test.js | 268 ++++++++++++------ 3 files changed, 214 insertions(+), 102 deletions(-) diff --git a/plasma_framework/contracts/mocks/utils/BondSizeMock.sol b/plasma_framework/contracts/mocks/utils/BondSizeMock.sol index c28375b9b..f4114cb9b 100644 --- a/plasma_framework/contracts/mocks/utils/BondSizeMock.sol +++ b/plasma_framework/contracts/mocks/utils/BondSizeMock.sol @@ -7,8 +7,8 @@ contract BondSizeMock { BondSize.Params public bond; - constructor (uint128 _initialBondSize, uint16 _lowerBoundDivisor, uint16 _upperBoundMultiplier) public { - bond = BondSize.buildParams(_initialBondSize, _lowerBoundDivisor, _upperBoundMultiplier); + constructor (uint128 initialBondSize, uint16 lowerBoundDivisor, uint16 upperBoundMultiplier) public { + bond = BondSize.buildParams(initialBondSize, lowerBoundDivisor, upperBoundMultiplier); } function bondSize() public view returns (uint128) { diff --git a/plasma_framework/contracts/src/exits/utils/BondSize.sol b/plasma_framework/contracts/src/exits/utils/BondSize.sol index 6307de41d..5b6fc1a1e 100644 --- a/plasma_framework/contracts/src/exits/utils/BondSize.sol +++ b/plasma_framework/contracts/src/exits/utils/BondSize.sol @@ -26,52 +26,56 @@ library BondSize { uint16 upperBoundMultiplier; } - function buildParams(uint128 _initialBondSize, uint16 _lowerBoundDivisor, uint16 _upperBoundMultiplier) + function buildParams(uint128 initialBondSize, uint16 lowerBoundDivisor, uint16 upperBoundMultiplier) internal pure returns (Params memory) { + require(initialBondSize > 0, "initialBondSize cannot be 0"); + require(lowerBoundDivisor > 0, "lowerBoundDivisor cannot be 0"); + require(upperBoundMultiplier > 0, "upperBoundMultiplier cannot be 0"); + // Set the initial value to far in the future uint128 initialEffectiveUpdateTime = 2 ** 63; return Params({ - previousBondSize: _initialBondSize, + previousBondSize: initialBondSize, updatedBondSize: 0, effectiveUpdateTime: initialEffectiveUpdateTime, - lowerBoundDivisor: _lowerBoundDivisor, - upperBoundMultiplier: _upperBoundMultiplier + lowerBoundDivisor: lowerBoundDivisor, + upperBoundMultiplier: upperBoundMultiplier }); } /** * @notice Updates the bond size. - * @notice The new value is bounded by 0.5 and 2x of current bond size. - * @notice There is a waiting period of 2 days before the new value goes into effect. + * @dev There is a waiting period of 2 days before the new value goes into effect. * @param newBondSize the new bond size. */ - function updateBondSize(Params storage _self, uint128 newBondSize) internal { - validateBondSize(_self, newBondSize); + function updateBondSize(Params storage self, uint128 newBondSize) internal { + validateBondSize(self, newBondSize); - if (_self.updatedBondSize != 0 && now > _self.effectiveUpdateTime) { - _self.previousBondSize = _self.updatedBondSize; + if (self.updatedBondSize != 0 && now > self.effectiveUpdateTime) { + self.previousBondSize = self.updatedBondSize; } - _self.updatedBondSize = newBondSize; - _self.effectiveUpdateTime = uint64(now) + WAITING_PERIOD; + self.updatedBondSize = newBondSize; + self.effectiveUpdateTime = uint64(now) + WAITING_PERIOD; } /** * @notice Returns the current bond size. */ - function bondSize(Params memory _self) internal view returns (uint128) { - if (now < _self.effectiveUpdateTime) { - return _self.previousBondSize; + function bondSize(Params memory self) internal view returns (uint128) { + if (now < self.effectiveUpdateTime) { + return self.previousBondSize; } else { - return _self.updatedBondSize; + return self.updatedBondSize; } } - function validateBondSize(Params memory _self, uint128 newBondSize) private view { - uint128 currentBondSize = bondSize(_self); - require(newBondSize >= currentBondSize / _self.lowerBoundDivisor, "Bond size is too low"); - require(uint256(newBondSize) <= uint256(currentBondSize) * _self.upperBoundMultiplier, "Bond size is too high"); + function validateBondSize(Params memory self, uint128 newBondSize) private view { + uint128 currentBondSize = bondSize(self); + require(newBondSize > 0, "Bond size cannot be zero"); + require(newBondSize >= currentBondSize / self.lowerBoundDivisor, "Bond size is too low"); + require(uint256(newBondSize) <= uint256(currentBondSize) * self.upperBoundMultiplier, "Bond size is too high"); } } diff --git a/plasma_framework/test/src/utils/BondSize.test.js b/plasma_framework/test/src/utils/BondSize.test.js index 0914d5602..7b071bf76 100644 --- a/plasma_framework/test/src/utils/BondSize.test.js +++ b/plasma_framework/test/src/utils/BondSize.test.js @@ -6,87 +6,195 @@ contract('BondSize', () => { const WAITING_PERIOD = time.duration.days(2); const HALF_WAITING_PERIOD = WAITING_PERIOD.divn(2); - beforeEach(async () => { - this.initialBondSize = new BN(20000000000); - this.lowerBoundDivisor = 2; - this.upperBoundMultiplier = 3; - this.contract = await BondSizeMock.new( - this.initialBondSize, - this.lowerBoundDivisor, - this.upperBoundMultiplier, - ); + describe('buildParams', () => { + it('should fail when setting initialBondSize to 0', async () => { + const initialBondSize = new BN(0); + const lowerBoundDivisor = 2; + const upperBoundMultiplier = 3; + + // BondSizeMock constructor would call buildParams under the hood + await expectRevert( + BondSizeMock.new( + initialBondSize, + lowerBoundDivisor, + upperBoundMultiplier, + ), + 'initialBondSize cannot be 0', + ); + }); + + it('should fail when setting lowerBoundDivisor to 0', async () => { + const initialBondSize = new BN(20000000000); + const lowerBoundDivisor = 0; + const upperBoundMultiplier = 3; + + // BondSizeMock constructor would call buildParams under the hood + await expectRevert( + BondSizeMock.new( + initialBondSize, + lowerBoundDivisor, + upperBoundMultiplier, + ), + 'lowerBoundDivisor cannot be 0', + ); + }); + + it('should fail when setting upperBoundMultiplier to 0', async () => { + const initialBondSize = new BN(20000000000); + const lowerBoundDivisor = 2; + const upperBoundMultiplier = 0; + + // BondSizeMock constructor would call buildParams under the hood + await expectRevert( + BondSizeMock.new( + initialBondSize, + lowerBoundDivisor, + upperBoundMultiplier, + ), + 'upperBoundMultiplier cannot be 0', + ); + }); }); - it('should return the initial bond size', async () => { - const bondSize = await this.contract.bondSize(); - expect(bondSize).to.be.bignumber.equal(this.initialBondSize); - }); - - it('should be able to update the bond to upperBoundMultiplier times its current value', async () => { - const newBondSize = new BN(this.initialBondSize).muln(this.upperBoundMultiplier); - await this.contract.updateBondSize(newBondSize); - }); - - it('should fail to update bond to more than upperBoundMultiplier times its current value', async () => { - const newBondSize = new BN(this.initialBondSize).muln(this.upperBoundMultiplier).addn(1); - await expectRevert( - this.contract.updateBondSize(newBondSize), - 'Bond size is too high', - ); - }); - - it('should be able to update the bond to lowerBoundDivisor of its current value', async () => { - const newBondSize = new BN(this.initialBondSize).divn(this.lowerBoundDivisor); - await this.contract.updateBondSize(newBondSize); - }); - - it('should fail to update bond to less than lowerBoundDivisor of its current value', async () => { - const newBondSize = new BN(this.initialBondSize).divn(this.lowerBoundDivisor).subn(1); - await expectRevert( - this.contract.updateBondSize(newBondSize), - 'Bond size is too low', - ); - }); - - it('should not update the actual bond value until after the waiting period', async () => { - const newBondSize = new BN(this.initialBondSize).muln(2); - await this.contract.updateBondSize(newBondSize); - - await time.increase(WAITING_PERIOD.sub(time.duration.seconds(5))); - const bondSize = await this.contract.bondSize(); - expect(bondSize).to.be.bignumber.equal(this.initialBondSize); - }); - - it('should update the actual bond value after the waiting period', async () => { - const newBondSize = new BN(this.initialBondSize).muln(2); - await this.contract.updateBondSize(newBondSize); - - await time.increase(WAITING_PERIOD); - const bondSize = await this.contract.bondSize(); - expect(bondSize).to.be.bignumber.equal(newBondSize); - }); - - it('should update the actual bond value after the waiting period', async () => { - const newBondSize = new BN(this.initialBondSize).muln(2); - await this.contract.updateBondSize(newBondSize); - - // Wait half the waiting period - await time.increase(HALF_WAITING_PERIOD); - - // Update again while the first update is in progress. - const secondNewBondSize = new BN(this.initialBondSize).muln(1.5); - await this.contract.updateBondSize(secondNewBondSize); - - // Wait half the waiting period again. - await time.increase(HALF_WAITING_PERIOD); - // Even though the first update's waiting period is over, the second - // update is in progress so the bond size should not have changed yet. - let bondSize = await this.contract.bondSize(); - expect(bondSize).to.be.bignumber.equal(this.initialBondSize); - - // Wait the remaining waiting period - await time.increase(HALF_WAITING_PERIOD); - bondSize = await this.contract.bondSize(); - expect(bondSize).to.be.bignumber.equal(secondNewBondSize); + describe('updateBondSize', () => { + describe('in general...', () => { + beforeEach(async () => { + this.initialBondSize = new BN(20000000000); + this.lowerBoundDivisor = 2; + this.upperBoundMultiplier = 3; + this.contract = await BondSizeMock.new( + this.initialBondSize, + this.lowerBoundDivisor, + this.upperBoundMultiplier, + ); + }); + + it('should return the initial bond size', async () => { + const bondSize = await this.contract.bondSize(); + expect(bondSize).to.be.bignumber.equal(this.initialBondSize); + }); + + it('should be able to update the bond to upperBoundMultiplier times its current value', async () => { + const newBondSize = new BN(this.initialBondSize).muln(this.upperBoundMultiplier); + await this.contract.updateBondSize(newBondSize); + }); + + it('should fail to update bond to more than upperBoundMultiplier times its current value', async () => { + const newBondSize = new BN(this.initialBondSize).muln(this.upperBoundMultiplier).addn(1); + await expectRevert( + this.contract.updateBondSize(newBondSize), + 'Bond size is too high', + ); + }); + + it('should be able to update the bond to lowerBoundDivisor of its current value', async () => { + const newBondSize = new BN(this.initialBondSize).divn(this.lowerBoundDivisor); + await this.contract.updateBondSize(newBondSize); + }); + + it('should fail to update bond to less than lowerBoundDivisor of its current value', async () => { + const newBondSize = new BN(this.initialBondSize).divn(this.lowerBoundDivisor).subn(1); + await expectRevert( + this.contract.updateBondSize(newBondSize), + 'Bond size is too low', + ); + }); + + it('should not update the actual bond value until after the waiting period', async () => { + const newBondSize = new BN(this.initialBondSize).muln(2); + await this.contract.updateBondSize(newBondSize); + + await time.increase(WAITING_PERIOD.sub(time.duration.seconds(5))); + const bondSize = await this.contract.bondSize(); + expect(bondSize).to.be.bignumber.equal(this.initialBondSize); + }); + + it('should update the actual bond value after the waiting period', async () => { + const newBondSize = new BN(this.initialBondSize).muln(2); + await this.contract.updateBondSize(newBondSize); + + await time.increase(WAITING_PERIOD); + const bondSize = await this.contract.bondSize(); + expect(bondSize).to.be.bignumber.equal(newBondSize); + }); + + it('should update the actual bond value after the waiting period', async () => { + const newBondSize = new BN(this.initialBondSize).muln(2); + await this.contract.updateBondSize(newBondSize); + + // Wait half the waiting period + await time.increase(HALF_WAITING_PERIOD); + + // Update again while the first update is in progress. + const secondNewBondSize = new BN(this.initialBondSize).muln(1.5); + await this.contract.updateBondSize(secondNewBondSize); + + // Wait half the waiting period again. + await time.increase(HALF_WAITING_PERIOD); + // Even though the first update's waiting period is over, the second + // update is in progress so the bond size should not have changed yet. + let bondSize = await this.contract.bondSize(); + expect(bondSize).to.be.bignumber.equal(this.initialBondSize); + + // Wait the remaining waiting period + await time.increase(HALF_WAITING_PERIOD); + bondSize = await this.contract.bondSize(); + expect(bondSize).to.be.bignumber.equal(secondNewBondSize); + }); + }); + + describe('with boundary size of numbers', () => { + it('should able to update to max number of uint128 without having overflow issue', async () => { + const maxSizeOfUint256 = (new BN(2)).pow(new BN(128)).sub(new BN(1)); // 2^128 - 1 + + const initialBondSize = maxSizeOfUint256.sub(new BN(10000)); + const lowerBoundDivisor = 2; + const upperBoundMultiplier = 3; + const contract = await BondSizeMock.new( + initialBondSize, + lowerBoundDivisor, + upperBoundMultiplier, + ); + + await contract.updateBondSize(maxSizeOfUint256); + await time.increase(WAITING_PERIOD + 1); + const bondSize = await contract.bondSize(); + expect(bondSize).to.be.bignumber.equal(maxSizeOfUint256); + }); + + it('should able to update to 1', async () => { + const initialBondSize = new BN(2); + const lowerBoundDivisor = 2; + const upperBoundMultiplier = 3; + const contract = await BondSizeMock.new( + initialBondSize, + lowerBoundDivisor, + upperBoundMultiplier, + ); + const bondSizeOne = new BN(1); + + await contract.updateBondSize(bondSizeOne); + await time.increase(WAITING_PERIOD + 1); + const bondSize = await contract.bondSize(); + expect(bondSize).to.be.bignumber.equal(bondSizeOne); + }); + + it('should NOT be able to update to 0', async () => { + const initialBondSize = new BN(2); + const lowerBoundDivisor = 2; + const upperBoundMultiplier = 3; + const contract = await BondSizeMock.new( + initialBondSize, + lowerBoundDivisor, + upperBoundMultiplier, + ); + const bondSizeZero = new BN(0); + + await expectRevert( + contract.updateBondSize(bondSizeZero), + 'Bond size cannot be zero', + ); + }); + }); }); });