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

fix: price upsert error #907

Merged
merged 1 commit into from
Dec 27, 2024
Merged

fix: price upsert error #907

merged 1 commit into from
Dec 27, 2024

Conversation

chedieck
Copy link
Collaborator

Related to #905

Description

When adding a big address (like ecash:prfhcnyqnl5cgrnmlfmms675w93ld7mvvqd0y8lz07), if the syncing process encountered some missing prices, this would lead to the error mentioned on #905.

This PR fixes this error, by uncoupling the process of filling up the missing prices and connecting it to the transaction.

Test plan

On master, after restarting the containers, try adding the aforementioned address to a button. After adding 200 txs around 10 times, it should result on the error happening multiple times.

Then restart the containers again on this branch, it should work fine.

@Klakurka Klakurka self-requested a review December 27, 2024 09:34
@Klakurka
Copy link
Member

Potential options for speeding up the syncing process? We're currently doing 200 tx/s. Do we run into issues above this?

@chedieck
Copy link
Collaborator Author

chedieck commented Dec 27, 2024

Potential options for speeding up the syncing process? We're currently doing 200 tx/s. Do we run into issues above this?

It's the Chronik's limit of how many txs to fetch per call. Then it takes a while between each subsequent call because we have to connect all these txs to addresses and prices we have in the DB, but I can have a look in another PR into making each one of these iterations take less time.

Copy link
Member

@Klakurka Klakurka left a comment

Choose a reason for hiding this comment

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

The speed isn't really that bad and IMO it's totally fine if we can improve the UI (eg. #903). Being able to submit a new address and have it sync in the bg would be even better.

@Klakurka Klakurka merged commit 2071390 into master Dec 27, 2024
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants