Commit "minimal" and "latest" lockfiles#591
Merged
apoelstra merged 5 commits intorust-bitcoin:masterfrom Mar 30, 2023
Merged
Conversation
The `cbor` crate has been unmaintained for several years, and depends on the ancient `rustc_serialize` crate which (a) doesn't build on WASM, and (b) doesn't build when we use a minimal-dep Cargo.lock. (The latter is because cbor specifies rustc_serialize 0.3.0 when it should specify 0.3.1, but there is nothing we can do to fix that when cbor is unmaintained.) This changes a hardcoded value in a regression test, but it's because we're replacing the serialization engine rather than changing our code, so this is not actually a change.
tcharding
previously approved these changes
Mar 19, 2023
Member
|
The idea is then that we would each publish crev reviews for all the crates (and versions) in both lock files, right? |
Kixunil
previously approved these changes
Mar 19, 2023
| // Secret key is 32 bytes. CBOR adds a byte of metadata for 20 of these bytes, | ||
| // (Apparently, any byte whose value is <24 gets an extra byte.) | ||
| // It also adds a 1-byte length prefix and a byte of metadata for the whole vector. | ||
| assert_eq!(e.len(), 54); |
Collaborator
There was a problem hiding this comment.
It's really weird that different engines encode it differently but I don't care much; it's not our fault.
Member
Author
|
Great, thanks both of you! We need a rust-secp maintainer to ACK before I can merge but it sounds like we're agreed on strategy here. |
This is only needed for the serde test, so don't bother putting it in the README. Downstream users won't encounter this dependency and don't need to care about it.
Member
Author
|
@elichai @sanket1729 @jonasnick could one of you take a look and ACK this? |
sanket1729
approved these changes
Mar 30, 2023
Member
sanket1729
left a comment
There was a problem hiding this comment.
ACK bd9d3c9. Verified the lockfiles. The latest one has a couple lines diff, but that is expected :) . minimal one is the same
chain-forgexcr45
added a commit
to chain-forgexcr45/rust-secp256k1
that referenced
this pull request
Sep 28, 2025
…lockfiles
bd9d3c9de7bc1d98b3d82fc4434ca997bc705ce7 test: pin 'half' dependency on 1.41.1. (Andrew Poelstra)
7fc84191eeee452e955a09a6ff5d68f5f689a835 cargo fmt (Andrew Poelstra)
1b12cc5f5854efdd14153b0937bb4ae8a2c43abf contrib: commit "minimal" and "latest" tested lockfiles (Andrew Poelstra)
0494f50b1a4507fff0a0fbb1e3dacdc49a8d6572 fix correct minimal versions for serde crates (Andrew Poelstra)
b03602bfaabf20f809f0ce7df2eb8275b4235cc1 tests: replace cbor with more-recently-deprecated serde_cbor (Andrew Poelstra)
Pull request description:
This is a proposed strategy for maintaining tested lockfiles in the rust-bitcoin ecosystem. The idea is that we would have both a "minimal" and a "latest" lockfile, and in both cases we have trusted crev reviews for all dependencies (this is not implemented here, I'd like to start by committing the lockfiles so we can agree what to review).
Periodically we can update the "latest" one to reflect new versions of deps that we've gotten around to reviewing.
I have local nix build scripts that are able to test every commit of proposed PRs against both lockfiles. In CI it is probably reasonable to at least do `cargo test --locked --all-features` with both of them, to make sure that the tests at least pass on each PR with them.
Thoughts?
ACKs for top commit:
sanket1729:
ACK bd9d3c9de7bc1d98b3d82fc4434ca997bc705ce7. Verified the lockfiles. The latest one has a couple lines diff, but that is expected :) . minimal one is the same
Tree-SHA512: 6f14406a595aa6a6006b35828080b00b1b87209cb3dd6512c0e08eb92ae1ff27df005494189504cd5654eac1607cc98e902ccdd62b221cb865652c29dd958463
william2332-limf
added a commit
to william2332-limf/rust-secp256k1
that referenced
this pull request
Oct 2, 2025
…lockfiles
bd9d3c9de7bc1d98b3d82fc4434ca997bc705ce7 test: pin 'half' dependency on 1.41.1. (Andrew Poelstra)
7fc84191eeee452e955a09a6ff5d68f5f689a835 cargo fmt (Andrew Poelstra)
1b12cc5f5854efdd14153b0937bb4ae8a2c43abf contrib: commit "minimal" and "latest" tested lockfiles (Andrew Poelstra)
0494f50b1a4507fff0a0fbb1e3dacdc49a8d6572 fix correct minimal versions for serde crates (Andrew Poelstra)
b03602bfaabf20f809f0ce7df2eb8275b4235cc1 tests: replace cbor with more-recently-deprecated serde_cbor (Andrew Poelstra)
Pull request description:
This is a proposed strategy for maintaining tested lockfiles in the rust-bitcoin ecosystem. The idea is that we would have both a "minimal" and a "latest" lockfile, and in both cases we have trusted crev reviews for all dependencies (this is not implemented here, I'd like to start by committing the lockfiles so we can agree what to review).
Periodically we can update the "latest" one to reflect new versions of deps that we've gotten around to reviewing.
I have local nix build scripts that are able to test every commit of proposed PRs against both lockfiles. In CI it is probably reasonable to at least do `cargo test --locked --all-features` with both of them, to make sure that the tests at least pass on each PR with them.
Thoughts?
ACKs for top commit:
sanket1729:
ACK bd9d3c9de7bc1d98b3d82fc4434ca997bc705ce7. Verified the lockfiles. The latest one has a couple lines diff, but that is expected :) . minimal one is the same
Tree-SHA512: 6f14406a595aa6a6006b35828080b00b1b87209cb3dd6512c0e08eb92ae1ff27df005494189504cd5654eac1607cc98e902ccdd62b221cb865652c29dd958463
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This is a proposed strategy for maintaining tested lockfiles in the rust-bitcoin ecosystem. The idea is that we would have both a "minimal" and a "latest" lockfile, and in both cases we have trusted crev reviews for all dependencies (this is not implemented here, I'd like to start by committing the lockfiles so we can agree what to review).
Periodically we can update the "latest" one to reflect new versions of deps that we've gotten around to reviewing.
I have local nix build scripts that are able to test every commit of proposed PRs against both lockfiles. In CI it is probably reasonable to at least do
cargo test --locked --all-featureswith both of them, to make sure that the tests at least pass on each PR with them.Thoughts?