FIX: DelegateAddress Arbitary (#1693)

* FIX: DelegateAddress Arbitary instances generated inconsistent length and buffer.

* FIX: Clippy suggestions

* FIX: Wider clippy checks in make

Makefile

@@ -12,7 +12,7 @@ clean:
 
 lint:
 	cargo fmt --all
-	cargo clippy --all -- -D warnings -A clippy::upper_case_acronyms
+	cargo clippy --all --all-targets -- -D warnings -A clippy::upper_case_acronyms
 
 license:
 	./scripts/add_license.sh

shared/Cargo.toml

@@ -24,7 +24,7 @@ anyhow = "1.0.51"
 fvm_ipld_encoding = { version = "0.3", path = "../ipld/encoding" }
 serde = { version = "1", default-features = false }
 serde_tuple = "0.5"
-arbitrary = { version = "1.1", optional = true, features = ["derive"]}
+arbitrary = { version = "1.1", optional = true, features = ["derive"] }
 quickcheck = { version = "1", optional = true }
 bitflags = "1.3.2"
 
@@ -40,6 +40,9 @@ rand = "0.8"
 rand_chacha = "0.3"
 serde_json = "1.0.56"
 multihash = { version = "0.16.3", default-features = false, features = ["multihash-impl", "sha2", "sha3", "ripemd"] }
+quickcheck_macros = "1"
+
+fvm_shared = { path = ".", features = ["arb"] }
 
 [features]
 default = []

shared/src/address/payload.rs

@@ -25,22 +25,30 @@ pub struct DelegatedAddress {
 #[cfg(feature = "arb")]
 impl quickcheck::Arbitrary for DelegatedAddress {
     fn arbitrary(g: &mut quickcheck::Gen) -> Self {
+        let length = usize::arbitrary(g) % (MAX_SUBADDRESS_LEN + 1);
+        let buffer = from_fn(|idx| if idx < length { u8::arbitrary(g) } else { 0u8 });
         Self {
             namespace: ActorID::arbitrary(g),
-            length: usize::arbitrary(g) % (MAX_SUBADDRESS_LEN + 1),
-            buffer: from_fn(|_| u8::arbitrary(g)),
+            length,
+            buffer,
         }
     }
 }
 
 #[cfg(feature = "arb")]
 impl<'a> arbitrary::Arbitrary<'a> for DelegatedAddress {
     fn arbitrary(u: &mut arbitrary::Unstructured<'a>) -> arbitrary::Result<Self> {
-        Ok(DelegatedAddress {
+        let length = u.int_in_range(0usize..=MAX_SUBADDRESS_LEN)?;
+        let mut buffer = [0u8; MAX_SUBADDRESS_LEN];
+        for b in buffer.iter_mut().take(length) {
+            *b = arbitrary::Arbitrary::arbitrary(u)?;
+        }
+        let addr = DelegatedAddress {
             namespace: arbitrary::Arbitrary::arbitrary(u)?,
-            length: u.int_in_range(0usize..=MAX_SUBADDRESS_LEN)?,
-            buffer: arbitrary::Arbitrary::arbitrary(u)?,
-        })
+            length,
+            buffer,
+        };
+        Ok(addr)
     }
 }
 

shared/src/sector/post.rs

@@ -53,7 +53,7 @@ impl quickcheck::Arbitrary for PoStProof {
             ])
             .unwrap();
         PoStProof {
-            post_proof: (*registered_postproof).into(),
+            post_proof: *registered_postproof,
             proof_bytes: Vec::arbitrary(g),
         }
     }

shared/tests/address_test.rs

@@ -10,6 +10,7 @@ use fvm_ipld_encoding::{from_slice, to_vec};
 use fvm_shared::address::{
     Address, Error, Protocol, BLS_PUB_LEN, MAX_SUBADDRESS_LEN, PAYLOAD_HASH_LEN, SECP_PUB_LEN,
 };
+use quickcheck_macros::quickcheck;
 
 #[test]
 fn bytes() {
@@ -608,3 +609,14 @@ fn invalid_strings_tests() {
         assert!(Address::from_str(st).is_err());
     }
 }
+
+#[quickcheck]
+fn prop_address_roundtrip(addr0: Address) -> Result<(), String> {
+    let bz = addr0.to_bytes();
+    let addr1 =
+        Address::from_bytes(&bz).map_err(|e| format!("error deserializing address: {e}"))?;
+    if addr1 != addr0 {
+        return Err("address differs after roundtrip".to_owned());
+    }
+    Ok(())
+}