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

L1-Scan guest code to prove range of N blocks #541

Merged
merged 6 commits into from
Jan 7, 2025
Merged

L1-Scan guest code to prove range of N blocks #541

merged 6 commits into from
Jan 7, 2025

Conversation

MdTeach
Copy link
Contributor

@MdTeach MdTeach commented Dec 14, 2024 •
edited
Loading

Description

  • Upgrades the current L1-scan guest code and prover manager to support proving the range of L1 blocks.
  • Refactor proof job in terms of the block IDs for L1 batch

Type of Change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature/Enhancement (non-breaking change which adds functionality or enhances an existing one)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Documentation update
  • Refactor
  • New or updated tests
  • Dependency Update

Notes to Reviewers

Checklist

  • I have performed a self-review of my code.
  • I have commented my code where necessary.
  • I have updated the documentation if needed.
  • My changes do not introduce new warnings.
  • I have added tests that prove my changes are effective or that my feature works.
  • New and existing tests pass with my changes.

Related Issues

STR-798

@MdTeach MdTeach marked this pull request as ready for review December 17, 2024 22:18
@MdTeach MdTeach requested review from a team as code owners December 17, 2024 22:18
@codecov Codecov
Copy link

codecov bot commented Dec 17, 2024 •
edited
Loading

Codecov Report

Attention: Patch coverage is 26.95652% with 84 lines in your changes missing coverage. Please review.

Project coverage is 56.33%. Comparing base (c465477) to head (88ec164).
Report is 3 commits behind head on main.

Files with missing lines Patch % Lines
bin/prover-client/src/operators/l1_batch.rs 0.00% 55 Missing ⚠️
bin/prover-client/src/operators/checkpoint.rs 0.00% 13 Missing ⚠️
bin/prover-client/src/operators/btc.rs 0.00% 11 Missing ⚠️
bin/prover-client/src/rpc_server.rs 0.00% 3 Missing ⚠️
bin/prover-client/src/hosts/native.rs 0.00% 1 Missing ⚠️
bin/prover-client/src/operators/operator.rs 0.00% 1 Missing ⚠️ 
@@            Coverage Diff             @@
##             main     #541      +/-   ##
==========================================
- Coverage   56.34%   56.33%   -0.02%     
==========================================
  Files         308      309       +1     
  Lines       32075    32050      -25     
==========================================
- Hits        18073    18055      -18     
+ Misses      14002    13995       -7
Files with missing lines Coverage Δ
crates/primitives/src/proof.rs 40.00% <ø> (ø)
crates/proof-impl/btc-blockspace/src/logic.rs 87.50% <100.00%> (-9.87%) ⬇️
crates/proof-impl/btc-blockspace/src/prover.rs 100.00% <ø> (ø)
crates/proof-impl/btc-blockspace/src/scan.rs 100.00% <100.00%> (ø)
crates/proof-impl/l1-batch/src/logic.rs 97.05% <100.00%> (-0.31%) ⬇️
crates/proof-impl/l1-batch/src/prover.rs 100.00% <100.00%> (+4.00%) ⬆️
crates/zkvm/hosts/src/native.rs 100.00% <100.00%> (ø)
bin/prover-client/src/hosts/native.rs 0.00% <0.00%> (ø)
bin/prover-client/src/operators/operator.rs 0.00% <0.00%> (ø)
bin/prover-client/src/rpc_server.rs 0.00% <0.00%> (ø)
... and 3 more

... and 4 files with indirect coverage changes

prajwolrg
prajwolrg requested changes Dec 18, 2024
View reviewed changes
Copy link
Contributor

@prajwolrg prajwolrg 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! Mostly nits!

bin/prover-client/src/proving_ops/l1_batch_ops.rs Outdated Show resolved Hide resolved
crates/proof-impl/btc-blockspace/src/logic.rs Outdated Show resolved Hide resolved
crates/proof-impl/btc-blockspace/src/logic.rs Outdated Show resolved Hide resolved
crates/proof-impl/btc-blockspace/src/prover.rs Outdated Show resolved Hide resolved
@MdTeach MdTeach requested a review from prajwolrg December 18, 2024 16:17
delbonis
delbonis requested changes Dec 18, 2024
View reviewed changes
Copy link
Contributor

