Skip to content

Str 1289 asm base #806

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
Merged
merged 28 commits into from
May 28, 2025
Merged

Str 1289 asm base #806

merged 28 commits into from
May 28, 2025

Conversation

prajwolrg
Copy link
Contributor

Description

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 (where necessary) tests that prove my changes are effective or that my feature works.
  • New and existing tests pass with my changes.

Related Issues

@prajwolrg prajwolrg requested a review from delbonis May 15, 2025 10:22
@codecov Codecov
Copy link

codecov bot commented May 15, 2025 •
edited
Loading

Codecov Report

Attention: Patch coverage is 2.88462% with 303 lines in your changes missing coverage. Please review.

Project coverage is 52.28%. Comparing base (5afc95b) to head (fc6d2b3).
Report is 3 commits behind head on main.

Files with missing lines Patch % Lines
crates/asm/stf/src/manager.rs 0.00% 118 Missing ⚠️
crates/asm/stf/src/tx_filter.rs 0.00% 35 Missing ⚠️
crates/asm/stf/src/stage.rs 0.00% 34 Missing ⚠️
crates/asm/stf/src/transition.rs 0.00% 34 Missing ⚠️
crates/asm/common/src/tx.rs 0.00% 24 Missing ⚠️
crates/asm/common/src/msg.rs 30.00% 21 Missing ⚠️
crates/asm/common/src/state.rs 0.00% 19 Missing ⚠️
crates/asm/subprotocols/bridge-v1/src/lib.rs 0.00% 11 Missing ⚠️
crates/asm/subprotocols/core/src/lib.rs 0.00% 7 Missing ⚠️ 
@@            Coverage Diff             @@
##             main     #806      +/-   ##
==========================================
- Coverage   52.62%   52.28%   -0.35%     
==========================================
  Files         307      316       +9     
  Lines       33467    33850     +383     
==========================================
+ Hits        17613    17697      +84     
- Misses      15854    16153     +299
Files with missing lines Coverage Δ
crates/asm/subprotocols/core/src/lib.rs 0.00% <0.00%> (ø)
crates/asm/subprotocols/bridge-v1/src/lib.rs 0.00% <0.00%> (ø)
crates/asm/common/src/state.rs 0.00% <0.00%> (ø)
crates/asm/common/src/msg.rs 30.00% <30.00%> (ø)
crates/asm/common/src/tx.rs 0.00% <0.00%> (ø)
crates/asm/stf/src/stage.rs 0.00% <0.00%> (ø)
crates/asm/stf/src/transition.rs 0.00% <0.00%> (ø)
crates/asm/stf/src/tx_filter.rs 0.00% <0.00%> (ø)
crates/asm/stf/src/manager.rs 0.00% <0.00%> (ø)

... and 5 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented May 15, 2025 •
edited
Loading

Commit: d357de6

SP1 Execution Results

program cycles success
Bitcoin Blockspace 71,672 ✅
EVM EE STF 277,604 ✅
CL STF 167,016 ✅
Checkpoint 4,252 ✅

@prajwolrg prajwolrg force-pushed the STR-1289-asm-base branch from 195a449 to d460eb7 Compare May 15, 2025 11:12
@prajwolrg prajwolrg marked this pull request as ready for review May 15, 2025 15:38
@prajwolrg prajwolrg requested review from a team as code owners May 15, 2025 15:38
delbonis
delbonis requested changes May 15, 2025
View reviewed changes
@prajwolrg prajwolrg requested a review from delbonis May 16, 2025 09:52
@prajwolrg prajwolrg force-pushed the STR-1289-asm-base branch 2 times, most recently from b23a295 to 0192489 Compare May 16, 2025 10:05
delbonis
delbonis requested changes May 16, 2025
View reviewed changes
@prajwolrg prajwolrg requested a review from delbonis May 17, 2025 04:34
@delbonis
Copy link
Contributor

I'll make a commit later to reflect what's in the notion doc.

@delbonis
Copy link
Contributor

Okay take a look over on that. It does rely on dyn a bit more than I wish it did but the number of vtable calls is linear in the number of inter-proto messages being sent, which we expect to actually be pretty uncommon in practice.

@prajwolrg prajwolrg force-pushed the STR-1289-asm-base branch from 8aaa87b to 33391c6 Compare May 20, 2025 16:05
@prajwolrg
Copy link
Contributor Author

@delbonis I tried to implement asm_stf function with the new changes.

delbonis
delbonis requested changes May 20, 2025
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.

I have a quick idea I want to try here to maybe avoid an extra clone.

@prajwolrg prajwolrg force-pushed the STR-1289-asm-base branch from 277c48e to 50daa9b Compare May 21, 2025 04:24
@prajwolrg prajwolrg requested a review from delbonis May 21, 2025 04:25
sapinb
sapinb reviewed May 21, 2025
View reviewed changes
delbonis
delbonis requested changes May 21, 2025
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.

Mostly just cleanup at this point.

@prajwolrg prajwolrg requested review from sapinb and delbonis May 22, 2025 05:31
@prajwolrg prajwolrg force-pushed the STR-1289-asm-base branch from e23938b to c084930 Compare May 22, 2025 09:24
delbonis
delbonis requested changes May 22, 2025
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.

I should have been more clear, my mistake.

@prajwolrg prajwolrg requested a review from delbonis May 24, 2025 04:17
bewakes
bewakes reviewed May 25, 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.

This is good! The ASM stf with staged processing is neat. A few more comments.

@irnb irnb self-requested a review May 26, 2025 08:24
@prajwolrg prajwolrg requested a review from bewakes May 26, 2025 16:32
@prajwolrg prajwolrg force-pushed the STR-1289-asm-base branch from fc6d2b3 to 26e6998 Compare May 26, 2025 16:32
bewakes
bewakes reviewed May 27, 2025
View reviewed changes
@prajwolrg prajwolrg force-pushed the STR-1289-asm-base branch from 26e6998 to 199394f Compare May 27, 2025 12:11
@prajwolrg prajwolrg requested a review from bewakes May 27, 2025 13:13
delbonis
delbonis reviewed May 27, 2025
View reviewed changes
@prajwolrg prajwolrg added this pull request to the merge queue May 28, 2025
Merged via the queue into main with commit b3ccdda May 28, 2025
16 checks passed
@prajwolrg prajwolrg deleted the STR-1289-asm-base branch May 28, 2025 03:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@delbonis delbonis delbonis approved these changes

@sapinb sapinb Awaiting requested review from sapinb

@irnb irnb Awaiting requested review from irnb

@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.
4 participants
@prajwolrg @delbonis @bewakes @sapinb