This repository has been archived by the owner on Feb 26, 2024. It is now read-only.
Add in-memory cache for forking requests #1248
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
MicaiahReid
approved these changes
Sep 24, 2021
src/chains/ethereum/ethereum/src/forking/handlers/base-handler.ts
Outdated
Show resolved
Hide resolved
Co-authored-by: Micaiah Reid <micaiahreid@gmail.com>
Co-authored-by: Micaiah Reid <micaiahreid@gmail.com>
…he into perf/forking-memcache
davidmurdoch
commented
Sep 24, 2021
Comment on lines
+121
to
+131
| let blockNumber: string; | ||
| if (typeof tagOrBlockNumber === "string") { | ||
| blockNumber = tagOrBlockNumber; | ||
| } else if (tagOrBlockNumber.toBigInt() > fallback.blockNumber.toBigInt()) { | ||
| // don't get the block if the requested block is _after_ our fallback's | ||
| // blocknumber because it doesn't exist in our local chain. | ||
| return null; | ||
| } else { | ||
| blockNumber = tagOrBlockNumber.toString(); | ||
| } | ||
|
|
There was a problem hiding this comment.
previously, requesting blocks that exist on the original chain, but not locally, would always just return our lcoal block's "latest" instead of null. this fixes it -- probably. :-)
Comment on lines
+13
to
+38
| import { ProviderHandler } from "./handlers/provider-handler"; | ||
|
|
||
| function fetchChainId(fork: Fork) { | ||
| return fork | ||
| .request<string>("eth_chainId", []) | ||
| .then(chainIdHex => parseInt(chainIdHex, 16)); | ||
| async function fetchChainId(fork: Fork) { | ||
| const chainIdHex = await fork.request<string>("eth_chainId", []); | ||
| return parseInt(chainIdHex, 16); | ||
| } | ||
| function fetchNetworkId(fork: Fork) { | ||
| return fork | ||
| .request<string>("net_version", []) | ||
| .then(networkIdStr => parseInt(networkIdStr, 10)); | ||
| async function fetchNetworkId(fork: Fork) { | ||
| const networkIdStr = await fork.request<string>("net_version", []); | ||
| return parseInt(networkIdStr, 10); | ||
| } | ||
| function fetchBlockNumber(fork: Fork) { | ||
| return fork.request<string>("eth_blockNumber", []); | ||
| } | ||
| function fetchBlock(fork: Fork, blockNumber: Quantity | Tag.LATEST) { | ||
| return fork.request<any>("eth_getBlockByNumber", [blockNumber, true]); | ||
| } | ||
| function fetchNonce( | ||
| async function fetchNonce( | ||
| fork: Fork, | ||
| address: Address, | ||
| blockNumber: Quantity | Tag.LATEST | ||
| ) { | ||
| return fork | ||
| .request<string>("eth_getTransactionCount", [address, blockNumber]) | ||
| .then(nonce => Quantity.from(nonce)); | ||
| const nonce = await fork.request<string>("eth_getTransactionCount", [ | ||
| address, | ||
| blockNumber | ||
| ]); | ||
| return Quantity.from(nonce); |
There was a problem hiding this comment.
vscode suggested it could convert all these to async`, so I clicked the button, and this is what it made. I like it.
| this.valueCache = new LRU({ | ||
| max: 1_073_741_824, // 1 gigabyte | ||
| length: (value, key) => { | ||
| return value.length + key.length; |
There was a problem hiding this comment.
To measure the cache size we measure the string length of the key and string length of the value. it's technically not perfect, but should be close enough for these purposes.
There was a problem hiding this comment.
Your comment should be part of the code
| const cached = this.getFromCache<T>(key); | ||
| if (cached !== undefined) return cached; | ||
|
|
||
| const promise = this.limiter.handle(send).then(({ response, raw }) => { |
There was a problem hiding this comment.
The send function actually does the sending, the limiter just schedules when to call that funciton. The send function returns a {response, raw} object here, raw is just used for the cache, and is a string or Buffer.
I did this because we can't measure the size of a json object for the cache, but we can for a string and Buffer.
davidmurdoch
added a commit
that referenced
this pull request
Oct 8, 2021
* perf: add an in-memory LRU cache for forking requests * add forking tests and fix bugs * clarify comment * `hasOwn` does null/undefined checks already * hasOwn already checks for null/undefined * Update src/chains/ethereum/ethereum/tests/forking/forking.test.ts Co-authored-by: Micaiah Reid <micaiahreid@gmail.com> * Update src/chains/ethereum/ethereum/src/forking/handlers/base-handler.ts Co-authored-by: Micaiah Reid <micaiahreid@gmail.com> * remove unused imports Co-authored-by: Micaiah Reid <micaiahreid@gmail.com>
Merged
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
closes #1224