IS-1208: Fix getBalance bug (#506)

* Fix a incorrect return value of getBalance caused by state mismatch between coin_part and stake_part

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

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:

iconservice/icx/icx_account.py

@@ -138,13 +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')
 
-            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:
@@ -168,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:

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,26 @@ 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:
+            """
+            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
 
         if AccountPartFlag.STAKE in part_flags:

tests/integrate_test/iiss/decentralized/test_iiss_unstake.py

@@ -0,0 +1,171 @@
+# -*- 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 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 TestIISSUnStake(TestIISSBase):
+    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
+        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
+        target_stake: int = 8
+        stake: int = target_stake * 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.MULTIPLE_UNSTAKE.value)
+
+        for i in range(6):
+            self.set_stake(
+                from_=self._accounts[0],
+                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 = 14
+        expected_res = {
+            "stake": stake - 1 * ICX_IN_LOOP * 6,
+            "unstakes": [
+                {"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)
+
+        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)
+
+        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)
+
+        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_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(
+            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
+        )
+
+        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)

tests/unit_test/icx/test_icx_account.py

@@ -0,0 +1,85 @@
+# -*- 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 copy
+from unittest.mock import PropertyMock
+
+import pytest
+
+from iconservice import Address
+from iconservice.base.block import Block
+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}")
+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()
+
+
+class TestAccount:
+
+    @pytest.mark.parametrize("revision", [
+        revision.value for revision in Revision if revision.value >= Revision.MULTIPLE_UNSTAKE.value
+    ])
+    @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(
+            ADDRESS, current_block_height, revision, coin_part=coin_part, stake_part=stake_part)
+
+        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.normalize(revision)
+
+        assert account.stake == stake
+        assert account.balance == expected_balance
+        assert account.unstakes_info == remaining_unstakes