@delbonis delbonis left a comment

Choose a reason for hiding this comment

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

Jose is also gonna ask for docstrings on a lot of this.

bin/prover-client/src/proving_ops/btc_ops.rs Outdated Show resolved Hide resolved
@MdTeach MdTeach force-pushed the STR-698-l1scan-prove-range branch from 89a232b to 2dccc11 Compare December 19, 2024 19:17
delbonis
delbonis requested changes Dec 19, 2024
View reviewed changes
functional-tests/fn_prover_l1_dispatch.py Outdated Show resolved Hide resolved
crates/proof-impl/l1-batch/src/logic.rs Outdated Show resolved Hide resolved
@john-light john-light changed the title Str 698 L1-Scan guest code to prove range of N blocks L1-Scan guest code to prove range of N blocks Dec 25, 2024
@MdTeach MdTeach force-pushed the STR-698-l1scan-prove-range branch from 2dccc11 to 4aa65bf Compare January 2, 2025 04:56
@MdTeach MdTeach requested a review from a team as a code owner January 2, 2025 04:56
@MdTeach MdTeach force-pushed the STR-698-l1scan-prove-range branch from 4aa65bf to 7dfaff9 Compare January 2, 2025 05:04
@MdTeach MdTeach requested a review from a team as a code owner January 2, 2025 05:04
bewakes
bewakes reviewed Jan 2, 2025
View reviewed changes
Copy link
Contributor

@bewakes bewakes left a comment

Choose a reason for hiding this comment

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

LGTM.

crates/proof-impl/btc-blockspace/src/logic.rs Show resolved Hide resolved
@MdTeach MdTeach marked this pull request as draft January 2, 2025 10:23
@MdTeach MdTeach force-pushed the STR-698-l1scan-prove-range branch from d10bb3a to 3d25d3a Compare January 3, 2025 10:47
@MdTeach MdTeach requested review from delbonis and bewakes January 3, 2025 10:47
@MdTeach MdTeach marked this pull request as ready for review January 3, 2025 10:48
@MdTeach MdTeach force-pushed the STR-698-l1scan-prove-range branch from 3d25d3a to 59d1007 Compare January 3, 2025 10:49
delbonis
delbonis previously approved these changes Jan 3, 2025
View reviewed changes
@MdTeach MdTeach force-pushed the STR-698-l1scan-prove-range branch from 59d1007 to 7f44cfa Compare January 5, 2025 06:17
@MdTeach MdTeach force-pushed the STR-698-l1scan-prove-range branch from 7f44cfa to 88ec164 Compare January 6, 2025 17:04
@MdTeach MdTeach requested review from storopoli and delbonis January 6, 2025 17:06
storopoli
storopoli approved these changes Jan 6, 2025
View reviewed changes
Copy link
Member

@storopoli storopoli left a comment

Choose a reason for hiding this comment

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

ACK 88ec164

@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Jan 6, 2025

Commit: 18bb376

SP1 Performance Test Results

program cycles success
BTC_BLOCKSPACE 7,239,295 ✅
EL_BLOCK 100,761 ✅
CL_BLOCK 56,358 ✅
L1_BATCH 12,459,517 ✅
L2_BATCH 5,443 ✅
CHECKPOINT 16,119 ✅

@storopoli storopoli enabled auto-merge January 6, 2025 19:50
@storopoli storopoli added this pull request to the merge queue Jan 7, 2025
mukeshdroid
mukeshdroid approved these changes Jan 7, 2025
View reviewed changes
Copy link

@mukeshdroid mukeshdroid left a comment

Choose a reason for hiding this comment

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

Only a doc string has been updated in the file owned by crypto.

Merged via the queue into main with commit 6a47763 Jan 7, 2025
18 of 19 checks passed
@storopoli storopoli deleted the STR-698-l1scan-prove-range branch January 7, 2025 04:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@prajwolrg prajwolrg prajwolrg approved these changes

@delbonis delbonis delbonis approved these changes

@storopoli storopoli storopoli approved these changes

@mukeshdroid mukeshdroid mukeshdroid approved these changes

@bewakes bewakes Awaiting requested review from bewakes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@MdTeach @bewakes @prajwolrg @delbonis @storopoli @mukeshdroid