separate functions for different signature types (#4354)

cost-model/src/transaction_cost.rs

@@ -176,8 +176,8 @@ pub struct WritableKeysTransaction(pub Vec<Pubkey>);
 
 #[cfg(feature = "dev-context-only-utils")]
 impl solana_svm_transaction::svm_message::SVMMessage for WritableKeysTransaction {
-    fn num_total_signatures(&self) -> u64 {
-        unimplemented!("WritableKeysTransaction::num_total_signatures")
+    fn num_transaction_signatures(&self) -> u64 {
+        unimplemented!("WritableKeysTransaction::num_transaction_signatures")
     }
 
     fn num_write_locks(&self) -> u64 {

fee/src/lib.rs

@@ -28,13 +28,91 @@ pub fn calculate_fee_details(
     if zero_fees_for_test {
         return FeeDetails::default();
     }
-    let signature_fee = message
-        .num_total_signatures()
-        .saturating_mul(lamports_per_signature);
 
     FeeDetails::new(
-        signature_fee,
+        calculate_signature_fee(SignatureCounts::from(message), lamports_per_signature),
         prioritization_fee,
         remove_rounding_in_fee_calculation,
     )
 }
+
+/// Calculate fees from signatures.
+fn calculate_signature_fee(
+    SignatureCounts {
+        num_transaction_signatures,
+        num_ed25519_signatures,
+        num_secp256k1_signatures,
+    }: SignatureCounts,
+    lamports_per_signature: u64,
+) -> u64 {
+    let signature_count = num_transaction_signatures
+        .saturating_add(num_ed25519_signatures)
+        .saturating_add(num_secp256k1_signatures);
+
+    signature_count.saturating_mul(lamports_per_signature)
+}
+
+struct SignatureCounts {
+    pub num_transaction_signatures: u64,
+    pub num_ed25519_signatures: u64,
+    pub num_secp256k1_signatures: u64,
+}
+
+impl<Tx: SVMMessage> From<&Tx> for SignatureCounts {
+    fn from(message: &Tx) -> Self {
+        Self {
+            num_transaction_signatures: message.num_transaction_signatures(),
+            num_ed25519_signatures: message.num_ed25519_signatures(),
+            num_secp256k1_signatures: message.num_secp256k1_signatures(),
+        }
+    }
+}
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+
+    #[test]
+    fn test_calculate_signature_fee() {
+        const LAMPORTS_PER_SIGNATURE: u64 = 5_000;
+
+        // Impossible case - 0 signatures.
+        assert_eq!(
+            calculate_signature_fee(
+                SignatureCounts {
+                    num_transaction_signatures: 0,
+                    num_ed25519_signatures: 0,
+                    num_secp256k1_signatures: 0,
+                },
+                LAMPORTS_PER_SIGNATURE,
+            ),
+            0
+        );
+
+        // Simple signature
+        assert_eq!(
+            calculate_signature_fee(
+                SignatureCounts {
+                    num_transaction_signatures: 1,
+                    num_ed25519_signatures: 0,
+                    num_secp256k1_signatures: 0,
+                },
+                LAMPORTS_PER_SIGNATURE,
+            ),
+            LAMPORTS_PER_SIGNATURE
+        );
+
+        // Pre-compile signatures.
+        assert_eq!(
+            calculate_signature_fee(
+                SignatureCounts {
+                    num_transaction_signatures: 1,
+                    num_ed25519_signatures: 2,
+                    num_secp256k1_signatures: 3,
+                },
+                LAMPORTS_PER_SIGNATURE,
+            ),
+            6 * LAMPORTS_PER_SIGNATURE
+        );
+    }
+}

runtime-transaction/src/runtime_transaction.rs

@@ -61,9 +61,26 @@ impl<T> Deref for RuntimeTransaction<T> {
 }
 
 impl<T: SVMMessage> SVMMessage for RuntimeTransaction<T> {
+    fn num_transaction_signatures(&self) -> u64 {
+        self.transaction.num_transaction_signatures()
+    }
+    // override to access from the cached meta instead of re-calculating
+    fn num_ed25519_signatures(&self) -> u64 {
+        self.meta
+            .signature_details
+            .num_ed25519_instruction_signatures()
+    }
     // override to access from the cached meta instead of re-calculating
-    fn num_total_signatures(&self) -> u64 {
-        self.meta.signature_details.total_signatures()
+    fn num_secp256k1_signatures(&self) -> u64 {
+        self.meta
+            .signature_details
+            .num_secp256k1_instruction_signatures()
+    }
+    // override to access form the cached meta instead of re-calculating
+    fn num_secp256r1_signatures(&self) -> u64 {
+        self.meta
+            .signature_details
+            .num_secp256r1_instruction_signatures()
     }
 
     fn num_write_locks(&self) -> u64 {

svm-transaction/src/svm_message.rs

@@ -6,7 +6,7 @@ use {
     solana_hash::Hash,
     solana_message::AccountKeys,
     solana_pubkey::Pubkey,
-    solana_sdk_ids::system_program,
+    solana_sdk_ids::{ed25519_program, secp256k1_program, secp256r1_program, system_program},
 };
 
 mod sanitized_message;
@@ -21,10 +21,20 @@ const NONCED_TX_MARKER_IX_INDEX: u8 = 0;
 
 // - Debug to support legacy logging
 pub trait SVMMessage: Debug {
-    /// Returns the total number of signatures in the message.
-    /// This includes required transaction signatures as well as any
-    /// pre-compile signatures that are attached in instructions.
-    fn num_total_signatures(&self) -> u64;
+    /// Return the number of transaction-level signatures in the message.
+    fn num_transaction_signatures(&self) -> u64;
+    /// Return the number of ed25519 precompile signatures in the message.
+    fn num_ed25519_signatures(&self) -> u64 {
+        default_precompile_signature_count(&ed25519_program::ID, self.program_instructions_iter())
+    }
+    /// Return the number of secp256k1 precompile signatures in the message.
+    fn num_secp256k1_signatures(&self) -> u64 {
+        default_precompile_signature_count(&secp256k1_program::ID, self.program_instructions_iter())
+    }
+    /// Return the number of secp256r1 precompile signatures in the message.
+    fn num_secp256r1_signatures(&self) -> u64 {
+        default_precompile_signature_count(&secp256r1_program::ID, self.program_instructions_iter())
+    }
 
     /// Returns the number of requested write-locks in this message.
     /// This does not consider if write-locks are demoted.
@@ -125,3 +135,13 @@ pub trait SVMMessage: Debug {
     /// Get message address table lookups used in the message
     fn message_address_table_lookups(&self) -> impl Iterator<Item = SVMMessageAddressTableLookup>;
 }
+
+fn default_precompile_signature_count<'a>(
+    precompile: &Pubkey,
+    instructions: impl Iterator<Item = (&'a Pubkey, SVMInstruction<'a>)>,
+) -> u64 {
+    instructions
+        .filter(|(program_id, _)| *program_id == precompile)
+        .map(|(_, ix)| u64::from(ix.data.first().copied().unwrap_or(0)))
+        .sum()
+}

svm-transaction/src/svm_message/sanitized_message.rs

@@ -10,8 +10,8 @@ use {
 
 // Implement for the "reference" `SanitizedMessage` type.
 impl SVMMessage for SanitizedMessage {
-    fn num_total_signatures(&self) -> u64 {
-        SanitizedMessage::num_total_signatures(self)
+    fn num_transaction_signatures(&self) -> u64 {
+        u64::from(self.header().num_required_signatures)
     }
 
     fn num_write_locks(&self) -> u64 {

svm-transaction/src/svm_message/sanitized_transaction.rs

@@ -10,8 +10,8 @@ use {
 };
 
 impl SVMMessage for SanitizedTransaction {
-    fn num_total_signatures(&self) -> u64 {
-        SVMMessage::num_total_signatures(SanitizedTransaction::message(self))
+    fn num_transaction_signatures(&self) -> u64 {
+        SVMMessage::num_transaction_signatures(SanitizedTransaction::message(self))
     }
 
     fn num_write_locks(&self) -> u64 {

transaction-view/src/resolved_transaction_view.rs

@@ -10,11 +10,9 @@ use {
         ops::Deref,
     },
     solana_hash::Hash,
-    solana_message::{v0::LoadedAddresses, AccountKeys, TransactionSignatureDetails},
+    solana_message::{v0::LoadedAddresses, AccountKeys},
     solana_pubkey::Pubkey,
-    solana_sdk_ids::{
-        bpf_loader_upgradeable, ed25519_program, secp256k1_program, secp256r1_program,
-    },
+    solana_sdk_ids::bpf_loader_upgradeable,
     solana_signature::Signature,
     solana_svm_transaction::{
         instruction::SVMInstruction, message_address_table_lookup::SVMMessageAddressTableLookup,
@@ -159,43 +157,11 @@ impl<D: TransactionData> ResolvedTransactionView<D> {
     pub fn loaded_addresses(&self) -> Option<&LoadedAddresses> {
         self.resolved_addresses.as_ref()
     }
-
-    fn signature_details(&self) -> TransactionSignatureDetails {
-        // counting the number of pre-processor operations separately
-        let mut num_secp256k1_instruction_signatures: u64 = 0;
-        let mut num_ed25519_instruction_signatures: u64 = 0;
-        let mut num_secp256r1_instruction_signatures: u64 = 0;
-        for (program_id, instruction) in self.program_instructions_iter() {
-            if secp256k1_program::check_id(program_id) {
-                if let Some(num_verifies) = instruction.data.first() {
-                    num_secp256k1_instruction_signatures =
-                        num_secp256k1_instruction_signatures.wrapping_add(u64::from(*num_verifies));
-                }
-            } else if ed25519_program::check_id(program_id) {
-                if let Some(num_verifies) = instruction.data.first() {
-                    num_ed25519_instruction_signatures =
-                        num_ed25519_instruction_signatures.wrapping_add(u64::from(*num_verifies));
-                }
-            } else if secp256r1_program::check_id(program_id) {
-                if let Some(num_verifies) = instruction.data.first() {
-                    num_secp256r1_instruction_signatures =
-                        num_secp256r1_instruction_signatures.wrapping_add(u64::from(*num_verifies));
-                }
-            }
-        }
-
-        TransactionSignatureDetails::new(
-            u64::from(self.view.num_required_signatures()),
-            num_secp256k1_instruction_signatures,
-            num_ed25519_instruction_signatures,
-            num_secp256r1_instruction_signatures,
-        )
-    }
 }
 
 impl<D: TransactionData> SVMMessage for ResolvedTransactionView<D> {
-    fn num_total_signatures(&self) -> u64 {
-        self.signature_details().total_signatures()
+    fn num_transaction_signatures(&self) -> u64 {
+        u64::from(self.view.num_required_signatures())
     }
 
     fn num_write_locks(&self) -> u64 {