Cache RPC responses #19
Conversation
chore: use double quotes in "action.yml" docs: add brief description about Foundry in README docs: fix typos in README docs: remove optional input "version" from example script refactor: set "nightly" as default value for "version"
build: add "@actions/cache" and "@actions/github" as deps build: add "prettier" as dev dep build: regenerate "dist" files chore: add Prettier configuration and Prettier ignore file docs: change author to "Foundry" feat: run "save" function on post style: format JavaScript files with Prettier
README.md
Outdated
|
|
||
| - Always load the latest valid cache, and always create a new one with the updated cache. | ||
| - When there are no changes to the fork tests, the cache does not change but the restore key does, since the key is based on the commit hash. | ||
| - When the fork tests are changed, both the cache and the store key are updated. |
There was a problem hiding this comment.
What does this bullet mean / what is the store key and restore key? More generally I think I'm unclear on why the logic isn't simpler to just "always cache all responses, clear cache if user changes the cache key". Basically just want to sanity check that the cache isn't deleted if I add/remove new fork blocks or networks to tests.
There was a problem hiding this comment.
It can't be that simple, unfortunately.
As Twitter user ultrasecreth explained here, we want a mechanism whereby if the fork tests change, the cache is updated too.
If you don't specify the git commit in the cache key, then the cache will never update, even if you completely redesign your tests. Quote from the README:
If the provided key matches an existing cache, a new cache is not created.
There was a problem hiding this comment.
Got it, I see. And is "store key" the same as the "restore keys" in the actions/cache repo?
There was a problem hiding this comment.
Oh my goodness, I have completely butchered the terminology in the proposed README. I'm sorry, I must have been off-caffeine on the day when I opened the PR.
I have pushed a commit to fix the wording, you can see the diff here:
Basically, I should have just said "key", because the "restore keys" never change on the same OS platform. It's the "key" (simple key, no restore) that changes all the time because it's dependent upon the commit hash.
There was a problem hiding this comment.
Heh got it, thanks! Deferring to @gakonst and co. for review/merge 🙂
There was a problem hiding this comment.
Thanks @mds1 for the review thus far.
| const github = require("@actions/github"); | ||
| const os = require("os"); | ||
|
|
||
| const CACHE_PATHS = ["~/.foundry/cache/rpc"]; |
There was a problem hiding this comment.
Does this also cache any etherscan queries? Should we add support for that?
There was a problem hiding this comment.
Good idea, I didn't think about Etherscan. Opened an issue for tracking:
| async function save() { | ||
| await saveCache(); | ||
| } |
There was a problem hiding this comment.
Nit - redundant? Can export saveCache as alias?
There was a problem hiding this comment.
Not redundant, I'm afraid.
The save function needs to kept separate because we have to run it at the end of the GA workflow execution - see the runs.post field I added in action.yml. There is no other way to run the save cache step at the end of the workflow execution.
Related - this is also how the official GitHub cache does this.
As discussed here and here, this PR upstreams my RPC caching method in
foundry-toolchainitself.I explained how the caching works in the README, but I will paste it here as well:
Side note: this PR also installs Prettier and adds a basic config file.
I tested this in my private repo that has fork tests, and both restoring the cache and saving it worked as expected:
Feel free to test in your repos, too: