From 5be4138814426d4668b926e7cc897105066be5f1 Mon Sep 17 00:00:00 2001 From: Darko Kolev Date: Tue, 5 Jul 2022 09:18:03 +0300 Subject: [PATCH 1/3] added days modifier in period setters --- .../smart-contracts/src/contracts/ERC20EscrowToPay.sol | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/packages/smart-contracts/src/contracts/ERC20EscrowToPay.sol b/packages/smart-contracts/src/contracts/ERC20EscrowToPay.sol index a961db772..8c03944fe 100644 --- a/packages/smart-contracts/src/contracts/ERC20EscrowToPay.sol +++ b/packages/smart-contracts/src/contracts/ERC20EscrowToPay.sol @@ -151,13 +151,19 @@ contract ERC20EscrowToPay is Ownable { revert('not payable receive'); } + /** + * @notice Sets duration of emergency period, with minimum value of 30 days. + */ function setEmergencyClaimPeriod(uint256 _emergencyClaimPeriod) external onlyOwner { - require(_emergencyClaimPeriod >= 30, 'emergency period too short'); + require(_emergencyClaimPeriod >= 30 days, 'emergency period too short'); emergencyClaimPeriod = _emergencyClaimPeriod; } + /** + * @notice Sets duration of freeze period, with minimum value of 30 days. + */ function setFrozenPeriod(uint256 _frozenPeriod) external onlyOwner { - require(_frozenPeriod >= 30, 'frozen period too short'); + require(_frozenPeriod >= 30 days, 'frozen period too short'); frozenPeriod = _frozenPeriod; } From 6c5198a5b1a021d5c42dd5cc1ec5af57593b4318 Mon Sep 17 00:00:00 2001 From: Darko Kolev Date: Tue, 5 Jul 2022 09:23:21 +0300 Subject: [PATCH 2/3] formatting fix --- packages/smart-contracts/src/contracts/ERC20EscrowToPay.sol | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/smart-contracts/src/contracts/ERC20EscrowToPay.sol b/packages/smart-contracts/src/contracts/ERC20EscrowToPay.sol index 8c03944fe..cda270218 100644 --- a/packages/smart-contracts/src/contracts/ERC20EscrowToPay.sol +++ b/packages/smart-contracts/src/contracts/ERC20EscrowToPay.sol @@ -152,7 +152,7 @@ contract ERC20EscrowToPay is Ownable { } /** - * @notice Sets duration of emergency period, with minimum value of 30 days. + * @notice Sets duration of emergency period, with minimum value of 30 days. */ function setEmergencyClaimPeriod(uint256 _emergencyClaimPeriod) external onlyOwner { require(_emergencyClaimPeriod >= 30 days, 'emergency period too short'); @@ -160,7 +160,7 @@ contract ERC20EscrowToPay is Ownable { } /** - * @notice Sets duration of freeze period, with minimum value of 30 days. + * @notice Sets duration of freeze period, with minimum value of 30 days. */ function setFrozenPeriod(uint256 _frozenPeriod) external onlyOwner { require(_frozenPeriod >= 30 days, 'frozen period too short'); From 3a20ad07ee779b004661c03e4af6ee98ee01ccee Mon Sep 17 00:00:00 2001 From: Darko Kolev Date: Tue, 5 Jul 2022 17:21:42 +0300 Subject: [PATCH 3/3] fixed and improved escrow contract tests --- .../test/contracts/ERC20EscrowToPay.test.ts | 30 ++++++++++++++----- 1 file changed, 22 insertions(+), 8 deletions(-) diff --git a/packages/smart-contracts/test/contracts/ERC20EscrowToPay.test.ts b/packages/smart-contracts/test/contracts/ERC20EscrowToPay.test.ts index 0f3ac5148..4cca90f13 100644 --- a/packages/smart-contracts/test/contracts/ERC20EscrowToPay.test.ts +++ b/packages/smart-contracts/test/contracts/ERC20EscrowToPay.test.ts @@ -259,19 +259,33 @@ describe('Contract: ERC20EscrowToPay', () => { }); }); describe('Admin should be able to change emergency and freeze periods', () => { - it('Should be able to change emergency period', async () => { + it('Minimum emergency period is 30 days', async () => { const twoDaysInSeconds = 3600 * 24 * 2; - await erc20EscrowToPay.connect(admin).setEmergencyClaimPeriod(twoDaysInSeconds); + expect( + erc20EscrowToPay.connect(admin).setEmergencyClaimPeriod(twoDaysInSeconds), + ).to.be.revertedWith('emergency period too short'); + }); + + it('Minimum frozen period is 30 days', async () => { + const twentyNineDaysInSeconds = 3600 * 24 * 29; + expect( + erc20EscrowToPay.connect(admin).setFrozenPeriod(twentyNineDaysInSeconds), + ).to.be.revertedWith('frozen period too short'); + }); + + it('Should be able to adjust emergency period', async () => { + const thirtyOneDaysInSeconds = 3600 * 24 * 31; + await erc20EscrowToPay.connect(admin).setEmergencyClaimPeriod(thirtyOneDaysInSeconds); const contractEmergencyPeriod = await erc20EscrowToPay.connect(payee).emergencyClaimPeriod(); - expect(contractEmergencyPeriod).to.be.equal(twoDaysInSeconds); + expect(contractEmergencyPeriod).to.be.equal(thirtyOneDaysInSeconds); }); - it('Should be able to adjust freeze period, only admin', async () => { - const twoDaysInSeconds = 3600 * 24 * 2; - await erc20EscrowToPay.connect(admin).setFrozenPeriod(twoDaysInSeconds); + it('Should be able to adjust freeze period', async () => { + const thirtyOneDaysInSeconds = 3600 * 24 * 31; + await erc20EscrowToPay.connect(admin).setFrozenPeriod(thirtyOneDaysInSeconds); const contractFreezePeriod = await erc20EscrowToPay.connect(payee).frozenPeriod(); - expect(contractFreezePeriod).to.be.equal(twoDaysInSeconds); + expect(contractFreezePeriod).to.be.equal(thirtyOneDaysInSeconds); }); - it('Contract creator should not be able to change periods', async () => { + it('Contract creator who is not the owner, should not be able to change periods', async () => { expect(erc20EscrowToPay.connect(owner).setEmergencyClaimPeriod(100)).to.be.revertedWith( 'Ownable: caller is not the owner', );