Skip to content

Cache RPC responses #19

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.

Sign up for GitHub

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

Merged
gakonst merged 4 commits into from
Jan 9, 2023
Merged

Cache RPC responses #19

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
d81f1d5
Fix branding color
PaulRBerg Jan 7, 2023
23f9eb2
refactor: make "version" input not required
PaulRBerg Jan 7, 2023
c604ee8
feat: cache RPC responses
PaulRBerg Jan 7, 2023
607b0c5
docs: fix terminology in caching explanation
PaulRBerg Jan 9, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions .prettierignore
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
dist
node_modules
7 changes: 7 additions & 0 deletions .prettierrc.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
bracketSpacing: true
printWidth: 120
proseWrap: "always"
singleQuote: false
tabWidth: 2
trailingComma: "all"
useTabs: false
28 changes: 21 additions & 7 deletions README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
## `foundry-toolchain` Action

This GitHub Action installs [Foundry](https://github.com/foundry-rs/foundry).
This GitHub Action installs [Foundry](https://github.com/foundry-rs/foundry), the blazing fast, portable and modular toolkit for Ethereum application development.

### Example workflow

Expand All @@ -20,8 +20,6 @@ jobs:

- name: Install Foundry
uses: foundry-rs/foundry-toolchain@v1
with:
version: nightly

- name: Run tests
run: forge test -vvv
Expand All @@ -32,10 +30,26 @@ jobs:

### Inputs

| **Name** | **Required** | **Description** | **Type** |
|-----------|--------------|---------------------------------------------------------------------------------------------------------------|----------|
| `version` | Yes | Version to install, e.g. `nightly` or `1.0.0`. **Note:** Foundry only has nightly builds for the time being. | string |
| **Name** | **Required** | **Default** | **Description** | **Type** |
| --------- | ------------ | ----------- | ------------------------------------------------------------------------------------------------------------ | -------- |
| `version` | No | `nightly` | Version to install, e.g. `nightly` or `1.0.0`. **Note:** Foundry only has nightly builds for the time being. | string |

### RPC Caching

This action matches Forge's behavior and caches all RPC responses in the `~/.foundry/cache/rpc` directory. This is done to
speed up the tests and avoid hitting the rate limit of your RPC provider.

The logic of the caching is as follows:

- 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.
Copy link

@mds1 mds1 Jan 9, 2023

Choose a reason for hiding this comment

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

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.

Copy link
Contributor Author

@PaulRBerg PaulRBerg Jan 9, 2023

Choose a reason for hiding this comment

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

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.

Copy link

@mds1 mds1 Jan 9, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

Got it, I see. And is "store key" the same as the "restore keys" in the actions/cache repo?

Copy link
Contributor Author

@PaulRBerg PaulRBerg Jan 9, 2023

Choose a reason for hiding this comment

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

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:

607b0c5

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.

Copy link

@mds1 mds1 Jan 9, 2023

Choose a reason for hiding this comment

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

Heh got it, thanks! Deferring to @gakonst and co. for review/merge 🙂

Copy link
Contributor Author

@PaulRBerg PaulRBerg Jan 9, 2023

Choose a reason for hiding this comment

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

Thanks @mds1 for the review thus far.


#### Fuzzing

Note that if you are fuzzing in your fork tests, the RPC cache strategy above will not work unless you set a
[fuzz seed](https://book.getfoundry.sh/reference/config/testing#seed). You might also want to reduce your number
of RPC calls by using [Multicall](https://github.com/mds1/multicall).

### Summaries

Expand All @@ -48,4 +62,4 @@ For example, to add the output of `forge snapshot` to a summary, you would chang
run: NO_COLOR=1 forge snapshot >> $GITHUB_STEP_SUMMARY
```

See the offical [GitHub docs](https://docs.github.com/en/actions/using-workflows/workflow-commands-for-github-actions#adding-a-job-summary) for more information.
See the official [GitHub docs](https://docs.github.com/en/actions/using-workflows/workflow-commands-for-github-actions#adding-a-job-summary) for more information.
19 changes: 12 additions & 7 deletions action.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,17 +1,22 @@
name: 'foundry-toolchain'
description: 'Install Foundry'
author: 'Oliver Nordbjerg'
name: "foundry-toolchain"
description: "Install Foundry"
author: "Foundry"
branding:
icon: play-circle
color: black
color: "gray-dark"
icon: "play-circle"

inputs:
version:
default: "nightly"
description: |
Foundry version.

This version number has to match a released version of Foundry.
Alternatively you can also specify `nightly` for the latest nightly build.
required: false

runs:
using: node16
main: dist/index.js
using: "node16"
main: "dist/index.js"
post: "dist/save/index.js"
post-if: "success()"
Loading