deprecates Signature::new in favor of Signature::{try_,}from

Signature::new lacks type-safety and may panic. The commit deprecates this interface and instead implements Signature::{try_,}from.
solana-labs · Jul 13, 2023 · 1729526 · 1729526
1 parent 09ddbd7
commit 1729526
Show file tree

Hide file tree

Showing 19 changed files with 137 additions and 100 deletions.
diff --git a/banking-bench/src/main.rs b/banking-bench/src/main.rs
@@ -134,7 +134,7 @@ fn make_accounts_txs(
                 hash,
                 compute_unit_price,
             );
-            let sig: Vec<u8> = (0..64).map(|_| thread_rng().gen::<u8>()).collect();
+            let sig: [u8; 64] = std::array::from_fn(|_| thread_rng().gen::<u8>());
             new.message.account_keys[0] = pubkey::new_rand();
             new.message.account_keys[1] = match contention {
                 WriteLockContention::None => pubkey::new_rand(),
@@ -148,7 +148,7 @@ fn make_accounts_txs(
                 }
                 WriteLockContention::Full => to_pubkey,
             };
-            new.signatures = vec![Signature::new(&sig[0..64])];
+            new.signatures = vec![Signature::from(sig)];
             new
         })
         .collect()
@@ -220,8 +220,8 @@ impl PacketsPerIteration {
     fn refresh_blockhash(&mut self, new_blockhash: Hash) {
         for tx in self.transactions.iter_mut() {
             tx.message.recent_blockhash = new_blockhash;
-            let sig: Vec<u8> = (0..64).map(|_| thread_rng().gen::<u8>()).collect();
-            tx.signatures[0] = Signature::new(&sig[0..64]);
+            let sig: [u8; 64] = std::array::from_fn(|_| thread_rng().gen::<u8>());
+            tx.signatures[0] = Signature::from(sig);
         }
         self.packet_batches = to_packet_batches(&self.transactions, self.packets_per_batch);
     }
@@ -377,8 +377,8 @@ fn main() {
                     genesis_config.hash(),
                 );
                 // Ignore any pesky duplicate signature errors in the case we are using single-payer
-                let sig: Vec<u8> = (0..64).map(|_| thread_rng().gen::<u8>()).collect();
-                fund.signatures = vec![Signature::new(&sig[0..64])];
+                let sig: [u8; 64] = std::array::from_fn(|_| thread_rng().gen::<u8>());
+                fund.signatures = vec![Signature::from(sig)];
                 bank.process_transaction(&fund).unwrap();
             });
     });

diff --git a/bloom/benches/bloom.rs b/bloom/benches/bloom.rs
@@ -61,7 +61,7 @@ fn bench_sigs_bloom(bencher: &mut Bencher) {
         id = hash(id.as_ref());
         sigbytes.extend(id.as_ref());
 
-        let sig = Signature::new(&sigbytes);
+        let sig = Signature::try_from(sigbytes).unwrap();
         if sigs.contains(&sig) {
             falses += 1;
         }
@@ -89,7 +89,7 @@ fn bench_sigs_hashmap(bencher: &mut Bencher) {
         id = hash(id.as_ref());
         sigbytes.extend(id.as_ref());
 
-        let sig = Signature::new(&sigbytes);
+        let sig = Signature::try_from(sigbytes).unwrap();
         if sigs.contains(&sig) {
             falses += 1;
         }

diff --git a/cli-output/src/cli_output.rs b/cli-output/src/cli_output.rs
@@ -3052,7 +3052,7 @@ mod tests {
             }
 
             fn try_sign_message(&self, _message: &[u8]) -> Result<Signature, SignerError> {
-                Ok(Signature::new(&[1u8; 64]))
+                Ok(Signature::from([1u8; 64]))
             }
 
             fn is_interactive(&self) -> bool {

diff --git a/cli/src/cli.rs b/cli/src/cli.rs
@@ -1898,7 +1898,7 @@ mod tests {
         );
 
         // Test Confirm Subcommand
-        let signature = Signature::new(&[1; 64]);
+        let signature = Signature::from([1; 64]);
         let signature_string = format!("{signature:?}");
         let test_confirm =
             test_commands
@@ -2053,7 +2053,11 @@ mod tests {
         };
         assert_eq!(process_command(&config).unwrap(), "0.00000005 SOL");
 
-        let good_signature = Signature::new(&bs58::decode(SIGNATURE).into_vec().unwrap());
+        let good_signature = bs58::decode(SIGNATURE)
+            .into_vec()
+            .map(Signature::try_from)
+            .unwrap()
+            .unwrap();
         config.command = CliCommand::Confirm(good_signature);
         assert_eq!(
             process_command(&config).unwrap(),
@@ -2283,13 +2287,21 @@ mod tests {
 
         // sig_not_found case
         config.rpc_client = Some(Arc::new(RpcClient::new_mock("sig_not_found".to_string())));
-        let missing_signature = Signature::new(&bs58::decode("5VERv8NMvzbJMEkV8xnrLkEaWRtSz9CosKDYjCJjBRnbJLgp8uirBgmQpjKhoR4tjF3ZpRzrFmBV6UjKdiSZkQUW").into_vec().unwrap());
+        let missing_signature = bs58::decode("5VERv8NMvzbJMEkV8xnrLkEaWRtSz9CosKDYjCJjBRnbJLgp8uirBgmQpjKhoR4tjF3ZpRzrFmBV6UjKdiSZkQUW")
+            .into_vec()
+            .map(Signature::try_from)
+            .unwrap()
+            .unwrap();
         config.command = CliCommand::Confirm(missing_signature);
         assert_eq!(process_command(&config).unwrap(), "Not found");
 
         // Tx error case
         config.rpc_client = Some(Arc::new(RpcClient::new_mock("account_in_use".to_string())));
-        let any_signature = Signature::new(&bs58::decode(SIGNATURE).into_vec().unwrap());
+        let any_signature = bs58::decode(SIGNATURE)
+            .into_vec()
+            .map(Signature::try_from)
+            .unwrap()
+            .unwrap();
         config.command = CliCommand::Confirm(any_signature);
         assert_eq!(
             process_command(&config).unwrap(),

diff --git a/core/benches/banking_stage.rs b/core/benches/banking_stage.rs
@@ -132,10 +132,10 @@ fn make_accounts_txs(txes: usize, mint_keypair: &Keypair, hash: Hash) -> Vec<Tra
         .into_par_iter()
         .map(|_| {
             let mut new = dummy.clone();
-            let sig: Vec<_> = (0..64).map(|_| thread_rng().gen::<u8>()).collect();
+            let sig: [u8; 64] = std::array::from_fn(|_| thread_rng().gen::<u8>());
             new.message.account_keys[0] = pubkey::new_rand();
             new.message.account_keys[1] = pubkey::new_rand();
-            new.signatures = vec![Signature::new(&sig[0..64])];
+            new.signatures = vec![Signature::from(sig)];
             new
         })
         .collect()

diff --git a/core/src/verified_vote_packets.rs b/core/src/verified_vote_packets.rs
@@ -338,7 +338,7 @@ mod tests {
             vote_account_key,
             vote: VoteTransaction::from(vote.clone()),
             packet_batch: PacketBatch::default(),
-            signature: Signature::new(&[1u8; 64]),
+            signature: Signature::from([1u8; 64]),
         }])
         .unwrap();
         verified_vote_packets
@@ -358,7 +358,7 @@ mod tests {
             vote_account_key,
             vote: VoteTransaction::from(vote),
             packet_batch: PacketBatch::default(),
-            signature: Signature::new(&[1u8; 64]),
+            signature: Signature::from([1u8; 64]),
         }])
         .unwrap();
         verified_vote_packets
@@ -380,7 +380,7 @@ mod tests {
             vote_account_key,
             vote: VoteTransaction::from(vote),
             packet_batch: PacketBatch::default(),
-            signature: Signature::new(&[1u8; 64]),
+            signature: Signature::from([1u8; 64]),
         }])
         .unwrap();
         verified_vote_packets
@@ -403,7 +403,7 @@ mod tests {
             vote_account_key,
             vote: VoteTransaction::from(vote),
             packet_batch: PacketBatch::default(),
-            signature: Signature::new(&[2u8; 64]),
+            signature: Signature::from([2u8; 64]),
         }])
         .unwrap();
         verified_vote_packets
@@ -442,7 +442,7 @@ mod tests {
                 vote_account_key,
                 vote: VoteTransaction::from(vote),
                 packet_batch: PacketBatch::default(),
-                signature: Signature::new(&[1u8; 64]),
+                signature: Signature::from([1u8; 64]),
             }])
             .unwrap();
         }
@@ -628,7 +628,7 @@ mod tests {
                 vote_account_key,
                 vote: VoteTransaction::from(vote),
                 packet_batch: PacketBatch::default(),
-                signature: Signature::new(&[1u8; 64]),
+                signature: Signature::from([1u8; 64]),
             }])
             .unwrap();
         }
@@ -651,7 +651,7 @@ mod tests {
             vote_account_key,
             vote: VoteTransaction::from(third_vote.clone()),
             packet_batch: PacketBatch::default(),
-            signature: Signature::new(&[1u8; 64]),
+            signature: Signature::from([1u8; 64]),
         }])
         .unwrap();
 
@@ -671,7 +671,7 @@ mod tests {
                 vote_account_key,
                 vote: VoteTransaction::from(vote),
                 packet_batch: PacketBatch::default(),
-                signature: Signature::new(&[1u8; 64]),
+                signature: Signature::from([1u8; 64]),
             }])
             .unwrap();
         }
@@ -702,7 +702,7 @@ mod tests {
             vote_account_key,
             vote: VoteTransaction::from(vote),
             packet_batch: PacketBatch::default(),
-            signature: Signature::new(&[1u8; 64]),
+            signature: Signature::from([1u8; 64]),
         }])
         .unwrap();
         verified_vote_packets
@@ -803,7 +803,7 @@ mod tests {
                 vote_account_key,
                 vote,
                 packet_batch: PacketBatch::default(),
-                signature: Signature::new(&[1u8; 64]),
+                signature: Signature::from([1u8; 64]),
             }])
             .unwrap();
         }
@@ -838,7 +838,7 @@ mod tests {
                 vote_account_key,
                 vote,
                 packet_batch: PacketBatch::default(),
-                signature: Signature::new(&[1u8; 64]),
+                signature: Signature::from([1u8; 64]),
             }])
             .unwrap();
         }
@@ -866,7 +866,7 @@ mod tests {
             vote_account_key,
             vote,
             packet_batch: PacketBatch::default(),
-            signature: Signature::new(&[1u8; 64]),
+            signature: Signature::from([1u8; 64]),
         }])
         .unwrap();
 
@@ -895,7 +895,7 @@ mod tests {
             vote_account_key,
             vote,
             packet_batch: PacketBatch::default(),
-            signature: Signature::new(&[1u8; 64]),
+            signature: Signature::from([1u8; 64]),
         }])
         .unwrap();
 
@@ -918,7 +918,7 @@ mod tests {
             vote_account_key,
             vote,
             packet_batch: PacketBatch::default(),
-            signature: Signature::new(&[1u8; 64]),
+            signature: Signature::from([1u8; 64]),
         }])
         .unwrap();
 
@@ -939,7 +939,7 @@ mod tests {
             vote_account_key,
             vote,
             packet_batch: PacketBatch::default(),
-            signature: Signature::new(&[1u8; 64]),
+            signature: Signature::from([1u8; 64]),
         }])
         .unwrap();