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

Add blockhash cache and allow user-supplied blockhash #102

Merged
merged 6 commits into from
Sep 20, 2021

Conversation

kevinheavey
Copy link
Collaborator

Two changes here, for people who want to reduce RPC calls from fetching recent blockhashes:

Firstly, allow the user to pass a recent blockhash to .send_transaction and its dependent functions themselves. Useful for those who may want to run a separate loop to maintain a recent blockhash.

Secondly, introduce an experimental (for now) blockhash cache that the user can opt in to. The blockhash cache works as follows:

  1. Retrieve the oldest unused cached blockhash that is younger than ttl seconds (defaults to 60).
    We prefer unused blockhashes because reusing blockhashes can cause errors in some edge cases,
    and we prefer slightly older blockhashes because they're more likely to be accepted by every validator.
  2. If there are no unused blockhashes in the cache, take the oldest used
    blockhash that is younger than ttl seconds.
  3. Fetch a new recent blockhash after sending the transaction. This is to keep the cache up-to-date.

More discussion on the blockhash caching here https://discordapp.com/channels/791995070613159966/882624712813998121/888379125679685692

Bonus changes:

  1. Added pytest-cov as a dev dependency. May as well have it in there since it's in the CI pipeline.
  2. Renamed the alt_stubbed_sender fixture to async_stubbed_sender just so it's a little clearer what it's for.

@codecov
Copy link

codecov bot commented Sep 19, 2021

Codecov Report

Merging #102 (f5beadc) into master (7bf0ee0) will increase coverage by 0.68%.
The diff coverage is 93.75%.

@@            Coverage Diff             @@
##           master     #102      +/-   ##
==========================================
+ Coverage   88.02%   88.71%   +0.68%     
==========================================
  Files          32       32              
  Lines        2130     2179      +49     
==========================================
+ Hits         1875     1933      +58     
+ Misses        255      246       -9     

Copy link
Owner

@michaelhly michaelhly left a comment

Choose a reason for hiding this comment

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

LGTM! Just a small nit on type hinting.

self,
endpoint: Optional[str] = None,
commitment: Optional[Commitment] = None,
blockhash_cache: Union[BlockhashCache, bool] = False,
Copy link
Owner

@michaelhly michaelhly Sep 20, 2021

Choose a reason for hiding this comment

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

Suggested change
blockhash_cache: Union[BlockhashCache, bool] = False,
blockhash_cache: Optional[BlockhashCache] = None,

What is the purpose of Union[BlockhashCache, bool]?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yeah, the idea here is just that the user can say blockhash_cache=True and get the default cache behaviour without having to import the BlockhashCache class.

@michaelhly michaelhly merged commit 3de15cd into michaelhly:master Sep 20, 2021
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