Skip to content
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

Jump to bottom

chore: prover node has no claims or bonds #11811

Merged
merged 6 commits into from
Feb 12, 2025
Merged

chore: prover node has no claims or bonds #11811

merged 6 commits into from
Feb 12, 2025

Conversation

just-mitch
Copy link
Contributor

Please read contributing guidelines and remove this line.

@just-mitch Graphite App
Copy link
Contributor Author

Warning

This pull request is not mergeable via GitHub because a downstack PR is open. Once all requirements are satisfied, merge this PR as a stack on Graphite.
Learn more

This stack of pull requests is managed by Graphite. Learn more about stacking.

@just-mitch just-mitch mentioned this pull request Feb 7, 2025
Merged
@just-mitch just-mitch changed the title prover node has no claims or bonds chore: prover node has no claims or bonds Feb 7, 2025
@just-mitch just-mitch force-pushed the 02-07-prover_node_has_no_claims_or_bonds branch from b41185c to 5fbcb67 Compare February 10, 2025 17:04
@LHerskind LHerskind force-pushed the lh/multiproof-ep-1 branch 2 times, most recently from 2b203e0 to eb4ba26 Compare February 10, 2025 20:10
@just-mitch just-mitch force-pushed the 02-07-prover_node_has_no_claims_or_bonds branch from 5fbcb67 to a2bd753 Compare February 10, 2025 20:18
@just-mitch just-mitch added the e2e-all CI: Deprecated, use ci-full. Enable all master checks. label Feb 10, 2025
@just-mitch just-mitch force-pushed the 02-07-prover_node_has_no_claims_or_bonds branch 2 times, most recently from 213be4f to e2f51cf Compare February 11, 2025 01:37
@LHerskind LHerskind marked this pull request as ready for review February 11, 2025 06:34
LHerskind
LHerskind approved these changes Feb 11, 2025
View reviewed changes
Copy link
Contributor

@LHerskind LHerskind left a comment

Choose a reason for hiding this comment

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

Looks good, think there might be a one-off in the tests which seems to match what I see in the runs.

yarn-project/circuit-types/src/interfaces/pxe.test.ts
@@ -306,7 +305,7 @@ describe('PXESchema', () => {
});

it('getPrivateEvents', async () => {
const result = await context.client.getPrivateEvents<EpochProofQuote>(
const result = await context.client.getPrivateEvents<{ value: bigint }>(
Copy link
Contributor

Choose a reason for hiding this comment

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

Not clear to me why this was in here to begin with? Why would the PXE ask for stuff here 🤷

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah no idea.

yarn-project/end-to-end/src/e2e_block_building.test.ts Outdated
@@ -577,7 +577,7 @@ describe('e2e_block_building', () => {
// Now move to a new epoch and past the proof claim window to cause a reorg
logger.info('Advancing past the proof claim window');
await cheatCodes.rollup.advanceToNextEpoch();
await cheatCodes.rollup.advanceSlots(aztecEpochProofClaimWindowInL2Slots + 1); // off-by-one?
await cheatCodes.rollup.advanceSlots(aztecProofSubmissionWindow + 1); // off-by-one?
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we update the comment here

yarn-project/end-to-end/src/e2e_prover/full.test.ts Outdated

// And wait for the first pair of txs to be proven
logger.info(`Awaiting proof for the previous epoch`);
await Promise.all(txs.map(tx => tx.wait({ timeout: 300, interval: 10, proven: true, provenTimeout: 3000 })));

const provenBn = await rollup.read.getProvenBlockNumber();
const balanceAfterCoinbase = await feeJuice.read.balanceOf([COINBASE_ADDRESS.toString()]);
const balanceAfterProver = await feeJuice.read.balanceOf([t.proverAddress.toString()]);
expect(provenBn + 1n).toBe(await rollup.read.getPendingBlockNumber());
Copy link
Contributor

Choose a reason for hiding this comment

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

Why would we have a +1 in there? If we have just waited until all have been proven, the pending block number should be the same as the proven?

Copy link
Contributor

Choose a reason for hiding this comment

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

Think it is a leftover or something that relied on the quotes only being passed along with a proposal, so an extra block was added.

yarn-project/end-to-end/src/prover-coordination/e2e_prover_coordination.test.ts Outdated
Copy link
Contributor

Choose a reason for hiding this comment

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

There is still a ci job for running this test 👀 we should also get rid of that one.

yarn-project/prover-node/src/prover-node.ts
const partialQuote = await this.quoteProvider.getQuote(Number(epochNumber), blocks);
if (!partialQuote) {
this.log.info(`No quote produced for epoch ${epochNumber}`);
this.log.debug('jobs', JSON.stringify(this.jobs, null, 2));
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we keep this one?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think it's okay- it's at debug level, only once per epoch, and is pretty informative.

@LHerskind LHerskind force-pushed the 02-07-prover_node_has_no_claims_or_bonds branch from e2f51cf to 7e17c08 Compare February 11, 2025 11:06
@just-mitch just-mitch force-pushed the 02-07-prover_node_has_no_claims_or_bonds branch from a25050d to 33fd80c Compare February 11, 2025 20:50
@LHerskind LHerskind merged commit 761d0d1 into lh/multiproof-ep-1 Feb 12, 2025
80 of 81 checks passed
@LHerskind LHerskind deleted the 02-07-prover_node_has_no_claims_or_bonds branch February 12, 2025 10:31
LHerskind added a commit that referenced this pull request Feb 12, 2025
@just-mitch @LHerskind
chore: prover node has no claims or bonds (#11811)
4de747a 
Please read [contributing guidelines](CONTRIBUTING.md) and remove this
line.

---------

Co-authored-by: LHerskind <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@LHerskind LHerskind LHerskind approved these changes

Assignees
No one assigned
Labels
e2e-all CI: Deprecated, use ci-full. Enable all master checks.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@just-mitch @LHerskind