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

Release/rate limiter #5295

Merged
merged 12 commits into from
Apr 24, 2024
Merged

Release/rate limiter #5295

merged 12 commits into from
Apr 24, 2024

Conversation

@pete-watters
Copy link
Contributor

Code LGTM. I took it for a spin testing it with a few wallets including Ledger (STX only, BTC only + both)

I was able to perform a swap, initiate send ordinal etc.

All looks good but when testing a wallet with a lot of accounts it was taking a long time to load balances sometimes. They do load eventually. Maybe thats improved in #5296?

I also noticed these console errors which I'm not sure if they are new or not.

Alex error about market data:
Screenshot 2024-04-23 at 15 10 20

When increasing STX fee I had a 429 and the spinner kept showing:
Screenshot 2024-04-23 at 15 07 33

@kyranjamie
Copy link
Collaborator

cc/ @fbwoolf on market data error. Do we expect this to happen? If so, maybe a different technique to handle is needed besides logger.error

@fbwoolf
Copy link
Contributor

fbwoolf commented Apr 24, 2024

cc/ @fbwoolf on market data error. Do we expect this to happen? If so, maybe a different technique to handle is needed besides logger.error

Let me look at this, it can likely be removed and is just a Stacks ft alex isn't handling. The logger.error seems unec, but I want to make sure I'm right here. I'll put up a quick PR this morn to remove it so it can get added here.

@fbwoolf
Copy link
Contributor

fbwoolf commented Apr 24, 2024

PR here to include: #5309

@fbwoolf fbwoolf merged commit 36c0ae2 into main Apr 24, 2024
69 checks passed
@fbwoolf fbwoolf deleted the release/rate-limiter branch April 24, 2024 14:54
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.

4 participants