From 246873acf91a8bda8400207732a3a8d5d9695543 Mon Sep 17 00:00:00 2001 From: Marc Doerflinger Date: Wed, 10 Aug 2022 19:46:01 +0000 Subject: [PATCH 1/6] Add unit test to configure two products with same riskpool --- contracts/test/TestCoin.sol | 19 ++++++++- scripts/product.py | 2 +- tests/conftest.py | 5 +++ tests/test_treasury_module.py | 80 +++++++++++++++++++++++++++++++++-- 4 files changed, 101 insertions(+), 5 deletions(-) diff --git a/contracts/test/TestCoin.sol b/contracts/test/TestCoin.sol index 8bf1859..446cd76 100644 --- a/contracts/test/TestCoin.sol +++ b/contracts/test/TestCoin.sol @@ -18,4 +18,21 @@ contract TestCoin is ERC20 { INITIAL_SUPPLY ); } -} \ No newline at end of file +} + +contract TestCoinX is ERC20 { + + string public constant NAME = "Test Dummy X"; + string public constant SYMBOL = "TDX"; + + uint256 public constant INITIAL_SUPPLY = 10**24; + + constructor() + ERC20(NAME, SYMBOL) + { + _mint( + _msgSender(), + INITIAL_SUPPLY + ); + } +} diff --git a/scripts/product.py b/scripts/product.py index 52eb11f..dac49c3 100644 --- a/scripts/product.py +++ b/scripts/product.py @@ -239,4 +239,4 @@ def getContract(self) -> TestProduct: return self.product def getPolicy(self, policyId: str): - return self.policy.getPolicy(policyId) \ No newline at end of file + return self.policy.getPolicy(policyId) diff --git a/tests/conftest.py b/tests/conftest.py index 2b7dbd3..9d81b69 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -25,6 +25,7 @@ InstanceOperatorService, InstanceService, TestCoin, + TestCoinX, TestRiskpool, TestOracle, TestProduct, @@ -238,6 +239,10 @@ def registry(registryController, owner) -> RegistryController: def testCoin(owner) -> TestCoin: return TestCoin.deploy({'from': owner}) +@pytest.fixture(scope="module") +def testCoinX(owner) -> TestCoinX: + return TestCoinX.deploy({'from': owner}) + @pytest.fixture(scope="module") def bundleToken(owner) -> BundleToken: return BundleToken.deploy({'from': owner}) diff --git a/tests/test_treasury_module.py b/tests/test_treasury_module.py index 4c81a71..d8c36f5 100644 --- a/tests/test_treasury_module.py +++ b/tests/test_treasury_module.py @@ -18,7 +18,7 @@ def test_bundle_creation_with_instance_wallet_not_set( capitalOwner: Account, ): withRiskpoolWallet = True - (gifProduct, gifRiskpool) = getProductAndRiskpool( + (gifProduct, gifRiskpool, gifOracle) = getProductAndRiskpool( instanceNoInstanceWallet, owner, testCoin, @@ -61,7 +61,7 @@ def test_bundle_creation_with_riskpool_wallet_not_set( feeOwner: Account, ): withRiskpoolWallet = False - (gifProduct, gifRiskpool) = getProductAndRiskpool( + (gifProduct, gifRiskpool, gifOracle) = getProductAndRiskpool( instance, owner, testCoin, @@ -94,6 +94,79 @@ def test_bundle_creation_with_riskpool_wallet_not_set( {'from': bundleOwner}) +def test_two_products_different_coin_same_riskpool( + instance: GifInstance, + owner: Account, + testCoinX, + productOwner: Account, + oracleProvider: Account, + riskpoolKeeper: Account, + capitalOwner: Account, +): + withRiskpoolWallet = True + (gifProduct, gifRiskpool, gifOracle) = getProductAndRiskpool( + instance, + owner, + testCoinX, + productOwner, + oracleProvider, + riskpoolKeeper, + capitalOwner, + withRiskpoolWallet + ) + + product = gifProduct.getContract() + riskpool = gifRiskpool.getContract() + riskpoolId = riskpool.getId() + + with brownie.reverts("ERROR:TRS-013:RISKPOOL_TOKEN_ALREADY_SET"): + GifTestProduct( + instance, + testCoinX, + capitalOwner, + productOwner, + gifOracle, + gifRiskpool, + 'Test.Product2') + + +def test_two_products_same_riskpool( + instance: GifInstance, + owner: Account, + testCoin, + productOwner: Account, + oracleProvider: Account, + riskpoolKeeper: Account, + capitalOwner: Account, +): + withRiskpoolWallet = True + (gifProduct, gifRiskpool, gifOracle) = getProductAndRiskpool( + instance, + owner, + testCoin, + productOwner, + oracleProvider, + riskpoolKeeper, + capitalOwner, + withRiskpoolWallet + ) + + product = gifProduct.getContract() + riskpool = gifRiskpool.getContract() + riskpoolId = riskpool.getId() + + GifTestProduct( + instance, + testCoin, + capitalOwner, + productOwner, + gifOracle, + gifRiskpool, + 'Test.Product2') + + # TODO create two ppolicies and check claims work + + def getProductAndRiskpool( instance: GifInstance, owner: Account, @@ -126,5 +199,6 @@ def getProductAndRiskpool( return ( gifProduct, - gifRiskpool + gifRiskpool, + gifOracle ) From 597aa88c7300533fd9e943c703eaa552099d4fdb Mon Sep 17 00:00:00 2001 From: Marc Doerflinger Date: Thu, 11 Aug 2022 07:16:43 +0000 Subject: [PATCH 2/6] Allow to create product on same riskpool only when token match --- contracts/modules/TreasuryModule.sol | 9 ++++++++- tests/test_treasury_module.py | 5 +++-- 2 files changed, 11 insertions(+), 3 deletions(-) diff --git a/contracts/modules/TreasuryModule.sol b/contracts/modules/TreasuryModule.sol index 00e6704..0724170 100644 --- a/contracts/modules/TreasuryModule.sol +++ b/contracts/modules/TreasuryModule.sol @@ -8,6 +8,7 @@ import "./PoolController.sol"; import "../shared/CoreController.sol"; import "@etherisc/gif-interface/contracts/components/IComponent.sol"; +import "@etherisc/gif-interface/contracts/components/IProduct.sol"; import "@etherisc/gif-interface/contracts/modules/IPolicy.sol"; import "@etherisc/gif-interface/contracts/modules/ITreasury.sol"; @@ -72,7 +73,13 @@ contract TreasuryModule is require(address(_componentToken[productId]) == address(0), "ERROR:TRS-012:PRODUCT_TOKEN_ALREADY_SET"); uint256 riskpoolId = _pool.getRiskPoolForProduct(productId); - require(address(_componentToken[riskpoolId]) == address(0), "ERROR:TRS-013:RISKPOOL_TOKEN_ALREADY_SET"); + + // If riskpool token is already set and product token does not match riskpool token, + // then revert (only product with same token are allowed in same riskpool) + if (address(_componentToken[riskpoolId]) != address(0) + && address(_componentToken[riskpoolId]) != address(IProduct(address(component)).getToken())) { + revert("ERROR:TRS-013:TOKEN_ADDRESS_NOT_MACHING"); + } _componentToken[productId] = IERC20(erc20Address); _componentToken[riskpoolId] = IERC20(erc20Address); diff --git a/tests/test_treasury_module.py b/tests/test_treasury_module.py index d8c36f5..0541a95 100644 --- a/tests/test_treasury_module.py +++ b/tests/test_treasury_module.py @@ -97,6 +97,7 @@ def test_bundle_creation_with_riskpool_wallet_not_set( def test_two_products_different_coin_same_riskpool( instance: GifInstance, owner: Account, + testCoin, testCoinX, productOwner: Account, oracleProvider: Account, @@ -107,7 +108,7 @@ def test_two_products_different_coin_same_riskpool( (gifProduct, gifRiskpool, gifOracle) = getProductAndRiskpool( instance, owner, - testCoinX, + testCoin, productOwner, oracleProvider, riskpoolKeeper, @@ -119,7 +120,7 @@ def test_two_products_different_coin_same_riskpool( riskpool = gifRiskpool.getContract() riskpoolId = riskpool.getId() - with brownie.reverts("ERROR:TRS-013:RISKPOOL_TOKEN_ALREADY_SET"): + with brownie.reverts("ERROR:TRS-013:TOKEN_ADDRESS_NOT_MACHING"): GifTestProduct( instance, testCoinX, From 2ddfe40edbfb0e994b3dd209f60daf42bc7f360d Mon Sep 17 00:00:00 2001 From: Marc Doerflinger Date: Thu, 11 Aug 2022 08:07:41 +0000 Subject: [PATCH 3/6] fix unit test by running in isolation and asserting product are different --- tests/test_treasury_module.py | 20 ++++++++++++++------ 1 file changed, 14 insertions(+), 6 deletions(-) diff --git a/tests/test_treasury_module.py b/tests/test_treasury_module.py index 0541a95..5dd6067 100644 --- a/tests/test_treasury_module.py +++ b/tests/test_treasury_module.py @@ -1,4 +1,5 @@ import brownie +import pytest from brownie.network.account import Account @@ -8,6 +9,16 @@ from scripts.product import GifTestOracle, GifTestProduct, GifTestRiskpool from scripts.util import b2s +from scripts.setup import ( + fund_riskpool, + apply_for_policy, +) + +# enforce function isolation for tests below +@pytest.fixture(autouse=True) +def isolation(fn_isolation): + pass + def test_bundle_creation_with_instance_wallet_not_set( instanceNoInstanceWallet: GifInstance, owner: Account, @@ -139,6 +150,7 @@ def test_two_products_same_riskpool( oracleProvider: Account, riskpoolKeeper: Account, capitalOwner: Account, + customer: Account, ): withRiskpoolWallet = True (gifProduct, gifRiskpool, gifOracle) = getProductAndRiskpool( @@ -152,11 +164,7 @@ def test_two_products_same_riskpool( withRiskpoolWallet ) - product = gifProduct.getContract() - riskpool = gifRiskpool.getContract() - riskpoolId = riskpool.getId() - - GifTestProduct( + gifProduct2 = GifTestProduct( instance, testCoin, capitalOwner, @@ -165,7 +173,7 @@ def test_two_products_same_riskpool( gifRiskpool, 'Test.Product2') - # TODO create two ppolicies and check claims work + assert gifProduct2.getContract().getId() != gifProduct.getContract().getId() def getProductAndRiskpool( From 71028b96fd25d99b6024be0563aaeb59f852bb0c Mon Sep 17 00:00:00 2001 From: Marc Doerflinger Date: Thu, 11 Aug 2022 08:21:01 +0000 Subject: [PATCH 4/6] add comments to testcases --- tests/test_treasury_module.py | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/tests/test_treasury_module.py b/tests/test_treasury_module.py index 5dd6067..f1aa57a 100644 --- a/tests/test_treasury_module.py +++ b/tests/test_treasury_module.py @@ -116,6 +116,8 @@ def test_two_products_different_coin_same_riskpool( capitalOwner: Account, ): withRiskpoolWallet = True + + # setup riskpool with a product (gifProduct, gifRiskpool, gifOracle) = getProductAndRiskpool( instance, owner, @@ -127,10 +129,7 @@ def test_two_products_different_coin_same_riskpool( withRiskpoolWallet ) - product = gifProduct.getContract() - riskpool = gifRiskpool.getContract() - riskpoolId = riskpool.getId() - + # ensure that creating of another product with different token is not allowed with brownie.reverts("ERROR:TRS-013:TOKEN_ADDRESS_NOT_MACHING"): GifTestProduct( instance, @@ -153,6 +152,8 @@ def test_two_products_same_riskpool( customer: Account, ): withRiskpoolWallet = True + + # setup riskpool with a product (gifProduct, gifRiskpool, gifOracle) = getProductAndRiskpool( instance, owner, @@ -164,6 +165,7 @@ def test_two_products_same_riskpool( withRiskpoolWallet ) + # ensure that creating of another product with different token succeeds gifProduct2 = GifTestProduct( instance, testCoin, @@ -173,6 +175,7 @@ def test_two_products_same_riskpool( gifRiskpool, 'Test.Product2') + # ensure the two products are different assert gifProduct2.getContract().getId() != gifProduct.getContract().getId() From 32d63253df2172890573326ad845c5e479ad2fcf Mon Sep 17 00:00:00 2001 From: Marc Doerflinger Date: Thu, 11 Aug 2022 08:37:20 +0000 Subject: [PATCH 5/6] fix duplicate error id --- contracts/modules/TreasuryModule.sol | 8 ++++---- tests/test_treasury_module.py | 2 +- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/contracts/modules/TreasuryModule.sol b/contracts/modules/TreasuryModule.sol index 0724170..182face 100644 --- a/contracts/modules/TreasuryModule.sol +++ b/contracts/modules/TreasuryModule.sol @@ -78,7 +78,7 @@ contract TreasuryModule is // then revert (only product with same token are allowed in same riskpool) if (address(_componentToken[riskpoolId]) != address(0) && address(_componentToken[riskpoolId]) != address(IProduct(address(component)).getToken())) { - revert("ERROR:TRS-013:TOKEN_ADDRESS_NOT_MACHING"); + revert("ERROR:TRS-014:TOKEN_ADDRESS_NOT_MACHING"); } _componentToken[productId] = IERC20(erc20Address); @@ -91,7 +91,7 @@ contract TreasuryModule is external override onlyInstanceOperator { - require(instanceWalletAddress != address(0), "ERROR:TRS-014:WALLET_ADDRESS_ZERO"); + require(instanceWalletAddress != address(0), "ERROR:TRS-015:WALLET_ADDRESS_ZERO"); _instanceWalletAddress = instanceWalletAddress; emit LogTreasuryInstanceWalletSet (instanceWalletAddress); @@ -102,8 +102,8 @@ contract TreasuryModule is onlyInstanceOperator { IComponent component = _component.getComponent(riskpoolId); - require(component.isRiskpool(), "ERROR:TRS-015:NOT_RISKPOOL"); - require(riskpoolWalletAddress != address(0), "ERROR:TRS-016:WALLET_ADDRESS_ZERO"); + require(component.isRiskpool(), "ERROR:TRS-016:NOT_RISKPOOL"); + require(riskpoolWalletAddress != address(0), "ERROR:TRS-017:WALLET_ADDRESS_ZERO"); _riskpoolWallet[riskpoolId] = riskpoolWalletAddress; emit LogTreasuryRiskpoolWalletSet (riskpoolId, riskpoolWalletAddress); diff --git a/tests/test_treasury_module.py b/tests/test_treasury_module.py index f1aa57a..6fc46a7 100644 --- a/tests/test_treasury_module.py +++ b/tests/test_treasury_module.py @@ -130,7 +130,7 @@ def test_two_products_different_coin_same_riskpool( ) # ensure that creating of another product with different token is not allowed - with brownie.reverts("ERROR:TRS-013:TOKEN_ADDRESS_NOT_MACHING"): + with brownie.reverts("ERROR:TRS-014:TOKEN_ADDRESS_NOT_MACHING"): GifTestProduct( instance, testCoinX, From eb91f6ae12512ec902a738f293c5d97a8252ba4c Mon Sep 17 00:00:00 2001 From: Marc Doerflinger Date: Fri, 12 Aug 2022 08:31:12 +0000 Subject: [PATCH 6/6] Change if to require --- contracts/modules/TreasuryModule.sol | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/contracts/modules/TreasuryModule.sol b/contracts/modules/TreasuryModule.sol index 182face..ac90721 100644 --- a/contracts/modules/TreasuryModule.sol +++ b/contracts/modules/TreasuryModule.sol @@ -74,13 +74,11 @@ contract TreasuryModule is uint256 riskpoolId = _pool.getRiskPoolForProduct(productId); - // If riskpool token is already set and product token does not match riskpool token, - // then revert (only product with same token are allowed in same riskpool) - if (address(_componentToken[riskpoolId]) != address(0) - && address(_componentToken[riskpoolId]) != address(IProduct(address(component)).getToken())) { - revert("ERROR:TRS-014:TOKEN_ADDRESS_NOT_MACHING"); - } - + // require if riskpool token is already set and product token does match riskpool token + require(address(_componentToken[riskpoolId]) == address(0) + || address(_componentToken[riskpoolId]) == address(IProduct(address(component)).getToken()), + "ERROR:TRS-014:TOKEN_ADDRESS_NOT_MACHING"); + _componentToken[productId] = IERC20(erc20Address); _componentToken[riskpoolId] = IERC20(erc20Address);