From e25faedaf7bd248231575fb1b764deb2634b6fc0 Mon Sep 17 00:00:00 2001 From: Alicepoltora Date: Tue, 22 Sep 2026 20:00:22 +0700 Subject: [PATCH 1/2] fix(accounts): normalize address in raw-SQL balance operations debit_account_balance(), credit_account_balance() and credit_tx_value_once() execute raw SQL keyed by the address, but unlike get_account()/create_new_account_with_address() they never passed the address through to_checksum_address(). Accounts are stored keyed by the checksummed id, so a caller passing a lowercase address (e.g. from a JSON-RPC 'from' field) would: - debit_account_balance(): match no row and silently return False even when the account has sufficient balance; - credit_account_balance(): hit ON CONFLICT DO NOTHING on the checksummed row's id and instead INSERT a new duplicate row keyed by the lowercase address, splitting the balance across two accounts; - refund_tx_value(): credit the refund onto the lowercase duplicate row, losing the funds from the sender's perspective. _normalize_address() centralizes the to_checksum_address() normalization with the same fallback-on-error semantics get_account() already used, and is applied to every raw-SQL balance path. Unit tests (no DB required) verify the SQL parameters are normalized and that non-address values are passed through unchanged. --- backend/database_handler/accounts_manager.py | 19 +++-- .../test_accounts_manager_normalization.py | 79 +++++++++++++++++++ 2 files changed, 93 insertions(+), 5 deletions(-) create mode 100644 tests/unit/test_accounts_manager_normalization.py diff --git a/backend/database_handler/accounts_manager.py b/backend/database_handler/accounts_manager.py index 6169b5171..42a569efc 100644 --- a/backend/database_handler/accounts_manager.py +++ b/backend/database_handler/accounts_manager.py @@ -62,12 +62,19 @@ def create_new_account_with_address(self, address: str) -> Account: def is_valid_address(self, address: str) -> bool: return is_address(address) - def get_account(self, account_address: str) -> CurrentState | None: - """Private method to retrieve an account from the data base""" + @staticmethod + def _normalize_address(account_address: str) -> str: + """Normalize to the checksum form used as the primary key, matching + get_account()/create_new_account_with_address(). Falls back to the + original value if checksumming fails.""" try: - normalized = to_checksum_address(account_address) + return to_checksum_address(account_address) except Exception: - normalized = account_address + return account_address + + def get_account(self, account_address: str) -> CurrentState | None: + """Private method to retrieve an account from the data base""" + normalized = self._normalize_address(account_address) account = ( self.session.query(CurrentState) .filter(CurrentState.id == normalized) @@ -106,7 +113,7 @@ def debit_account_balance(self, account_address: str, amount: int) -> bool: "UPDATE current_state SET balance = balance - :amount " "WHERE id = :addr AND balance >= :amount" ), - {"amount": amount, "addr": account_address}, + {"amount": amount, "addr": self._normalize_address(account_address)}, ) return result.rowcount > 0 @@ -114,6 +121,7 @@ def credit_account_balance(self, account_address: str, amount: int): """Atomic credit. Creates account if it doesn't exist.""" if amount <= 0: return + account_address = self._normalize_address(account_address) self.session.execute( text( "INSERT INTO current_state (id, data, balance) " @@ -137,6 +145,7 @@ def credit_tx_value_once( Sets value_credited=true atomically. Returns True if credit was applied.""" if amount <= 0: return False + target_address = self._normalize_address(target_address) # Ensure target account exists self.session.execute( text( diff --git a/tests/unit/test_accounts_manager_normalization.py b/tests/unit/test_accounts_manager_normalization.py new file mode 100644 index 000000000..64ec7bfa2 --- /dev/null +++ b/tests/unit/test_accounts_manager_normalization.py @@ -0,0 +1,79 @@ +"""Unit tests for AccountsManager address normalization in raw-SQL paths. + +debit_account_balance / credit_account_balance / credit_tx_value_once use +raw SQL and previously received the address verbatim, while ORM paths +(get_account / create_new_account_with_address) store rows keyed by +to_checksum_address(). A lowercase address therefore created a duplicate +account row and split balances. These tests exercise the fix with a mocked +SQLAlchemy session — no database required. +""" + +from unittest.mock import MagicMock + +from eth_utils import to_checksum_address + +from backend.database_handler.accounts_manager import AccountsManager + +RAW_ADDRESS = "0x71c7656ec7ab88b098defb751b7401b5f6d8976f" # lowercase on purpose +CHECKSUMMED = to_checksum_address(RAW_ADDRESS) +assert RAW_ADDRESS != CHECKSUMMED # fixture sanity: casing must differ + + +def _executed_params(session: MagicMock) -> list[dict]: + return [call.args[1] for call in session.execute.call_args_list] + + +def test_debit_normalizes_address(): + session = MagicMock() + session.execute.return_value = MagicMock(rowcount=1) + assert AccountsManager(session).debit_account_balance(RAW_ADDRESS, 5) is True + + (params,) = [ + p for p in _executed_params(session) if p.get("addr") + ] + assert params["addr"] == CHECKSUMMED + + +def test_credit_normalizes_address_before_insert_and_update(): + session = MagicMock() + AccountsManager(session).credit_account_balance(RAW_ADDRESS, 7) + + params = [p for p in _executed_params(session) if p.get("addr")] + assert len(params) == 2 # INSERT ... ON CONFLICT + UPDATE + assert all(p["addr"] == CHECKSUMMED for p in params) + + +def test_credit_tx_value_once_normalizes_target(): + session = MagicMock() + # value_credited update must match so the credit path is taken + session.execute.return_value = MagicMock(rowcount=1) + + manager = AccountsManager(session) + trx = manager.credit_tx_value_once("0x12" + "34" * 16, RAW_ADDRESS, 3) + + assert trx is True + params = [p for p in _executed_params(session) if p.get("addr")] + assert params + assert all(p["addr"] == CHECKSUMMED for p in params) + + +def test_credit_zero_or_negative_amount_is_noop(): + session = MagicMock() + manager = AccountsManager(session) + + manager.credit_account_balance(RAW_ADDRESS, 0) + manager.credit_account_balance(RAW_ADDRESS, -10) + assert manager.debit_account_balance(RAW_ADDRESS, 0) is True + + session.execute.assert_not_called() + + +def test_normalize_fallback_keeps_original(): + """A value that to_checksum_address rejects must be used unchanged, + preserving the pre-existing behavior for non-address identifiers.""" + session = MagicMock() + session.execute.return_value = MagicMock(rowcount=1) + AccountsManager(session).debit_account_balance("not-an-address", 5) + + (params,) = [p for p in _executed_params(session) if p.get("addr")] + assert params["addr"] == "not-an-address" From a91e6bbfedbdc74f8dca1cbcd489192959c6a662 Mon Sep 17 00:00:00 2001 From: Daniel Rojas Date: Wed, 21 Oct 2026 13:18:47 -0500 Subject: [PATCH 2/2] style: apply black to accounts manager normalization test --- tests/unit/test_accounts_manager_normalization.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/tests/unit/test_accounts_manager_normalization.py b/tests/unit/test_accounts_manager_normalization.py index 64ec7bfa2..b7678b45b 100644 --- a/tests/unit/test_accounts_manager_normalization.py +++ b/tests/unit/test_accounts_manager_normalization.py @@ -28,9 +28,7 @@ def test_debit_normalizes_address(): session.execute.return_value = MagicMock(rowcount=1) assert AccountsManager(session).debit_account_balance(RAW_ADDRESS, 5) is True - (params,) = [ - p for p in _executed_params(session) if p.get("addr") - ] + (params,) = [p for p in _executed_params(session) if p.get("addr")] assert params["addr"] == CHECKSUMMED