From 66a6b8cdb3d309ebb10cfafc21b3baaa3b4cfaa1 Mon Sep 17 00:00:00 2001 From: leeheonseung Date: Tue, 25 Aug 2020 19:42:04 +0900 Subject: [PATCH 01/17] IS-1208: Fix Balance bug according to revision 11, branch logic has_unstake --- iconservice/icon_constant.py | 6 +- iconservice/icx/icx_account.py | 9 ++- .../iiss/prevote/test_iiss_unstake.py | 70 +++++++++++++++++++ 3 files changed, 82 insertions(+), 3 deletions(-) create mode 100644 tests/integrate_test/iiss/prevote/test_iiss_unstake.py diff --git a/iconservice/icon_constant.py b/iconservice/icon_constant.py index 8bf2c99b1..75682beec 100644 --- a/iconservice/icon_constant.py +++ b/iconservice/icon_constant.py @@ -134,10 +134,12 @@ class Revision(Enum): FIX_COIN_PART_BYTES_ENCODING = 9 STRICT_SCORE_DECORATOR_CHECK = 9 - LOCK_ADDRESS = 10 FIX_UNSTAKE_BUG = 10 + LOCK_ADDRESS = 10 + + FIX_BALANCE_BUG = 11 - LATEST = 9 + LATEST = 11 RC_DB_VERSION_0 = 0 diff --git a/iconservice/icx/icx_account.py b/iconservice/icx/icx_account.py index 404d88f53..6b16c0012 100644 --- a/iconservice/icx/icx_account.py +++ b/iconservice/icx/icx_account.py @@ -143,7 +143,14 @@ def normalize(self, revision: int): if self.coin_part is None: raise InvalidParamsException('Failed to normalize: no coin part') - self.coin_part.toggle_has_unstake(False) + if revision >= Revision.FIX_BALANCE_BUG.value: + if self.stake_part.total_unstake == 0: + self.coin_part.toggle_has_unstake(False) + else: + # recover has_unstake flag + self.coin_part.toggle_has_unstake(True) + else: + self.coin_part.toggle_has_unstake(False) self.coin_part.deposit(balance) def set_stake(self, context: "IconScoreContext", value: int, unstake_lock_period: int): diff --git a/tests/integrate_test/iiss/prevote/test_iiss_unstake.py b/tests/integrate_test/iiss/prevote/test_iiss_unstake.py new file mode 100644 index 000000000..6ef08dc74 --- /dev/null +++ b/tests/integrate_test/iiss/prevote/test_iiss_unstake.py @@ -0,0 +1,70 @@ +# -*- coding: utf-8 -*- + +# Copyright 2018 ICON Foundation +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +"""IconScoreEngine testcase +""" +from typing import TYPE_CHECKING, List +from unittest.mock import patch + +from iconservice import SYSTEM_SCORE_ADDRESS +from iconservice.icon_constant import Revision, ICX_IN_LOOP +from tests.integrate_test.iiss.test_iiss_base import TestIISSBase + +if TYPE_CHECKING: + from iconservice.iconscore.icon_score_result import TransactionResult + + +class TestIISSStake(TestIISSBase): + def test_unstake1(self): + self.update_governance() + + # set Revision REV_IISS + self.set_revision(Revision.IISS.value) + + # gain 10 icx + balance: int = 10 * ICX_IN_LOOP + self.distribute_icx( + accounts=self._accounts[:1], + init_balance=balance + ) + + # set stake + stake: int = 4 * ICX_IN_LOOP + tx_results: List['TransactionResult'] = self.set_stake( + from_=self._accounts[0], + value=stake + ) + fee = tx_results[0].step_used * tx_results[0].step_price + expected_balance: int = balance - stake - fee + response: int = self.get_balance(self._accounts[0]) + self.assertEqual(expected_balance, response) + + self.set_revision(Revision.FIX_BALANCE_BUG.value) + + for i in range(1, 5): + self.set_stake( + from_=self._accounts[0], + value=stake - i + ) + + self.make_empty_blocks(16) + last_balance: int = self.get_balance(self._accounts[0]) + tx_results = self.transfer_icx(from_=self._accounts[0], to_=self._accounts[0], value=0) + fee = tx_results[0].step_used * tx_results[0].step_price + + self.make_empty_blocks(1) + balance: int = self.get_balance(self._accounts[0]) + self.assertEqual(last_balance - fee + 2, balance) From 9d8ef7573ba71beef3a2548a7a0289484ce0519d Mon Sep 17 00:00:00 2001 From: leeheonseung Date: Tue, 25 Aug 2020 19:44:51 +0900 Subject: [PATCH 02/17] append comment --- tests/integrate_test/iiss/prevote/test_iiss_unstake.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tests/integrate_test/iiss/prevote/test_iiss_unstake.py b/tests/integrate_test/iiss/prevote/test_iiss_unstake.py index 6ef08dc74..72ae0a4ef 100644 --- a/tests/integrate_test/iiss/prevote/test_iiss_unstake.py +++ b/tests/integrate_test/iiss/prevote/test_iiss_unstake.py @@ -65,6 +65,8 @@ def test_unstake1(self): tx_results = self.transfer_icx(from_=self._accounts[0], to_=self._accounts[0], value=0) fee = tx_results[0].step_used * tx_results[0].step_price + # apply expire balance self.make_empty_blocks(1) + balance: int = self.get_balance(self._accounts[0]) self.assertEqual(last_balance - fee + 2, balance) From 16e3c30aff3c9f867dec71e09524c6d75678e19f Mon Sep 17 00:00:00 2001 From: leeheonseung Date: Wed, 26 Aug 2020 12:37:40 +0900 Subject: [PATCH 03/17] feedback apply deprecated has_unstake flag --- iconservice/icx/coin_part.py | 8 ++++++-- iconservice/icx/icx_account.py | 19 ++++++------------- iconservice/icx/storage.py | 7 +++++-- .../iiss/prevote/test_iiss_unstake.py | 5 +++++ 4 files changed, 22 insertions(+), 17 deletions(-) diff --git a/iconservice/icx/coin_part.py b/iconservice/icx/coin_part.py index 576d1c292..b370d32d2 100644 --- a/iconservice/icx/coin_part.py +++ b/iconservice/icx/coin_part.py @@ -49,7 +49,7 @@ def __str__(self) -> str: class CoinPartFlag(Flag): NONE = 0 - HAS_UNSTAKE = 1 + HAS_UNSTAKE = 1 # deprecated class CoinPart(BasePart): @@ -130,7 +130,11 @@ def flags(self) -> 'CoinPartFlag': """ return self._flags - def toggle_has_unstake(self, on: bool): + def toggle_has_unstake(self, on: bool, revision: int): + # deprecated flag + if revision >= Revision.FIX_BALANCE_BUG.value: + return + new_flags = set_flag(self._flags, CoinPartFlag.HAS_UNSTAKE, on) if self._flags != new_flags: diff --git a/iconservice/icx/icx_account.py b/iconservice/icx/icx_account.py index 6b16c0012..e8fecc3c3 100644 --- a/iconservice/icx/icx_account.py +++ b/iconservice/icx/icx_account.py @@ -138,20 +138,13 @@ def normalize(self, revision: int): if self.coin_part is None or self.stake_part is None: return - balance: int = self.stake_part.normalize(self._current_block_height, revision) - if balance > 0: + expired_unstaked_balance: int = self.stake_part.normalize(self._current_block_height, revision) + if expired_unstaked_balance > 0: if self.coin_part is None: raise InvalidParamsException('Failed to normalize: no coin part') - if revision >= Revision.FIX_BALANCE_BUG.value: - if self.stake_part.total_unstake == 0: - self.coin_part.toggle_has_unstake(False) - else: - # recover has_unstake flag - self.coin_part.toggle_has_unstake(True) - else: - self.coin_part.toggle_has_unstake(False) - self.coin_part.deposit(balance) + self.coin_part.toggle_has_unstake(False, revision) + self.coin_part.deposit(expired_unstaked_balance) def set_stake(self, context: "IconScoreContext", value: int, unstake_lock_period: int): if self.coin_part is None or self.stake_part is None: @@ -175,12 +168,12 @@ def set_stake(self, context: "IconScoreContext", value: int, unstake_lock_period self.stake_part.reset_unstake() else: unlock_block_height: int = self._current_block_height + unstake_lock_period - self.coin_part.toggle_has_unstake(True) + self.coin_part.toggle_has_unstake(True, context.revision) if context.revision >= Revision.MULTIPLE_UNSTAKE.value: self.stake_part.set_unstakes_info(unlock_block_height, -offset, context.unstake_slot_max) else: - self.stake_part.set_unstake(unlock_block_height, -offset) + self.stake_part.set_unstake(unlock_block_height, -offset) def update_delegated_amount(self, offset: int): if self.delegation_part is None: diff --git a/iconservice/icx/storage.py b/iconservice/icx/storage.py index ab907673a..cddac56d1 100644 --- a/iconservice/icx/storage.py +++ b/iconservice/icx/storage.py @@ -27,7 +27,8 @@ from ..base.ComponentBase import StorageBase from ..base.address import Address from ..base.block import Block, NULL_BLOCK -from ..icon_constant import DEFAULT_BYTE_SIZE, DATA_BYTE_ORDER, ICX_LOG_TAG, ROLLBACK_LOG_TAG, IconScoreContextType +from ..icon_constant import DEFAULT_BYTE_SIZE, DATA_BYTE_ORDER, ICX_LOG_TAG, ROLLBACK_LOG_TAG, IconScoreContextType, \ + Revision from ..utils import bytes_to_hex if TYPE_CHECKING: @@ -269,7 +270,9 @@ def get_account(self, if AccountPartFlag.COIN in part_flags: coin_part: 'CoinPart' = self._get_part(context, CoinPart, address) - if CoinPartFlag.HAS_UNSTAKE in coin_part.flags: + # TODO Comment + if context.revision >= Revision.FIX_BALANCE_BUG.value or \ + CoinPartFlag.HAS_UNSTAKE in coin_part.flags: part_flags |= AccountPartFlag.STAKE if AccountPartFlag.STAKE in part_flags: diff --git a/tests/integrate_test/iiss/prevote/test_iiss_unstake.py b/tests/integrate_test/iiss/prevote/test_iiss_unstake.py index 72ae0a4ef..d7fd8843f 100644 --- a/tests/integrate_test/iiss/prevote/test_iiss_unstake.py +++ b/tests/integrate_test/iiss/prevote/test_iiss_unstake.py @@ -60,12 +60,17 @@ def test_unstake1(self): value=stake - i ) + # 1st expired unstake self.make_empty_blocks(16) last_balance: int = self.get_balance(self._accounts[0]) tx_results = self.transfer_icx(from_=self._accounts[0], to_=self._accounts[0], value=0) fee = tx_results[0].step_used * tx_results[0].step_price + # todo unstake info + # todo unittest about account + # apply expire balance + # 2st expired unstake self.make_empty_blocks(1) balance: int = self.get_balance(self._accounts[0]) From 840e5ae1ea1f658b1963b68165a594b39412dba9 Mon Sep 17 00:00:00 2001 From: leeheonseung Date: Wed, 26 Aug 2020 15:19:40 +0900 Subject: [PATCH 04/17] append comment --- iconservice/icx/storage.py | 19 +++++++++++- .../iiss/prevote/test_iiss_unstake.py | 30 ++++++++++++++++--- 2 files changed, 44 insertions(+), 5 deletions(-) diff --git a/iconservice/icx/storage.py b/iconservice/icx/storage.py index cddac56d1..b6e7e25f6 100644 --- a/iconservice/icx/storage.py +++ b/iconservice/icx/storage.py @@ -270,7 +270,24 @@ def get_account(self, if AccountPartFlag.COIN in part_flags: coin_part: 'CoinPart' = self._get_part(context, CoinPart, address) - # TODO Comment + """ + Ref IS-1208. + Mismatch has_unstake flag and actual unstake info below rev 10. + The reason that branch logic by revision is backward compatibility process on transfer ICX logic. + + The below code is a logic that determines in advance whether it is possible or not before ICX transfer. + + ### _process_transaction in icon_service_engine.py ### + # Checks the balance only on the invoke context(skip estimate context) + if context.type == IconScoreContextType.INVOKE: + tmp_context: 'IconScoreContext' = IconScoreContext(IconScoreContextType.QUERY) + tmp_context.block = self._get_last_block() + # Check if from account can charge a tx fee + self._icon_pre_validator.execute_to_check_out_of_balance( + context if context.revision >= Revision.THREE.value else tmp_context, + params, + step_price=context.step_counter.step_price) + """ if context.revision >= Revision.FIX_BALANCE_BUG.value or \ CoinPartFlag.HAS_UNSTAKE in coin_part.flags: part_flags |= AccountPartFlag.STAKE diff --git a/tests/integrate_test/iiss/prevote/test_iiss_unstake.py b/tests/integrate_test/iiss/prevote/test_iiss_unstake.py index d7fd8843f..b642c7d12 100644 --- a/tests/integrate_test/iiss/prevote/test_iiss_unstake.py +++ b/tests/integrate_test/iiss/prevote/test_iiss_unstake.py @@ -28,7 +28,7 @@ class TestIISSStake(TestIISSBase): - def test_unstake1(self): + def test_unstake(self): self.update_governance() # set Revision REV_IISS @@ -60,14 +60,36 @@ def test_unstake1(self): value=stake - i ) + actual_res: dict = self.get_stake(self._accounts[0]) + first_remaining_blocks: int = 16 + expected_res = { + "stake": stake - 1 * 4, + "unstakes": [ + {"unstake": 1, "unstakeBlockHeight": 25, "remainingBlocks": first_remaining_blocks}, + {"unstake": 1, "unstakeBlockHeight": 26, "remainingBlocks": first_remaining_blocks+1}, + {"unstake": 1, "unstakeBlockHeight": 27, "remainingBlocks": first_remaining_blocks+2}, + {"unstake": 1, "unstakeBlockHeight": 28, "remainingBlocks": first_remaining_blocks+3}, + ] + } + self.assertEqual(expected_res, actual_res) + # 1st expired unstake - self.make_empty_blocks(16) + self.make_empty_blocks(first_remaining_blocks) last_balance: int = self.get_balance(self._accounts[0]) tx_results = self.transfer_icx(from_=self._accounts[0], to_=self._accounts[0], value=0) fee = tx_results[0].step_used * tx_results[0].step_price - # todo unstake info - # todo unittest about account + actual_res: dict = self.get_stake(self._accounts[0]) + remaining_blocks: int = 0 + expected_res = { + "stake": stake - 1 * 4, + "unstakes": [ + {"unstake": 1, "unstakeBlockHeight": 26, "remainingBlocks": remaining_blocks}, + {"unstake": 1, "unstakeBlockHeight": 27, "remainingBlocks": remaining_blocks+1}, + {"unstake": 1, "unstakeBlockHeight": 28, "remainingBlocks": remaining_blocks+2}, + ] + } + self.assertEqual(expected_res, actual_res) # apply expire balance # 2st expired unstake From 91ff93ad68b71448228c44c78d97ace3df25ccf7 Mon Sep 17 00:00:00 2001 From: inwonkim Date: Wed, 26 Aug 2020 16:15:10 +0900 Subject: [PATCH 05/17] Write test for Account(unstake and balance) --- tests/unit_test/icx/test_icx_account.py | 79 +++++++++++++++++++++++++ 1 file changed, 79 insertions(+) create mode 100644 tests/unit_test/icx/test_icx_account.py diff --git a/tests/unit_test/icx/test_icx_account.py b/tests/unit_test/icx/test_icx_account.py new file mode 100644 index 000000000..c0d58f683 --- /dev/null +++ b/tests/unit_test/icx/test_icx_account.py @@ -0,0 +1,79 @@ +# -*- coding: utf-8 -*- +# +# Copyright 2019 ICON Foundation +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +from unittest.mock import PropertyMock + +import pytest + +from iconservice import Address +from iconservice.base.block import Block +from iconservice.base.exception import OutOfBalanceException +from iconservice.icon_constant import Revision, IconScoreContextType +from iconservice.iconscore.context.context import ContextContainer +from iconservice.iconscore.icon_score_context import IconScoreContext +from iconservice.icx.coin_part import CoinPart, CoinPartFlag, CoinPartType +from iconservice.icx.icx_account import Account +from iconservice.icx.stake_part import StakePart + +ADDRESS = Address.from_string(f"hx{'1234'*10}") + + +@pytest.fixture(scope="function") +def context(): + ctx = IconScoreContext(IconScoreContextType.DIRECT) + block = Block(0, None, 0, None, 0) + ctx.block = block + ContextContainer._push_context(ctx) + yield ctx + ContextContainer._pop_context() + + +class TestMultipleUnstake: + + @pytest.mark.parametrize("revision", [ + revision.value for revision in Revision if revision.value == Revision.FIX_BALANCE_BUG.value + ]) + @pytest.mark.parametrize("balance, withdrawal_amount", [(0, 10)]) + @pytest.mark.parametrize("unstake_info, current_block_height", [ + ([], 20), + ([[10, 20]], 5), + ([[10, 20]], 15), + ([[10, 20], [10, 40]], 5), + ([[10, 20], [10, 40]], 15), + ([[10, 20], [10, 40]], 25), + ]) + def test_multiple_unstake_deposit_( + self, context, mocker, revision, balance, withdrawal_amount, unstake_info, current_block_height): + mocker.patch.object(IconScoreContext, "revision", PropertyMock(return_value=revision)) + coin_part = CoinPart(CoinPartType.GENERAL, CoinPartFlag.NONE, balance) + stake = 100 + unstake_lock_period = 20 + account = Account(ADDRESS, + current_block_height, + revision, + coin_part=coin_part, + stake_part=StakePart(stake=stake, unstake=0, unstake_block_height=0)) + + for info in unstake_info: + context.block._height = info[1] - unstake_lock_period + stake = stake - info[0] + account.set_stake(context, stake, unstake_lock_period) + + if revision >= Revision.FIX_BALANCE_BUG.value: + account.normalize(revision) + account.withdraw(withdrawal_amount) + else: + with pytest.raises(OutOfBalanceException) as e: + account.withdraw(withdrawal_amount) From 5a61df62af6d071fe2ddc02857e558fc09103350 Mon Sep 17 00:00:00 2001 From: inwonkim Date: Wed, 26 Aug 2020 17:13:23 +0900 Subject: [PATCH 06/17] Write test for Account related unstake and balance --- tests/unit_test/icx/test_icx_account.py | 40 ++++++++++++++----------- 1 file changed, 22 insertions(+), 18 deletions(-) diff --git a/tests/unit_test/icx/test_icx_account.py b/tests/unit_test/icx/test_icx_account.py index c0d58f683..f0805ca10 100644 --- a/tests/unit_test/icx/test_icx_account.py +++ b/tests/unit_test/icx/test_icx_account.py @@ -19,7 +19,6 @@ from iconservice import Address from iconservice.base.block import Block -from iconservice.base.exception import OutOfBalanceException from iconservice.icon_constant import Revision, IconScoreContextType from iconservice.iconscore.context.context import ContextContainer from iconservice.iconscore.icon_score_context import IconScoreContext @@ -28,6 +27,8 @@ from iconservice.icx.stake_part import StakePart ADDRESS = Address.from_string(f"hx{'1234'*10}") +UNSTAKE_LOCK_PERIOD = 20 +WITHDRAWAL_AMOUNT = 10 @pytest.fixture(scope="function") @@ -43,37 +44,40 @@ def context(): class TestMultipleUnstake: @pytest.mark.parametrize("revision", [ - revision.value for revision in Revision if revision.value == Revision.FIX_BALANCE_BUG.value + revision.value for revision in Revision if revision.value >= Revision.MULTIPLE_UNSTAKE.value ]) - @pytest.mark.parametrize("balance, withdrawal_amount", [(0, 10)]) - @pytest.mark.parametrize("unstake_info, current_block_height", [ + @pytest.mark.parametrize("unstakes_info, current_block_height", [ ([], 20), ([[10, 20]], 5), ([[10, 20]], 15), - ([[10, 20], [10, 40]], 5), - ([[10, 20], [10, 40]], 15), - ([[10, 20], [10, 40]], 25), + ([[10, 20], [10, 30]], 15), + ([[10, 20], [10, 30]], 25), + ([[10, 20], [10, 30]], 35), ]) def test_multiple_unstake_deposit_( - self, context, mocker, revision, balance, withdrawal_amount, unstake_info, current_block_height): + self, context, mocker, revision, unstakes_info, current_block_height): mocker.patch.object(IconScoreContext, "revision", PropertyMock(return_value=revision)) + stake, balance = 100, 0 coin_part = CoinPart(CoinPartType.GENERAL, CoinPartFlag.NONE, balance) - stake = 100 - unstake_lock_period = 20 account = Account(ADDRESS, current_block_height, revision, coin_part=coin_part, stake_part=StakePart(stake=stake, unstake=0, unstake_block_height=0)) - for info in unstake_info: - context.block._height = info[1] - unstake_lock_period + for info in unstakes_info: + context.block._height = info[1] - UNSTAKE_LOCK_PERIOD stake = stake - info[0] - account.set_stake(context, stake, unstake_lock_period) + # set_stake method refer account._current_block_height + account._current_block_height = context.block.height + account.set_stake(context, stake, UNSTAKE_LOCK_PERIOD) if revision >= Revision.FIX_BALANCE_BUG.value: - account.normalize(revision) - account.withdraw(withdrawal_amount) - else: - with pytest.raises(OutOfBalanceException) as e: - account.withdraw(withdrawal_amount) + account._current_block_height = current_block_height + expired_unstake = sum((unstake_info[0] for unstake_info in unstakes_info + if unstake_info[1] < current_block_height)) + balance = expired_unstake + + account.normalize(revision) + + assert balance == account.balance From 86d050403007e8db0733ecfb3b158b8cbf4bc39a Mon Sep 17 00:00:00 2001 From: inwonkim Date: Wed, 26 Aug 2020 17:15:40 +0900 Subject: [PATCH 07/17] Write test for Account related unstake and balance(fix argument) --- tests/unit_test/icx/test_icx_account.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/unit_test/icx/test_icx_account.py b/tests/unit_test/icx/test_icx_account.py index f0805ca10..68f92c78d 100644 --- a/tests/unit_test/icx/test_icx_account.py +++ b/tests/unit_test/icx/test_icx_account.py @@ -49,7 +49,7 @@ class TestMultipleUnstake: @pytest.mark.parametrize("unstakes_info, current_block_height", [ ([], 20), ([[10, 20]], 5), - ([[10, 20]], 15), + ([[10, 20]], 25), ([[10, 20], [10, 30]], 15), ([[10, 20], [10, 30]], 25), ([[10, 20], [10, 30]], 35), From 20d12917f237930c80a10d7c475bd93ebf3dca99 Mon Sep 17 00:00:00 2001 From: inwonkim Date: Wed, 26 Aug 2020 18:53:12 +0900 Subject: [PATCH 08/17] Write test for normalizing Account --- tests/unit_test/icx/test_icx_account.py | 37 +++++++++++++++---------- 1 file changed, 22 insertions(+), 15 deletions(-) diff --git a/tests/unit_test/icx/test_icx_account.py b/tests/unit_test/icx/test_icx_account.py index 68f92c78d..50610bd56 100644 --- a/tests/unit_test/icx/test_icx_account.py +++ b/tests/unit_test/icx/test_icx_account.py @@ -46,24 +46,25 @@ class TestMultipleUnstake: @pytest.mark.parametrize("revision", [ revision.value for revision in Revision if revision.value >= Revision.MULTIPLE_UNSTAKE.value ]) - @pytest.mark.parametrize("unstakes_info, current_block_height", [ - ([], 20), - ([[10, 20]], 5), - ([[10, 20]], 25), - ([[10, 20], [10, 30]], 15), - ([[10, 20], [10, 30]], 25), - ([[10, 20], [10, 30]], 35), + @pytest.mark.parametrize("unstakes_info, current_block_height, has_unstake", [ + ([], 20, False), + ([[10, 20]], 5, True), + ([[10, 20]], 25, False), + ([[10, 20], [10, 30]], 15, True), + ([[10, 20], [10, 30]], 25, False), + ([[10, 20], [10, 30]], 35, False), ]) def test_multiple_unstake_deposit_( - self, context, mocker, revision, unstakes_info, current_block_height): + self, context, mocker, revision, unstakes_info, current_block_height, has_unstake): mocker.patch.object(IconScoreContext, "revision", PropertyMock(return_value=revision)) stake, balance = 100, 0 coin_part = CoinPart(CoinPartType.GENERAL, CoinPartFlag.NONE, balance) + stake_part = StakePart(stake=stake, unstake=0, unstake_block_height=0) account = Account(ADDRESS, current_block_height, revision, coin_part=coin_part, - stake_part=StakePart(stake=stake, unstake=0, unstake_block_height=0)) + stake_part=stake_part) for info in unstakes_info: context.block._height = info[1] - UNSTAKE_LOCK_PERIOD @@ -72,12 +73,18 @@ def test_multiple_unstake_deposit_( account._current_block_height = context.block.height account.set_stake(context, stake, UNSTAKE_LOCK_PERIOD) - if revision >= Revision.FIX_BALANCE_BUG.value: - account._current_block_height = current_block_height - expired_unstake = sum((unstake_info[0] for unstake_info in unstakes_info - if unstake_info[1] < current_block_height)) - balance = expired_unstake + stake_part = account.stake_part + remaining_unstakes = [unstake_info for unstake_info in unstakes_info if unstake_info[1] > current_block_height] + expired_unstake = sum((unstake_info[0] for unstake_info in unstakes_info + if unstake_info[1] < current_block_height)) + balance = expired_unstake - account.normalize(revision) + account = Account(ADDRESS, current_block_height, revision, coin_part=coin_part, + stake_part=stake_part) + if revision >= Revision.FIX_BALANCE_BUG.value: + has_unstake = False + assert stake == account.stake assert balance == account.balance + assert remaining_unstakes == account.unstakes_info + assert (CoinPartFlag.HAS_UNSTAKE in account.coin_part.flags) == has_unstake From b63bb00707eb39583dbf419708ef05923ba40be9 Mon Sep 17 00:00:00 2001 From: Chiwon Cho Date: Wed, 26 Aug 2020 21:43:48 +0900 Subject: [PATCH 09/17] Add unit-test for StakePart --- tests/unit_test/icx/__init__.py | 0 tests/unit_test/icx/test_stake_part.py | 80 ++++++++++++++++++++++++++ 2 files changed, 80 insertions(+) create mode 100644 tests/unit_test/icx/__init__.py create mode 100644 tests/unit_test/icx/test_stake_part.py diff --git a/tests/unit_test/icx/__init__.py b/tests/unit_test/icx/__init__.py new file mode 100644 index 000000000..e69de29bb diff --git a/tests/unit_test/icx/test_stake_part.py b/tests/unit_test/icx/test_stake_part.py new file mode 100644 index 000000000..5d0899347 --- /dev/null +++ b/tests/unit_test/icx/test_stake_part.py @@ -0,0 +1,80 @@ +# -*- coding: utf-8 -*- + +import pytest + +from iconservice.icon_constant import Revision +from iconservice.icx.stake_part import StakePart + + +class TestStakePart: + @pytest.mark.parametrize( + "block_height,params,expected", + [ + ( + 999, + {"stake": 0, "unstake": 100, "unstake_block_height": 1000}, + {"stake": 0, "unstake": 100, "unstake_block_height": 1000, "expired_unstake": 0}, + ), + ( + 999, + {"stake": 50, "unstake": 100, "unstake_block_height": 1000}, + {"stake": 50, "unstake": 100, "unstake_block_height": 1000, "expired_unstake": 0}, + ), + ( + 1000, + {"stake": 0, "unstake": 100, "unstake_block_height": 1000}, + {"stake": 0, "unstake": 100, "unstake_block_height": 1000, "expired_unstake": 0}, + ), + ( + 1000, + {"stake": 50, "unstake": 100, "unstake_block_height": 1000}, + {"stake": 50, "unstake": 100, "unstake_block_height": 1000, "expired_unstake": 0}, + ), + ( + 1001, + {"stake": 0, "unstake": 100, "unstake_block_height": 1000}, + {"stake": 0, "unstake": 0, "unstake_block_height": 0, "expired_unstake": 100}, + ), + ( + 1001, + {"stake": 50, "unstake": 100, "unstake_block_height": 1000}, + {"stake": 50, "unstake": 0, "unstake_block_height": 0, "expired_unstake": 100}, + ), + ] + ) + def test_stake_part_on_rev_8(self, block_height, params, expected): + revision = 8 + stake_part = StakePart(**params) + expired_unstake: int = stake_part.normalize(block_height=block_height, revision=revision) + + assert expired_unstake == expected["expired_unstake"] + assert stake_part.stake == expected["stake"] + assert stake_part.total_stake == expected["stake"] + expected["unstake"] + assert stake_part.unstake == expected["unstake"] + assert stake_part.total_unstake == expected["unstake"] + assert stake_part.unstake_block_height == expected["unstake_block_height"] + + @pytest.mark.parametrize( + "stake,unstakes_info,amount,expected_unstakes_info", + [ + (0, [[100, 1000]], 10, [[90, 1000]]), + (0, [[100, 1000], [200, 1500]], 50, [[100, 1000], [150, 1500]]), + (0, [[100, 1000], [200, 1500]], 200, [[100, 1000]]), + (0, [[100, 1000], [200, 1500]], 299, [[1, 1000]]), + (0, [[100, 1000], [200, 1500], [50, 2000]], 20, [[100, 1000], [200, 1500], [30, 2000]]), + (0, [[100, 1000], [200, 1500], [50, 2000]], 50, [[100, 1000], [200, 1500]]), + (0, [[100, 1000], [200, 1500], [50, 2000]], 100, [[100, 1000], [150, 1500]]), + (0, [[100, 1000], [200, 1500], [50, 2000]], 250, [[100, 1000]]), + (0, [[100, 1000], [200, 1500], [50, 2000]], 270, [[80, 1000]]), + ] + ) + def test_withdraw_unstake(self, stake, unstakes_info, amount, expected_unstakes_info): + expected_total_unstake = sum(unstake for unstake, _ in expected_unstakes_info) + + stake_part = StakePart(stake=stake, unstakes_info=unstakes_info) + stake_part.normalize(block_height=500, revision=Revision.FIX_UNSTAKE_BUG.value) + stake_part.withdraw_unstake(amount) + + assert stake_part.stake == stake + assert stake_part.unstakes_info == expected_unstakes_info + assert stake_part.total_unstake == expected_total_unstake From 69b7da50c7607cfb6fd3770e60bb14f66fd14bad Mon Sep 17 00:00:00 2001 From: inwonkim Date: Wed, 26 Aug 2020 22:50:14 +0900 Subject: [PATCH 10/17] Write test for getting Account in IcxStorage --- tests/unit_test/icx/test_icx_account.py | 14 ++-- tests/unit_test/icx/test_icx_storage.py | 89 +++++++++++++++++++++++++ 2 files changed, 94 insertions(+), 9 deletions(-) create mode 100644 tests/unit_test/icx/test_icx_storage.py diff --git a/tests/unit_test/icx/test_icx_account.py b/tests/unit_test/icx/test_icx_account.py index 50610bd56..545d14164 100644 --- a/tests/unit_test/icx/test_icx_account.py +++ b/tests/unit_test/icx/test_icx_account.py @@ -41,7 +41,7 @@ def context(): ContextContainer._pop_context() -class TestMultipleUnstake: +class TestAccount: @pytest.mark.parametrize("revision", [ revision.value for revision in Revision if revision.value >= Revision.MULTIPLE_UNSTAKE.value @@ -54,17 +54,14 @@ class TestMultipleUnstake: ([[10, 20], [10, 30]], 25, False), ([[10, 20], [10, 30]], 35, False), ]) - def test_multiple_unstake_deposit_( + def test_normalize( self, context, mocker, revision, unstakes_info, current_block_height, has_unstake): mocker.patch.object(IconScoreContext, "revision", PropertyMock(return_value=revision)) stake, balance = 100, 0 coin_part = CoinPart(CoinPartType.GENERAL, CoinPartFlag.NONE, balance) stake_part = StakePart(stake=stake, unstake=0, unstake_block_height=0) - account = Account(ADDRESS, - current_block_height, - revision, - coin_part=coin_part, - stake_part=stake_part) + account = Account( + ADDRESS, current_block_height, revision, coin_part=coin_part, stake_part=stake_part) for info in unstakes_info: context.block._height = info[1] - UNSTAKE_LOCK_PERIOD @@ -79,8 +76,7 @@ def test_multiple_unstake_deposit_( if unstake_info[1] < current_block_height)) balance = expired_unstake - account = Account(ADDRESS, current_block_height, revision, coin_part=coin_part, - stake_part=stake_part) + account = Account(ADDRESS, current_block_height, revision, coin_part=coin_part, stake_part=stake_part) if revision >= Revision.FIX_BALANCE_BUG.value: has_unstake = False diff --git a/tests/unit_test/icx/test_icx_storage.py b/tests/unit_test/icx/test_icx_storage.py new file mode 100644 index 000000000..084b8de0c --- /dev/null +++ b/tests/unit_test/icx/test_icx_storage.py @@ -0,0 +1,89 @@ +# -*- coding: utf-8 -*- +# +# Copyright 2019 ICON Foundation +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +import shutil +from unittest.mock import PropertyMock + +import pytest + +from iconservice import Address +from iconservice.base.block import Block +from iconservice.database.db import ContextDatabase +from iconservice.icon_constant import Revision, IconScoreContextType +from iconservice.iconscore.context.context import ContextContainer +from iconservice.iconscore.icon_score_context import IconScoreContext +from iconservice.icx.coin_part import CoinPart, CoinPartFlag, CoinPartType +from iconservice.icx.icx_account import Account +from iconservice.icx.stake_part import StakePart +from iconservice.icx.storage import Storage, Intent +from iconservice.utils import ContextStorage + +ADDRESS = Address.from_string(f"hx{'1234'*10}") +UNSTAKE_LOCK_PERIOD = 20 +WITHDRAWAL_AMOUNT = 10 + + +@pytest.fixture(scope="function") +def context(): + ctx = IconScoreContext(IconScoreContextType.DIRECT) + block = Block(0, None, 0, None, 0) + ctx.block = block + ContextContainer._push_context(ctx) + yield ctx + ContextContainer._pop_context() + + +@pytest.fixture(scope="function") +def storage(context): + db_name = 'icx.db' + db = ContextDatabase.from_path(db_name) + storage = Storage(db) + context.storage = ContextStorage(icx=storage) + storage.open(context) + yield storage + storage.close(context) + shutil.rmtree(db_name) + + +class TestIcxStorage: + + @pytest.mark.parametrize("unstakes_info, current_block_height, flag, expected_balance", [ + ([], 20, CoinPartFlag.NONE, 100), + ([[10, 20]], 5, CoinPartFlag.HAS_UNSTAKE, 100), + ([[10, 20]], 25, CoinPartFlag.NONE, 110), + ([[10, 20], [10, 30]], 15, CoinPartFlag.HAS_UNSTAKE, 100), + ([[10, 20], [10, 30]], 25, CoinPartFlag.NONE, 110), + ([[10, 20], [10, 30]], 35, CoinPartFlag.NONE, 120), + ]) + def test_get_account( + self, storage, context, mocker, unstakes_info, current_block_height, flag, expected_balance): + revision = Revision.FIX_BALANCE_BUG.value + mocker.patch.object(IconScoreContext, "revision", PropertyMock(return_value=revision)) + stake, balance = 100, 100 + coin_part = CoinPart(CoinPartType.GENERAL, CoinPartFlag.NONE, balance) + coin_part.set_dirty(True) + stake_part = StakePart(stake=stake, unstake=0, unstake_block_height=0, unstakes_info=unstakes_info) + stake_part.set_dirty(True) + account = Account( + ADDRESS, current_block_height, revision, coin_part=coin_part, stake_part=stake_part) + account.coin_part._flags = flag + context.block._height = current_block_height + storage.put_account(context, account) + + remaining_unstakes = [unstake_info for unstake_info in unstakes_info if unstake_info[1] > current_block_height] + + account = storage.get_account(context, ADDRESS) + assert account.balance == expected_balance + assert account.unstakes_info == remaining_unstakes From 5599a16b96a481536a8fe371165dc5705c2f0687 Mon Sep 17 00:00:00 2001 From: inwonkim Date: Wed, 26 Aug 2020 23:24:15 +0900 Subject: [PATCH 11/17] Add test case when current block == unstakeBlockHeight --- tests/unit_test/icx/test_icx_storage.py | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/tests/unit_test/icx/test_icx_storage.py b/tests/unit_test/icx/test_icx_storage.py index 084b8de0c..6d3d0b1aa 100644 --- a/tests/unit_test/icx/test_icx_storage.py +++ b/tests/unit_test/icx/test_icx_storage.py @@ -60,15 +60,21 @@ def storage(context): class TestIcxStorage: @pytest.mark.parametrize("unstakes_info, current_block_height, flag, expected_balance", [ + (None, 20, CoinPartFlag.NONE, 100), ([], 20, CoinPartFlag.NONE, 100), + ([], 20, CoinPartFlag.HAS_UNSTAKE, 100), ([[10, 20]], 5, CoinPartFlag.HAS_UNSTAKE, 100), + ([[10, 20]], 20, CoinPartFlag.HAS_UNSTAKE, 100), ([[10, 20]], 25, CoinPartFlag.NONE, 110), ([[10, 20], [10, 30]], 15, CoinPartFlag.HAS_UNSTAKE, 100), + ([[10, 20], [10, 30]], 20, CoinPartFlag.HAS_UNSTAKE, 100), ([[10, 20], [10, 30]], 25, CoinPartFlag.NONE, 110), + ([[10, 20], [10, 30]], 30, CoinPartFlag.NONE, 110), ([[10, 20], [10, 30]], 35, CoinPartFlag.NONE, 120), ]) def test_get_account( self, storage, context, mocker, unstakes_info, current_block_height, flag, expected_balance): + # test whether the `Account` saved in the wrong format is properly got on revision11. revision = Revision.FIX_BALANCE_BUG.value mocker.patch.object(IconScoreContext, "revision", PropertyMock(return_value=revision)) stake, balance = 100, 100 @@ -82,7 +88,12 @@ def test_get_account( context.block._height = current_block_height storage.put_account(context, account) - remaining_unstakes = [unstake_info for unstake_info in unstakes_info if unstake_info[1] > current_block_height] + if unstakes_info is None: + remaining_unstakes = [] + else: + remaining_unstakes = [ + unstake_info for unstake_info in unstakes_info if unstake_info[1] >= current_block_height + ] account = storage.get_account(context, ADDRESS) assert account.balance == expected_balance From 12bbf56b276a928de3542e2a38928d6a45c90d8c Mon Sep 17 00:00:00 2001 From: inwonkim Date: Thu, 27 Aug 2020 00:05:46 +0900 Subject: [PATCH 12/17] Improve test for normalizing Account --- tests/unit_test/icx/test_icx_account.py | 58 ++++++++++++------------- 1 file changed, 28 insertions(+), 30 deletions(-) diff --git a/tests/unit_test/icx/test_icx_account.py b/tests/unit_test/icx/test_icx_account.py index 545d14164..3ce7d9423 100644 --- a/tests/unit_test/icx/test_icx_account.py +++ b/tests/unit_test/icx/test_icx_account.py @@ -13,6 +13,7 @@ # WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. # See the License for the specific language governing permissions and # limitations under the License. +import copy from unittest.mock import PropertyMock import pytest @@ -46,41 +47,38 @@ class TestAccount: @pytest.mark.parametrize("revision", [ revision.value for revision in Revision if revision.value >= Revision.MULTIPLE_UNSTAKE.value ]) - @pytest.mark.parametrize("unstakes_info, current_block_height, has_unstake", [ - ([], 20, False), - ([[10, 20]], 5, True), - ([[10, 20]], 25, False), - ([[10, 20], [10, 30]], 15, True), - ([[10, 20], [10, 30]], 25, False), - ([[10, 20], [10, 30]], 35, False), + @pytest.mark.parametrize("unstakes_info, current_block_height, flag, expected_balance", [ + (None, 20, CoinPartFlag.NONE, 100), + ([], 20, CoinPartFlag.NONE, 100), + ([], 20, CoinPartFlag.HAS_UNSTAKE, 100), + ([[10, 20]], 5, CoinPartFlag.HAS_UNSTAKE, 100), + ([[10, 20]], 20, CoinPartFlag.HAS_UNSTAKE, 100), + ([[10, 20]], 25, CoinPartFlag.NONE, 110), + ([[10, 20], [10, 30]], 15, CoinPartFlag.HAS_UNSTAKE, 100), + ([[10, 20], [10, 30]], 20, CoinPartFlag.HAS_UNSTAKE, 100), + ([[10, 20], [10, 30]], 25, CoinPartFlag.NONE, 110), + ([[10, 20], [10, 30]], 30, CoinPartFlag.NONE, 110), + ([[10, 20], [10, 30]], 35, CoinPartFlag.NONE, 120), ]) def test_normalize( - self, context, mocker, revision, unstakes_info, current_block_height, has_unstake): + self, context, mocker, revision, unstakes_info, current_block_height, flag, expected_balance): + unstakes_info = copy.deepcopy(unstakes_info) mocker.patch.object(IconScoreContext, "revision", PropertyMock(return_value=revision)) - stake, balance = 100, 0 - coin_part = CoinPart(CoinPartType.GENERAL, CoinPartFlag.NONE, balance) - stake_part = StakePart(stake=stake, unstake=0, unstake_block_height=0) + stake, balance = 100, 100 + coin_part = CoinPart(CoinPartType.GENERAL, flag, balance) + stake_part = StakePart(stake=stake, unstake=0, unstake_block_height=0, unstakes_info=unstakes_info) account = Account( ADDRESS, current_block_height, revision, coin_part=coin_part, stake_part=stake_part) - for info in unstakes_info: - context.block._height = info[1] - UNSTAKE_LOCK_PERIOD - stake = stake - info[0] - # set_stake method refer account._current_block_height - account._current_block_height = context.block.height - account.set_stake(context, stake, UNSTAKE_LOCK_PERIOD) + if unstakes_info is None: + remaining_unstakes = [] + else: + remaining_unstakes = [ + unstake_info for unstake_info in unstakes_info if unstake_info[1] >= current_block_height + ] - stake_part = account.stake_part - remaining_unstakes = [unstake_info for unstake_info in unstakes_info if unstake_info[1] > current_block_height] - expired_unstake = sum((unstake_info[0] for unstake_info in unstakes_info - if unstake_info[1] < current_block_height)) - balance = expired_unstake + account.normalize(revision) - account = Account(ADDRESS, current_block_height, revision, coin_part=coin_part, stake_part=stake_part) - if revision >= Revision.FIX_BALANCE_BUG.value: - has_unstake = False - - assert stake == account.stake - assert balance == account.balance - assert remaining_unstakes == account.unstakes_info - assert (CoinPartFlag.HAS_UNSTAKE in account.coin_part.flags) == has_unstake + assert account.stake == stake + assert account.balance == expected_balance + assert account.unstakes_info == remaining_unstakes From c83ce1e6918126863e4123f3128fdd9fb9b868b9 Mon Sep 17 00:00:00 2001 From: Chiwon Cho Date: Thu, 27 Aug 2020 00:05:48 +0900 Subject: [PATCH 13/17] Update unit-tests for IcxStorage and StakePart --- tests/unit_test/icx/test_icx_storage.py | 77 +++++++++++++------------ tests/unit_test/icx/test_stake_part.py | 38 ++++++++++++ 2 files changed, 79 insertions(+), 36 deletions(-) diff --git a/tests/unit_test/icx/test_icx_storage.py b/tests/unit_test/icx/test_icx_storage.py index 6d3d0b1aa..aa8872d72 100644 --- a/tests/unit_test/icx/test_icx_storage.py +++ b/tests/unit_test/icx/test_icx_storage.py @@ -13,6 +13,7 @@ # WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. # See the License for the specific language governing permissions and # limitations under the License. +import copy import shutil from unittest.mock import PropertyMock @@ -59,42 +60,46 @@ def storage(context): class TestIcxStorage: - @pytest.mark.parametrize("unstakes_info, current_block_height, flag, expected_balance", [ - (None, 20, CoinPartFlag.NONE, 100), - ([], 20, CoinPartFlag.NONE, 100), - ([], 20, CoinPartFlag.HAS_UNSTAKE, 100), - ([[10, 20]], 5, CoinPartFlag.HAS_UNSTAKE, 100), - ([[10, 20]], 20, CoinPartFlag.HAS_UNSTAKE, 100), - ([[10, 20]], 25, CoinPartFlag.NONE, 110), - ([[10, 20], [10, 30]], 15, CoinPartFlag.HAS_UNSTAKE, 100), - ([[10, 20], [10, 30]], 20, CoinPartFlag.HAS_UNSTAKE, 100), - ([[10, 20], [10, 30]], 25, CoinPartFlag.NONE, 110), - ([[10, 20], [10, 30]], 30, CoinPartFlag.NONE, 110), - ([[10, 20], [10, 30]], 35, CoinPartFlag.NONE, 120), + @pytest.mark.parametrize("flag", [CoinPartFlag.NONE, CoinPartFlag.HAS_UNSTAKE]) + @pytest.mark.parametrize("unstakes_info, current_block_height, expected_balance", [ + (None, 20, 100), + ([], 20, 100), + ([], 20, 100), + ([[10, 20]], 5, 100), + ([[10, 20]], 20, 100), + ([[10, 20]], 25, 110), + ([[10, 20], [10, 30]], 15, 100), + ([[10, 20], [10, 30]], 20, 100), + ([[10, 20], [10, 30]], 25, 110), + ([[10, 20], [10, 30]], 30, 110), + ([[10, 20], [10, 30]], 35, 120), ]) - def test_get_account( - self, storage, context, mocker, unstakes_info, current_block_height, flag, expected_balance): - # test whether the `Account` saved in the wrong format is properly got on revision11. - revision = Revision.FIX_BALANCE_BUG.value - mocker.patch.object(IconScoreContext, "revision", PropertyMock(return_value=revision)) - stake, balance = 100, 100 - coin_part = CoinPart(CoinPartType.GENERAL, CoinPartFlag.NONE, balance) - coin_part.set_dirty(True) - stake_part = StakePart(stake=stake, unstake=0, unstake_block_height=0, unstakes_info=unstakes_info) - stake_part.set_dirty(True) - account = Account( - ADDRESS, current_block_height, revision, coin_part=coin_part, stake_part=stake_part) - account.coin_part._flags = flag - context.block._height = current_block_height - storage.put_account(context, account) + def test_get_account(self, + storage, context, mocker, flag, unstakes_info, current_block_height, expected_balance): + unstakes_info = copy.deepcopy(unstakes_info) - if unstakes_info is None: - remaining_unstakes = [] - else: - remaining_unstakes = [ - unstake_info for unstake_info in unstakes_info if unstake_info[1] >= current_block_height - ] + # test whether the `Account` saved in the wrong format is properly got on revision11. + revision = Revision.FIX_BALANCE_BUG.value + mocker.patch.object(IconScoreContext, "revision", PropertyMock(return_value=revision)) - account = storage.get_account(context, ADDRESS) - assert account.balance == expected_balance - assert account.unstakes_info == remaining_unstakes + stake, balance = 100, 100 + coin_part = CoinPart(CoinPartType.GENERAL, flag, balance) + coin_part.set_dirty(True) + stake_part = StakePart(stake=stake, unstake=0, unstake_block_height=0, unstakes_info=unstakes_info) + stake_part.set_dirty(True) + account = Account( + ADDRESS, current_block_height, revision, coin_part=coin_part, stake_part=stake_part) + account.coin_part._flags = flag + context.block._height = current_block_height + storage.put_account(context, account) + + if unstakes_info is None: + remaining_unstakes = [] + else: + remaining_unstakes = [ + unstake_info for unstake_info in unstakes_info if unstake_info[1] >= current_block_height + ] + + account = storage.get_account(context, ADDRESS) + assert account.balance == expected_balance + assert account.unstakes_info == remaining_unstakes diff --git a/tests/unit_test/icx/test_stake_part.py b/tests/unit_test/icx/test_stake_part.py index 5d0899347..46e1649d9 100644 --- a/tests/unit_test/icx/test_stake_part.py +++ b/tests/unit_test/icx/test_stake_part.py @@ -1,5 +1,6 @@ # -*- coding: utf-8 -*- +import random import pytest from iconservice.icon_constant import Revision @@ -73,8 +74,45 @@ def test_withdraw_unstake(self, stake, unstakes_info, amount, expected_unstakes_ stake_part = StakePart(stake=stake, unstakes_info=unstakes_info) stake_part.normalize(block_height=500, revision=Revision.FIX_UNSTAKE_BUG.value) + + # withdraw() assumes that amount is less than total_stake + assert stake_part.total_unstake > amount stake_part.withdraw_unstake(amount) assert stake_part.stake == stake assert stake_part.unstakes_info == expected_unstakes_info assert stake_part.total_unstake == expected_total_unstake + + @pytest.mark.parametrize( + "unstakes_info, new_total_unstake, unstake_block_height, expected_unstakes_info", + [ + (None, 100, 1000, [[100, 1000]]), + ([], 50, 5000, [[50, 5000]]), + + ([[100, 1000]], 300, 2000, [[100, 1000], [200, 2000]]), + ([[100, 1000]], 300, 800, [[200, 800], [100, 1000]]), + + ([[100, 2000], [200, 3000]], 600, 1000, [[300, 1000], [100, 2000], [200, 3000]]), + ([[100, 1000], [200, 3000]], 600, 2000, [[100, 1000], [300, 2000], [200, 3000]]), + ([[100, 1000], [200, 2000]], 600, 3000, [[100, 1000], [200, 2000], [300, 3000]]), + + ( + [[100, 1000], [200, 1500], [300, 2000]], 550, 3000, [[100, 1000], [200, 1500], [250, 2000]] + ), + ] + ) + def test_set_unstakes_info(self, unstakes_info, new_total_unstake, unstake_block_height, expected_unstakes_info): + old_total_unsake: int = sum(info[0] for info in unstakes_info) if unstakes_info else 0 + assert new_total_unstake != old_total_unsake + + stake = random.randint(0, 100) + slot_max = 3 + revision = Revision.FIX_UNSTAKE_BUG.value + block_height = 500 + + stake_part = StakePart(stake=stake, unstakes_info=unstakes_info) + stake_part.normalize(block_height, revision) + + stake_part.set_unstakes_info(unstake_block_height, new_total_unstake, slot_max) + assert stake_part.is_dirty() + assert stake_part.unstakes_info == expected_unstakes_info From 469d4665eefcddbd8e93febbac7ba5f697a773bf Mon Sep 17 00:00:00 2001 From: Chiwon Cho Date: Thu, 27 Aug 2020 02:52:13 +0900 Subject: [PATCH 14/17] Unit-test for StakePart is done --- tests/unit_test/icx/test_stake_part.py | 151 ++++++++++++++++++++++++- 1 file changed, 145 insertions(+), 6 deletions(-) diff --git a/tests/unit_test/icx/test_stake_part.py b/tests/unit_test/icx/test_stake_part.py index 46e1649d9..643f0b79b 100644 --- a/tests/unit_test/icx/test_stake_part.py +++ b/tests/unit_test/icx/test_stake_part.py @@ -1,6 +1,8 @@ # -*- coding: utf-8 -*- +import copy import random + import pytest from iconservice.icon_constant import Revision @@ -55,6 +57,92 @@ def test_stake_part_on_rev_8(self, block_height, params, expected): assert stake_part.total_unstake == expected["unstake"] assert stake_part.unstake_block_height == expected["unstake_block_height"] + @pytest.mark.parametrize( + "revision, block_height," + "unstakes_info," + "expected_unstakes_info, expected_expired_unstake, expected_dirty", + [ + ( + Revision.MULTIPLE_UNSTAKE.value, 500, + [[100, 1000]], + [[100, 1000]], 0, False + ), + ( + Revision.FIX_UNSTAKE_BUG.value, 500, + [[100, 1000]], + [[100, 1000]], 0, False + ), + + ( + # BUG case + Revision.MULTIPLE_UNSTAKE.value, 1001, + [[100, 1000]], + [], 100, False + ), + ( + Revision.FIX_UNSTAKE_BUG.value, 1001, + [[100, 1000]], + [], 100, True + ), + + ( + Revision.MULTIPLE_UNSTAKE.value, 999, + [[100, 1000], [200, 2000]], + [[100, 1000], [200, 2000]], 0, False + ), + ( + Revision.FIX_UNSTAKE_BUG.value, 1000, + [[100, 1000], [200, 2000]], + [[100, 1000], [200, 2000]], 0, False + ), + + ( + Revision.MULTIPLE_UNSTAKE.value, 1001, + [[100, 1000], [200, 2000]], + [[200, 2000]], 100, True + ), + ( + Revision.FIX_UNSTAKE_BUG.value, 1001, + [[100, 1000], [200, 2000]], + [[200, 2000]], 100, True + ), + + ( + Revision.MULTIPLE_UNSTAKE.value, 2000, + [[100, 1000], [200, 2000]], + [[200, 2000]], 100, True + ), + ( + Revision.FIX_UNSTAKE_BUG.value, 2000, + [[100, 1000], [200, 2000]], + [[200, 2000]], 100, True + ), + + ( + # BUG Case + Revision.MULTIPLE_UNSTAKE.value, 2001, + [[100, 1000], [200, 2000]], + [], 300, False + ), + ( + Revision.FIX_UNSTAKE_BUG.value, 2001, + [[100, 1000], [200, 2000]], + [], 300, True + ), + ] + ) + def test_normalize( + self, revision, block_height, + unstakes_info, + expected_expired_unstake, expected_unstakes_info, expected_dirty): + stake = random.randint(0, 100) + + stake_part = StakePart(stake=stake, unstakes_info=copy.deepcopy(unstakes_info)) + expired_unstake = stake_part.normalize(block_height, revision) + assert expired_unstake == expected_expired_unstake + assert stake_part.unstakes_info == expected_unstakes_info + assert stake_part.is_dirty() == expected_dirty + @pytest.mark.parametrize( "stake,unstakes_info,amount,expected_unstakes_info", [ @@ -89,15 +177,62 @@ def test_withdraw_unstake(self, stake, unstakes_info, amount, expected_unstakes_ (None, 100, 1000, [[100, 1000]]), ([], 50, 5000, [[50, 5000]]), - ([[100, 1000]], 300, 2000, [[100, 1000], [200, 2000]]), - ([[100, 1000]], 300, 800, [[200, 800], [100, 1000]]), + ( + [[100, 1000]], + 300, 2000, + [[100, 1000], [200, 2000]] + ), + ( + [[100, 1000]], + 300, 800, + [[200, 800], [100, 1000]] + ), - ([[100, 2000], [200, 3000]], 600, 1000, [[300, 1000], [100, 2000], [200, 3000]]), - ([[100, 1000], [200, 3000]], 600, 2000, [[100, 1000], [300, 2000], [200, 3000]]), - ([[100, 1000], [200, 2000]], 600, 3000, [[100, 1000], [200, 2000], [300, 3000]]), + ( + [[100, 2000], [200, 3000]], + 600, 1000, + [[300, 1000], [100, 2000], [200, 3000]] + ), + ( + [[100, 1000], [200, 3000]], + 600, 2000, + [[100, 1000], [300, 2000], [200, 3000]] + ), + ( + [[100, 1000], [200, 2000]], + 600, 3000, + [[100, 1000], [200, 2000], [300, 3000]] + ), ( - [[100, 1000], [200, 1500], [300, 2000]], 550, 3000, [[100, 1000], [200, 1500], [250, 2000]] + [[100, 1000], [200, 1500], [300, 2000]], + 700, 3000, + [[100, 1000], [200, 1500], [400, 3000]] + ), + ( + [[100, 1000], [200, 1500], [300, 2000]], + 550, 3000, + [[100, 1000], [200, 1500], [250, 2000]] + ), + ( + [[100, 1000], [200, 1500], [300, 2000]], + 300, 3000, + [[100, 1000], [200, 1500]] + ), + ( + [[100, 1000], [200, 1500], [300, 2000]], + 250, 3000, + [[100, 1000], [150, 1500]] + ), + ( + [[100, 1000], [200, 1500], [300, 2000]], + 100, 3000, + [[100, 1000]] + ), + ( + [[100, 1000], [200, 1500], [300, 2000]], + 50, 3000, + [[50, 1000]] ), ] ) @@ -113,6 +248,10 @@ def test_set_unstakes_info(self, unstakes_info, new_total_unstake, unstake_block stake_part = StakePart(stake=stake, unstakes_info=unstakes_info) stake_part.normalize(block_height, revision) + unstake_delta: int = new_total_unstake - old_total_unsake stake_part.set_unstakes_info(unstake_block_height, new_total_unstake, slot_max) assert stake_part.is_dirty() + assert stake_part.stake == stake - unstake_delta assert stake_part.unstakes_info == expected_unstakes_info + assert stake_part.total_unstake == new_total_unstake + assert stake_part.total_stake == stake_part.stake + stake_part.total_unstake From 5b7c35e2c4c482a3d5a9dda981b8ed91134e104f Mon Sep 17 00:00:00 2001 From: Chiwon Cho Date: Thu, 27 Aug 2020 03:13:05 +0900 Subject: [PATCH 15/17] Update unit-test for Account --- tests/unit_test/icx/test_icx_account.py | 25 +++++++++++++------------ 1 file changed, 13 insertions(+), 12 deletions(-) diff --git a/tests/unit_test/icx/test_icx_account.py b/tests/unit_test/icx/test_icx_account.py index 3ce7d9423..35bdf9a17 100644 --- a/tests/unit_test/icx/test_icx_account.py +++ b/tests/unit_test/icx/test_icx_account.py @@ -47,24 +47,25 @@ class TestAccount: @pytest.mark.parametrize("revision", [ revision.value for revision in Revision if revision.value >= Revision.MULTIPLE_UNSTAKE.value ]) - @pytest.mark.parametrize("unstakes_info, current_block_height, flag, expected_balance", [ - (None, 20, CoinPartFlag.NONE, 100), - ([], 20, CoinPartFlag.NONE, 100), - ([], 20, CoinPartFlag.HAS_UNSTAKE, 100), - ([[10, 20]], 5, CoinPartFlag.HAS_UNSTAKE, 100), - ([[10, 20]], 20, CoinPartFlag.HAS_UNSTAKE, 100), - ([[10, 20]], 25, CoinPartFlag.NONE, 110), - ([[10, 20], [10, 30]], 15, CoinPartFlag.HAS_UNSTAKE, 100), - ([[10, 20], [10, 30]], 20, CoinPartFlag.HAS_UNSTAKE, 100), - ([[10, 20], [10, 30]], 25, CoinPartFlag.NONE, 110), - ([[10, 20], [10, 30]], 30, CoinPartFlag.NONE, 110), - ([[10, 20], [10, 30]], 35, CoinPartFlag.NONE, 120), + @pytest.mark.parametrize("flag", [CoinPartFlag.NONE, CoinPartFlag.HAS_UNSTAKE]) + @pytest.mark.parametrize("unstakes_info, current_block_height, expected_balance", [ + (None, 20, 100), + ([], 20, 100), + ([[10, 20]], 5, 100), + ([[10, 20]], 20, 100), + ([[10, 20]], 25, 110), + ([[10, 20], [10, 30]], 15, 100), + ([[10, 20], [10, 30]], 20, 100), + ([[10, 20], [10, 30]], 25, 110), + ([[10, 20], [10, 30]], 30, 110), + ([[10, 20], [10, 30]], 35, 120), ]) def test_normalize( self, context, mocker, revision, unstakes_info, current_block_height, flag, expected_balance): unstakes_info = copy.deepcopy(unstakes_info) mocker.patch.object(IconScoreContext, "revision", PropertyMock(return_value=revision)) stake, balance = 100, 100 + coin_part = CoinPart(CoinPartType.GENERAL, flag, balance) stake_part = StakePart(stake=stake, unstake=0, unstake_block_height=0, unstakes_info=unstakes_info) account = Account( From a8bb9030a8876f990c64f2de0a4619984ac09f7a Mon Sep 17 00:00:00 2001 From: leeheonseung Date: Thu, 27 Aug 2020 11:36:39 +0900 Subject: [PATCH 16/17] append integrate test --- .../iiss/prevote/test_iiss_unstake.py | 92 ++++++++++++------- 1 file changed, 60 insertions(+), 32 deletions(-) diff --git a/tests/integrate_test/iiss/prevote/test_iiss_unstake.py b/tests/integrate_test/iiss/prevote/test_iiss_unstake.py index b642c7d12..10ee42996 100644 --- a/tests/integrate_test/iiss/prevote/test_iiss_unstake.py +++ b/tests/integrate_test/iiss/prevote/test_iiss_unstake.py @@ -17,9 +17,7 @@ """IconScoreEngine testcase """ from typing import TYPE_CHECKING, List -from unittest.mock import patch -from iconservice import SYSTEM_SCORE_ADDRESS from iconservice.icon_constant import Revision, ICX_IN_LOOP from tests.integrate_test.iiss.test_iiss_base import TestIISSBase @@ -28,7 +26,26 @@ class TestIISSStake(TestIISSBase): - def test_unstake(self): + def test_unstake_balance_rev_10(self): + self._test_unstake_balance( + rev=Revision.FIX_UNSTAKE_BUG.value, + expected_expired_ustake_cnt=2, + expected_last_balance=0 + ) + + def test_unstake_balance_rev_11(self): + self._test_unstake_balance( + rev=Revision.FIX_BALANCE_BUG.value, + expected_expired_ustake_cnt=3, + expected_last_balance=1 * ICX_IN_LOOP + ) + + def _test_unstake_balance( + self, + rev: int, + expected_expired_ustake_cnt: int, + expected_last_balance: int + ): self.update_governance() # set Revision REV_IISS @@ -42,7 +59,8 @@ def test_unstake(self): ) # set stake - stake: int = 4 * ICX_IN_LOOP + target_stake: int = 8 + stake: int = target_stake * ICX_IN_LOOP tx_results: List['TransactionResult'] = self.set_stake( from_=self._accounts[0], value=stake @@ -52,48 +70,58 @@ def test_unstake(self): response: int = self.get_balance(self._accounts[0]) self.assertEqual(expected_balance, response) - self.set_revision(Revision.FIX_BALANCE_BUG.value) + self.set_revision(Revision.MULTIPLE_UNSTAKE.value) - for i in range(1, 5): + for i in range(6): self.set_stake( from_=self._accounts[0], - value=stake - i + value=stake - (i+1) * ICX_IN_LOOP ) + last_balance: int = self.get_balance(self._accounts[0]) + actual_res: dict = self.get_stake(self._accounts[0]) - first_remaining_blocks: int = 16 + first_remaining_blocks: int = 14 expected_res = { - "stake": stake - 1 * 4, + "stake": stake - 1 * ICX_IN_LOOP * 6, "unstakes": [ - {"unstake": 1, "unstakeBlockHeight": 25, "remainingBlocks": first_remaining_blocks}, - {"unstake": 1, "unstakeBlockHeight": 26, "remainingBlocks": first_remaining_blocks+1}, - {"unstake": 1, "unstakeBlockHeight": 27, "remainingBlocks": first_remaining_blocks+2}, - {"unstake": 1, "unstakeBlockHeight": 28, "remainingBlocks": first_remaining_blocks+3}, + {"unstake": 1 * ICX_IN_LOOP, "unstakeBlockHeight": 25, "remainingBlocks": first_remaining_blocks}, + {"unstake": 1 * ICX_IN_LOOP, "unstakeBlockHeight": 26, "remainingBlocks": first_remaining_blocks+1}, + {"unstake": 1 * ICX_IN_LOOP, "unstakeBlockHeight": 27, "remainingBlocks": first_remaining_blocks+2}, + {"unstake": 1 * ICX_IN_LOOP, "unstakeBlockHeight": 28, "remainingBlocks": first_remaining_blocks+3}, + {"unstake": 1 * ICX_IN_LOOP, "unstakeBlockHeight": 29, "remainingBlocks": first_remaining_blocks+4}, + {"unstake": 1 * ICX_IN_LOOP, "unstakeBlockHeight": 30, "remainingBlocks": first_remaining_blocks+5}, ] } self.assertEqual(expected_res, actual_res) + self.set_revision(rev) + # 1st expired unstake self.make_empty_blocks(first_remaining_blocks) - last_balance: int = self.get_balance(self._accounts[0]) - tx_results = self.transfer_icx(from_=self._accounts[0], to_=self._accounts[0], value=0) - fee = tx_results[0].step_used * tx_results[0].step_price - - actual_res: dict = self.get_stake(self._accounts[0]) - remaining_blocks: int = 0 - expected_res = { - "stake": stake - 1 * 4, - "unstakes": [ - {"unstake": 1, "unstakeBlockHeight": 26, "remainingBlocks": remaining_blocks}, - {"unstake": 1, "unstakeBlockHeight": 27, "remainingBlocks": remaining_blocks+1}, - {"unstake": 1, "unstakeBlockHeight": 28, "remainingBlocks": remaining_blocks+2}, - ] - } - self.assertEqual(expected_res, actual_res) + # len(unstakes_info) = 6 + tx_results = self.transfer_icx(from_=self._accounts[0], to_=self._accounts[1], value=0) + # len(unstakes_info) = 5 + estimate_fee = tx_results[0].step_used * tx_results[0].step_price - # apply expire balance - # 2st expired unstake + # 2nd expired unstake self.make_empty_blocks(1) - + # len(unstakes_info) = 4 + + current_balance: int = self.get_balance(self._accounts[0]) + expected_balance: int = last_balance + 1 * ICX_IN_LOOP * expected_expired_ustake_cnt - estimate_fee + self.assertEqual(current_balance, expected_balance) + + self.transfer_icx( + from_=self._accounts[0], + to_=self._accounts[1], + value=expected_balance-estimate_fee, + disable_pre_validate=True, + step_limit=1 * 10 ** 5, + expected_status=True + ) + + # len(unstakes_info) = 3 + balance: int = self.get_balance(self._accounts[0]) - self.assertEqual(last_balance - fee + 2, balance) + self.assertEqual(balance, expected_last_balance) From 751f622cdf19909cbee6c8d46e1fe01829922c20 Mon Sep 17 00:00:00 2001 From: leeheonseung Date: Thu, 27 Aug 2020 12:20:36 +0900 Subject: [PATCH 17/17] move file path -> prevote -> decentralize append unstake check logic --- .../test_iiss_unstake.py | 56 +++++++++++++++++-- 1 file changed, 50 insertions(+), 6 deletions(-) rename tests/integrate_test/iiss/{prevote => decentralized}/test_iiss_unstake.py (64%) diff --git a/tests/integrate_test/iiss/prevote/test_iiss_unstake.py b/tests/integrate_test/iiss/decentralized/test_iiss_unstake.py similarity index 64% rename from tests/integrate_test/iiss/prevote/test_iiss_unstake.py rename to tests/integrate_test/iiss/decentralized/test_iiss_unstake.py index 10ee42996..cc9c15a17 100644 --- a/tests/integrate_test/iiss/prevote/test_iiss_unstake.py +++ b/tests/integrate_test/iiss/decentralized/test_iiss_unstake.py @@ -25,7 +25,7 @@ from iconservice.iconscore.icon_score_result import TransactionResult -class TestIISSStake(TestIISSBase): +class TestIISSUnStake(TestIISSBase): def test_unstake_balance_rev_10(self): self._test_unstake_balance( rev=Revision.FIX_UNSTAKE_BUG.value, @@ -99,17 +99,53 @@ def _test_unstake_balance( # 1st expired unstake self.make_empty_blocks(first_remaining_blocks) - # len(unstakes_info) = 6 + + res: dict = self.get_stake(self._accounts[0]) + expected_res = { + "stake": stake - 1 * ICX_IN_LOOP * 6, + "unstakes": [ + {"unstake": 1 * ICX_IN_LOOP, "unstakeBlockHeight": 26, "remainingBlocks": 0}, + {"unstake": 1 * ICX_IN_LOOP, "unstakeBlockHeight": 27, "remainingBlocks": 1}, + {"unstake": 1 * ICX_IN_LOOP, "unstakeBlockHeight": 28, "remainingBlocks": 2}, + {"unstake": 1 * ICX_IN_LOOP, "unstakeBlockHeight": 29, "remainingBlocks": 3}, + {"unstake": 1 * ICX_IN_LOOP, "unstakeBlockHeight": 30, "remainingBlocks": 4}, + ] + } + self.assertEqual(res, expected_res) + tx_results = self.transfer_icx(from_=self._accounts[0], to_=self._accounts[1], value=0) - # len(unstakes_info) = 5 + + res: dict = self.get_stake(self._accounts[0]) + expected_res = { + "stake": stake - 1 * ICX_IN_LOOP * 6, + "unstakes": [ + {"unstake": 1 * ICX_IN_LOOP, "unstakeBlockHeight": 27, "remainingBlocks": 0}, + {"unstake": 1 * ICX_IN_LOOP, "unstakeBlockHeight": 28, "remainingBlocks": 1}, + {"unstake": 1 * ICX_IN_LOOP, "unstakeBlockHeight": 29, "remainingBlocks": 2}, + {"unstake": 1 * ICX_IN_LOOP, "unstakeBlockHeight": 30, "remainingBlocks": 3}, + ] + } + self.assertEqual(res, expected_res) + estimate_fee = tx_results[0].step_used * tx_results[0].step_price # 2nd expired unstake self.make_empty_blocks(1) - # len(unstakes_info) = 4 + + res: dict = self.get_stake(self._accounts[0]) + expected_res = { + "stake": stake - 1 * ICX_IN_LOOP * 6, + "unstakes": [ + {"unstake": 1 * ICX_IN_LOOP, "unstakeBlockHeight": 28, "remainingBlocks": 0}, + {"unstake": 1 * ICX_IN_LOOP, "unstakeBlockHeight": 29, "remainingBlocks": 1}, + {"unstake": 1 * ICX_IN_LOOP, "unstakeBlockHeight": 30, "remainingBlocks": 2}, + ] + } + self.assertEqual(res, expected_res) current_balance: int = self.get_balance(self._accounts[0]) - expected_balance: int = last_balance + 1 * ICX_IN_LOOP * expected_expired_ustake_cnt - estimate_fee + expected_exipred_stake_balance: int = 1 * ICX_IN_LOOP * expected_expired_ustake_cnt + expected_balance: int = last_balance + expected_exipred_stake_balance - estimate_fee self.assertEqual(current_balance, expected_balance) self.transfer_icx( @@ -121,7 +157,15 @@ def _test_unstake_balance( expected_status=True ) - # len(unstakes_info) = 3 + res: dict = self.get_stake(self._accounts[0]) + expected_res = { + "stake": stake - 1 * ICX_IN_LOOP * 6, + "unstakes": [ + {"unstake": 1 * ICX_IN_LOOP, "unstakeBlockHeight": 29, "remainingBlocks": 0}, + {"unstake": 1 * ICX_IN_LOOP, "unstakeBlockHeight": 30, "remainingBlocks": 1}, + ] + } + self.assertEqual(res, expected_res) balance: int = self.get_balance(self._accounts[0]) self.assertEqual(balance, expected_last_balance)