Skip to content

Commit

Permalink
feat: move precompile verification to SVM (#2441)
Browse files Browse the repository at this point in the history
  • Loading branch information
jstarry authored Aug 27, 2024
1 parent 669d1bc commit 3bf82c9
Show file tree
Hide file tree
Showing 12 changed files with 175 additions and 66 deletions.
2 changes: 2 additions & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

10 changes: 8 additions & 2 deletions banks-server/src/banks_server.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ use {
account::Account,
clock::Slot,
commitment_config::CommitmentLevel,
feature_set::FeatureSet,
feature_set::{self, FeatureSet},
hash::Hash,
message::{Message, SanitizedMessage},
pubkey::Pubkey,
Expand Down Expand Up @@ -163,7 +163,13 @@ fn verify_transaction(
feature_set: &Arc<FeatureSet>,
) -> transaction::Result<()> {
transaction.verify()?;
transaction.verify_precompiles(feature_set)?;

let move_precompile_verification_to_svm =
feature_set.is_active(&feature_set::move_precompile_verification_to_svm::id());
if !move_precompile_verification_to_svm {
transaction.verify_precompiles(feature_set)?;
}

Ok(())
}

Expand Down
21 changes: 18 additions & 3 deletions core/src/banking_stage/consumer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -392,11 +392,20 @@ impl Consumer {
let check_results =
bank.check_transactions(txs, &pre_results, MAX_PROCESSING_AGE, &mut error_counters);
// If checks passed, verify pre-compiles and continue processing on success.
let move_precompile_verification_to_svm = bank
.feature_set
.is_active(&feature_set::move_precompile_verification_to_svm::id());
let check_results: Vec<_> = txs
.iter()
.zip(check_results)
.map(|(tx, result)| match result {
Ok(_) => tx.verify_precompiles(&bank.feature_set),
Ok(_) => {
if !move_precompile_verification_to_svm {
tx.verify_precompiles(&bank.feature_set)
} else {
Ok(())
}
}
Err(err) => Err(err),
})
.collect();
Expand All @@ -421,7 +430,10 @@ impl Consumer {
txs: &[SanitizedTransaction],
max_slot_ages: &[Slot],
) -> ProcessTransactionBatchOutput {
// Verify pre-compiles.
let move_precompile_verification_to_svm = bank
.feature_set
.is_active(&feature_set::move_precompile_verification_to_svm::id());

// Need to filter out transactions since they were sanitized earlier.
// This means that the transaction may cross and epoch boundary (not allowed),
// or account lookup tables may have been closed.
Expand All @@ -439,7 +451,10 @@ impl Consumer {
}
} else {
// Verify pre-compiles.
tx.verify_precompiles(&bank.feature_set)?;
if !move_precompile_verification_to_svm {
tx.verify_precompiles(&bank.feature_set)?;
}

// Any transaction executed between sanitization time and now may have closed the lookup table(s).
// Above re-sanitization already loads addresses, so don't need to re-check in that case.
let lookup_tables = tx.message().message_address_table_lookups();
Expand Down
31 changes: 30 additions & 1 deletion program-runtime/src/invoke_context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,10 +22,11 @@ use {
bpf_loader_deprecated,
clock::Slot,
epoch_schedule::EpochSchedule,
feature_set::FeatureSet,
feature_set::{self, FeatureSet},
hash::Hash,
instruction::{AccountMeta, InstructionError},
native_loader,
precompiles::Precompile,
pubkey::Pubkey,
saturating_add_assign,
stable_layout::stable_instruction::StableInstruction,
Expand Down Expand Up @@ -465,6 +466,34 @@ impl<'a> InvokeContext<'a> {
.and(self.pop())
}

/// Processes a precompile instruction
pub fn process_precompile<'ix_data>(
&mut self,
precompile: &Precompile,
instruction_data: &[u8],
instruction_accounts: &[InstructionAccount],
program_indices: &[IndexOfAccount],
message_instruction_datas_iter: impl Iterator<Item = &'ix_data [u8]>,
) -> Result<(), InstructionError> {
self.transaction_context
.get_next_instruction_context()?
.configure(program_indices, instruction_accounts, instruction_data);
self.push()?;

let feature_set = self.get_feature_set();
let move_precompile_verification_to_svm =
feature_set.is_active(&feature_set::move_precompile_verification_to_svm::id());
if move_precompile_verification_to_svm {
let instruction_datas: Vec<_> = message_instruction_datas_iter.collect();
precompile
.verify(instruction_data, &instruction_datas, feature_set)
.map_err(InstructionError::from)
.and(self.pop())
} else {
self.pop()
}
}

/// Calls the instruction's program entrypoint method
fn process_executable_chain(
&mut self,
Expand Down
38 changes: 37 additions & 1 deletion programs/ed25519-tests/tests/process_transaction.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,9 @@ use {
solana_program_test::*,
solana_sdk::{
ed25519_instruction::new_ed25519_instruction,
feature_set,
instruction::InstructionError,
precompiles::PrecompileError,
signature::Signer,
transaction::{Transaction, TransactionError},
},
Expand Down Expand Up @@ -44,6 +47,37 @@ async fn test_success() {
assert_matches!(client.process_transaction(transaction).await, Ok(()));
}

#[tokio::test]
async fn test_failure_without_move_precompiles_feature() {
let mut program_test = ProgramTest::default();
program_test.deactivate_feature(feature_set::move_precompile_verification_to_svm::id());
let mut context = program_test.start_with_context().await;

let client = &mut context.banks_client;
let payer = &context.payer;
let recent_blockhash = context.last_blockhash;

let privkey = generate_keypair();
let message_arr = b"hello";
let mut instruction = new_ed25519_instruction(&privkey, message_arr);

instruction.data[0] += 1;

let transaction = Transaction::new_signed_with_payer(
&[instruction],
Some(&payer.pubkey()),
&[payer],
recent_blockhash,
);

assert_matches!(
client.process_transaction(transaction).await,
Err(BanksClientError::TransactionError(
TransactionError::InvalidAccountIndex
))
);
}

#[tokio::test]
async fn test_failure() {
let mut context = ProgramTest::default().start_with_context().await;
Expand All @@ -68,7 +102,9 @@ async fn test_failure() {
assert_matches!(
client.process_transaction(transaction).await,
Err(BanksClientError::TransactionError(
TransactionError::InvalidAccountIndex
TransactionError::InstructionError(0, InstructionError::Custom(3))
))
);
// this assert is for documenting the matched error code above
assert_eq!(3, PrecompileError::InvalidDataOffsets as u32);
}
2 changes: 2 additions & 0 deletions programs/sbf/Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

8 changes: 6 additions & 2 deletions rpc/src/rpc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2251,8 +2251,12 @@ fn verify_transaction(
return Err(RpcCustomError::TransactionSignatureVerificationFailure.into());
}

if let Err(e) = transaction.verify_precompiles(feature_set) {
return Err(RpcCustomError::TransactionPrecompileVerificationFailure(e).into());
let move_precompile_verification_to_svm =
feature_set.is_active(&feature_set::move_precompile_verification_to_svm::id());
if !move_precompile_verification_to_svm {
if let Err(e) = transaction.verify_precompiles(feature_set) {
return Err(RpcCustomError::TransactionPrecompileVerificationFailure(e).into());
}
}

Ok(())
Expand Down
10 changes: 7 additions & 3 deletions runtime/src/bank.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5518,9 +5518,13 @@ impl Bank {
)
}?;

if verification_mode == TransactionVerificationMode::HashAndVerifyPrecompiles
|| verification_mode == TransactionVerificationMode::FullVerification
{
let move_precompile_verification_to_svm = self
.feature_set
.is_active(&feature_set::move_precompile_verification_to_svm::id());
if !move_precompile_verification_to_svm && {
verification_mode == TransactionVerificationMode::HashAndVerifyPrecompiles
|| verification_mode == TransactionVerificationMode::FullVerification
} {
sanitized_tx.verify_precompiles(&self.feature_set)?;
}

Expand Down
2 changes: 2 additions & 0 deletions sdk/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,8 @@ lazy_static = { workspace = true }
libsecp256k1 = { workspace = true, optional = true, features = ["hmac"] }
log = { workspace = true }
memmap2 = { workspace = true, optional = true }
num-derive = { workspace = true }
num-traits = { workspace = true }
num_enum = { workspace = true }
pbkdf2 = { workspace = true }
qstring = { workspace = true }
Expand Down
5 changes: 5 additions & 0 deletions sdk/src/feature_set.rs
Original file line number Diff line number Diff line change
Expand Up @@ -841,6 +841,10 @@ pub mod vote_only_retransmitter_signed_fec_sets {
solana_sdk::declare_id!("RfEcA95xnhuwooVAhUUksEJLZBF7xKCLuqrJoqk4Zph");
}

pub mod move_precompile_verification_to_svm {
solana_sdk::declare_id!("9ypxGLzkMxi89eDerRKXWDXe44UY2z4hBig4mDhNq5Dp");
}

pub mod enable_transaction_loading_failure_fees {
solana_sdk::declare_id!("PaymEPK2oqwT9TXAVfadjztH2H6KfLEB9Hhd5Q5frvP");
}
Expand Down Expand Up @@ -1058,6 +1062,7 @@ lazy_static! {
(move_stake_and_move_lamports_ixs::id(), "Enable MoveStake and MoveLamports stake program instructions #1610"),
(ed25519_precompile_verify_strict::id(), "Use strict verification in ed25519 precompile SIMD-0152"),
(vote_only_retransmitter_signed_fec_sets::id(), "vote only on retransmitter signed fec sets"),
(move_precompile_verification_to_svm::id(), "SIMD-0159: Move precompile verification into SVM"),
(enable_transaction_loading_failure_fees::id(), "Enable fees for some additional transaction failures SIMD-0082"),
(enable_turbine_extended_fanout_experiments::id(), "enable turbine extended fanout experiments #"),
(deprecate_legacy_vote_ixs::id(), "Deprecate legacy vote instructions"),
Expand Down
17 changes: 15 additions & 2 deletions sdk/src/precompiles.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,14 +3,16 @@
#![cfg(feature = "full")]

use {
crate::{feature_set::FeatureSet, instruction::CompiledInstruction, pubkey::Pubkey},
crate::{feature_set::FeatureSet, pubkey::Pubkey},
lazy_static::lazy_static,
num_derive::{FromPrimitive, ToPrimitive},
solana_decode_error::DecodeError,
solana_program::instruction::CompiledInstruction,
thiserror::Error,
};

/// Precompile errors
#[derive(Error, Debug, Clone, PartialEq, Eq)]
#[derive(Error, Debug, Clone, PartialEq, Eq, FromPrimitive, ToPrimitive)]
pub enum PrecompileError {
#[error("public key is not valid")]
InvalidPublicKey,
Expand All @@ -23,6 +25,7 @@ pub enum PrecompileError {
#[error("instruction is incorrect size")]
InvalidInstructionDataSize,
}

impl<T> DecodeError<T> for PrecompileError {
fn type_of() -> &'static str {
"PrecompileError"
Expand Down Expand Up @@ -96,6 +99,16 @@ where
.any(|precompile| precompile.check_id(program_id, |feature_id| is_enabled(feature_id)))
}

/// Find an enabled precompiled program
pub fn get_precompile<F>(program_id: &Pubkey, is_enabled: F) -> Option<&Precompile>
where
F: Fn(&Pubkey) -> bool,
{
PRECOMPILES
.iter()
.find(|precompile| precompile.check_id(program_id, |feature_id| is_enabled(feature_id)))
}

pub fn get_precompiles<'a>() -> &'a [Precompile] {
&PRECOMPILES
}
Expand Down
Loading

0 comments on commit 3bf82c9

Please sign in to comment.