-
Notifications
You must be signed in to change notification settings - Fork 293
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
feat: Test against Sepolia #8176
Conversation
Changes to circuit sizes
🧾 Summary (100% most significant diffs)
Full diff report 👇
|
Benchmark resultsMetrics with a significant change:
Detailed resultsAll benchmarks are run on txs on the This benchmark source data is available in JSON format on S3 here. Proof generationEach column represents the number of threads used in proof generation.
L2 block published to L1Each column represents the number of txs on an L2 block published to L1.
L2 chain processingEach column represents the number of blocks on the L2 chain where each block has 8 txs.
Circuits statsStats on running time and I/O sizes collected for every kernel circuit run across all benchmarks.
Stats on running time collected for app circuits
AVM SimulationTime to simulate various public functions in the AVM.
Public DB AccessTime to access various public DBs.
Tree insertion statsThe duration to insert a fixed batch of leaves into each tree type.
MiscellaneousTransaction sizes based on how many contract classes are registered in the tx.
Transaction size based on fee payment method | Metric | | |
// process.env.SEQ_PUBLISHER_PRIVATE_KEY = '<PRIVATE_KEY_WITH_SEPOLIA_ETH>'; | ||
// process.env.PROVER_PUBLISHER_PRIVATE_KEY = '<PRIVATE_KEY_WITH_SEPOLIA_ETH>'; | ||
// process.env.ETHEREUM_HOST= 'https://sepolia.infura.io/v3/<API_KEY>'; | ||
// process.env.L1_CHAIN_ID = '11155111'; |
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.
Where is this getting set for the nightly run?
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.
Currently it's not as leaving everything defaulted causes the test to run against a local Anvil. Once I have seen it run successfully against Anvil I will put in another PR to set these values in the sepolia-test.yml
const chain = chainId == sepolia.id ? sepolia : foundry; // Not the best way of doing this. | ||
({ logger, pxe, teardown, config, aztecNode } = await setup( | ||
0, | ||
{ skipProtocolContracts: true, stateLoad: undefined, salt: 1 }, |
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.
The salt: 1
means that we'll consistently reuse the same set of L1 rollup contracts, which means each test run won't be independent from the previous one. Are we sure about this? I'm worried that we could have breaking changes that screw up the node or prover-node syncing if we keep using the same rollup.
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.
Ahh yeah. You make a good point. I was hoping to save on ETH by not re-deploying everytime but that's probably not desirable.
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 had the same concern.
# Uncomment the following to run against Sepolia | ||
# SEQ_PUBLISHER_PRIVATE_KEY: ${{ secrets.SEPOLIA_SEQ_PRIVATE_KEY }} | ||
# PROVER_PUBLISHER_PRIVATE_KEY: ${{ secrets.SEPOLIA_PROVER_PRIVATE_KEY }} | ||
# ETHEREUM_HOST: 'https://sepolia.infura.io/v3/${{ secrets.SEPOLIA_API_KEY }}'; | ||
# L1_CHAIN_ID: '11155111' # Sepolia Chain ID |
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.
Should we uncomment?
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.
Not just yet. Once I have seen a successful Anvil run I will uncomment, and will have to load some ETH.
await teardown(); | ||
}); | ||
|
||
it('calls a private function', async () => { |
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've feel we've written variations of this test a dozen times already
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.
Possibly. The difference here really is that this uses the EasyPrivateTokenContract
which isn't used anywhere else. It has no public bytecode that needs deploying. Once we are able to deploy public bytecode, I'll probably just use the standard token contract and an existing test.
.github/workflows/sepolia-test.yml
Outdated
#ref: "${{ env.GIT_COMMIT }}" | ||
ref: pw/new-test-2 |
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.
Does the cron run if it is defined in a PR? If not, let's just change this to env.GIT_COMMIT
now, since it won't need this branch, right?
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.
Good catch, this was a oversight.
builder_type: builder-x86 | ||
# these are copied to the tester and expected by the earthly command below | ||
# if they fail to copy, it will try to build them on the tester and fail | ||
builder_images_to_copy: aztecprotocol/end-to-end:${{ env.GIT_COMMIT }} |
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.
e2e
also pulls aztecprotocol/aztec
in addition to aztecprotocol/end-to-end
. Is it needed here?
This PR creates an e2e test that can be used against Sepolia. It also sets up an automated job for the test.