Merge #200: A few cleanup commits

b2252b7 ci: upgrade upload-artifact and download-artifact to v4 (Andrew Poelstra)
207c6dd field: add Sum trait (Andrew Poelstra)
53664cf fuzz: remove `afl` feature, clean up fuzztests a bit (Andrew Poelstra)
665f864 clippy: shorten a doccomment's first line (Andrew Poelstra)

Pull request description:

  This extracts 3 (hopefully) simple and non-controversial commits from my error-correction branch.

  The first is just a clippy fix.

  The second cleans up and simplifies the fuzztests (it does *not* bring over the new fuzztesting scripts from rust-bitcoin, use the bitcoin-maintainer-tools repo, introduce libfuzzer, etc.; it just does some basic cleanups to make things more useful).

  And the third adds a few new bounds to the `Field` trait, specifically `Neg<Output = Self>` and `Sum`. (This is a breaking change.)

ACKs for top commit:
  tcharding:
    ACK b2252b7
  clarkmoody:
    ACK b2252b7

Tree-SHA512: cad04dcf67f90acd5dcac1cf4a1ce0cd23e6171270e95c425b591b7b41bd1b6611b7c2147082ff68f0dd0a5ad2c483f2ec1a92bcc4aad3eb31c1289e02f4cc98

Author: apoelstra

.github/workflows/fuzz.yml

@@ -28,7 +28,7 @@ jobs:
       - name: fuzz
         run: cd fuzz && ./fuzz.sh "${{ matrix.fuzz_target }}"
       - run: echo "${{ matrix.fuzz_target }}.rs" >executed_${{ matrix.fuzz_target }}
-      - uses: actions/upload-artifact@v2
+      - uses: actions/upload-artifact@v4
         with:
           name: executed_${{ matrix.fuzz_target }}
           path: executed_${{ matrix.fuzz_target }}
@@ -39,7 +39,7 @@ jobs:
     runs-on: ubuntu-latest
     steps:
       - uses: actions/checkout@v2
-      - uses: actions/download-artifact@v2
+      - uses: actions/download-artifact@v4
       - name: Display structure of downloaded files
         run: ls -R
       - run: find executed_* -type f -exec cat {} + | sort > executed

fuzz/Cargo.toml

@@ -1,20 +1,16 @@
 [package]
 name = "bech32-fuzz"
+edition = "2021"
+rust-version = "1.56.1"
 version = "0.0.1"
 authors = ["Automatically generated"]
 publish = false
 
 [package.metadata]
 cargo-fuzz = true
 
-[features]
-afl_fuzz = ["afl"]
-honggfuzz_fuzz = ["honggfuzz"]
-
 [dependencies]
-honggfuzz = { version = "0.5", default-features = false, optional = true }
-afl = { version = "0.3", optional = true }
-libc = "0.2"
+honggfuzz = { version = "0.5.55", default-features = false }
 bech32 = { path = ".." }
 
 # Prevent this from interfering with workspaces

fuzz/fuzz.sh

