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

BitVM2: bump rust nightly #551

Merged
merged 7 commits into from
Dec 19, 2024
Merged

BitVM2: bump rust nightly #551

merged 7 commits into from
Dec 19, 2024

Conversation

storopoli
Copy link
Member

@storopoli storopoli commented Dec 19, 2024

Description

Same version as main.

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

@storopoli storopoli requested a review from Rajil1213 December 19, 2024 08:44
Copy link

codecov bot commented Dec 19, 2024

Codecov Report

Attention: Patch coverage is 16.66667% with 5 lines in your changes missing coverage. Please review.

Project coverage is 56.36%. Comparing base (a6af246) to head (9146439).
Report is 9 commits behind head on bitvm2.

Files with missing lines Patch % Lines
bin/strata-cli/src/recovery.rs 0.00% 3 Missing ⚠️
crates/reth/exex/src/cache_db_provider.rs 0.00% 1 Missing ⚠️
crates/state/src/operation.rs 0.00% 1 Missing ⚠️
@@            Coverage Diff             @@
##           bitvm2     #551      +/-   ##
==========================================
- Coverage   56.64%   56.36%   -0.29%     
==========================================
  Files         249      249              
  Lines       28064    28062       -2     
==========================================
- Hits        15897    15816      -81     
- Misses      12167    12246      +79     
Files with missing lines Coverage Δ
crates/btcio/src/rpc/types.rs 51.85% <ø> (-1.24%) ⬇️
crates/state/src/bridge_state.rs 57.69% <ø> (-1.29%) ⬇️
crates/storage/src/cache.rs 78.19% <ø> (ø)
crates/storage/src/exec.rs 47.54% <ø> (ø)
crates/tasks/src/pending_tasks.rs 94.11% <ø> (ø)
crates/test-utils/src/bitcoin.rs 78.80% <100.00%> (ø)
crates/zkvm/adapters/sp1/src/input.rs 0.00% <ø> (ø)
crates/reth/exex/src/cache_db_provider.rs 0.00% <0.00%> (ø)
crates/state/src/operation.rs 31.53% <0.00%> (-2.31%) ⬇️
bin/strata-cli/src/recovery.rs 0.00% <0.00%> (ø)

... and 46 files with indirect coverage changes

Copy link
Contributor

@Rajil1213 Rajil1213 left a comment

Choose a reason for hiding this comment

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

The plan is to cherry-pick changes from this branch into main soon. During this process, we will definitely need to bump the nightly version so we don't get rug-pulled. Thank you for taking up this task.

Have left some nits and questions. And the CI needs fixing too.

@@ -94,6 +94,7 @@ impl OperatorTable {
}

/// Sanity checks the operator table for sensibility.
#[allow(dead_code)] // #FIXME: remove this.
Copy link
Contributor

Choose a reason for hiding this comment

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

Moving forward we should use #[expect(dead_code)] instead. And then, we don't need to rely on FIXME:. The compiler will tell us if this can be removed.

#![allow(dead_code)] // TODO: remove once the bridge state `sanity_check` fn is used.
#![feature(is_sorted)] // TODO: switch to using crate
#![allow(stable_features)] // FIX: this is needed for sp1 toolchain.
#![feature(is_sorted, is_none_or)]
Copy link
Contributor

@Rajil1213 Rajil1213 Dec 19, 2024

Choose a reason for hiding this comment

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

Could you add a comment as to why this one (is_none_or) is needed too? Should also replace allow with expect here.

Copy link
Member Author

Choose a reason for hiding this comment

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

The problem is that this will conflict with main:

#![allow(stable_features)] // FIX: this is needed for sp1 toolchain.
#![feature(is_sorted, is_none_or)]

Copy link
Contributor

Choose a reason for hiding this comment

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

Okay then. We can leave this as is for now.

@@ -85,13 +85,15 @@ impl<K: Clone + Eq + Hash, V: Clone> CacheTable<K, V> {

/// Gets the number of elements in the cache.
// TODO replace this with an atomic we update after every op
#[allow(dead_code)] // #FIXME: remove this.
Copy link
Contributor

Choose a reason for hiding this comment

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

Same comment about replacing this with expect.

pub async fn get_len_async(&self) -> usize {
let cache = self.cache.lock().await;
cache.len()
}

/// Gets the number of elements in the cache.
// TODO replace this with an atomic we update after every op
#[allow(dead_code)] // #FIXME: remove this.
Copy link
Contributor

Choose a reason for hiding this comment

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

Here too.

@@ -110,13 +112,15 @@ impl<K: Clone + Eq + Hash, V: Clone> CacheTable<K, V> {
}

/// Inserts an entry into the table, dropping the previous value.
#[allow(dead_code)] // #FIXME: remove this.
Copy link
Contributor

Choose a reason for hiding this comment

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

And here...

@@ -21,6 +21,7 @@ pub struct CheckpointProofGenerator {
}

impl CheckpointProofGenerator {
#[allow(dead_code)] // #FIXME: remove this.
Copy link
Contributor

Choose a reason for hiding this comment

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

eight

@@ -20,6 +20,7 @@ pub struct ClProofGenerator {
}

impl ClProofGenerator {
#[allow(dead_code)] // #FIXME: remove this.
Copy link
Contributor

Choose a reason for hiding this comment

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

nine

@@ -14,6 +14,7 @@ use crate::helpers::proof_generator::ProofGenerator;
pub struct ElProofGenerator;

impl ElProofGenerator {
#[allow(dead_code)] // #FIXME: remove this.
Copy link
Contributor

Choose a reason for hiding this comment

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

ten

@@ -19,6 +19,7 @@ pub struct L1BatchProofGenerator {
}

impl L1BatchProofGenerator {
#[allow(dead_code)] // #FIXME: remove this.
Copy link
Contributor

Choose a reason for hiding this comment

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

eleven

@@ -16,6 +16,7 @@ pub struct L2BatchProofGenerator {
}

impl L2BatchProofGenerator {
#[allow(dead_code)] // #FIXME: remove this.
Copy link
Contributor

Choose a reason for hiding this comment

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

last one...

@storopoli
Copy link
Member Author

All of the expect comments are valid but they will be off from the main branch

@storopoli
Copy link
Member Author

This might not be needed since https://alpenlabs.atlassian.net/browse/STR-769

@Rajil1213
Copy link
Contributor

This might not be needed since https://alpenlabs.atlassian.net/browse/STR-769

Yeah the plan is to tackle all the changes at once and create a single PR but those changes don't include bumping the nightly version. If you want to merge this one before that ticket, that works too. if not, you can go ahead and close this ticket and I or somebody else will pickup the other one.

@storopoli storopoli merged commit a64a5b0 into bitvm2 Dec 19, 2024
15 of 17 checks passed
@storopoli storopoli deleted the bivm2/bump-rust-nightly branch December 19, 2024 22:46
Rajil1213 pushed a commit that referenced this pull request Dec 30, 2024
* chore: bump nightly to 2024-12-12

* chore: clippy lints

* fix: stable_features for the sp1 toolchain

* chore: allow dead_code lints

* chore: fmt

* chore: more clippy lints

* chore: fmt
sapinb pushed a commit that referenced this pull request Jan 8, 2025
* Merge pull request #450 from alpenlabs/fix/add-getters-for-withdrawal-info-fields

fix: add accessors for all fields in the CooperativeWithdrawalInfo

* Merge pull request #451 from alpenlabs/bitvm2-poc-checkpoint-state-info

poc checkpoint state info

* Derive copy and clone traits (#464)

* export hashed chain state
* derive clone and copy

* Feat/export hashed chain state (#465)

* export hashed chain state

* derive clone and copy

* derive brosh deserialize

* use separate bridge wallets for local dev

* Update local sequencer config to use wallet name

* feat: use separate bridge wallets for local dev
* feat: update rpc-url to include wallet-name for sequencer

* chore(deps): bump bitcoin to 0.32.5 (#548)

* BitVM2: bump rust nightly (#551)

* chore: bump nightly to 2024-12-12

* chore: clippy lints

* fix: stable_features for the sp1 toolchain

* chore: allow dead_code lints

* chore: fmt

* chore: more clippy lints

* chore: fmt

* feat(fn-tests): use fast-batch in checkpoint-related tests

* fix(chain_state): correct typos

* fix(csm): use correct check

* chore: remove unnecessary patches

* chore(fn-tests): increase timeouts

* refactor(fn-tests): remove unncessary method

* chore: ignore _dd from codespell check

* style(fn-tests): run ruff fmt

* refactor(strata-rpc-server): use status_channel for chain state

* fix(fn-tests): account for checkpointing in bridge duty reassignment

* refactor(strata-rpc-server): get latest checkpointed state from status_channel

* fix(fn-tests): wait longer to republish after reorg

* revert(fn-tests): reset timeout values to original

* style(fn-tests): apply formatting

* fix(docker): update entrypoint script

* fix(datatool): encode xpriv instead of converting to bytes

* fix(strata-client): trim whitespace from keyfile

* fix(Makefile): use correct docker datadir

* style(storage): remove hash

* feat(consensus): include block height and position of checkpoint tx in info

* chore(fn-tests): remove unused function param

* fix(proof-impl): remove last proved l1_blockid from checkpoint batch info

* fix(fn-tests): increase timeout for cl checkpoint finalization

* fix(docker): update sequencer keygen script in local env

---------

Co-authored-by: Jose Storopoli <[email protected]>
Co-authored-by: bbist <[email protected]>
Co-authored-by: Jose Storopoli <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants