-
Notifications
You must be signed in to change notification settings - Fork 12
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
refactor: setup for standardizing benchmarks #2
Changes from 4 commits
8fb0bd6
55ea90f
87c81a0
f7f3b78
63c94fd
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,6 @@ | ||
export ETH_RPC_URL=https://eth-mainnet.alchemyapi.io/v2/yourAlchemyApiKey | ||
export FORK_BLOCK=13724056 | ||
export GAS_LIMIT=30000000 | ||
|
||
# skip type-checking to avoid its perfomance impact | ||
export TS_NODE_TRANSPILE_ONLY=1 |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,4 +1,5 @@ | ||
all :; dapp build | ||
clean :; dapp clean | ||
test :; dapp test --rpc-url ${ETH_RPC_URL} | ||
deploy :; dapp create Convex | ||
benchmark-all :; bash scripts/benchmark-all.sh | ||
benchmark-dapptools :; bash scripts/benchmark-dapptools.sh | ||
benchmark-foundry :; bash scripts/benchmark-foundry.sh | ||
benchmark-ganache :; bash scripts/benchmark-ganache.sh | ||
benchmark-hardhat :; bash scripts/benchmark-hardhat.sh |
This file was deleted.
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,23 @@ | ||
import '@nomiclabs/hardhat-waffle'; | ||
import { HardhatUserConfig } from 'hardhat/config'; | ||
|
||
const config: HardhatUserConfig = { | ||
solidity: '0.8.10', | ||
defaultNetwork: 'hardhat', | ||
networks: { | ||
hardhat: { | ||
accounts: { mnemonic: 'test test test test test test test test test test test junk' }, | ||
chainId: 31337, | ||
gas: Number(process.env.GAS_LIMIT), // set to block gas limit, which avoids calls to eth_estimateGas (results in same behavior as dapptools/foundry) | ||
forking: { | ||
url: String(process.env.ETH_RPC_URL), | ||
blockNumber: Number(process.env.FORK_BLOCK), | ||
}, | ||
}, | ||
}, | ||
mocha: { | ||
timeout: 0, | ||
}, | ||
}; | ||
|
||
export default config; |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -5,11 +5,8 @@ | |
"repository": "https://github.com/mds1/convex-shutdown-simulation", | ||
"author": "Matt Solomon <[email protected]>", | ||
"license": "MIT", | ||
"scripts": { | ||
"benchmark:ganache": "ts-node ./benchmark-ganache.ts" | ||
}, | ||
"devDependencies": { | ||
"@nomiclabs/hardhat-ethers": "^2.0.2", | ||
"@nomiclabs/hardhat-ethers": "^2.0.3", | ||
"@nomiclabs/hardhat-waffle": "^2.0.1", | ||
"chai": "^4.3.4", | ||
"chalk": "^4", | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,11 @@ | ||
echo -e "\n--- BENCHMARKING DAPPTOOLS ---" | ||
bash scripts/benchmark-dapptools.sh | ||
|
||
echo -e "\n--- BENCHMARKING FOUNDRY ---" | ||
bash scripts/benchmark-foundry.sh | ||
|
||
echo -e "\n--- BENCHMARKING GANACHE ---" | ||
bash scripts/benchmark-ganache.sh | ||
|
||
echo -e "\n--- BENCHMARKING HARDHAT ---" | ||
bash scripts/benchmark-hardhat.sh |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,5 @@ | ||
. ./.env | ||
|
||
# dapptools has no option to set gas limit, but it's hardcoded by default | ||
# anyway so shouldn't be make a difference | ||
time dapp test --rpc-url $ETH_RPC_URL --rpc-block $FORK_BLOCK |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,3 @@ | ||
. ./.env | ||
|
||
time forge test --rpc-url $ETH_RPC_URL --fork-block-number $FORK_BLOCK --gas-limit $GAS_LIMIT |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,4 @@ | ||
. ./.env | ||
|
||
# Uses Ganache v7 (@beta) | ||
time yarn ts-node ./scripts/convex.ganache.ts | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I assume we're okay with just running I was thinking it might be neat to benchmark startup time separately from simulation time, but it'd require that each script output its own metrics broken out. Probably more trouble than it's worth, though. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Agreed, I'd like to benchmark that as well. I wasn't sure if each tool exposed that easily so figured it's worth looking at in a future PR, with |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,3 @@ | ||
. ./.env | ||
|
||
time TS_NODE_FILES=true yarn ts-node ./scripts/convex.hardhat.ts |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,25 @@ | ||
import { ethers, network } from 'hardhat'; | ||
import '@nomiclabs/hardhat-ethers'; | ||
|
||
async function main(): Promise<void> { | ||
// Impersonate owner | ||
const abi = ['function shutdownSystem() external', 'function owner() external view returns (address)']; | ||
const convex = new ethers.Contract('0xF403C135812408BFbE8713b5A23a04b3D48AAE31', abi, ethers.provider); | ||
const ownerAddr = await convex.owner(); | ||
await network.provider.request({ method: 'hardhat_impersonateAccount', params: [ownerAddr] }); | ||
const owner = await ethers.getSigner(ownerAddr); | ||
|
||
// Fund owner (it's a contract) | ||
await network.provider.send('hardhat_setBalance', [owner.address, '0xffffffffffffffffffff']); | ||
|
||
// Execute transaction | ||
const tx = await convex.connect(owner).shutdownSystem(); | ||
const receipt = await ethers.provider.getTransactionReceipt(tx.hash); | ||
} | ||
|
||
main() | ||
.then(() => process.exit(0)) | ||
.catch((error: Error) => { | ||
console.error(error); | ||
process.exit(1); | ||
}); |
This file was deleted.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Aside: does this repo need to do any results verification? One or more tool's simulation could be totally inaccurate and we wouldn't know!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yep true! I was thinking of also reporting the
gasUsed
of each tool in a future PR, but open to other ideas here also. Not sure if verifying state changes or comparing transaction traces is excessive for a benchmarking repo 🤔There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was just thinking... if we're forking at the same block for each tool, just count the logs and make sure all tools report the same number of logs. Seems quick and dirty? Could also do a deep-equals on the JSON, but that's perhaps more prone to minute differences.
Anyway, this also might be something to manually test once and then not worry about until/unless this repo becomes used more seriously.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
True, just verifying once might be sufficient for now! Either way, now tracking all potential metrics in #3