Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add a table for not valid tokens #2326

Merged
merged 1 commit into from
Nov 18, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -200,6 +200,9 @@ https://docs.safe.global/api-supported-networks
### What means banned field in SafeContract model?
The `banned` field in the `SafeContract` model is used to prevent indexing of certain Safes that have an unsupported `MasterCopy` or unverified proxies that have issues during indexing. This field does not remove the banned Safe and indexing can be resumed once the issue has been resolved.

### Why is my ERC20 token not indexed?
For an ERC20 token to be indexed it needs to have `name`, `symbol`, `decimals` and `balanceOf()`, otherwise the service will ignore it and add it to the `TokenNotValid` model.

## Troubleshooting

### Issues installing grpc on an Apple silicon system
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ def __init__(self, ethereum_client: EthereumClient, redis: Redis):
maxsize=4096, ttl=60 * 30
) # 2 hours of caching

def _filter_addresses(
def _filter_tokens(
self,
erc20_addresses: Sequence[ChecksumAddress],
only_trusted: bool,
Expand Down Expand Up @@ -200,7 +200,7 @@ def _get_page_erc20_balances(
for address in all_erc20_addresses:
# Store tokens in database if not present
self.get_token_info(address) # This is cached
erc20_addresses = self._filter_addresses(
erc20_addresses = self._filter_tokens(
all_erc20_addresses, only_trusted, exclude_spam
)
# Total count should take into account the request filters
Expand Down
10 changes: 5 additions & 5 deletions safe_transaction_service/history/tests/test_balance_service.py
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ def test_get_token_info(self):
self.assertEqual(token_info.symbol, token_db.symbol)
self.assertEqual(token_info.decimals, token_db.decimals)

def test_filter_addresses(self):
def test_filter_tokens(self):
balance_service = self.balance_service
db_not_trusted_addresses = [
TokenFactory(trusted=False, spam=False).address for _ in range(3)
Expand Down Expand Up @@ -65,20 +65,20 @@ def test_filter_addresses(self):
)

self.assertCountEqual(
balance_service._filter_addresses(addresses, False, False), expected_address
balance_service._filter_tokens(addresses, False, False), expected_address
)

expected_address = db_trusted_addresses
self.assertCountEqual(
balance_service._filter_addresses(addresses, True, False), expected_address
balance_service._filter_tokens(addresses, True, False), expected_address
)

Token.objects.filter(address=db_events_bugged_erc20_address).update(
trusted=True
)
expected_address = db_trusted_addresses + [db_events_bugged_erc20_address]
self.assertCountEqual(
balance_service._filter_addresses(addresses, True, False), expected_address
balance_service._filter_tokens(addresses, True, False), expected_address
)

expected_address = (
Expand All @@ -87,5 +87,5 @@ def test_filter_addresses(self):
+ [db_events_bugged_erc20_address]
)
self.assertCountEqual(
balance_service._filter_addresses(addresses, False, True), expected_address
balance_service._filter_tokens(addresses, False, True), expected_address
)
2 changes: 1 addition & 1 deletion safe_transaction_service/history/tests/test_views_v2.py
Original file line number Diff line number Diff line change
Expand Up @@ -684,7 +684,7 @@ def test_safe_pagination_balances_view(self):
ERC20TransferFactory(address=other_erc20.address, to=safe_address)

with mock.patch(
"safe_transaction_service.history.services.balance_service.BalanceService._filter_addresses",
"safe_transaction_service.history.services.balance_service.BalanceService._filter_tokens",
return_value=[erc20.address, other_erc20.address],
):
response = self.client.get(
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
# Generated by Django 5.0.9 on 2024-11-12 13:38

from django.db import migrations

import safe_eth.eth.django.models


class Migration(migrations.Migration):

dependencies = [
("tokens", "0013_token_logo_uri"),
]

operations = [
migrations.CreateModel(
name="TokenNotValid",
fields=[
(
"address",
safe_eth.eth.django.models.EthereumAddressBinaryField(
primary_key=True, serialize=False
),
),
],
),
]
15 changes: 15 additions & 0 deletions safe_transaction_service/tokens/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,10 @@ def create(self, **kwargs):
def create_from_blockchain(
self, token_address: ChecksumAddress
) -> Optional["Token"]:
if TokenNotValid.objects.filter(address=token_address).exists():
# If token is not valid, ignore it
return None

ethereum_client = get_auto_ethereum_client()
if token_address in ENS_CONTRACTS_WITH_TLD: # Special case for ENS
return self.create(
Expand Down Expand Up @@ -118,13 +122,15 @@ def create_from_blockchain(
logger.debug(
"Cannot find anything on blockchain for token=%s", token_address
)
TokenNotValid.objects.create(address=token_address)
return None

# Ignore tokens with empty name or symbol
if not erc_info.name or not erc_info.symbol:
logger.warning(
"Token with address=%s has not name or symbol", token_address
)
TokenNotValid.objects.create(address=token_address)
return None

name_and_symbol: list[str] = []
Expand Down Expand Up @@ -308,6 +314,15 @@ def get_price_address(self) -> ChecksumAddress:
return self.copy_price or self.address


class TokenNotValid(models.Model):
"""
Stores information about tokens not valid (missing name or symbol, for example), so they are not requested
again to the RPC
"""

address = EthereumAddressBinaryField(primary_key=True)


class TokenListToken(TypedDict):
symbol: str
name: str
Expand Down
14 changes: 8 additions & 6 deletions safe_transaction_service/tokens/tests/test_models.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
ZerionUniswapV2TokenAdapterClient,
)
from ..exceptions import TokenListRetrievalException
from ..models import Token, TokenManager
from ..models import Token, TokenManager, TokenNotValid
from ..models import logger as token_model_logger
from .factories import TokenFactory, TokenListFactory
from .mocks import token_list_mock
Expand Down Expand Up @@ -196,11 +196,10 @@ def test_create_from_blockchain_with_string_null_data(self, get_info: MagicMock)
),
)
def test_create_from_blockchain_with_empty_name(self, get_info: MagicMock):
self.assertIsNone(
Token.objects.create_from_blockchain(
"0xBB9bc244D798123fDe783fCc1C72d3Bb8C189413"
)
)
self.assertEqual(TokenNotValid.objects.count(), 0)
address = "0xBB9bc244D798123fDe783fCc1C72d3Bb8C189413"
self.assertIsNone(Token.objects.create_from_blockchain(address))
self.assertEqual(TokenNotValid.objects.get(address=address).address, address)

@mock.patch.object(TokenManager, "create", side_effect=ValueError)
@mock.patch.object(
Expand All @@ -225,6 +224,9 @@ def test_create_from_blockchain_error(self, get_info: MagicMock, create: MagicMo
cm.output[0],
)

# Token should not be marked as not valid if there's a blockchain error
self.assertEqual(TokenNotValid.objects.count(), 0)


class TestTokenListModel(TestCase):
@mock.patch("requests.get")
Expand Down