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: dynamic vault address retrieval #3

Merged
merged 6 commits into from
Jan 9, 2024

Conversation

fabianschu
Copy link
Contributor

@fabianschu fabianschu commented Jan 8, 2024

@Another-DevX
Copy link
Member

Everything seems to be falling into place. Overall, I notice that to consume the backend correctly, I must pass the ChainID.
I'll to Merge this PR to start working on it!

@Another-DevX
Copy link
Member

Pls fix that issue with the unit testing

@fabianschu
Copy link
Contributor Author

Re: unit tests
I fixed two type errors that I introduced in this PR in my last commit (thanks for pointing me at tests once more!). However the unit tests still fail, which is expected. Imo the problem is the RPC rate limitations by Alchemy. The tests use forks of goerli / mainnet, which in turn rely on that external RPC. In my last PR, I tried to use a free plan API key (I also added it to the Github CI), but unfortunately the free plan doesn't allow for a sufficient amount of requests for the tests to run through. Because this is the only issue that I'm currently having with RPC call rate limits, I'm hesitant to switch over to a paid plan atm. Instead, I would say let's live with the failing tests for now and merge.

@Another-DevX Another-DevX merged commit 8ca52a7 into master Jan 9, 2024
2 of 3 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