Fix ed25519 builtin program handling (solana-labs#23182)

* Fix ed25519 builtin program handling

* Fix tests

* Add integration tests for processing transactions with ed25519 ixs

* Fix another test

* fix formatting

Cargo.toml

@@ -48,6 +48,7 @@ members = [
     "program-test",
     "programs/address-lookup-table",
     "programs/address-lookup-table-tests",
+    "programs/ed25519-tests",
     "programs/bpf_loader",
     "programs/bpf_loader/gen-syscall-list",
     "programs/compute-budget",

program-test/src/lib.rs

@@ -25,6 +25,7 @@ use {
         account_info::AccountInfo,
         clock::Slot,
         entrypoint::{ProgramResult, SUCCESS},
+        feature_set::FEATURE_NAMES,
         fee_calculator::{FeeCalculator, FeeRateGovernor},
         genesis_config::{ClusterType, GenesisConfig},
         hash::Hash,
@@ -57,7 +58,10 @@ use {
     tokio::task::JoinHandle,
 };
 // Export types so test clients can limit their solana crate dependencies
-pub use {solana_banks_client::BanksClient, solana_program_runtime::invoke_context::InvokeContext};
+pub use {
+    solana_banks_client::{BanksClient, BanksClientError},
+    solana_program_runtime::invoke_context::InvokeContext,
+};
 
 pub mod programs;
 
@@ -448,6 +452,7 @@ pub struct ProgramTest {
     compute_max_units: Option<u64>,
     prefer_bpf: bool,
     use_bpf_jit: bool,
+    deactivate_feature_set: HashSet<Pubkey>,
 }
 
 impl Default for ProgramTest {
@@ -478,6 +483,7 @@ impl Default for ProgramTest {
             compute_max_units: None,
             prefer_bpf,
             use_bpf_jit: false,
+            deactivate_feature_set: HashSet::default(),
         }
     }
 }
@@ -707,6 +713,13 @@ impl ProgramTest {
             .push(Builtin::new(program_name, program_id, process_instruction));
     }
 
+    /// Deactivate a runtime feature.
+    ///
+    /// Note that all features are activated by default.
+    pub fn deactivate_feature(&mut self, feature_id: Pubkey) {
+        self.deactivate_feature_set.insert(feature_id);
+    }
+
     fn setup_bank(
         &self,
     ) -> (
@@ -746,6 +759,25 @@ impl ProgramTest {
             ClusterType::Development,
             vec![],
         );
+
+        // Remove features tagged to deactivate
+        for deactivate_feature_pk in &self.deactivate_feature_set {
+            if FEATURE_NAMES.contains_key(deactivate_feature_pk) {
+                match genesis_config.accounts.remove(deactivate_feature_pk) {
+                    Some(_) => debug!("Feature for {:?} deactivated", deactivate_feature_pk),
+                    None => warn!(
+                        "Feature {:?} set for deactivation not found in genesis_config account list, ignored.",
+                        deactivate_feature_pk
+                    ),
+                }
+            } else {
+                warn!(
+                    "Feature {:?} set for deactivation is not a known Feature public key",
+                    deactivate_feature_pk
+                );
+            }
+        }
+
         let target_tick_duration = Duration::from_micros(100);
         genesis_config.poh_config = PohConfig::new_sleep(target_tick_duration);
         debug!("Payer address: {}", mint_keypair.pubkey());

programs/ed25519-tests/Cargo.toml

@@ -0,0 +1,19 @@
+[package]
+name = "solana-ed25519-program-tests"
+version = "1.10.0"
+authors = ["Solana Maintainers <[email protected]>"]
+repository = "https://github.com/solana-labs/solana"
+license = "Apache-2.0"
+homepage = "https://solana.com/"
+edition = "2021"
+publish = false
+
+[dev-dependencies]
+assert_matches = "1.5.0"
+ed25519-dalek = "=1.0.1"
+rand = "0.7.0"
+solana-program-test = { path = "../../program-test", version = "=1.10.0" }
+solana-sdk = { path = "../../sdk", version = "=1.10.0" }
+
+[package.metadata.docs.rs]
+targets = ["x86_64-unknown-linux-gnu"]

programs/ed25519-tests/tests/process_transaction.rs

@@ -0,0 +1,86 @@
+use {
+    assert_matches::assert_matches,
+    rand::thread_rng,
+    solana_program_test::*,
+    solana_sdk::{
+        ed25519_instruction::new_ed25519_instruction,
+        feature_set,
+        signature::Signer,
+        transaction::{Transaction, TransactionError},
+    },
+};
+
+#[tokio::test]
+async fn test_success() {
+    let mut context = ProgramTest::default().start_with_context().await;
+
+    let client = &mut context.banks_client;
+    let payer = &context.payer;
+    let recent_blockhash = context.last_blockhash;
+
+    let privkey = ed25519_dalek::Keypair::generate(&mut thread_rng());
+    let message_arr = b"hello";
+    let instruction = new_ed25519_instruction(&privkey, message_arr);
+
+    let transaction = Transaction::new_signed_with_payer(
+        &[instruction],
+        Some(&payer.pubkey()),
+        &[payer],
+        recent_blockhash,
+    );
+
+    assert_matches!(client.process_transaction(transaction).await, Ok(()));
+}
+
+#[tokio::test]
+async fn test_failure() {
+    let mut context = ProgramTest::default().start_with_context().await;
+
+    let client = &mut context.banks_client;
+    let payer = &context.payer;
+    let recent_blockhash = context.last_blockhash;
+
+    let privkey = ed25519_dalek::Keypair::generate(&mut thread_rng());
+    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_success_call_builtin_program() {
+    let mut program_test = ProgramTest::default();
+    program_test.deactivate_feature(feature_set::prevent_calling_precompiles_as_programs::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 = ed25519_dalek::Keypair::generate(&mut thread_rng());
+    let message_arr = b"hello";
+    let instruction = new_ed25519_instruction(&privkey, message_arr);
+
+    let transaction = Transaction::new_signed_with_payer(
+        &[instruction],
+        Some(&payer.pubkey()),
+        &[payer],
+        recent_blockhash,
+    );
+
+    assert_matches!(client.process_transaction(transaction).await, Ok(()));
+}

runtime/src/bank.rs

@@ -12555,25 +12555,25 @@ pub(crate) mod tests {
             if bank.slot == 0 {
                 assert_eq!(
                     bank.hash().to_string(),
-                    "2VLeMNvmpPEDy7sWw48gUVzjMa3WmgoegjavyhLTCnq7"
+                    "HREoNvUAuqqGdJxYTgnFqjTxsuuBVUFFDNLGR2cdrtMf"
                 );
             }
             if bank.slot == 32 {
                 assert_eq!(
                     bank.hash().to_string(),
-                    "AYnKo6pV8WJy5yXiSkYk79LWnCUCG714fQG7Xq2EizLv"
+                    "J8kjxLMMrpEVQUbX54zDALkXidjdXyFSL5wD2d7xUaWL"
                 );
             }
             if bank.slot == 64 {
                 assert_eq!(
                     bank.hash().to_string(),
-                    "37iX2uYecqAzxwyeWL8FCFeWg31B8u9hQhCRF76atrWy"
+                    "yPCTEPtNi2DJb8KyqPKgBK7HCfiEpH2oS3Nn12LPBHm"
                 );
             }
             if bank.slot == 128 {
                 assert_eq!(
                     bank.hash().to_string(),
-                    "4TDaCAvTJJg1YZ6aDhoM33Hk2cDzaraaxtGMJCmXG4wf"
+                    "2oG1rmA59tmr457oK4oF6C6TcM2gyy2ZAKeJoUhMNb1L"
                 );
                 break;
             }
@@ -12802,7 +12802,7 @@ pub(crate) mod tests {
         // No more slots should be shrunk
         assert_eq!(bank2.shrink_candidate_slots(), 0);
         // alive_counts represents the count of alive accounts in the three slots 0,1,2
-        assert_eq!(alive_counts, vec![10, 1, 7]);
+        assert_eq!(alive_counts, vec![11, 1, 7]);
     }
 
     #[test]
@@ -12850,7 +12850,7 @@ pub(crate) mod tests {
             .map(|_| bank.process_stale_slot_with_budget(0, force_to_return_alive_account))
             .sum();
         // consumed_budgets represents the count of alive accounts in the three slots 0,1,2
-        assert_eq!(consumed_budgets, 11);
+        assert_eq!(consumed_budgets, 12);
     }
 
     #[test]

runtime/src/builtins.rs

@@ -129,10 +129,15 @@ fn genesis_builtins() -> Vec<Builtin> {
             solana_sdk::secp256k1_program::id(),
             dummy_process_instruction,
         ),
+        Builtin::new(
+            "ed25519_program",
+            solana_sdk::ed25519_program::id(),
+            dummy_process_instruction,
+        ),
     ]
 }
 
-/// place holder for secp256k1, remove when the precompile program is deactivated via feature activation
+/// place holder for precompile programs, remove when the precompile program is deactivated via feature activation
 fn dummy_process_instruction(
     _first_instruction_account: usize,
     _data: &[u8],
@@ -172,6 +177,18 @@ fn feature_builtins() -> Vec<(Builtin, Pubkey, ActivationType)> {
             feature_set::prevent_calling_precompiles_as_programs::id(),
             ActivationType::RemoveProgram,
         ),
+        // TODO when feature `prevent_calling_precompiles_as_programs` is
+        // cleaned up also remove "ed25519_program" from the main builtins
+        // list
+        (
+            Builtin::new(
+                "ed25519_program",
+                solana_sdk::ed25519_program::id(),
+                dummy_process_instruction,
+            ),
+            feature_set::prevent_calling_precompiles_as_programs::id(),
+            ActivationType::RemoveProgram,
+        ),
         (
             Builtin::new(
                 "address_lookup_table_program",

runtime/src/genesis_utils.rs

@@ -26,7 +26,7 @@ pub fn bootstrap_validator_stake_lamports() -> u64 {
 
 // Number of lamports automatically used for genesis accounts
 pub const fn genesis_sysvar_and_builtin_program_lamports() -> u64 {
-    const NUM_BUILTIN_PROGRAMS: u64 = 5;
+    const NUM_BUILTIN_PROGRAMS: u64 = 6;
     const FEES_SYSVAR_MIN_BALANCE: u64 = 946_560;
     const STAKE_HISTORY_MIN_BALANCE: u64 = 114_979_200;
     const CLOCK_SYSVAR_MIN_BALANCE: u64 = 1_169_280;

sdk/src/precompiles.rs

@@ -5,9 +5,7 @@
 use {
     crate::{
         decode_error::DecodeError,
-        feature_set::{
-            ed25519_program_enabled, prevent_calling_precompiles_as_programs, FeatureSet,
-        },
+        feature_set::{prevent_calling_precompiles_as_programs, FeatureSet},
         instruction::CompiledInstruction,
         pubkey::Pubkey,
     },
@@ -88,7 +86,7 @@ lazy_static! {
         ),
         Precompile::new(
             crate::ed25519_program::id(),
-            Some(ed25519_program_enabled::id()),
+            Some(prevent_calling_precompiles_as_programs::id()),
             crate::ed25519_instruction::verify,
         ),
     ];