From 79e184f08af0aff0df87b911b4aae61d474e962b Mon Sep 17 00:00:00 2001 From: "Tobin C. Harding" Date: Tue, 8 Aug 2023 11:56:26 +1000 Subject: [PATCH 1/7] CI: Update MSRV pins Pinning is broken again, update the pins it CI so that the following sequence of commands would work ```bash rm Cargo.lock cargo +1.48 update -p wasm-bindgen-test --precise 0.3.34 cargo +1.48 update -p serde_test --precise 1.0.175 cargo +1.48 test --all-features ``` Note, solely out of interest, `cargo +1.48 build` does not need pinning (at the moment :) --- contrib/test.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/contrib/test.sh b/contrib/test.sh index e76c193..1ee60da 100755 --- a/contrib/test.sh +++ b/contrib/test.sh @@ -16,7 +16,7 @@ fi # Pin dependencies as required if we are using MSRV toolchain. if cargo --version | grep "1\.48"; then cargo update -p wasm-bindgen-test --precise 0.3.34 - cargo update -p serde --precise 1.0.156 + cargo update -p serde_test --precise 1.0.175 fi # Test if panic in C code aborts the process (either with a real panic or with SIGILL) From cff7a3dae79ab416a1de106c5736ae1bfb69503f Mon Sep 17 00:00:00 2001 From: "Tobin C. Harding" Date: Tue, 8 Aug 2023 11:40:24 +1000 Subject: [PATCH 2/7] CI: Grep for exact error message `cargo +nightly` output of panic recently changed breaking our grep statement by adding the code line and a newline. Grep for the exact second line of the error message. --- contrib/test.sh | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/contrib/test.sh b/contrib/test.sh index 1ee60da..13c216c 100755 --- a/contrib/test.sh +++ b/contrib/test.sh @@ -20,7 +20,9 @@ if cargo --version | grep "1\.48"; then fi # Test if panic in C code aborts the process (either with a real panic or with SIGILL) -cargo test -- --ignored --exact 'tests::test_panic_raw_ctx_should_terminate_abnormally' 2>&1 | tee /dev/stderr | grep "SIGILL\\|panicked at '\[libsecp256k1\]" +cargo test -- --ignored --exact 'tests::test_panic_raw_ctx_should_terminate_abnormally' 2>&1 \ + | tee /dev/stderr \ + | grep "SIGILL\\|\[libsecp256k1] illegal argument. !rustsecp256k1_v0_._._fe_is_zero(&ge->x)" # Make all cargo invocations verbose export CARGO_TERM_VERBOSE=true From 3bbf08348e9b11427ac8f1d13f1a975b3feee7ff Mon Sep 17 00:00:00 2001 From: "Tobin C. Harding" Date: Tue, 8 Aug 2023 12:23:01 +1000 Subject: [PATCH 3/7] no_std_test: Remove internal_features Remove the `internal_features` attribute, not sure what it was supposed to be doing but the crate works without it. --- no_std_test/src/main.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/no_std_test/src/main.rs b/no_std_test/src/main.rs index d98276c..2e2a3bd 100644 --- a/no_std_test/src/main.rs +++ b/no_std_test/src/main.rs @@ -27,7 +27,6 @@ //! * Requires linking with `libc` for calling `printf`. //! -#![feature(lang_items)] #![feature(start)] #![feature(core_intrinsics)] #![feature(panic_info_message)] From ec9c9643d794b7c5b2c4bbe4aeee8b70bf46ff1f Mon Sep 17 00:00:00 2001 From: "Tobin C. Harding" Date: Tue, 8 Aug 2023 12:24:24 +1000 Subject: [PATCH 4/7] Allow stuff after unconditional panic We have an unconditional panic for some combination of features, this causes clippy to give a bunch of useless warnings. Add allow attributes to quieten down clippy. --- src/key.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/key.rs b/src/key.rs index 0eb747e..1791dc1 100644 --- a/src/key.rs +++ b/src/key.rs @@ -1036,6 +1036,8 @@ impl serde::Serialize for KeyPair { } #[cfg(feature = "serde")] +#[allow(unused_variables)] // For `data` under some feature combinations (the unconditional panic below). +#[allow(unreachable_code)] // For `KeyPair::from_seckey_slice` after unconditional panic. impl<'de> serde::Deserialize<'de> for KeyPair { fn deserialize>(d: D) -> Result { if d.is_human_readable() { From cf5f1034cad0fbeae5e995442f7fd80e8eaa3edf Mon Sep 17 00:00:00 2001 From: "Tobin C. Harding" Date: Tue, 8 Aug 2023 12:23:33 +1000 Subject: [PATCH 5/7] Target panic message at lib users Currently the panic message refers to stuff related to development of the library, this is meaningless for users of the lib. Target panic message at secp users instead. --- src/key.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/key.rs b/src/key.rs index 1791dc1..ecec179 100644 --- a/src/key.rs +++ b/src/key.rs @@ -1053,7 +1053,7 @@ impl<'de> serde::Deserialize<'de> for KeyPair { let ctx = Secp256k1::signing_only(); #[cfg(not(any(feature = "global-context", feature = "alloc")))] - let ctx: Secp256k1 = panic!("The previous implementation was panicking too, please enable the global-context feature of rust-secp256k1"); + let ctx: Secp256k1 = panic!("cannot deserialize key pair without a context (please enable either the global-context or alloc feature)"); #[allow(clippy::needless_borrow)] KeyPair::from_seckey_slice(&ctx, data) From 5e6dd8a46783c8ab0aaf6a057cf6942ffc72fb2a Mon Sep 17 00:00:00 2001 From: "Tobin C. Harding" Date: Thu, 10 Aug 2023 09:48:02 +1000 Subject: [PATCH 6/7] CI: Use bash instead of sh In preparation for using `pushd`/`popd` use `bash` to run the CI script instead of `sh`. --- contrib/test.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/contrib/test.sh b/contrib/test.sh index 13c216c..0e403a3 100755 --- a/contrib/test.sh +++ b/contrib/test.sh @@ -1,4 +1,4 @@ -#!/bin/sh +#!/usr/bin/env bash set -ex From 92778efe926b673f04e2935af50314db99cf8244 Mon Sep 17 00:00:00 2001 From: "Tobin C. Harding" Date: Thu, 10 Aug 2023 09:16:23 +1000 Subject: [PATCH 7/7] CI: Pin cc in ASAN no_std_test crate Looks like a recent version of `cc` breaks our ASAN job. Pin to the previous version. --- contrib/test.sh | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/contrib/test.sh b/contrib/test.sh index 0e403a3..379059e 100755 --- a/contrib/test.sh +++ b/contrib/test.sh @@ -2,6 +2,7 @@ set -ex +REPO_DIR=$(git rev-parse --show-toplevel) FEATURES="bitcoin-hashes global-context lowmemory rand recovery serde std alloc bitcoin-hashes-std rand-std" cargo --version @@ -108,6 +109,12 @@ if [ "$DO_ASAN" = true ]; then CC='clang -fsanitize=memory -fno-omit-frame-pointer' \ RUSTFLAGS='-Zsanitizer=memory -Zsanitizer-memory-track-origins -Cforce-frame-pointers=yes -Cllvm-args=-msan-eager-checks=0' \ cargo test --lib --all --features="$FEATURES" -Zbuild-std --target x86_64-unknown-linux-gnu + + pushd "$REPO_DIR/no_std_test" > /dev/null || exit 1 + # See https://github.com/rust-bitcoin/rust-secp256k1/pull/641#issuecomment-1671598914 + cargo update -p cc --precise 1.0.79 + popd > /dev/null || exit 1 + cargo run --release --manifest-path=./no_std_test/Cargo.toml | grep -q "Verified Successfully" cargo run --release --features=alloc --manifest-path=./no_std_test/Cargo.toml | grep -q "Verified alloc Successfully" fi