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

improve(TokenClient): Cache allowance and bondToken eth_call data #1322

Merged
merged 18 commits into from
Mar 22, 2024

Conversation

nicholaspai
Copy link
Member

These values should rarely change. Follows strategy implemented in #1282 in the BaseAdapter (inventory manager)

nicholaspai and others added 2 commits March 20, 2024 11:50
These values should rarely change. Follows strategy implemented in #1282 in the BaseAdapter (inventory manager)
@nicholaspai nicholaspai marked this pull request as ready for review March 20, 2024 16:05
src/clients/TokenClient.ts Outdated Show resolved Hide resolved
src/clients/TokenClient.ts Outdated Show resolved Hide resolved
src/clients/TokenClient.ts Outdated Show resolved Hide resolved
Copy link
Contributor

@pxrl pxrl left a comment

Choose a reason for hiding this comment

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

LGTM!

nicholaspai and others added 7 commits March 21, 2024 20:03
While fixing the block tag to the nearest 100_000th block when querying bondToken is nice, it breaks tests and I don't think they're worth fixing. The main motivation for taking advantage of the eth_call specific-blockTag caching is to make the code cleaner but I think this commit's refactor is nice too
@nicholaspai
Copy link
Member Author

While fixing the block tag to the nearest 100_000th block when querying bondToken is nice, it breaks tests and I don't think they're worth fixing. The main motivation for taking advantage of the eth_call specific-blockTag caching is to make the code cleaner but I think this PR's refactor is now minimally invasive

@pxrl

@nicholaspai nicholaspai requested a review from pxrl March 22, 2024 00:38
Copy link
Contributor

@mrice32 mrice32 left a comment

Choose a reason for hiding this comment

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

Looks great, just one comment!

src/clients/TokenClient.ts Outdated Show resolved Hide resolved
src/clients/TokenClient.ts Show resolved Hide resolved
src/clients/TokenClient.ts Outdated Show resolved Hide resolved
src/clients/TokenClient.ts Outdated Show resolved Hide resolved
@nicholaspai nicholaspai merged commit 4e268da into master Mar 22, 2024
4 checks passed
@pxrl pxrl deleted the npai/cache-allowances branch September 5, 2024 10:27
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.

3 participants