@@ -2,17 +2,17 @@
 set -e
 cargo install --force honggfuzz --no-default-features
 for TARGET in fuzz_targets/*; do
-    FILENAME=$(basename $TARGET)
+    FILENAME=$(basename "$TARGET")
 	FILE="${FILENAME%.*}"
-	if [ -d hfuzz_input/$FILE ]; then
+	if [ -d "hfuzz_input/$FILE" ]; then
 	    HFUZZ_INPUT_ARGS="-f hfuzz_input/$FILE/input"
 	fi
-	HFUZZ_BUILD_ARGS="--features honggfuzz_fuzz" HFUZZ_RUN_ARGS="-N1000000 --exit_upon_crash -v $HFUZZ_INPUT_ARGS" cargo hfuzz run $FILE
+	HFUZZ_RUN_ARGS="--run_time 10 --exit_upon_crash -v $HFUZZ_INPUT_ARGS" cargo hfuzz run "$FILE"
 
-	if [ -f hfuzz_workspace/$FILE/HONGGFUZZ.REPORT.TXT ]; then
-		cat hfuzz_workspace/$FILE/HONGGFUZZ.REPORT.TXT
-		for CASE in hfuzz_workspace/$FILE/SIG*; do
-			cat $CASE | xxd -p
+	if [ -f "hfuzz_workspace/$FILE/HONGGFUZZ.REPORT.TXT" ]; then
+		cat "hfuzz_workspace/$FILE/HONGGFUZZ.REPORT.TXT"
+		for CASE in "hfuzz_workspace/$FILE/SIG"*; do
+			xxd -p < "$CASE"
 		done
 		exit 1
 	fi

fuzz/fuzz_targets/decode_rnd.rs

@@ -1,7 +1,6 @@
-extern crate bech32;
-
 use bech32::primitives::decode::{CheckedHrpstring, SegwitHrpstring, UncheckedHrpstring};
 use bech32::Bech32m;
+use honggfuzz::fuzz;
 
 // Checks that we do not crash if passed random data while decoding.
 fn do_test(data: &[u8]) {
@@ -11,19 +10,6 @@ fn do_test(data: &[u8]) {
     let _ = SegwitHrpstring::new(&data_str);
 }
 
-#[cfg(feature = "afl")]
-extern crate afl;
-#[cfg(feature = "afl")]
-fn main() {
-    afl::read_stdio_bytes(|data| {
-        do_test(&data);
-    });
-}
-
-#[cfg(feature = "honggfuzz")]
-#[macro_use]
-extern crate honggfuzz;
-#[cfg(feature = "honggfuzz")]
 fn main() {
     loop {
         fuzz!(|data| {
@@ -39,9 +25,9 @@ mod tests {
         for (idx, c) in hex.as_bytes().iter().enumerate() {
             b <<= 4;
             match *c {
-                b'A'...b'F' => b |= c - b'A' + 10,
-                b'a'...b'f' => b |= c - b'a' + 10,
-                b'0'...b'9' => b |= c - b'0',
+                b'A'..=b'F' => b |= c - b'A' + 10,
+                b'a'..=b'f' => b |= c - b'a' + 10,
+                b'0'..=b'9' => b |= c - b'0',
                 _ => panic!("Bad hex"),
             }
             if (idx & 1) == 1 {

fuzz/fuzz_targets/encode_decode.rs

@@ -1,11 +1,10 @@
-extern crate bech32;
-
 use std::str;
 
 use bech32::{Bech32m, Hrp};
+use honggfuzz::fuzz;
 
 fn do_test(data: &[u8]) {
-    if data.len() < 1 {
+    if data.is_empty() {
         return;
     }
 
@@ -17,35 +16,23 @@ fn do_test(data: &[u8]) {
 
     let dp = &data[hrp_end..];
 
-    match str::from_utf8(&data[1..hrp_end]) {
+    let s = match str::from_utf8(&data[1..hrp_end]) {
+        Ok(s) => s,
         Err(_) => return,
-        Ok(s) => {
-            match Hrp::parse(&s) {
-                Err(_) => return,
-                Ok(hrp) => {
-                    if let Ok(address) = bech32::encode::<Bech32m>(hrp, dp) {
-                        let (hrp, data) = bech32::decode(&address).expect("should be able to decode own encoding");
-                        assert_eq!(bech32::encode::<Bech32m>(hrp, &data).unwrap(), address);
-                    }
-                }
-            }
-        }
-    }
-}
+    };
+    let hrp = match Hrp::parse(s) {
+        Ok(hrp) => hrp,
+        Err(_) => return,
+    };
+    let address = match bech32::encode::<Bech32m>(hrp, dp) {
+        Ok(addr) => addr,
+        Err(_) => return,
+    };
 
-#[cfg(feature = "afl")]
-extern crate afl;
-#[cfg(feature = "afl")]
-fn main() {
-    afl::read_stdio_bytes(|data| {
-        do_test(&data);
-    });
+    let (hrp, data) = bech32::decode(&address).expect("should be able to decode own encoding");
+    assert_eq!(bech32::encode::<Bech32m>(hrp, &data).unwrap(), address);
 }
 
-#[cfg(feature = "honggfuzz")]
-#[macro_use]
-extern crate honggfuzz;
-#[cfg(feature = "honggfuzz")]
 fn main() {
     loop {
         fuzz!(|data| {
@@ -61,9 +48,9 @@ mod tests {
         for (idx, c) in hex.as_bytes().iter().filter(|&&c| c != b'\n').enumerate() {
             b <<= 4;
             match *c {
-                b'A'...b'F' => b |= c - b'A' + 10,
-                b'a'...b'f' => b |= c - b'a' + 10,
-                b'0'...b'9' => b |= c - b'0',
+                b'A'..=b'F' => b |= c - b'A' + 10,
+                b'a'..=b'f' => b |= c - b'a' + 10,
+                b'0'..=b'9' => b |= c - b'0',
                 _ => panic!("Bad hex"),
             }
             if (idx & 1) == 1 {

fuzz/fuzz_targets/parse_hrp.rs

@@ -1,6 +1,5 @@
-extern crate bech32;
-
 use bech32::Hrp;
+use honggfuzz::fuzz;
 
 fn do_test(data: &[u8]) {
     let s = String::from_utf8_lossy(data);
@@ -10,19 +9,6 @@ fn do_test(data: &[u8]) {
     let _ = Hrp::parse(&s);
 }
 
-#[cfg(feature = "afl")]
-extern crate afl;
-#[cfg(feature = "afl")]
-fn main() {
-    afl::read_stdio_bytes(|data| {
-        do_test(&data);
-    });
-}
-
-#[cfg(feature = "honggfuzz")]
-#[macro_use]
-extern crate honggfuzz;
-#[cfg(feature = "honggfuzz")]
 fn main() {
     loop {
         fuzz!(|data| {
@@ -38,9 +24,9 @@ mod tests {
         for (idx, c) in hex.as_bytes().iter().filter(|&&c| c != b'\n').enumerate() {
             b <<= 4;
             match *c {
-                b'A'...b'F' => b |= c - b'A' + 10,
-                b'a'...b'f' => b |= c - b'a' + 10,
-                b'0'...b'9' => b |= c - b'0',
+                b'A'..=b'F' => b |= c - b'A' + 10,
+                b'a'..=b'f' => b |= c - b'a' + 10,
+                b'0'..=b'9' => b |= c - b'0',
                 _ => panic!("Bad hex"),
             }
             if (idx & 1) == 1 {

src/primitives/encode.rs

@@ -169,9 +169,11 @@ where
     }
 }
 
-/// Iterator adaptor which takes a stream of field elements, converts it to characters prefixed by
-/// an HRP (and separator), and suffixed by the checksum i.e., converts the data in a stream of
-/// field elements into stream of characters representing the encoded bech32 string.
+/// Iterator adaptor which converts a stream of field elements to an iterator over the
+/// characters of an HRP-string.
+///
+/// Does so by converting the field elements to characters, prefixing an HRP, and suffixing
+/// a checksum.
 pub struct CharIter<'hrp, I, Ck>
 where
     I: Iterator<Item = Fe32>,

src/primitives/field.rs

@@ -2,7 +2,7 @@
 
 //! Generic Field Traits
 
-use core::{fmt, hash, ops};
+use core::{fmt, hash, iter, ops};
 
 /// A generic field.
 pub trait Field:
@@ -13,6 +13,8 @@ pub trait Field:
     + hash::Hash
     + fmt::Debug
     + fmt::Display
+    + iter::Sum
+    + for<'a> iter::Sum<&'a Self>
     + ops::Add<Self, Output = Self>
     + ops::Sub<Self, Output = Self>
     + ops::AddAssign
@@ -29,7 +31,7 @@ pub trait Field:
     + for<'a> ops::MulAssign<&'a Self>
     + for<'a> ops::Div<&'a Self, Output = Self>
     + for<'a> ops::DivAssign<&'a Self>
-    + ops::Neg
+    + ops::Neg<Output = Self>
 {
     /// The zero constant of the field.
     const ZERO: Self;
@@ -362,6 +364,19 @@ macro_rules! impl_ops_for_fe {
                 self._neg()
             }
         }
+
+        // sum
+        impl core::iter::Sum for $op {
+            fn sum<I: Iterator<Item = Self>>(iter: I) -> Self {
+                iter.fold(crate::primitives::Field::ZERO, |i, acc| i + acc)
+            }
+        }
+
+        impl<'s> core::iter::Sum<&'s Self> for $op {
+            fn sum<I: Iterator<Item = &'s Self>>(iter: I) -> Self {
+                iter.fold(crate::primitives::Field::ZERO, |i, acc| i + acc)
+            }
+        }
     };
 }
 pub(super) use impl_ops_for_fe;

src/primitives/polynomial.rs

@@ -90,8 +90,7 @@ impl<F: Field> Polynomial<F> {
         let mut cand = F::ONE;
         let mut eval = self.clone();
         for _ in 0..F::MULTIPLICATIVE_ORDER {
-            let sum = eval.inner.iter().cloned().fold(F::ZERO, F::add);
-            if sum == F::ZERO {
+            if eval.inner.iter().sum::<F>() == F::ZERO {
                 ret.push(cand.clone());
             }