From ecfb98b83d142da0656137e83712b50e86079af6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Oliver=20B=C3=BCnting?= Date: Mon, 19 Aug 2019 13:54:57 +0200 Subject: [PATCH 1/5] Fixed calculation of deposit_amount --- scenario_player/utils/token.py | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/scenario_player/utils/token.py b/scenario_player/utils/token.py index bbeb954b3..2afa05309 100644 --- a/scenario_player/utils/token.py +++ b/scenario_player/utils/token.py @@ -353,6 +353,10 @@ def effective_balance(self, at_target): """Get the effective balance of the target address.""" return self.contract_proxy.contract.functions.effectiveBalance(at_target).call() + def total_deposit(self, at_target): + """"Get the so far deposted amount""" + return self.contract_proxy.contract.functions.total_deposit(at_target).call() + def mint(self, target_address) -> Union[str, None]: """The mint function isn't present on the UDC, pass the UDTC address instead.""" return super(UserDepositContract, self).mint( @@ -397,6 +401,7 @@ def deposit(self, target_address) -> Union[str, None]: TODO: Allow setting max funding parameter, similar to the token `funding_min` setting. """ balance = self.effective_balance(target_address) + total_deposit = self.total_deposit(target_address) min_deposit = self.config.settings.services.udc.token.balance_per_node max_funding = self.config.settings.services.udc.token.max_funding log.debug( @@ -409,6 +414,6 @@ def deposit(self, target_address) -> Union[str, None]: return log.debug("deposit call required - insufficient funds") - deposit_amount = max_funding - balance + deposit_amount = total_deposit + max(max_funding, min_deposit) - balance params = {"amount": deposit_amount, "target_address": target_address} return self.transact("deposit", params) From 22fb50701e3f00b5080a4221be6930a64c666635 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Oliver=20B=C3=BCnting?= Date: Tue, 20 Aug 2019 16:50:17 +0200 Subject: [PATCH 2/5] Added an assert to check that enough tokens get minted --- scenario_player/utils/configuration/base.py | 2 +- .../utils/configuration/settings.py | 24 ++++++++++++++++++- scenario_player/utils/token.py | 2 +- 3 files changed, 25 insertions(+), 3 deletions(-) diff --git a/scenario_player/utils/configuration/base.py b/scenario_player/utils/configuration/base.py index 8412d36ce..e7b4c8a49 100644 --- a/scenario_player/utils/configuration/base.py +++ b/scenario_player/utils/configuration/base.py @@ -49,7 +49,7 @@ def assert_option(cls, expression, err: Optional[Union[str, Exception]] = None): exception = err raise exception from e - def validate(self): + def validate(self, **args): """Validate the configuration. Assert that all required keys are present, and no mutually exclusive diff --git a/scenario_player/utils/configuration/settings.py b/scenario_player/utils/configuration/settings.py index 168685c10..299e67e44 100644 --- a/scenario_player/utils/configuration/settings.py +++ b/scenario_player/utils/configuration/settings.py @@ -2,7 +2,7 @@ import structlog -from scenario_player.constants import GAS_STRATEGIES, TIMEOUT +from scenario_player.constants import DEFAULT_TOKEN_BALANCE_FUND, GAS_STRATEGIES, TIMEOUT from scenario_player.exceptions.config import ScenarioConfigurationError, ServiceConfigurationError from scenario_player.utils.configuration.base import ConfigMapping @@ -41,8 +41,30 @@ class UDCTokenSettings(ConfigMapping): def __init__(self, loaded_yaml: dict): udc_settings = ((loaded_yaml.get("settings") or {}).get("services") or {}).get("udc") or {} super(UDCTokenSettings, self).__init__(udc_settings.get("token")) + self.validate(loaded_yaml) print(self.dict) + def validate(self, loaded_yaml): + self.assert_option( + self.max_funding >= self.balance_per_node, + "max_funding needs to be greater or equal to balance_per_node", + ) + + # Get the amount of tokens that will be minted (min_balance) + try: + min_balance = loaded_yaml["token"]["min_balance"] + except (KeyError, TypeError): + min_balance = DEFAULT_TOKEN_BALANCE_FUND + # The amount of Tokens minted need to be greater than the amount of tokens + # that will be deposited into the UDC + self.assert_option( + self.max_funding <= min_balance, + "Token.max_funding needs to be greater than the Deposit amount." + f"The Deposit amount is max({DEFAULT_TOKEN_BALANCE_FUND}, " + "settings.services.token.udc.balance_per_node, " + "settings.services.token.udc.max_funding", + ) + @property def deposit(self): return self.get("deposit", False) diff --git a/scenario_player/utils/token.py b/scenario_player/utils/token.py index 2afa05309..979b7983d 100644 --- a/scenario_player/utils/token.py +++ b/scenario_player/utils/token.py @@ -414,6 +414,6 @@ def deposit(self, target_address) -> Union[str, None]: return log.debug("deposit call required - insufficient funds") - deposit_amount = total_deposit + max(max_funding, min_deposit) - balance + deposit_amount = total_deposit + (max_funding - balance) params = {"amount": deposit_amount, "target_address": target_address} return self.transact("deposit", params) From d1d4ba3c7a7fd2aa9cf7c0740ceb6514cb394cac Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Oliver=20B=C3=BCnting?= Date: Tue, 20 Aug 2019 16:50:41 +0200 Subject: [PATCH 3/5] WIP testing UDC deposit config --- .../unittests/utils/configuration/test_settings.py | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/tests/unittests/utils/configuration/test_settings.py b/tests/unittests/utils/configuration/test_settings.py index e9ec7f53f..4589cb054 100644 --- a/tests/unittests/utils/configuration/test_settings.py +++ b/tests/unittests/utils/configuration/test_settings.py @@ -1,8 +1,7 @@ -from unittest.mock import patch - import pytest from web3.gas_strategies.time_based import fast_gas_price_strategy, medium_gas_price_strategy +from scenario_player.exceptions.config import ConfigurationError from scenario_player.utils.configuration.base import ConfigMapping from scenario_player.utils.configuration.settings import ( PFSSettingsConfig, @@ -158,3 +157,14 @@ def test_attributes_whose_key_is_absent_return_expected_default( config = UDCTokenSettings(minimal_yaml_dict) MISSING = object() assert getattr(config, key, MISSING) == expected + + def test_balance_per_node_bigger_than_max_funding(self, minimal_yaml_dict): + minimal_yaml_dict["max_funding"] = 5000 + minimal_yaml_dict["balance_per_node"] = 5001 + assert UDCTokenSettings(minimal_yaml_dict).CONFIGURATION_ERROR == ConfigurationError + + # FIXME this test doesn't work for whatever reason + def test_insufficient_minting(self, minimal_yaml_dict): + minimal_yaml_dict["max_funding"] = 5000 + minimal_yaml_dict["token"]["min_balance"] = 4999 + assert UDCTokenSettings(minimal_yaml_dict).CONFIGURATION_ERROR == ConfigurationError From 64ab9a9ee3582a1c4bd93f0db492f109837c3551 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Oliver=20B=C3=BCnting?= Date: Wed, 21 Aug 2019 16:24:38 +0200 Subject: [PATCH 4/5] Added new config errors and implemented requested changes in PR #249 --- scenario_player/exceptions/config.py | 8 +++++ scenario_player/scenario.py | 10 ++++++ .../utils/configuration/settings.py | 28 ++++----------- .../utils/configuration/test_settings.py | 34 +++++++++++++------ 4 files changed, 49 insertions(+), 31 deletions(-) diff --git a/scenario_player/exceptions/config.py b/scenario_player/exceptions/config.py index d52a9f602..e5ff93dbb 100644 --- a/scenario_player/exceptions/config.py +++ b/scenario_player/exceptions/config.py @@ -2,6 +2,14 @@ class ConfigurationError(ValueError): """Generic error thrown if there was an error while reading the scenario file.""" +class UDCTokenConfigError(ConfigurationError): + """Invalid Configuration parameters. Most likely there is an issue with Token amounts""" + + +class InsufficientMintingAmount(ConfigurationError): + """The minted amount set in token.min_balance is not sufficient""" + + class NodeConfigurationError(ConfigurationError): """An error occurred while validating the nodes setting of a scenario file.""" diff --git a/scenario_player/scenario.py b/scenario_player/scenario.py index 8c92d5342..bcbcf1a09 100644 --- a/scenario_player/scenario.py +++ b/scenario_player/scenario.py @@ -6,6 +6,7 @@ import yaml from scenario_player.constants import GAS_LIMIT_FOR_TOKEN_CONTRACT_CALL +from scenario_player.exceptions.config import InsufficientMintingAmount from scenario_player.utils.configuration import ( NodesConfig, ScenarioConfig, @@ -33,6 +34,7 @@ def __init__(self, yaml_path: pathlib.Path, data_path: pathlib.Path) -> None: self.scenario = ScenarioConfig(self._loaded) self.token = TokenConfig(self._loaded, data_path.joinpath("token.info")) self.spaas = SPaaSConfig(self._loaded) + self.validate() self.gas_limit = GAS_LIMIT_FOR_TOKEN_CONTRACT_CALL * 2 @@ -40,3 +42,11 @@ def __init__(self, yaml_path: pathlib.Path, data_path: pathlib.Path) -> None: def name(self) -> str: """Return the name of the scenario file, sans extension.""" return self.path.stem + + def validate(self): + # The amount of Tokens minted need to be greater than the amount of tokens + # that will be deposited into the UDC + try: + assert self.token.min_balance >= self.settings.services.udc.token.max_funding + except AssertionError: + raise InsufficientMintingAmount diff --git a/scenario_player/utils/configuration/settings.py b/scenario_player/utils/configuration/settings.py index 299e67e44..d314a813c 100644 --- a/scenario_player/utils/configuration/settings.py +++ b/scenario_player/utils/configuration/settings.py @@ -2,8 +2,12 @@ import structlog -from scenario_player.constants import DEFAULT_TOKEN_BALANCE_FUND, GAS_STRATEGIES, TIMEOUT -from scenario_player.exceptions.config import ScenarioConfigurationError, ServiceConfigurationError +from scenario_player.constants import GAS_STRATEGIES, TIMEOUT +from scenario_player.exceptions.config import ( + ScenarioConfigurationError, + ServiceConfigurationError, + UDCTokenConfigError, +) from scenario_player.utils.configuration.base import ConfigMapping log = structlog.get_logger(__name__) @@ -45,25 +49,7 @@ def __init__(self, loaded_yaml: dict): print(self.dict) def validate(self, loaded_yaml): - self.assert_option( - self.max_funding >= self.balance_per_node, - "max_funding needs to be greater or equal to balance_per_node", - ) - - # Get the amount of tokens that will be minted (min_balance) - try: - min_balance = loaded_yaml["token"]["min_balance"] - except (KeyError, TypeError): - min_balance = DEFAULT_TOKEN_BALANCE_FUND - # The amount of Tokens minted need to be greater than the amount of tokens - # that will be deposited into the UDC - self.assert_option( - self.max_funding <= min_balance, - "Token.max_funding needs to be greater than the Deposit amount." - f"The Deposit amount is max({DEFAULT_TOKEN_BALANCE_FUND}, " - "settings.services.token.udc.balance_per_node, " - "settings.services.token.udc.max_funding", - ) + self.assert_option(self.max_funding >= self.balance_per_node, UDCTokenConfigError) @property def deposit(self): diff --git a/tests/unittests/utils/configuration/test_settings.py b/tests/unittests/utils/configuration/test_settings.py index 4589cb054..e90adad78 100644 --- a/tests/unittests/utils/configuration/test_settings.py +++ b/tests/unittests/utils/configuration/test_settings.py @@ -1,7 +1,9 @@ import pytest +import yaml from web3.gas_strategies.time_based import fast_gas_price_strategy, medium_gas_price_strategy -from scenario_player.exceptions.config import ConfigurationError +from scenario_player.exceptions.config import InsufficientMintingAmount, UDCTokenConfigError +from scenario_player.scenario import ScenarioYAML from scenario_player.utils.configuration.base import ConfigMapping from scenario_player.utils.configuration.settings import ( PFSSettingsConfig, @@ -13,6 +15,16 @@ ) +@pytest.fixture() +def file_for_insufficient_minting_test(tmp_path, minimal_yaml_dict): + minimal_yaml_dict["settings"] = {"services": {"udc": {"token": {"max_funding": 6000}}}} + minimal_yaml_dict["token"] = {"min_balance": 5999} + tmp_file = tmp_path.joinpath("tmp.yaml") + with open(tmp_file, "w") as outfile: + yaml.dump(minimal_yaml_dict, outfile, default_flow_style=False) + yield tmp_file + + class TestSettingsConfig: def test_is_subclass_of_config_mapping(self, minimal_yaml_dict): """The class is a subclass of :class:`ConfigMapping`.""" @@ -159,12 +171,14 @@ def test_attributes_whose_key_is_absent_return_expected_default( assert getattr(config, key, MISSING) == expected def test_balance_per_node_bigger_than_max_funding(self, minimal_yaml_dict): - minimal_yaml_dict["max_funding"] = 5000 - minimal_yaml_dict["balance_per_node"] = 5001 - assert UDCTokenSettings(minimal_yaml_dict).CONFIGURATION_ERROR == ConfigurationError - - # FIXME this test doesn't work for whatever reason - def test_insufficient_minting(self, minimal_yaml_dict): - minimal_yaml_dict["max_funding"] = 5000 - minimal_yaml_dict["token"]["min_balance"] = 4999 - assert UDCTokenSettings(minimal_yaml_dict).CONFIGURATION_ERROR == ConfigurationError + minimal_yaml_dict["settings"] = { + "services": {"udc": {"token": {"max_funding": 6000, "balance_per_node": 6001}}} + } + with pytest.raises(UDCTokenConfigError): + UDCTokenSettings(minimal_yaml_dict) + + def test_insufficient_minting(self, file_for_insufficient_minting_test): + with pytest.raises(InsufficientMintingAmount): + ScenarioYAML( + file_for_insufficient_minting_test, file_for_insufficient_minting_test.parent + ) From b9d96b16492004be7ba376130bdd97e63f47d20c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Oliver=20B=C3=BCnting?= Date: Wed, 21 Aug 2019 17:41:34 +0200 Subject: [PATCH 5/5] Removed unneeded leftovers and added validate dockstring --- scenario_player/scenario.py | 9 +++++++-- scenario_player/utils/configuration/base.py | 2 +- scenario_player/utils/configuration/settings.py | 4 ++-- tests/unittests/utils/configuration/test_settings.py | 2 +- 4 files changed, 11 insertions(+), 6 deletions(-) diff --git a/scenario_player/scenario.py b/scenario_player/scenario.py index bcbcf1a09..10567ce30 100644 --- a/scenario_player/scenario.py +++ b/scenario_player/scenario.py @@ -44,8 +44,13 @@ def name(self) -> str: return self.path.stem def validate(self): - # The amount of Tokens minted need to be greater than the amount of tokens - # that will be deposited into the UDC + """Validate cross-config section requirements of the scenario. + + :raises InsufficientMintingAmount: + If token.min_balance < settings.services.udc.token.max_funding + """ + + # Check that the amount of minted tokens is >= than the amount of deposited tokens try: assert self.token.min_balance >= self.settings.services.udc.token.max_funding except AssertionError: diff --git a/scenario_player/utils/configuration/base.py b/scenario_player/utils/configuration/base.py index e7b4c8a49..8412d36ce 100644 --- a/scenario_player/utils/configuration/base.py +++ b/scenario_player/utils/configuration/base.py @@ -49,7 +49,7 @@ def assert_option(cls, expression, err: Optional[Union[str, Exception]] = None): exception = err raise exception from e - def validate(self, **args): + def validate(self): """Validate the configuration. Assert that all required keys are present, and no mutually exclusive diff --git a/scenario_player/utils/configuration/settings.py b/scenario_player/utils/configuration/settings.py index d314a813c..906654e6e 100644 --- a/scenario_player/utils/configuration/settings.py +++ b/scenario_player/utils/configuration/settings.py @@ -45,10 +45,10 @@ class UDCTokenSettings(ConfigMapping): def __init__(self, loaded_yaml: dict): udc_settings = ((loaded_yaml.get("settings") or {}).get("services") or {}).get("udc") or {} super(UDCTokenSettings, self).__init__(udc_settings.get("token")) - self.validate(loaded_yaml) + self.validate() print(self.dict) - def validate(self, loaded_yaml): + def validate(self): self.assert_option(self.max_funding >= self.balance_per_node, UDCTokenConfigError) @property diff --git a/tests/unittests/utils/configuration/test_settings.py b/tests/unittests/utils/configuration/test_settings.py index e90adad78..a2e07fdbf 100644 --- a/tests/unittests/utils/configuration/test_settings.py +++ b/tests/unittests/utils/configuration/test_settings.py @@ -170,7 +170,7 @@ def test_attributes_whose_key_is_absent_return_expected_default( MISSING = object() assert getattr(config, key, MISSING) == expected - def test_balance_per_node_bigger_than_max_funding(self, minimal_yaml_dict): + def test_balance_per_node_must_not_be_greater_than_max_funding(self, minimal_yaml_dict): minimal_yaml_dict["settings"] = { "services": {"udc": {"token": {"max_funding": 6000, "balance_per_node": 6001}}} }