Small changes of multisig (MystenLabs#9927)

## Description 
Small changes of multisig

## Test Plan 
Unit tests

crates/sui-types/src/base_types.rs

@@ -466,7 +466,7 @@ impl From<&PublicKey> for SuiAddress {
     }
 }
 
-/// A MultiSig address is the first 20 bytes of the hash of
+/// A MultiSig address is the 32 bytes of the hash of
 /// `flag_MultiSig || threshold || flag_1 || pk_1 || weight_1 || ... || flag_n || pk_n || weight_n`
 /// of all participating public keys and its weight.
 impl From<MultiSigPublicKey> for SuiAddress {

crates/sui-types/src/multisig.rs

@@ -45,13 +45,10 @@ impl AsRef<[u8]> for MultiSig {
     fn as_ref(&self) -> &[u8] {
         self.bytes
             .get_or_try_init::<_, eyre::Report>(|| {
-                let mut bytes = Vec::new();
+                let as_bytes = bcs::to_bytes(self).expect("BCS serialization should not fail");
+                let mut bytes = Vec::with_capacity(1 + as_bytes.len());
                 bytes.push(SignatureScheme::MultiSig.flag());
-                bytes.extend_from_slice(
-                    bcs::to_bytes(self)
-                        .expect("BCS serialization should not fail")
-                        .as_slice(),
-                );
+                bytes.extend_from_slice(as_bytes.as_slice());
                 Ok(bytes)
             })
             .expect("OnceCell invariant violated")
@@ -123,7 +120,7 @@ impl AuthenticatorTrait for MultiSig {
         // Verify each signature against its corresponding signature scheme and public key.
         // TODO: further optimization can be done because multiple Ed25519 signatures can be batch verified.
         for (sig, i) in self.sigs.iter().zip(&self.bitmap) {
-            let pk_map =
+            let (pk, weight) =
                 self.multisig_pk
                     .pk_map
                     .get(i as usize)
@@ -132,23 +129,23 @@ impl AuthenticatorTrait for MultiSig {
                     })?;
             let res = match sig {
                 CompressedSignature::Ed25519(s) => {
-                    let pk = Ed25519PublicKey::from_bytes(pk_map.0.as_ref()).map_err(|_| {
+                    let pk = Ed25519PublicKey::from_bytes(pk.as_ref()).map_err(|_| {
                         SuiError::InvalidSignature {
                             error: "Invalid public key".to_string(),
                         }
                     })?;
                     pk.verify(&digest, &s.try_into()?)
                 }
                 CompressedSignature::Secp256k1(s) => {
-                    let pk = Secp256k1PublicKey::from_bytes(pk_map.0.as_ref()).map_err(|_| {
+                    let pk = Secp256k1PublicKey::from_bytes(pk.as_ref()).map_err(|_| {
                         SuiError::InvalidSignature {
                             error: "Invalid public key".to_string(),
                         }
                     })?;
                     pk.verify(&digest, &s.try_into()?)
                 }
                 CompressedSignature::Secp256r1(s) => {
-                    let pk = Secp256r1PublicKey::from_bytes(pk_map.0.as_ref()).map_err(|_| {
+                    let pk = Secp256r1PublicKey::from_bytes(pk.as_ref()).map_err(|_| {
                         SuiError::InvalidSignature {
                             error: "Invalid public key".to_string(),
                         }
@@ -157,10 +154,10 @@ impl AuthenticatorTrait for MultiSig {
                 }
             };
             if res.is_ok() {
-                weight_sum += pk_map.1 as u16;
+                weight_sum += *weight as u16;
             } else {
                 return Err(SuiError::InvalidSignature {
-                    error: format!("Invalid signature for pk={:?}", pk_map.0),
+                    error: format!("Invalid signature for pk={:?}", pk),
                 });
             }
         }
@@ -191,7 +188,7 @@ impl MultiSig {
             });
         }
         let mut bitmap = RoaringBitmap::new();
-        let mut sigs = Vec::new();
+        let mut sigs = Vec::with_capacity(full_sigs.len());
         for s in full_sigs {
             bitmap.insert(multisig_pk.get_index(s.to_public_key()?).ok_or(
                 SuiError::IncorrectSigner {
@@ -237,7 +234,12 @@ impl MultiSigPublicKey {
             || threshold == 0
             || pks.len() != weights.len()
             || pks.len() > MAX_SIGNER_IN_MULTISIG
-            || weights.iter().any(|w| w == &0)
+            || weights.iter().any(|w| *w == 0)
+            || weights
+                .iter()
+                .map(|w| *w as ThresholdUnit)
+                .sum::<ThresholdUnit>()
+                < threshold
         {
             return Err(SuiError::InvalidSignature {
                 error: "Invalid number of public keys".to_string(),
@@ -265,7 +267,13 @@ impl MultiSigPublicKey {
         if self.threshold == 0
             || self.pubkeys().is_empty()
             || self.pubkeys().len() > MAX_SIGNER_IN_MULTISIG
-            || self.pubkeys().iter().any(|pk_weight| pk_weight.1 == 0)
+            || self.pubkeys().iter().any(|(_pk, weight)| *weight == 0)
+            || self
+                .pubkeys()
+                .iter()
+                .map(|(_pk, weight)| *weight as ThresholdUnit)
+                .sum::<ThresholdUnit>()
+                < self.threshold
         {
             return Err(FastCryptoError::InvalidInput);
         }

crates/sui-types/src/signature.rs

@@ -54,6 +54,7 @@ impl ToFromBytes for GenericSignature {
                     Signature::from_bytes(bytes).map_err(|_| FastCryptoError::InvalidSignature)?,
                 )),
                 SignatureScheme::MultiSig => {
+                    // The flag is added to the bytes representation of MultiSig in MultiSig::as_ref().
                     let multisig: MultiSig =
                         bcs::from_bytes(bytes.get(1..).ok_or(FastCryptoError::InvalidInput)?)
                             .map_err(|_| FastCryptoError::InvalidSignature)?;

crates/sui-types/src/unit_tests/multisig_tests.rs

@@ -191,7 +191,7 @@ fn test_serde_roundtrip() {
         assert_eq!(generic_sig_bytes.first().unwrap(), &0x03);
     }
 
-    // Malformed multisig cannot be serialized
+    // Malformed multisig cannot be deserialized
     let multisig_pk = MultiSigPublicKey {
         pk_map: vec![(keys()[0].public(), 1)],
         threshold: 1,
@@ -207,7 +207,7 @@ fn test_serde_roundtrip() {
     let generic_sig_bytes = generic_sig.as_bytes();
     assert!(GenericSignature::from_bytes(generic_sig_bytes).is_err());
 
-    // Malformed multisig_pk cannot be serialized
+    // Malformed multisig_pk cannot be deserialized
     let multisig_pk_1 = MultiSigPublicKey {
         pk_map: vec![],
         threshold: 0,
@@ -330,11 +330,19 @@ fn test_max_sig() {
     )
     .is_err());
 
-    // multisig_pk with max weights for each pk and max threshold is ok.
+    // multisig_pk with unreachable threshold fails.
+    assert!(MultiSigPublicKey::new(
+        vec![keys[0].public(); 5],
+        vec![3; MAX_SIGNER_IN_MULTISIG],
+        16
+    )
+    .is_err());
+
+    // multisig_pk with max weights for each pk and max reachable threshold is ok.
     let high_threshold_pk = MultiSigPublicKey::new(
         vec![keys[0].public(); MAX_SIGNER_IN_MULTISIG],
-        vec![WeightUnit::MAX; 10],
-        ThresholdUnit::MAX,
+        vec![WeightUnit::MAX; MAX_SIGNER_IN_MULTISIG],
+        (WeightUnit::MAX as ThresholdUnit) * (MAX_SIGNER_IN_MULTISIG as ThresholdUnit),
     )
     .unwrap();
     let address: SuiAddress = high_threshold_pk.clone().into();