From 8a69f776106aee4cce5fc8252044e7a2212a5c60 Mon Sep 17 00:00:00 2001 From: Andrew Fitzgerald Date: Fri, 3 Jan 2025 16:54:04 -0600 Subject: [PATCH 1/4] separate functions for different signature types --- cost-model/src/transaction_cost.rs | 4 +- fee/src/lib.rs | 8 ++-- .../src/runtime_transaction.rs | 21 +++++++++- svm-transaction/src/svm_message.rs | 30 ++++++++++--- .../src/svm_message/sanitized_message.rs | 4 +- .../src/svm_message/sanitized_transaction.rs | 4 +- .../src/resolved_transaction_view.rs | 42 ++----------------- 7 files changed, 59 insertions(+), 54 deletions(-) diff --git a/cost-model/src/transaction_cost.rs b/cost-model/src/transaction_cost.rs index 00d4e1f71783f9..824f447392f594 100644 --- a/cost-model/src/transaction_cost.rs +++ b/cost-model/src/transaction_cost.rs @@ -176,8 +176,8 @@ pub struct WritableKeysTransaction(pub Vec); #[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 { diff --git a/fee/src/lib.rs b/fee/src/lib.rs index 8d250982614212..4a0e57cc6e2f2e 100644 --- a/fee/src/lib.rs +++ b/fee/src/lib.rs @@ -28,9 +28,11 @@ 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); + let signature_fee = (message + .num_transaction_signatures() + .saturating_add(message.num_ed25519_signatures()) + .saturating_add(message.num_secp256k1_signatures())) + .saturating_mul(lamports_per_signature); FeeDetails::new( signature_fee, diff --git a/runtime-transaction/src/runtime_transaction.rs b/runtime-transaction/src/runtime_transaction.rs index ba10ccee07879e..7273a67d670b03 100644 --- a/runtime-transaction/src/runtime_transaction.rs +++ b/runtime-transaction/src/runtime_transaction.rs @@ -61,9 +61,26 @@ impl Deref for RuntimeTransaction { } impl SVMMessage for RuntimeTransaction { + 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 { diff --git a/svm-transaction/src/svm_message.rs b/svm-transaction/src/svm_message.rs index 0e60567729b296..dfd02a131b073d 100644 --- a/svm-transaction/src/svm_message.rs +++ b/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; } + +fn default_precompile_signature_count<'a>( + precompile: &Pubkey, + instructions: impl Iterator)>, +) -> u64 { + instructions + .filter(|(program_id, _)| *program_id == precompile) + .map(|(_, ix)| u64::from(ix.data.first().copied().unwrap_or(0))) + .sum() +} diff --git a/svm-transaction/src/svm_message/sanitized_message.rs b/svm-transaction/src/svm_message/sanitized_message.rs index 75b98c3a9ca8e6..7224c1fe45aa62 100644 --- a/svm-transaction/src/svm_message/sanitized_message.rs +++ b/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 { diff --git a/svm-transaction/src/svm_message/sanitized_transaction.rs b/svm-transaction/src/svm_message/sanitized_transaction.rs index 4bbea93cb0d215..5537412bcc35c7 100644 --- a/svm-transaction/src/svm_message/sanitized_transaction.rs +++ b/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 { diff --git a/transaction-view/src/resolved_transaction_view.rs b/transaction-view/src/resolved_transaction_view.rs index 0eb69616c800cd..019e64d0147596 100644 --- a/transaction-view/src/resolved_transaction_view.rs +++ b/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 ResolvedTransactionView { 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 SVMMessage for ResolvedTransactionView { - 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 { From 7362ecd2cafdb5e1b56bb85267558dbd6c31edbc Mon Sep 17 00:00:00 2001 From: Andrew Fitzgerald Date: Wed, 15 Jan 2025 08:24:14 -0600 Subject: [PATCH 2/4] calculate_fee_details_internal --- fee/src/lib.rs | 43 +++++++++++++++++++++++++++++++++++++++---- 1 file changed, 39 insertions(+), 4 deletions(-) diff --git a/fee/src/lib.rs b/fee/src/lib.rs index 4a0e57cc6e2f2e..97a16a88ae0de1 100644 --- a/fee/src/lib.rs +++ b/fee/src/lib.rs @@ -28,10 +28,29 @@ pub fn calculate_fee_details( if zero_fees_for_test { return FeeDetails::default(); } - let signature_fee = (message - .num_transaction_signatures() - .saturating_add(message.num_ed25519_signatures()) - .saturating_add(message.num_secp256k1_signatures())) + + calculate_fee_details_internal( + SignatureCounts::from(message), + lamports_per_signature, + prioritization_fee, + remove_rounding_in_fee_calculation, + ) +} + +/// This function has the actual fee calculation. +fn calculate_fee_details_internal( + SignatureCounts { + num_transaction_signatures, + num_ed25519_signatures, + num_secp256k1_signatures, + }: SignatureCounts, + lamports_per_signature: u64, + prioritization_fee: u64, + remove_rounding_in_fee_calculation: bool, +) -> FeeDetails { + let signature_fee = (num_transaction_signatures + .saturating_add(num_ed25519_signatures) + .saturating_add(num_secp256k1_signatures)) .saturating_mul(lamports_per_signature); FeeDetails::new( @@ -40,3 +59,19 @@ pub fn calculate_fee_details( remove_rounding_in_fee_calculation, ) } + +struct SignatureCounts { + pub num_transaction_signatures: u64, + pub num_ed25519_signatures: u64, + pub num_secp256k1_signatures: u64, +} + +impl 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(), + } + } +} From 52cfd859b178c40b23c2b3d4595959ea032e6334 Mon Sep 17 00:00:00 2001 From: Andrew Fitzgerald Date: Wed, 15 Jan 2025 08:34:00 -0600 Subject: [PATCH 3/4] tests --- fee/src/lib.rs | 90 ++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 90 insertions(+) diff --git a/fee/src/lib.rs b/fee/src/lib.rs index 97a16a88ae0de1..9f91bf913a7892 100644 --- a/fee/src/lib.rs +++ b/fee/src/lib.rs @@ -75,3 +75,93 @@ impl From<&Tx> for SignatureCounts { } } } + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn test_calculate_fee_details_internal() { + const LAMPORTS_PER_SIGNATURE: u64 = 5_000; + + // Impossible case - 0 signatures. + { + let signature_counts = SignatureCounts { + num_transaction_signatures: 0, + num_ed25519_signatures: 0, + num_secp256k1_signatures: 0, + }; + let prioritization_fee = 0; + + assert_eq!( + calculate_fee_details_internal( + signature_counts, + LAMPORTS_PER_SIGNATURE, + prioritization_fee, + false + ), + FeeDetails::new(0, 0, false) + ); + } + + // Simple signature + { + let signature_counts = SignatureCounts { + num_transaction_signatures: 1, + num_ed25519_signatures: 0, + num_secp256k1_signatures: 0, + }; + let prioritization_fee = 0; + + assert_eq!( + calculate_fee_details_internal( + signature_counts, + LAMPORTS_PER_SIGNATURE, + prioritization_fee, + false + ), + FeeDetails::new(LAMPORTS_PER_SIGNATURE, 0, false) + ); + } + + // Simple signature and prioritization. + { + let signature_counts = SignatureCounts { + num_transaction_signatures: 1, + num_ed25519_signatures: 0, + num_secp256k1_signatures: 0, + }; + let prioritization_fee = 1_000; + + assert_eq!( + calculate_fee_details_internal( + signature_counts, + LAMPORTS_PER_SIGNATURE, + prioritization_fee, + false + ), + FeeDetails::new(LAMPORTS_PER_SIGNATURE, 1_000, false) + ); + } + + // Pre-compile signatures. + { + let signature_counts = SignatureCounts { + num_transaction_signatures: 1, + num_ed25519_signatures: 2, + num_secp256k1_signatures: 3, + }; + let prioritization_fee = 4_000; + + assert_eq!( + calculate_fee_details_internal( + signature_counts, + LAMPORTS_PER_SIGNATURE, + prioritization_fee, + false + ), + FeeDetails::new(6 * LAMPORTS_PER_SIGNATURE, 4_000, false) + ); + } + } +} From 10e32132381533f8297645a394abd34c883424d0 Mon Sep 17 00:00:00 2001 From: Andrew Fitzgerald Date: Wed, 15 Jan 2025 10:57:31 -0600 Subject: [PATCH 4/4] calculate_signature_fee --- fee/src/lib.rs | 133 ++++++++++++++++--------------------------------- 1 file changed, 42 insertions(+), 91 deletions(-) diff --git a/fee/src/lib.rs b/fee/src/lib.rs index 9f91bf913a7892..7f2f7ec072a492 100644 --- a/fee/src/lib.rs +++ b/fee/src/lib.rs @@ -29,35 +29,27 @@ pub fn calculate_fee_details( return FeeDetails::default(); } - calculate_fee_details_internal( - SignatureCounts::from(message), - lamports_per_signature, + FeeDetails::new( + calculate_signature_fee(SignatureCounts::from(message), lamports_per_signature), prioritization_fee, remove_rounding_in_fee_calculation, ) } -/// This function has the actual fee calculation. -fn calculate_fee_details_internal( +/// Calculate fees from signatures. +fn calculate_signature_fee( SignatureCounts { num_transaction_signatures, num_ed25519_signatures, num_secp256k1_signatures, }: SignatureCounts, lamports_per_signature: u64, - prioritization_fee: u64, - remove_rounding_in_fee_calculation: bool, -) -> FeeDetails { - let signature_fee = (num_transaction_signatures +) -> u64 { + let signature_count = num_transaction_signatures .saturating_add(num_ed25519_signatures) - .saturating_add(num_secp256k1_signatures)) - .saturating_mul(lamports_per_signature); + .saturating_add(num_secp256k1_signatures); - FeeDetails::new( - signature_fee, - prioritization_fee, - remove_rounding_in_fee_calculation, - ) + signature_count.saturating_mul(lamports_per_signature) } struct SignatureCounts { @@ -81,87 +73,46 @@ mod tests { use super::*; #[test] - fn test_calculate_fee_details_internal() { + fn test_calculate_signature_fee() { const LAMPORTS_PER_SIGNATURE: u64 = 5_000; // Impossible case - 0 signatures. - { - let signature_counts = SignatureCounts { - num_transaction_signatures: 0, - num_ed25519_signatures: 0, - num_secp256k1_signatures: 0, - }; - let prioritization_fee = 0; - - assert_eq!( - calculate_fee_details_internal( - signature_counts, - LAMPORTS_PER_SIGNATURE, - prioritization_fee, - false - ), - FeeDetails::new(0, 0, false) - ); - } + assert_eq!( + calculate_signature_fee( + SignatureCounts { + num_transaction_signatures: 0, + num_ed25519_signatures: 0, + num_secp256k1_signatures: 0, + }, + LAMPORTS_PER_SIGNATURE, + ), + 0 + ); // Simple signature - { - let signature_counts = SignatureCounts { - num_transaction_signatures: 1, - num_ed25519_signatures: 0, - num_secp256k1_signatures: 0, - }; - let prioritization_fee = 0; - - assert_eq!( - calculate_fee_details_internal( - signature_counts, - LAMPORTS_PER_SIGNATURE, - prioritization_fee, - false - ), - FeeDetails::new(LAMPORTS_PER_SIGNATURE, 0, false) - ); - } - - // Simple signature and prioritization. - { - let signature_counts = SignatureCounts { - num_transaction_signatures: 1, - num_ed25519_signatures: 0, - num_secp256k1_signatures: 0, - }; - let prioritization_fee = 1_000; - - assert_eq!( - calculate_fee_details_internal( - signature_counts, - LAMPORTS_PER_SIGNATURE, - prioritization_fee, - false - ), - FeeDetails::new(LAMPORTS_PER_SIGNATURE, 1_000, false) - ); - } + 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. - { - let signature_counts = SignatureCounts { - num_transaction_signatures: 1, - num_ed25519_signatures: 2, - num_secp256k1_signatures: 3, - }; - let prioritization_fee = 4_000; - - assert_eq!( - calculate_fee_details_internal( - signature_counts, - LAMPORTS_PER_SIGNATURE, - prioritization_fee, - false - ), - FeeDetails::new(6 * LAMPORTS_PER_SIGNATURE, 4_000, false) - ); - } + 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 + ); } }