Skip to content
New issue

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

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

Already on GitHub? # to your account

fix(solana token provider): await cached data #1828

Merged
merged 2 commits into from
Jan 5, 2025

Conversation

bentatum
Copy link
Contributor

@bentatum bentatum commented Jan 4, 2025

Relates to

Executing trades

Risks

low

Background

I was unable to execute a trade unless I made these changes

What does this PR do?

Awaits cached data because the function returns a promise. If we don't await, then the results are always not null and the logic is incorrect and likely fails downstream.

What kind of change is this?

It's a bug fix

Documentation changes needed?

None

Testing

Where should a reviewer start?

Detailed testing steps

  • Start the app with a character and a valid wallet address and private key
  • Start the UI
  • Use the following prompt: "buy 0.001 sol of GOAT CzLSujWBLFsSjncfkh59rUFqvafWcY5tzedWJSuypump"

monilpat
monilpat previously approved these changes Jan 4, 2025
Copy link
Collaborator

@monilpat monilpat left a comment

Choose a reason for hiding this comment

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

LGTM

@bentatum bentatum force-pushed the fix/await-cached-prices-2 branch from 668b688 to 0628d3a Compare January 4, 2025 23:38
@bentatum bentatum requested a review from monilpat January 5, 2025 00:56
@bentatum bentatum force-pushed the fix/await-cached-prices-2 branch 4 times, most recently from 4003263 to 7367010 Compare January 5, 2025 17:14
@bentatum
Copy link
Contributor Author

bentatum commented Jan 5, 2025

Thanks, @monilpat. I rebased a few times, so this will need your re-approval.

@bentatum bentatum force-pushed the fix/await-cached-prices-2 branch from 7367010 to 02384d4 Compare January 5, 2025 17:27
@shakkernerd shakkernerd merged commit 2724555 into elizaOS:develop Jan 5, 2025
3 checks passed
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants