Make it unappealing to build and sign transactions at the same time

Use a client to sign transactions. It'll need that keypair anyway
to resign new blockhashes on retries.

programs/budget_api/src/budget_transaction.rs

@@ -28,7 +28,7 @@ impl BudgetTransaction {
     ) -> Transaction {
         let from = from_keypair.pubkey();
         let space = serialized_size(&BudgetState::new(expr.clone())).unwrap();
-        TransactionBuilder::new(fee)
+        let mut tx = TransactionBuilder::new(fee)
             .push(SystemInstruction::new_program_account(
                 &from,
                 contract,
@@ -37,7 +37,10 @@ impl BudgetTransaction {
                 &id(),
             ))
             .push(BudgetInstruction::new_initialize_account(contract, expr))
-            .sign(&[from_keypair], recent_blockhash)
+            .compile();
+
+        tx.sign(&[from_keypair], recent_blockhash);
+        tx
     }
 
     /// Create and sign a new Transaction. Used for unit-testing.
@@ -69,11 +72,13 @@ impl BudgetTransaction {
         recent_blockhash: Hash,
     ) -> Transaction {
         let from = from_keypair.pubkey();
-        TransactionBuilder::default()
+        let mut tx = TransactionBuilder::default()
             .push(BudgetInstruction::new_apply_timestamp(
                 &from, contract, to, dt,
             ))
-            .sign(&[from_keypair], recent_blockhash)
+            .compile();
+        tx.sign(&[from_keypair], recent_blockhash);
+        tx
     }
 
     /// Create and sign a new Witness Signature. Used for unit-testing.
@@ -84,9 +89,11 @@ impl BudgetTransaction {
         recent_blockhash: Hash,
     ) -> Transaction {
         let from = from_keypair.pubkey();
-        TransactionBuilder::default()
+        let mut tx = TransactionBuilder::default()
             .push(BudgetInstruction::new_apply_signature(&from, contract, to))
-            .sign(&[from_keypair], recent_blockhash)
+            .compile();
+        tx.sign(&[from_keypair], recent_blockhash);
+        tx
     }
 
     /// Create and sign a postdated Transaction. Used for unit-testing.

programs/rewards_api/src/rewards_transaction.rs

@@ -40,11 +40,13 @@ impl RewardsTransaction {
         fee: u64,
     ) -> Transaction {
         let vote_id = vote_keypair.pubkey();
-        TransactionBuilder::new(fee)
+        let mut tx = TransactionBuilder::new(fee)
             .push(RewardsInstruction::new_redeem_vote_credits(
                 &vote_id, rewards_id,
             ))
             .push(VoteInstruction::new_clear_credits(&vote_id))
-            .sign(&[vote_keypair], blockhash)
+            .compile();
+        tx.sign(&[vote_keypair], blockhash);
+        tx
     }
 }

programs/vote_api/src/vote_transaction.rs

@@ -22,9 +22,11 @@ impl VoteTransaction {
         fee: u64,
     ) -> Transaction {
         let vote = Vote { slot };
-        TransactionBuilder::new(fee)
+        let mut tx = TransactionBuilder::new(fee)
             .push(VoteInstruction::new_vote(staking_account, vote))
-            .sign(&[authorized_voter_keypair], recent_blockhash)
+            .compile();
+        tx.sign(&[authorized_voter_keypair], recent_blockhash);
+        tx
     }
 
     /// Fund or create the staking account with lamports
@@ -37,7 +39,7 @@ impl VoteTransaction {
     ) -> Transaction {
         let from_id = from_keypair.pubkey();
         let space = VoteState::max_size() as u64;
-        TransactionBuilder::new(fee)
+        let mut tx = TransactionBuilder::new(fee)
             .push(SystemInstruction::new_program_account(
                 &from_id,
                 staker_id,
@@ -46,7 +48,9 @@ impl VoteTransaction {
                 &id(),
             ))
             .push(VoteInstruction::new_initialize_account(staker_id))
-            .sign(&[from_keypair], recent_blockhash)
+            .compile();
+        tx.sign(&[from_keypair], recent_blockhash);
+        tx
     }
 
     /// Fund or create the staking account with lamports
@@ -61,7 +65,7 @@ impl VoteTransaction {
         let from_id = from_keypair.pubkey();
         let voter_id = voter_keypair.pubkey();
         let space = VoteState::max_size() as u64;
-        TransactionBuilder::new(fee)
+        let mut tx = TransactionBuilder::new(fee)
             .push(SystemInstruction::new_program_account(
                 &from_id,
                 &voter_id,
@@ -71,7 +75,9 @@ impl VoteTransaction {
             ))
             .push(VoteInstruction::new_initialize_account(&voter_id))
             .push(VoteInstruction::new_delegate_stake(&voter_id, &delegate_id))
-            .sign(&[from_keypair, voter_keypair], recent_blockhash)
+            .compile();
+        tx.sign(&[from_keypair, voter_keypair], recent_blockhash);
+        tx
     }
 
     /// Choose a voter id to accept signed votes from
@@ -81,12 +87,14 @@ impl VoteTransaction {
         authorized_voter_id: &Pubkey,
         fee: u64,
     ) -> Transaction {
-        TransactionBuilder::new(fee)
+        let mut tx = TransactionBuilder::new(fee)
             .push(VoteInstruction::new_authorize_voter(
                 &vote_keypair.pubkey(),
                 authorized_voter_id,
             ))
-            .sign(&[vote_keypair], recent_blockhash)
+            .compile();
+        tx.sign(&[vote_keypair], recent_blockhash);
+        tx
     }
 
     /// Choose a node id to `delegate` or `assign` this vote account to
@@ -96,12 +104,14 @@ impl VoteTransaction {
         node_id: &Pubkey,
         fee: u64,
     ) -> Transaction {
-        TransactionBuilder::new(fee)
+        let mut tx = TransactionBuilder::new(fee)
             .push(VoteInstruction::new_delegate_stake(
                 &vote_keypair.pubkey(),
                 node_id,
             ))
-            .sign(&[vote_keypair], recent_blockhash)
+            .compile();
+        tx.sign(&[vote_keypair], recent_blockhash);
+        tx
     }
 
     fn get_vote(tx: &Transaction, ix_index: usize) -> Option<(Pubkey, Vote, Hash)> {

sdk/src/transaction.rs

@@ -641,4 +641,49 @@ mod tests {
             ]
         );
     }
+
+    #[test]
+    #[should_panic]
+    fn test_transaction_missing_key() {
+        let keypair = Keypair::new();
+        TransactionBuilder::default()
+            .compile()
+            .sign(&[&keypair], Hash::default());
+    }
+
+    #[test]
+    #[should_panic]
+    fn test_transaction_missing_keypair() {
+        let program_id = Pubkey::default();
+        let keypair0 = Keypair::new();
+        let id0 = keypair0.pubkey();
+        TransactionBuilder::default()
+            .push(Instruction::new(program_id, &0, vec![(id0, true)]))
+            .compile()
+            .sign(&Vec::<&Keypair>::new(), Hash::default());
+    }
+
+    #[test]
+    #[should_panic]
+    fn test_transaction_wrong_key() {
+        let program_id = Pubkey::default();
+        let keypair0 = Keypair::new();
+        let wrong_id = Pubkey::default();
+        TransactionBuilder::default()
+            .push(Instruction::new(program_id, &0, vec![(wrong_id, true)]))
+            .compile()
+            .sign(&[&keypair0], Hash::default());
+    }
+
+    #[test]
+    fn test_transaction_correct_key() {
+        let program_id = Pubkey::default();
+        let keypair0 = Keypair::new();
+        let id0 = keypair0.pubkey();
+        let mut tx = TransactionBuilder::default()
+            .push(Instruction::new(program_id, &0, vec![(id0, true)]))
+            .compile();
+        tx.sign(&[&keypair0], Hash::default());
+        assert_eq!(tx.instructions[0], Instruction::new(0, &0, vec![0]));
+    }
 }

sdk/src/transaction_builder.rs

@@ -2,7 +2,6 @@
 
 use crate::hash::Hash;
 use crate::pubkey::Pubkey;
-use crate::signature::KeypairUtil;
 use crate::transaction::{Instruction, Transaction};
 use itertools::Itertools;
 
@@ -104,13 +103,6 @@ impl TransactionBuilder {
             instructions,
         }
     }
-
-    /// Return a signed transaction.
-    pub fn sign<T: KeypairUtil>(&self, keypairs: &[&T], recent_blockhash: Hash) -> Transaction {
-        let mut tx = self.compile();
-        tx.sign(keypairs, recent_blockhash);
-        tx
-    }
 }
 
 #[cfg(test)]
@@ -227,50 +219,9 @@ mod tests {
         assert_eq!(tx.signatures.capacity(), 1);
     }
 
-    #[test]
-    #[should_panic]
-    fn test_transaction_builder_missing_key() {
-        let keypair = Keypair::new();
-        TransactionBuilder::default().sign(&[&keypair], Hash::default());
-    }
-
-    #[test]
-    #[should_panic]
-    fn test_transaction_builder_missing_keypair() {
-        let program_id = Pubkey::default();
-        let keypair0 = Keypair::new();
-        let id0 = keypair0.pubkey();
-        TransactionBuilder::default()
-            .push(Instruction::new(program_id, &0, vec![(id0, true)]))
-            .sign(&Vec::<&Keypair>::new(), Hash::default());
-    }
-
-    #[test]
-    #[should_panic]
-    fn test_transaction_builder_wrong_key() {
-        let program_id = Pubkey::default();
-        let keypair0 = Keypair::new();
-        let wrong_id = Pubkey::default();
-        TransactionBuilder::default()
-            .push(Instruction::new(program_id, &0, vec![(wrong_id, true)]))
-            .sign(&[&keypair0], Hash::default());
-    }
-
-    #[test]
-    fn test_transaction_builder_correct_key() {
-        let program_id = Pubkey::default();
-        let keypair0 = Keypair::new();
-        let id0 = keypair0.pubkey();
-        let tx = TransactionBuilder::default()
-            .push(Instruction::new(program_id, &0, vec![(id0, true)]))
-            .sign(&[&keypair0], Hash::default());
-        assert_eq!(tx.instructions[0], Instruction::new(0, &0, vec![0]));
-    }
-
     #[test]
     fn test_transaction_builder_fee() {
-        let tx = TransactionBuilder::new(42).sign(&Vec::<&Keypair>::new(), Hash::default());
+        assert_eq!(TransactionBuilder::new(42).compile().fee, 42);
-        assert_eq!(tx.fee, 42);
     }
 
     #[test]
@@ -284,7 +235,7 @@ mod tests {
             .push(Instruction::new(program_id0, &0, vec![(id0, false)]))
             .push(Instruction::new(program_id1, &0, vec![(id1, true)]))
             .push(Instruction::new(program_id0, &0, vec![(id1, false)]))
-            .sign(&[&keypair1], Hash::default());
+            .compile();
         assert_eq!(tx.instructions[0], Instruction::new(0, &0, vec![1]));
         assert_eq!(tx.instructions[1], Instruction::new(1, &0, vec![0]));
         assert_eq!(tx.instructions[2], Instruction::new(0, &0, vec![0]));

wallet/src/wallet.rs

@@ -455,7 +455,8 @@ fn process_configure_staking(
             &authorized_voter_id,
         ));
     }
-    let mut tx = tx.sign(&[&config.id], recent_blockhash);
+    let mut tx = tx.compile();
+    tx.sign(&[&config.id], recent_blockhash);
     let signature_str = send_and_confirm_transaction(&rpc_client, &mut tx, &config.id)?;
     Ok(signature_str.to_string())
 }