From ad028d19ca806e68630671f8c275cab21557ad9f Mon Sep 17 00:00:00 2001 From: brainbot-devops Date: Tue, 13 Aug 2019 13:45:52 +0200 Subject: [PATCH 1/4] Add 'node_balance' option to yaml config keywords for udc tokens. --- .../utils/configuration/settings.py | 21 ++++++++-- .../utils/configuration/test_settings.py | 41 +++++++++++++++---- 2 files changed, 50 insertions(+), 12 deletions(-) diff --git a/scenario_player/utils/configuration/settings.py b/scenario_player/utils/configuration/settings.py index 6df1df53d..63427808a 100644 --- a/scenario_player/utils/configuration/settings.py +++ b/scenario_player/utils/configuration/settings.py @@ -37,6 +37,21 @@ def url(self): return self.get("url") +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")) + print(self.dict) + + @property + def deposit(self): + return self.get("deposit", False) + + @property + def node_balance(self): + return self.get("node_balance", 5000) + + class UDCSettingsConfig(ConfigMapping): """UDC Service Settings interface. @@ -54,6 +69,7 @@ class UDCSettingsConfig(ConfigMapping): address: 0x1000001 token: deposit: True + node_balance: 5000 ... """ @@ -61,6 +77,7 @@ def __init__(self, loaded_yaml: dict): services_dict = (loaded_yaml.get("settings") or {}).get("services") or {} super(UDCSettingsConfig, self).__init__(services_dict.get("udc", {})) self.validate() + self.token = UDCTokenSettings(loaded_yaml) @property def enable(self): @@ -70,10 +87,6 @@ def enable(self): def address(self): return self.get("address") - @property - def token(self): - return self.get("token", {"deposit": False}) - class ServiceSettingsConfig(ConfigMapping): """Service Configuration Setting interface. diff --git a/tests/unittests/utils/configuration/test_settings.py b/tests/unittests/utils/configuration/test_settings.py index 01d5e8df7..959de746b 100644 --- a/tests/unittests/utils/configuration/test_settings.py +++ b/tests/unittests/utils/configuration/test_settings.py @@ -10,6 +10,7 @@ ServiceSettingsConfig, SettingsConfig, UDCSettingsConfig, + UDCTokenSettings, ) @@ -109,10 +110,10 @@ def test_is_subclass_of_config_mapping(self, minimal_yaml_dict): """The class is a subclass of :class:`ConfigMapping`.""" assert isinstance(UDCSettingsConfig(minimal_yaml_dict), ConfigMapping) - @pytest.mark.parametrize( - "key, expected", - argvalues=[("enable", False), ("address", None), ("token", {"deposit": False})], - ) + def test_token_attribute_is_an_instance_of_udctokenconfig(self, minimal_yaml_dict): + assert isinstance(UDCSettingsConfig(minimal_yaml_dict).token, UDCTokenSettings) + + @pytest.mark.parametrize("key, expected", argvalues=[("enable", False), ("address", None)]) def test_attributes_whose_key_is_absent_return_expected_default( self, key, expected, minimal_yaml_dict ): @@ -120,10 +121,7 @@ def test_attributes_whose_key_is_absent_return_expected_default( MISSING = object() assert getattr(config, key, MISSING) == expected - @pytest.mark.parametrize( - "key, expected", - argvalues=[("enable", True), ("address", "walahoo"), ("token", {"deposit": True})], - ) + @pytest.mark.parametrize("key, expected", argvalues=[("enable", True), ("address", "walahoo")]) def test_attributes_return_for_key_value_if_key_present( self, key, expected, minimal_yaml_dict ): @@ -131,3 +129,30 @@ def test_attributes_return_for_key_value_if_key_present( config = UDCSettingsConfig(minimal_yaml_dict) MISSING = object() assert getattr(config, key, MISSING) == expected + + +class TestUDCTokenConfig: + def test_is_subclass_of_config_mapping(self, minimal_yaml_dict): + """The class is a subclass of :class:`ConfigMapping`.""" + assert isinstance(UDCTokenSettings(minimal_yaml_dict), ConfigMapping) + + @pytest.mark.parametrize( + "key, expected", argvalues=[("deposit", True), ("node_balance", 1000)] + ) + def test_attributes_return_for_key_value_if_key_present( + self, key, expected, minimal_yaml_dict + ): + minimal_yaml_dict["settings"] = {"services": {"udc": {"token": {key: expected}}}} + config = UDCTokenSettings(minimal_yaml_dict) + MISSING = object() + assert getattr(config, key, MISSING) == expected + + @pytest.mark.parametrize( + "key, expected", argvalues=[("deposit", False), ("node_balance", 5000)] + ) + def test_attributes_whose_key_is_absent_return_expected_default( + self, key, expected, minimal_yaml_dict + ): + config = UDCTokenSettings(minimal_yaml_dict) + MISSING = object() + assert getattr(config, key, MISSING) == expected From 0104040f4fab0a6af8f0a62e2b8fe291fdb66e12 Mon Sep 17 00:00:00 2001 From: brainbot-devops Date: Tue, 13 Aug 2019 15:40:03 +0200 Subject: [PATCH 2/4] Use udc.token.node_balance in UserDepositContract class. --- .../utils/configuration/settings.py | 2 +- scenario_player/utils/token.py | 12 ++- tests/unittests/conftest.py | 2 +- tests/unittests/data/minimal.yaml | 9 ++ tests/unittests/utils/test_token.py | 90 ++++++++++++++++++- 5 files changed, 106 insertions(+), 9 deletions(-) create mode 100644 tests/unittests/data/minimal.yaml diff --git a/scenario_player/utils/configuration/settings.py b/scenario_player/utils/configuration/settings.py index 63427808a..2cb7ad90a 100644 --- a/scenario_player/utils/configuration/settings.py +++ b/scenario_player/utils/configuration/settings.py @@ -49,7 +49,7 @@ def deposit(self): @property def node_balance(self): - return self.get("node_balance", 5000) + return int(self.get("node_balance", 5000)) class UDCSettingsConfig(ConfigMapping): diff --git a/scenario_player/utils/token.py b/scenario_player/utils/token.py index 8765b6e3a..c7ca0eda6 100644 --- a/scenario_player/utils/token.py +++ b/scenario_player/utils/token.py @@ -349,6 +349,10 @@ def allowance(self): self._local_rpc_client.address, self.address ).call() + 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 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( @@ -362,7 +366,7 @@ def update_allowance(self) -> Union[str, None]: """ node_count = self.config.nodes.count udt_allowance = self.allowance - required_allowance = self.config.token.min_balance * node_count + required_allowance = self.config.settings.services.udc.token.node_balance * node_count log.debug( "Checking necessity of deposit request", @@ -375,7 +379,7 @@ def update_allowance(self) -> Union[str, None]: return log.debug("allowance update call required - insufficient allowance") - allow_amount = (self.config.token.max_funding * 10 * node_count) - udt_allowance + allow_amount = required_allowance - udt_allowance params = { "amount": allow_amount, "target_address": self.checksum_address, @@ -392,8 +396,8 @@ def deposit(self, target_address) -> Union[str, None]: TODO: Allow setting max funding parameter, similar to the token `funding_min` setting. """ - balance = self.contract_proxy.contract.functions.effectiveBalance(target_address).call() - min_deposit = self.config.token.min_balance // 2 + balance = self.effective_balance + min_deposit = self.config.settings.services.udc.token.node_balance log.debug( "Checking necessity of deposit request", required_balance=min_deposit, diff --git a/tests/unittests/conftest.py b/tests/unittests/conftest.py index 1461f7560..cef5f323f 100644 --- a/tests/unittests/conftest.py +++ b/tests/unittests/conftest.py @@ -15,7 +15,7 @@ def minimal_yaml_dict(): """A dictionary with the minimum required keys for instantiating any ConfigMapping.""" return { - "scenario": {"serial": {"runner": None, "config": "salami"}}, + "scenario": {"serial": {"tasks": {"wait_blocks": {"blocks": 5}}}}, "settings": {}, "token": {}, "nodes": {"count": 1}, diff --git a/tests/unittests/data/minimal.yaml b/tests/unittests/data/minimal.yaml new file mode 100644 index 000000000..82855aba8 --- /dev/null +++ b/tests/unittests/data/minimal.yaml @@ -0,0 +1,9 @@ +scenario: + - serial: + - tasks: None + - wait_block: {"blocks": 5} + settings: + token: + nodes": + - count: 1 + spaas: diff --git a/tests/unittests/utils/test_token.py b/tests/unittests/utils/test_token.py index af758ac64..490067124 100644 --- a/tests/unittests/utils/test_token.py +++ b/tests/unittests/utils/test_token.py @@ -1,5 +1,6 @@ import json -from unittest.mock import PropertyMock, patch +import pathlib +from unittest.mock import MagicMock, PropertyMock, patch import pytest from eth_utils.address import to_checksum_address @@ -22,9 +23,10 @@ TokenNotDeployed, TokenSourceCodeDoesNotExist, ) +from scenario_player.scenario import ScenarioYAML from scenario_player.utils.configuration.spaas import SPaaSConfig from scenario_player.utils.configuration.token import TokenConfig -from scenario_player.utils.token import Contract, Token +from scenario_player.utils.token import Contract, Token, UserDepositContract token_import_path = "scenario_player.utils.token" token_config_import_path = "scenario_player.utils.configuration.token" @@ -37,7 +39,10 @@ class Sentinel(Exception): @pytest.fixture def runner(dummy_scenario_runner, minimal_yaml_dict, token_info_path, tmp_path): token_config = TokenConfig(minimal_yaml_dict, token_info_path) - dummy_scenario_runner.yaml.spaas = SPaaSConfig(minimal_yaml_dict) + with patch("yaml.safe_load", return_value=minimal_yaml_dict): + tmp_file = tmp_path.joinpath("tmp.yaml") + tmp_file.touch() + dummy_scenario_runner.yaml = ScenarioYAML(tmp_file, tmp_path) dummy_scenario_runner.yaml.spaas.rpc.client_id = "the_client_id" dummy_scenario_runner.yaml.token = token_config @@ -468,3 +473,82 @@ def json(self): if reuse_token: return pytest.fail(f"save_token called, but reuse_token is {reuse_token}") + + +class TestUserDepositContract: + # TODO: Write tests to assert correctness of allowance and effective_balance properties + @pytest.fixture(autouse=True) + def set_up_udc_test_class(self, runner): + with patch( + "scenario_player.utils.token.UserDepositContract.effective_balance", + new_callable=PropertyMock, + ) as mock_eb, patch( + "scenario_player.utils.token.UserDepositContract.allowance", new_callable=PropertyMock + ) as mock_allowance, patch( + "scenario_player.utils.token.UserDepositContract.ud_token_address", + new_callable=PropertyMock, + ) as mock_ud_token_addr: + # We'll mock the properties accessing the contract proxies for now + # TODO: Properly mock them instead. + self.instance = UserDepositContract(runner, MagicMock(), MagicMock()) + self.mock_allowance = mock_allowance + self.mock_effective_balance = mock_eb + self.mock_ud_token_address = mock_ud_token_addr + yield + + @patch("scenario_player.utils.token.Contract.transact") + def test_update_allowance_is_noop_if_allowance_is_sufficient(self, mock_transact): + self.instance.config.nodes.dict["count"] = 2 + self.instance.config.settings.services.udc.token.dict["node_balance"] = 5_000 + self.mock_allowance.return_value = 10_000 + + self.instance.update_allowance() + + assert mock_transact.called is False + + @patch( + "scenario_player.utils.token.Contract.checksum_address", + new_callable=PropertyMock(return_value="ud_contract_addr"), + ) + @patch("scenario_player.utils.token.Contract.transact") + def test_update_allowance_updates_allowance_according_to_udc_token_node_balance_yaml_setting( + self, mock_transact, _ + ): + self.instance.config.nodes.dict["count"] = 2 + self.instance.config.settings.services.udc.token.dict["node_balance"] = 15_000 + self.mock_allowance.return_value = 5_000 + self.mock_ud_token_address.return_value = "ud_token_addr" + + # Expect an allowance transact request to be invoked, with its amount equal to: + # (node_balance * node_count) - current_allowance + # Targeting the UD Contract address, calling from the UD Token address. + expected_params = { + "amount": 25_000, + "target_address": "ud_contract_addr", + "contract_address": "ud_token_addr", + } + + self.instance.update_allowance() + + mock_transact.assert_called_once_with("allowance", expected_params) + + @patch("scenario_player.utils.token.Contract.transact") + def test_deposit_method_issues_deposit_request_if_node_funding_is_insufficient( + self, mock_transact + ): + self.instance.config.settings.services.udc.token.dict["node_balance"] = 5_000 + self.mock_effective_balance.return_value = 1000 + expected_params = {"amount": 4_000, "target_address": "some_address"} + + self.instance.deposit("some_address") + + mock_transact.assert_called_once_with("deposit", expected_params) + + @patch("scenario_player.utils.token.Contract.transact") + def test_deposit_methpd_is_noop_if_node_funding_is_sufficient(self, mock_transact): + self.instance.config.settings.services.udc.token.dict["node_balance"] = 5_000 + self.mock_effective_balance.return_value = 10_000 + + self.instance.deposit("some_address") + + assert mock_transact.called is False From e2b5a3447f4316759dadb604e53d30eb7db750ef Mon Sep 17 00:00:00 2001 From: brainbot-devops Date: Tue, 13 Aug 2019 16:01:52 +0200 Subject: [PATCH 3/4] Allow setting of a max_funding yaml option. --- scenario_player/runner.py | 4 ++-- .../utils/configuration/settings.py | 15 +++++++++--- scenario_player/utils/token.py | 9 +++---- .../utils/configuration/test_settings.py | 6 +++-- tests/unittests/utils/test_token.py | 24 ++++++++++++------- 5 files changed, 38 insertions(+), 20 deletions(-) diff --git a/scenario_player/runner.py b/scenario_player/runner.py index 9d3d2e63b..5bbdfc949 100644 --- a/scenario_player/runner.py +++ b/scenario_player/runner.py @@ -327,10 +327,10 @@ def _initialize_nodes( self.node_controller.initialize_nodes() node_addresses = self.node_controller.addresses node_count = len(self.node_controller) - node_balances = {address: self.client.balance(address) for address in node_addresses} + balance_per_nodes = {address: self.client.balance(address) for address in node_addresses} low_balances = { address: balance - for address, balance in node_balances.items() + for address, balance in balance_per_nodes.items() if balance < NODE_ACCOUNT_BALANCE_MIN } if low_balances: diff --git a/scenario_player/utils/configuration/settings.py b/scenario_player/utils/configuration/settings.py index 2cb7ad90a..168685c10 100644 --- a/scenario_player/utils/configuration/settings.py +++ b/scenario_player/utils/configuration/settings.py @@ -48,8 +48,17 @@ def deposit(self): return self.get("deposit", False) @property - def node_balance(self): - return int(self.get("node_balance", 5000)) + def balance_per_node(self): + """The required amount of UDC/RDN tokens required by each node.""" + return int(self.get("balance_per_node", 5000)) + + @property + def max_funding(self): + """The maximum amount to fund when depositing RDN tokens at a target. + + It defaults to :attr:`.balance_per_node`'s value. + """ + return int(self.get("max_funding", self.balance_per_node)) class UDCSettingsConfig(ConfigMapping): @@ -69,7 +78,7 @@ class UDCSettingsConfig(ConfigMapping): address: 0x1000001 token: deposit: True - node_balance: 5000 + balance_per_node: 5000 ... """ diff --git a/scenario_player/utils/token.py b/scenario_player/utils/token.py index c7ca0eda6..bbeb954b3 100644 --- a/scenario_player/utils/token.py +++ b/scenario_player/utils/token.py @@ -366,7 +366,7 @@ def update_allowance(self) -> Union[str, None]: """ node_count = self.config.nodes.count udt_allowance = self.allowance - required_allowance = self.config.settings.services.udc.token.node_balance * node_count + required_allowance = self.config.settings.services.udc.token.balance_per_node * node_count log.debug( "Checking necessity of deposit request", @@ -396,8 +396,9 @@ 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 - min_deposit = self.config.settings.services.udc.token.node_balance + balance = self.effective_balance(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( "Checking necessity of deposit request", required_balance=min_deposit, @@ -408,6 +409,6 @@ def deposit(self, target_address) -> Union[str, None]: return log.debug("deposit call required - insufficient funds") - deposit_amount = min_deposit - balance + deposit_amount = max_funding - balance params = {"amount": deposit_amount, "target_address": target_address} return self.transact("deposit", params) diff --git a/tests/unittests/utils/configuration/test_settings.py b/tests/unittests/utils/configuration/test_settings.py index 959de746b..e9ec7f53f 100644 --- a/tests/unittests/utils/configuration/test_settings.py +++ b/tests/unittests/utils/configuration/test_settings.py @@ -137,7 +137,8 @@ def test_is_subclass_of_config_mapping(self, minimal_yaml_dict): assert isinstance(UDCTokenSettings(minimal_yaml_dict), ConfigMapping) @pytest.mark.parametrize( - "key, expected", argvalues=[("deposit", True), ("node_balance", 1000)] + "key, expected", + argvalues=[("deposit", True), ("balance_per_node", 1000), ("max_funding", 10_000)], ) def test_attributes_return_for_key_value_if_key_present( self, key, expected, minimal_yaml_dict @@ -148,7 +149,8 @@ def test_attributes_return_for_key_value_if_key_present( assert getattr(config, key, MISSING) == expected @pytest.mark.parametrize( - "key, expected", argvalues=[("deposit", False), ("node_balance", 5000)] + "key, expected", + argvalues=[("deposit", False), ("balance_per_node", 5000), ("max_funding", 5000)], ) def test_attributes_whose_key_is_absent_return_expected_default( self, key, expected, minimal_yaml_dict diff --git a/tests/unittests/utils/test_token.py b/tests/unittests/utils/test_token.py index 490067124..0c754eab1 100644 --- a/tests/unittests/utils/test_token.py +++ b/tests/unittests/utils/test_token.py @@ -480,8 +480,7 @@ class TestUserDepositContract: @pytest.fixture(autouse=True) def set_up_udc_test_class(self, runner): with patch( - "scenario_player.utils.token.UserDepositContract.effective_balance", - new_callable=PropertyMock, + "scenario_player.utils.token.UserDepositContract.effective_balance" ) as mock_eb, patch( "scenario_player.utils.token.UserDepositContract.allowance", new_callable=PropertyMock ) as mock_allowance, patch( @@ -499,7 +498,7 @@ def set_up_udc_test_class(self, runner): @patch("scenario_player.utils.token.Contract.transact") def test_update_allowance_is_noop_if_allowance_is_sufficient(self, mock_transact): self.instance.config.nodes.dict["count"] = 2 - self.instance.config.settings.services.udc.token.dict["node_balance"] = 5_000 + self.instance.config.settings.services.udc.token.dict["balance_per_node"] = 5_000 self.mock_allowance.return_value = 10_000 self.instance.update_allowance() @@ -511,16 +510,16 @@ def test_update_allowance_is_noop_if_allowance_is_sufficient(self, mock_transact new_callable=PropertyMock(return_value="ud_contract_addr"), ) @patch("scenario_player.utils.token.Contract.transact") - def test_update_allowance_updates_allowance_according_to_udc_token_node_balance_yaml_setting( + def test_update_allowance_updates_allowance_according_to_udc_token_balance_per_node_yaml_setting( self, mock_transact, _ ): self.instance.config.nodes.dict["count"] = 2 - self.instance.config.settings.services.udc.token.dict["node_balance"] = 15_000 + self.instance.config.settings.services.udc.token.dict["balance_per_node"] = 15_000 self.mock_allowance.return_value = 5_000 self.mock_ud_token_address.return_value = "ud_token_addr" # Expect an allowance transact request to be invoked, with its amount equal to: - # (node_balance * node_count) - current_allowance + # (balance_per_node * node_count) - current_allowance # Targeting the UD Contract address, calling from the UD Token address. expected_params = { "amount": 25_000, @@ -536,9 +535,16 @@ def test_update_allowance_updates_allowance_according_to_udc_token_node_balance_ def test_deposit_method_issues_deposit_request_if_node_funding_is_insufficient( self, mock_transact ): - self.instance.config.settings.services.udc.token.dict["node_balance"] = 5_000 + """deposit() deposits the correct amount of UDC Tokens at the target node's address. + + amount = yaml.settings.services.udc.token.max_funding - target_node.effective_balance + """ + self.instance.config.settings.services.udc.token.dict["balance_per_node"] = 5_000 + self.instance.config.settings.services.udc.token.dict["max_funding"] = 10_000 self.mock_effective_balance.return_value = 1000 - expected_params = {"amount": 4_000, "target_address": "some_address"} + + # amount = max_funding - effective_balance + expected_params = {"amount": 9_000, "target_address": "some_address"} self.instance.deposit("some_address") @@ -546,7 +552,7 @@ def test_deposit_method_issues_deposit_request_if_node_funding_is_insufficient( @patch("scenario_player.utils.token.Contract.transact") def test_deposit_methpd_is_noop_if_node_funding_is_sufficient(self, mock_transact): - self.instance.config.settings.services.udc.token.dict["node_balance"] = 5_000 + self.instance.config.settings.services.udc.token.dict["balance_per_node"] = 5_000 self.mock_effective_balance.return_value = 10_000 self.instance.deposit("some_address") From 3b62117ef62bac78face208e2369121b3e97d0a8 Mon Sep 17 00:00:00 2001 From: brainbot-devops Date: Wed, 14 Aug 2019 07:35:15 +0200 Subject: [PATCH 4/4] fix keyword spelling. --- scenario_player/runner.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/scenario_player/runner.py b/scenario_player/runner.py index 5bbdfc949..53054801b 100644 --- a/scenario_player/runner.py +++ b/scenario_player/runner.py @@ -327,10 +327,10 @@ def _initialize_nodes( self.node_controller.initialize_nodes() node_addresses = self.node_controller.addresses node_count = len(self.node_controller) - balance_per_nodes = {address: self.client.balance(address) for address in node_addresses} + balance_per_node = {address: self.client.balance(address) for address in node_addresses} low_balances = { address: balance - for address, balance in balance_per_nodes.items() + for address, balance in balance_per_node.items() if balance < NODE_ACCOUNT_BALANCE_MIN } if low_balances: