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.

Sign up for GitHub

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

Small fixes and cleanup #79

Open
wants to merge 12 commits into
base: main
Choose a base branch
from
Open

Small fixes and cleanup #79

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
9b020de
small fixes and cleanup
harisang Oct 15, 2024
5ab545e
modify existing test
harisang Oct 16, 2024
e21eaa9
modify info msg
harisang Oct 16, 2024
950dcc1
black fix
harisang Oct 16, 2024
e2f328a
fix test again
harisang Oct 16, 2024
efc12f8
address comment
harisang Oct 16, 2024
3288000
rearrange get transaction tokens functionality
harisang Oct 16, 2024
e284c5a
Remove get transaction tokens test
harisang Oct 16, 2024
16a08f2
Add duplicate write prices test
harisang Oct 16, 2024
768f08b
remove comment
harisang Oct 16, 2024
bd1d5d5
remove another comment
harisang Oct 16, 2024
1f3bfe6
fetchone instead of fetchall in get prices query
harisang Oct 16, 2024
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
11 changes: 10 additions & 1 deletion src/helpers/database.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -173,11 +173,20 @@ def write_prices_new(self, prices: list[tuple[str, int, float, str]]) -> None:
)
for token_address, time, price, source in prices:
try:
date_time = datetime.fromtimestamp(time, tz=timezone.utc)
check_existence_query = f"SELECT * FROM prices WHERE token_address = '\\{token_address[1:]}' and time = '{date_time}' and source = '{source}'"
result = self.execute_query(check_existence_query, {}).fetchone()
fhenneke marked this conversation as resolved.
Show resolved Hide resolved
if result is not None:
logger.info(
"Skipping INSERT operation as entry already exists in PRICES table."
)
continue

self.execute_and_commit(
query,
{
"token_address": bytes.fromhex(token_address[2:]),
"time": datetime.fromtimestamp(time, tz=timezone.utc),
"time": date_time,
"price": price,
"source": source,
},
Expand Down
12 changes: 5 additions & 7 deletions src/transaction_processor.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -119,9 +119,7 @@ def process_single_transaction(
self.log_message = []
try:
# compute raw token imbalances
token_imbalances = self.process_token_imbalances(
tx_hash, auction_id, block_number
)
token_imbalances = self.process_token_imbalances(tx_hash)

# get transaction timestamp
transaction_timestamp = self.blockchain_data.get_transaction_timestamp(
Expand All @@ -134,9 +132,8 @@ def process_single_transaction(
# transaction_tokens = self.blockchain_data.get_transaction_tokens(tx_hash)
# store transaction tokens
transaction_tokens = []
for token_address, imbalance in token_imbalances.items():
if imbalance != 0:
transaction_tokens.append((tx_hash, token_address))
for token_address in token_imbalances.keys():
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would reorganize code here.

Instead of fetching token_imbalances in the beginning, and then having this loop here, both should be moved into a self.imbalances.get_transaction_tokens function. That function would then be called here. The function self.blockchain_data.get_transaction_tokens should then be deleted.

Copy link
Contributor Author

@harisang harisang Oct 16, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As it is still not clear where and how the raw imbalances will be used, i decided to keep it separate and not part of a get_transaction_tokens function.

Rearranged a few things and modified the test as well (not sure if you like how it looks like), in order to check the current version of the code. Ended up removing the test as it wasn't easy to make it work

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One reason for separating functionality into functions is that it makes it possible to test things.

Moving a function into the main body and removing tests is not ideal.

I can handle moving the function into token imbalances if you do not have the capacity.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok i am attempting to do this now

transaction_tokens.append((tx_hash, token_address))
self.db.write_transaction_tokens(transaction_tokens)

# update token decimals
Expand Down Expand Up @@ -202,7 +199,8 @@ def process_single_transaction(
return

def process_token_imbalances(
self, tx_hash: str, auction_id: int, block_number: int
self,
tx_hash: str,
) -> dict[str, int]:
"""Process token imbalances for a given transaction and return imbalances."""
try:
Expand Down
9 changes: 9 additions & 0 deletions tests/unit/test_database.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,7 @@ def tests_write_prices():
f"postgresql+psycopg://postgres:postgres@localhost:5432/mainnet"
)
db = Database(engine, "mainnet")
# list contains duplicate entry in order to test how this is handled
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would make this a separate test test_write_prices_duplicates. Otherwise we might get stuck with complicated tests as in solver-rewards where we have one huge test case with lots of settlements. It then becomes difficult to add new cases and a failed test does not show why it failed.

token_prices = [
(
"0xA0B86991C6218B36C1D19D4A2E9EB0CE3606EB48",
Expand All @@ -85,6 +86,12 @@ def tests_write_prices():
0.000000050569218629,
"moralis",
),
(
"0x68BBED6A47194EFF1CF514B50EA91895597FC91E",
int(datetime.fromisoformat("2024-10-10 16:49:47.000000").timestamp()),
0.000000050569218629,
"moralis",
),
]
# truncate table
with engine.connect() as conn:
Expand All @@ -97,6 +104,8 @@ def tests_write_prices():
res = conn.execute(
text("SELECT token_address, time, price, source FROM prices")
).all()
# cleaning up the duplicate entry
token_prices = token_prices[:2]
for i, (token_address, time, price, source) in enumerate(token_prices):
assert HexBytes(res[i][0]) == HexBytes(token_address)
fhenneke marked this conversation as resolved.
Show resolved Hide resolved
assert res[i][1].timestamp() == time
Expand Down
Loading