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.

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

Properly create L1 segment from sequencer's view of L1 #116

Merged
merged 27 commits into from
Aug 7, 2024

Conversation

delbonis
Copy link
Contributor

@delbonis delbonis commented Jul 11, 2024

Resolves #93
Resolves #132
Resolves #152
Resolves #131

@delbonis
Copy link
Contributor Author

@sapinb Had to rebase this, there was some extremely hairy change made to the block assembly code both here and in your PR that I merged earlier. Please give a scan over this to make sure it's still correct. One fairly trivial change I made was passing in the whole Arc<Params> instead of just passing in a RollupParams so that we only have to keep around a single instance of it. I might have broken something more significant, especially with regard to the block accessories.

@delbonis delbonis requested a review from sapinb July 31, 2024 20:49
@delbonis delbonis marked this pull request as ready for review August 1, 2024 14:12
@delbonis
Copy link
Contributor Author

delbonis commented Aug 1, 2024

This isn't complete yet but it's touching more parts so I could use some reviews.

@delbonis delbonis mentioned this pull request Aug 5, 2024
11 tasks
sapinb
sapinb previously approved these changes Aug 6, 2024
Copy link

codecov bot commented Aug 7, 2024

Codecov Report

Attention: Patch coverage is 34.31455% with 781 lines in your changes missing coverage. Please review.

Project coverage is 52.45%. Comparing base (0425777) to head (6b868f5).
Report is 2 commits behind head on master.

Files Patch % Lines
crates/consensus-logic/src/duty/block_assembly.rs 52.23% 203 Missing ⚠️
crates/consensus-logic/src/duty/worker.rs 0.00% 179 Missing ⚠️
crates/consensus-logic/src/chain_transition.rs 0.00% 102 Missing ⚠️
crates/state/src/state_op.rs 2.50% 78 Missing ⚠️
crates/btcio/src/reader/query.rs 0.00% 47 Missing ⚠️
crates/state/src/l1.rs 19.51% 33 Missing ⚠️
sequencer/src/rpc_server.rs 0.00% 19 Missing ⚠️
crates/consensus-logic/src/l1_handler.rs 0.00% 18 Missing ⚠️
crates/state/src/operation.rs 5.55% 17 Missing ⚠️
crates/state/src/bridge_state.rs 0.00% 12 Missing ⚠️
... and 14 more
@@            Coverage Diff             @@
##           master     #116      +/-   ##
==========================================
- Coverage   53.74%   52.45%   -1.30%     
==========================================
  Files          86       87       +1     
  Lines        7725     8425     +700     
==========================================
+ Hits         4152     4419     +267     
- Misses       3573     4006     +433     
Files Coverage Δ
crates/consensus-logic/src/errors.rs 0.00% <ø> (ø)
crates/primitives/src/hash.rs 50.00% <100.00%> (+20.00%) ⬆️
crates/rpc/api/src/lib.rs 0.00% <ø> (ø)
crates/test-utils/src/bitcoin.rs 100.00% <100.00%> (ø)
crates/test-utils/src/l2.rs 98.66% <100.00%> (ø)
crates/test-utils/src/lib.rs 94.23% <100.00%> (+1.37%) ⬆️
crates/consensus-logic/src/ctl.rs 0.00% <0.00%> (ø)
crates/consensus-logic/src/worker.rs 0.00% <0.00%> (ø)
crates/consensus-logic/src/genesis.rs 0.00% <0.00%> (ø)
crates/consensus-logic/src/duty/extractor.rs 0.00% <0.00%> (ø)
... and 20 more

... and 2 files with indirect coverage changes

@delbonis delbonis merged commit baf9947 into master Aug 7, 2024
7 of 8 checks passed
@delbonis
Copy link
Contributor Author

delbonis commented Aug 7, 2024

Force merged because CI was taking too long and everybody's blocked on this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants