-
-
Notifications
You must be signed in to change notification settings - Fork 490
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
Globaldb migration: new tables and move assets #3400
Changes from 8 commits
c3e7904
f03cb80
7eec111
bd58427
825a37a
a4baa31
6aa5f4a
5bb4fc4
e4b6379
f966b58
a478d07
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -6,6 +6,7 @@ on: | |
- master | ||
- develop | ||
- bugfixes | ||
- multichain_globaldb_hell | ||
|
||
jobs: | ||
check-changes: | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,12 +1,80 @@ | ||
from enum import Enum | ||
from typing import Optional | ||
|
||
from rotkehlchen.typing import ChecksumEthAddress | ||
|
||
EVM_CHAIN_DIRECTIVE = 'eip155' | ||
ETHEREUM_DIRECTIVE = '_ceth_' | ||
ETHEREUM_DIRECTIVE_LENGTH = len(ETHEREUM_DIRECTIVE) | ||
|
||
|
||
class CHAINID(Enum): | ||
ETHEREUM_CHAIN_IDENTIFIER = '1' | ||
OPTIMISM_CHAIN_IDENTIFIER = '10' | ||
BINANCE_CHAIN_IDENTIFIER = '56' | ||
OKEX_CHAIN_IDENTIFIER = '66' | ||
XDAI_CHAIN_IDENTIFIER = '100' | ||
MATIC_CHAIN_IDENTIFIER = '137' | ||
FANTOM_CHAIN_IDENTIFIER = '250' | ||
ARBITRIUM_CHAIN_IDENTIFIER = '42161' | ||
AVALANCHE_CHAIN_IDENTIFIER = '43114' | ||
yabirgb marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
@classmethod | ||
def deserialize_from_coingecko(cls, chain: str) -> 'CHAINID': | ||
if chain == 'ethereum': | ||
return CHAINID.ETHEREUM_CHAIN_IDENTIFIER | ||
if chain == 'binance-smart-chain': | ||
return CHAINID.BINANCE_CHAIN_IDENTIFIER | ||
if chain == 'avalanche': | ||
return CHAINID.AVALANCHE_CHAIN_IDENTIFIER | ||
if chain == 'polygon-pos': | ||
return CHAINID.MATIC_CHAIN_IDENTIFIER | ||
|
||
raise KeyError(f'Unknown chain {chain}') | ||
|
||
|
||
class EvmTokenKind(Enum): | ||
ERC20 = 'erc20' | ||
ERC721 = 'erc721' | ||
yabirgb marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
|
||
VALID_EVM_CHAINS = { | ||
'S': CHAINID.BINANCE_CHAIN_IDENTIFIER, | ||
'X': CHAINID.AVALANCHE_CHAIN_IDENTIFIER, | ||
} | ||
EVM_CHAINS_TO_DATABASE = {v: k for k, v in VALID_EVM_CHAINS.items()} | ||
|
||
|
||
def ethaddress_to_identifier(address: ChecksumEthAddress) -> str: | ||
return ETHEREUM_DIRECTIVE + address | ||
|
||
|
||
def strethaddress_to_identifier(address: str) -> str: | ||
return ETHEREUM_DIRECTIVE + address | ||
|
||
|
||
def evm_address_to_identifier( | ||
address: str, | ||
chain: CHAINID, | ||
token_type: EvmTokenKind, | ||
collectible_id: Optional[str] = None, | ||
) -> str: | ||
"""Format a EVM token information into the CAIPs identifier format""" | ||
yabirgb marked this conversation as resolved.
Show resolved
Hide resolved
|
||
ident = f'{EVM_CHAIN_DIRECTIVE}:{chain.value}/{token_type.value}:{address}' | ||
if collectible_id is not None: | ||
return ident + f'/{collectible_id}' | ||
return ident | ||
|
||
|
||
def translate_old_format_to_new(identifier: str) -> str: | ||
""" | ||
Given a identifier that is either in the _ceth_ format transalate it to the new format, | ||
otherwise leave it unmodified. | ||
""" | ||
if identifier.startswith(ETHEREUM_DIRECTIVE): | ||
return evm_address_to_identifier( | ||
identifier[6:], | ||
CHAINID.ETHEREUM_CHAIN_IDENTIFIER, | ||
EvmTokenKind.ERC20, | ||
) | ||
return identifier |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -10,17 +10,18 @@ | |
from rotkehlchen.assets.typing import AssetData, AssetType | ||
from rotkehlchen.chain.ethereum.typing import string_to_ethereum_address | ||
from rotkehlchen.constants.assets import CONSTANT_ASSETS | ||
from rotkehlchen.constants.resolver import ethaddress_to_identifier | ||
from rotkehlchen.constants.resolver import ethaddress_to_identifier, translate_old_format_to_new | ||
from rotkehlchen.errors import DeserializationError, InputError, UnknownAsset | ||
from rotkehlchen.globaldb.upgrades.v1_v2 import upgrade_ethereum_asset_ids | ||
from rotkehlchen.globaldb.upgrades.v2_v3 import migrate_to_v3 | ||
from rotkehlchen.history.typing import HistoricalPrice, HistoricalPriceOracle | ||
from rotkehlchen.typing import ChecksumEthAddress, Timestamp | ||
|
||
from .schema import DB_SCRIPT_CREATE_TABLES | ||
|
||
log = logging.getLogger(__name__) | ||
|
||
GLOBAL_DB_VERSION = 2 | ||
GLOBAL_DB_VERSION = 3 | ||
|
||
|
||
def _get_setting_value(cursor: sqlite3.Cursor, name: str, default_value: int) -> int: | ||
|
@@ -37,6 +38,7 @@ def _get_setting_value(cursor: sqlite3.Cursor, name: str, default_value: int) -> | |
|
||
def initialize_globaldb(dbpath: Path) -> sqlite3.Connection: | ||
connection = sqlite3.connect(dbpath) | ||
print(dbpath) | ||
yabirgb marked this conversation as resolved.
Show resolved
Hide resolved
|
||
connection.executescript(DB_SCRIPT_CREATE_TABLES) | ||
cursor = connection.cursor() | ||
db_version = _get_setting_value(cursor, 'version', GLOBAL_DB_VERSION) | ||
|
@@ -48,6 +50,8 @@ def initialize_globaldb(dbpath: Path) -> sqlite3.Connection: | |
|
||
if db_version == 1: | ||
upgrade_ethereum_asset_ids(connection) | ||
if db_version == 2: | ||
migrate_to_v3(connection) | ||
cursor.execute( | ||
'INSERT OR REPLACE INTO settings(name, value) VALUES(?, ?)', | ||
('version', str(GLOBAL_DB_VERSION)), | ||
|
@@ -222,21 +226,25 @@ def get_all_asset_data( | |
else: | ||
result = [] | ||
cursor = GlobalDBHandler()._conn.cursor() | ||
if specific_ids is not None: | ||
specific_ids = [translate_old_format_to_new(id) for id in specific_ids] | ||
Comment on lines
+228
to
+229
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. These are the temporary hacks you meant? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes |
||
specific_ids_query = '' | ||
if specific_ids is not None: | ||
specific_ids_query = f'AND A.identifier in ({",".join("?" * len(specific_ids))})' | ||
querystr = f""" | ||
SELECT A.identifier, A.type, B.address, B.decimals, A.name, A.symbol, A.started, null, A.swapped_for, A.coingecko, A.cryptocompare, B.protocol from assets as A LEFT OUTER JOIN ethereum_tokens as B | ||
ON B.address = A.details_reference WHERE A.type=? {specific_ids_query} | ||
SELECT A.identifier, A.type, B.address, B.decimals, C.name, C.symbol, A.started, null, A.swapped_for, C.coingecko, C.cryptocompare, B.protocol FROM assets as A JOIN evm_tokens as B | ||
ON B.identifier = A.identifier JOIN common_asset_details AS C ON C.identifier = B.identifier WHERE A.type=? {specific_ids_query} | ||
UNION ALL | ||
SELECT A.identifier, A.type, null, null, A.name, A.symbol, A.started, B.forked, A.swapped_for, A.coingecko, A.cryptocompare, null from assets as A LEFT OUTER JOIN common_asset_details as B | ||
ON B.asset_id = A.identifier WHERE A.type!=? {specific_ids_query}; | ||
SELECT A.identifier, A.type, null, null, B.name, B.symbol, A.started, B.forked, A.swapped_for, B.coingecko, B.cryptocompare, null from assets as A JOIN common_asset_details as B | ||
ON B.identifier = A.identifier WHERE A.type!=? {specific_ids_query}; | ||
""" # noqa: E501 | ||
|
||
eth_token_type = AssetType.ETHEREUM_TOKEN.serialize_for_db() # pylint: disable=no-member | ||
if specific_ids is not None: | ||
bindings = (eth_token_type, *specific_ids, eth_token_type, *specific_ids) | ||
else: | ||
bindings = (eth_token_type, eth_token_type) | ||
|
||
query = cursor.execute(querystr, bindings) | ||
for entry in query: | ||
asset_type = AssetType.deserialize_from_db(entry[1]) | ||
|
@@ -275,13 +283,17 @@ def get_asset_data( | |
|
||
Returns None if identifier can't be matched to an asset | ||
""" | ||
# TODO: This is a safe for the identifier and need to be removed | ||
yabirgb marked this conversation as resolved.
Show resolved
Hide resolved
|
||
identifier = translate_old_format_to_new(identifier) | ||
cursor = GlobalDBHandler()._conn.cursor() | ||
query = cursor.execute( | ||
'SELECT identifier, type, name, symbol, started, swapped_for, coingecko, ' | ||
'cryptocompare, details_reference from assets WHERE identifier=?;', | ||
'SELECT A.identifier, A.type, B.name, B.symbol, A.started, A.swapped_for, ' | ||
'B.coingecko, B.cryptocompare, A.common_details_id from assets AS A JOIN ' | ||
'common_asset_details AS B ON A.identifier = B.identifier WHERE A.identifier=?;', | ||
(identifier,), | ||
) | ||
result = query.fetchone() | ||
|
||
yabirgb marked this conversation as resolved.
Show resolved
Hide resolved
|
||
if result is None: | ||
return None | ||
|
||
|
@@ -294,11 +306,10 @@ def get_asset_data( | |
swapped_for = result[5] | ||
coingecko = result[6] | ||
cryptocompare = result[7] | ||
details_reference = result[8] | ||
forked = None | ||
decimals = None | ||
protocol = None | ||
ethereum_address = None | ||
evm_address = None | ||
|
||
try: | ||
asset_type = AssetType.deserialize_from_db(db_serialized_type) | ||
|
@@ -309,11 +320,17 @@ def get_asset_data( | |
) | ||
return None | ||
|
||
if asset_type == AssetType.ETHEREUM_TOKEN: | ||
ethereum_address = details_reference | ||
evm_assets = ( | ||
AssetType.ETHEREUM_TOKEN, | ||
AssetType.POLYGON_TOKEN, | ||
AssetType.XDAI_TOKEN, | ||
AssetType.AVALANCHE_TOKEN, | ||
) | ||
yabirgb marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
if asset_type in evm_assets: | ||
cursor.execute( | ||
'SELECT decimals, protocol from ethereum_tokens ' | ||
'WHERE address=?', (ethereum_address,), | ||
'SELECT decimals, protocol, address from evm_tokens ' | ||
'WHERE identifier=?', (saved_identifier,), | ||
) | ||
result = query.fetchone() | ||
if result is None: | ||
|
@@ -325,18 +342,19 @@ def get_asset_data( | |
|
||
decimals = result[0] | ||
protocol = result[1] | ||
evm_address = result[2] | ||
missing_basic_data = name is None or symbol is None or decimals is None | ||
if missing_basic_data and form_with_incomplete_data is False: | ||
log.debug( | ||
f'Considering ethereum token with address {details_reference} ' | ||
f'Considering ethereum token with address {saved_identifier} ' | ||
yabirgb marked this conversation as resolved.
Show resolved
Hide resolved
|
||
f'as unknown since its missing either decimals or name or symbol', | ||
) | ||
return None | ||
else: | ||
cursor = GlobalDBHandler()._conn.cursor() | ||
query = cursor.execute( | ||
'SELECT forked FROM common_asset_details WHERE asset_id = ?;', | ||
(details_reference,), | ||
'SELECT forked FROM common_asset_details WHERE identifier = ?;', | ||
(saved_identifier,), | ||
) | ||
result = query.fetchone() | ||
if result is None: | ||
|
@@ -355,7 +373,7 @@ def get_asset_data( | |
started=started, | ||
forked=forked, | ||
swapped_for=swapped_for, | ||
ethereum_address=ethereum_address, | ||
ethereum_address=evm_address, | ||
decimals=decimals, | ||
coingecko=coingecko, | ||
cryptocompare=cryptocompare, | ||
|
@@ -364,12 +382,15 @@ def get_asset_data( | |
|
||
@staticmethod | ||
def fetch_underlying_tokens( | ||
address: ChecksumEthAddress, | ||
address: str, | ||
yabirgb marked this conversation as resolved.
Show resolved
Hide resolved
|
||
) -> Optional[List[UnderlyingToken]]: | ||
"""Fetch underlying tokens for a token address if they exist""" | ||
""" | ||
Fetch underlying tokens for a token address if they exist. | ||
TODO assets: review type of argument. Shouldn't be str | ||
""" | ||
cursor = GlobalDBHandler()._conn.cursor() | ||
query = cursor.execute( | ||
'SELECT address, weight from underlying_tokens_list WHERE parent_token_entry=?;', | ||
'SELECT identifier, weight from underlying_tokens_list WHERE parent_token_entry=?;', | ||
(address,), | ||
) | ||
results = query.fetchall() | ||
|
@@ -1206,11 +1227,10 @@ def get_historical_price_data(source: HistoricalPriceOracle) -> List[Dict[str, A | |
|
||
def _reload_constant_assets(globaldb: GlobalDBHandler) -> None: | ||
"""Reloads the details of the constant declared assets after reading from the DB""" | ||
identifiers = [x.identifier for x in CONSTANT_ASSETS] | ||
identifiers = [translate_old_format_to_new(x.identifier) for x in CONSTANT_ASSETS] | ||
db_data = globaldb.get_all_asset_data(mapping=True, serialized=False, specific_ids=identifiers) # type: ignore # noqa: E501 | ||
|
||
for entry in CONSTANT_ASSETS: | ||
db_entry = db_data.get(entry.identifier) | ||
db_entry = db_data.get(translate_old_format_to_new(entry.identifier)) | ||
if db_entry is None: | ||
log.critical( | ||
f'Constant declared asset with id {entry.identifier} has no corresponding DB entry' | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,12 +1,12 @@ | ||
DB_CREATE_ETHEREUM_TOKENS_LIST = """ | ||
CREATE TABLE IF NOT EXISTS underlying_tokens_list ( | ||
address VARCHAR[42] NOT NULL, | ||
identifier TEXT NOT NULL, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is this table only for ethereum tokens? Or for all evm tokens? Cause it will need renaming if it's for all. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why renaming it? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
weight TEXT NOT NULL, | ||
parent_token_entry TEXT NOT NULL, | ||
FOREIGN KEY(parent_token_entry) REFERENCES ethereum_tokens(address) | ||
FOREIGN KEY(parent_token_entry) REFERENCES evm_tokens(identifier) | ||
ON DELETE CASCADE ON UPDATE CASCADE | ||
FOREIGN KEY(address) REFERENCES ethereum_tokens(address) ON UPDATE CASCADE | ||
PRIMARY KEY(address, parent_token_entry) | ||
FOREIGN KEY(identifier) REFERENCES evm_tokens(identifier) ON UPDATE CASCADE | ||
PRIMARY KEY(identifier, parent_token_entry) | ||
); | ||
""" # noqa: E501 | ||
|
||
|
@@ -74,6 +74,18 @@ | |
INSERT OR IGNORE INTO asset_types(type, seq) VALUES ('X', 24); | ||
/* SOLANA TOKEN */ | ||
INSERT OR IGNORE INTO asset_types(type, seq) VALUES ('Y', 25); | ||
/* MATIC TOKEN */ | ||
INSERT OR IGNORE INTO asset_types(type, seq) VALUES ('Z', 26); | ||
/* XDAI TOKEN */ | ||
INSERT OR IGNORE INTO asset_types(type, seq) VALUES ('[', 27); | ||
/* OKEX TOKEN */ | ||
INSERT OR IGNORE INTO asset_types(type, seq) VALUES ('\', 28); | ||
/* FANTOM TOKEN */ | ||
INSERT OR IGNORE INTO asset_types(type, seq) VALUES (']', 29); | ||
/* ARBITRIUM TOKEN */ | ||
yabirgb marked this conversation as resolved.
Show resolved
Hide resolved
|
||
INSERT OR IGNORE INTO asset_types(type, seq) VALUES ('^', 30); | ||
/* OPTIMISM TOKEN */ | ||
INSERT OR IGNORE INTO asset_types(type, seq) VALUES ('_', 31); | ||
""" | ||
|
||
# Using asset_id as a primary key here since nothing else is guaranteed to be unique | ||
|
@@ -114,7 +126,7 @@ | |
|
||
DB_CREATE_USER_OWNED_ASSETS = """ | ||
CREATE TABLE IF NOT EXISTS user_owned_assets ( | ||
asset_id VARCHAR[24] NOT NULL PRIMARY KEY, | ||
asset_id TEXT NOT NULL PRIMARY KEY, | ||
FOREIGN KEY(asset_id) REFERENCES assets(identifier) ON UPDATE CASCADE | ||
); | ||
""" | ||
|
@@ -148,6 +160,59 @@ | |
); | ||
""" | ||
|
||
DB_V3_CREATE_COMMON_ASSETS_DETAILS = """ | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. And this won't stay right? We willl only have 1 version of creation command per table. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I was thinking in someone comming from an old version but I believe it doesn't make sense? We can go ahead and remove the versions in the names There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. it doesn't make sense. We never keep old version creation schemas around. That's what DB upgrades are for. |
||
CREATE TABLE IF NOT EXISTS common_asset_details( | ||
identifier TEXT PRIMARY KEY NOT NULL COLLATE NOCASE, | ||
name TEXT, | ||
symbol TEXT, | ||
coingecko TEXT, | ||
cryptocompare TEXT, | ||
forked TEXT, | ||
FOREIGN KEY(forked) REFERENCES assets(identifier) ON UPDATE CASCADE | ||
); | ||
""" | ||
|
||
DB_V3_CREATE_ASSETS = """ | ||
CREATE TABLE IF NOT EXISTS assets ( | ||
identifier TEXT PRIMARY KEY NOT NULL COLLATE NOCASE, | ||
type CHAR(1) NOT NULL DEFAULT('A') REFERENCES asset_types(type), | ||
started INTEGER, | ||
swapped_for TEXT, | ||
common_details_id TEXT, | ||
FOREIGN KEY(common_details_id) REFERENCES common_asset_details(identifier) ON UPDATE CASCADE | ||
); | ||
""" | ||
|
||
DB_V3_CREATE_EVM_TOKENS = """ | ||
CREATE TABLE IF NOT EXISTS evm_tokens ( | ||
identifier TEXT PRIMARY KEY NOT NULL COLLATE NOCASE, | ||
token_type TEXT NOT NULL, | ||
chain TEXT NOT NULL, | ||
address VARCHAR[42] NOT NULL, | ||
decimals INTEGER, | ||
protocol TEXT | ||
); | ||
""" | ||
|
||
DB_V3_CREATE_ASSETS_EVM_TOKENS = """ | ||
CREATE TABLE IF NOT EXISTS assets_evm_tokens( | ||
yabirgb marked this conversation as resolved.
Show resolved
Hide resolved
|
||
asset_id TEXT, | ||
evm_token_id TEXT, | ||
FOREIGN KEY(asset_id) REFERENCES assets(identifier) ON UPDATE CASCADE ON DELETE CASCADE | ||
FOREIGN KEY(evm_token_id) REFERENCES evm_tokens(identifier) ON UPDATE CASCADE ON DELETE CASCADE | ||
PRIMARY KEY(asset_id, evm_token_id) | ||
); | ||
""" | ||
|
||
DB_V3_CREATE_MULTIASSETS = """ | ||
CREATE TABLE IF NOT EXISTS multiasset_collector( | ||
identifier TEXT NOT NULL, | ||
child_asset_id TEXT, | ||
FOREIGN KEY(child_asset_id) REFERENCES assets(identifier) ON UPDATE CASCADE | ||
PRIMARY KEY(identifier, child_asset_id) | ||
); | ||
""" | ||
|
||
DB_SCRIPT_CREATE_TABLES = """ | ||
PRAGMA foreign_keys=off; | ||
BEGIN TRANSACTION; | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also above here: https://github.com/rotki/rotki/pull/3400/files#diff-d838b4670ae94dd2743ece1ac79b73a99865b761474821ed39d7d3498d830338R616
Shouldn't it be talking about an EVM token?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Need to rename some things. Will do in the next PR