Feature - Program redeployment cooldown (#29136)

* Registers the feature enable_program_redeployment_cooldown.

* Adds redeployment slot constraint.

* Adds test to assert that closed programs can not be reopened.

* Ensure that program close truncates the account data.

* Adds set_sysvar_for_tests() to SBF program tests.

programs/bpf_loader/src/lib.rs

@@ -43,8 +43,8 @@ use {
             cap_accounts_data_allocations_per_transaction, cap_bpf_program_instruction_accounts,
             check_slice_translation_size, disable_deploy_of_alloc_free_syscall,
             disable_deprecated_loader, enable_bpf_loader_extend_program_ix,
-            enable_bpf_loader_set_authority_checked_ix, limit_max_instruction_trace_length,
+            enable_bpf_loader_set_authority_checked_ix, enable_program_redeployment_cooldown,
-            FeatureSet,
+            limit_max_instruction_trace_length, FeatureSet,
         },
         instruction::{AccountMeta, InstructionError},
         loader_instruction::LoaderInstruction,
@@ -601,7 +601,6 @@ fn process_loader_upgradeable_instruction(
             }
 
             // Create ProgramData account
-
             let (derived_address, bump_seed) =
                 Pubkey::find_program_address(&[new_program_id.as_ref()], program_id);
             if derived_address != programdata_key {
@@ -801,10 +800,18 @@ fn process_loader_upgradeable_instruction(
                 return Err(InstructionError::InsufficientFunds);
             }
             if let UpgradeableLoaderState::ProgramData {
-                slot: _,
+                slot,
                 upgrade_authority_address,
             } = programdata.get_state()?
             {
+                if invoke_context
+                    .feature_set
+                    .is_active(&enable_program_redeployment_cooldown::id())
+                    && clock.slot == slot
+                {
+                    ic_logger_msg!(log_collector, "Program was deployed in this block already");
+                    return Err(InstructionError::InvalidArgument);
+                }
                 if upgrade_authority_address.is_none() {
                     ic_logger_msg!(log_collector, "Program not upgradeable");
                     return Err(InstructionError::Immutable);
@@ -1048,7 +1055,14 @@ fn process_loader_upgradeable_instruction(
             let mut close_account =
                 instruction_context.try_borrow_instruction_account(transaction_context, 0)?;
             let close_key = *close_account.get_key();
-            match close_account.get_state()? {
+            let close_account_state = close_account.get_state()?;
+            if invoke_context
+                .feature_set
+                .is_active(&enable_program_redeployment_cooldown::id())
+            {
+                close_account.set_data_length(UpgradeableLoaderState::size_of_uninitialized())?;
+            }
+            match close_account_state {
                 UpgradeableLoaderState::Uninitialized => {
                     let mut recipient_account = instruction_context
                         .try_borrow_instruction_account(transaction_context, 1)?;
@@ -1518,7 +1532,7 @@ mod tests {
             instruction::{AccountMeta, InstructionError},
             pubkey::Pubkey,
             rent::Rent,
-            sysvar,
+            system_program, sysvar,
         },
         std::{fs::File, io::Read, ops::Range},
     };
@@ -2299,7 +2313,7 @@ mod tests {
             let spill_account = AccountSharedData::new(0, 0, &Pubkey::new_unique());
             let rent_account = create_account_for_test(&rent);
             let clock_account = create_account_for_test(&Clock {
-                slot: SLOT,
+                slot: SLOT.saturating_add(1),
                 ..Clock::default()
             });
             let upgrade_authority_account = AccountSharedData::new(1, 0, &Pubkey::new_unique());
@@ -2394,7 +2408,7 @@ mod tests {
         assert_eq!(
             state,
             UpgradeableLoaderState::ProgramData {
-                slot: SLOT,
+                slot: SLOT.saturating_add(1),
                 upgrade_authority_address: Some(upgrade_authority_address)
             }
         );
@@ -3529,7 +3543,7 @@ mod tests {
         let recipient_account = AccountSharedData::new(1, 0, &Pubkey::new_unique());
         let buffer_address = Pubkey::new_unique();
         let mut buffer_account =
-            AccountSharedData::new(1, UpgradeableLoaderState::size_of_buffer(0), &loader_id);
+            AccountSharedData::new(1, UpgradeableLoaderState::size_of_buffer(128), &loader_id);
         buffer_account
             .set_state(&UpgradeableLoaderState::Buffer {
                 authority_address: Some(authority_address),
@@ -3547,7 +3561,7 @@ mod tests {
         let programdata_address = Pubkey::new_unique();
         let mut programdata_account = AccountSharedData::new(
             1,
-            UpgradeableLoaderState::size_of_programdata(0),
+            UpgradeableLoaderState::size_of_programdata(128),
             &loader_id,
         );
         programdata_account
@@ -3603,6 +3617,10 @@ mod tests {
         assert_eq!(2, accounts.get(1).unwrap().lamports());
         let state: UpgradeableLoaderState = accounts.first().unwrap().state().unwrap();
         assert_eq!(state, UpgradeableLoaderState::Uninitialized);
+        assert_eq!(
+            UpgradeableLoaderState::size_of_uninitialized(),
+            accounts.first().unwrap().data().len()
+        );
 
         // Case: close with wrong authority
         process_instruction(
@@ -3651,6 +3669,10 @@ mod tests {
         assert_eq!(2, accounts.get(1).unwrap().lamports());
         let state: UpgradeableLoaderState = accounts.first().unwrap().state().unwrap();
         assert_eq!(state, UpgradeableLoaderState::Uninitialized);
+        assert_eq!(
+            UpgradeableLoaderState::size_of_uninitialized(),
+            accounts.first().unwrap().data().len()
+        );
 
         // Case: close a program account
         let accounts = process_instruction(
@@ -3659,8 +3681,8 @@ mod tests {
             &instruction,
             vec![
                 (programdata_address, programdata_account.clone()),
-                (recipient_address, recipient_account),
+                (recipient_address, recipient_account.clone()),
-                (authority_address, authority_account),
+                (authority_address, authority_account.clone()),
                 (program_address, program_account.clone()),
             ],
             vec![
@@ -3683,8 +3705,14 @@ mod tests {
         assert_eq!(2, accounts.get(1).unwrap().lamports());
         let state: UpgradeableLoaderState = accounts.first().unwrap().state().unwrap();
         assert_eq!(state, UpgradeableLoaderState::Uninitialized);
+        assert_eq!(
+            UpgradeableLoaderState::size_of_uninitialized(),
+            accounts.first().unwrap().data().len()
+        );
 
         // Try to invoke closed account
+        programdata_account = accounts.first().unwrap().clone();
+        program_account = accounts.get(3).unwrap().clone();
         process_instruction(
             &program_address,
             &[0, 1],
@@ -3696,6 +3724,78 @@ mod tests {
             Vec::new(),
             Err(InstructionError::InvalidAccountData),
         );
+
+        // Case: Reopen should fail
+        process_instruction(
+            &loader_id,
+            &[],
+            &bincode::serialize(&UpgradeableLoaderInstruction::DeployWithMaxDataLen {
+                max_data_len: 0,
+            })
+            .unwrap(),
+            vec![
+                (recipient_address, recipient_account),
+                (programdata_address, programdata_account),
+                (program_address, program_account),
+                (buffer_address, buffer_account),
+                (
+                    sysvar::rent::id(),
+                    create_account_for_test(&Rent::default()),
+                ),
+                (
+                    sysvar::clock::id(),
+                    create_account_for_test(&Clock::default()),
+                ),
+                (
+                    system_program::id(),
+                    AccountSharedData::new(0, 0, &system_program::id()),
+                ),
+                (authority_address, authority_account),
+            ],
+            vec![
+                AccountMeta {
+                    pubkey: recipient_address,
+                    is_signer: true,
+                    is_writable: true,
+                },
+                AccountMeta {
+                    pubkey: programdata_address,
+                    is_signer: false,
+                    is_writable: true,
+                },
+                AccountMeta {
+                    pubkey: program_address,
+                    is_signer: false,
+                    is_writable: true,
+                },
+                AccountMeta {
+                    pubkey: buffer_address,
+                    is_signer: false,
+                    is_writable: false,
+                },
+                AccountMeta {
+                    pubkey: sysvar::rent::id(),
+                    is_signer: false,
+                    is_writable: false,
+                },
+                AccountMeta {
+                    pubkey: sysvar::clock::id(),
+                    is_signer: false,
+                    is_writable: false,
+                },
+                AccountMeta {
+                    pubkey: system_program::id(),
+                    is_signer: false,
+                    is_writable: false,
+                },
+                AccountMeta {
+                    pubkey: authority_address,
+                    is_signer: false,
+                    is_writable: false,
+                },
+            ],
+            Err(InstructionError::AccountAlreadyInitialized),
+        );
     }
 
     /// fuzzing utility function

programs/sbf/tests/programs.rs

@@ -163,6 +163,10 @@ fn load_upgradeable_sbf_program(
         authority_keypair,
         elf,
     );
+    bank_client.set_sysvar_for_tests(&clock::Clock {
+        slot: 1,
+        ..clock::Clock::default()
+    });
 }
 
 #[cfg(feature = "sbf_rust")]
@@ -2002,6 +2006,10 @@ fn test_program_sbf_upgrade() {
         &authority_keypair,
         "solana_sbf_rust_upgraded",
     );
+    bank_client.set_sysvar_for_tests(&clock::Clock {
+        slot: 2,
+        ..clock::Clock::default()
+    });
 
     // Call upgraded program
     instruction.data[0] += 1;
@@ -2179,6 +2187,10 @@ fn test_program_sbf_invoke_upgradeable_via_cpi() {
         &authority_keypair,
         "solana_sbf_rust_upgraded",
     );
+    bank_client.set_sysvar_for_tests(&clock::Clock {
+        slot: 2,
+        ..clock::Clock::default()
+    });
 
     // Call the upgraded program
     instruction.data[0] += 1;

runtime/src/bank_client.rs

@@ -14,6 +14,7 @@ use {
         signature::{Keypair, Signature, Signer},
         signers::Signers,
         system_instruction,
+        sysvar::{Sysvar, SysvarId},
         transaction::{self, Transaction, VersionedTransaction},
         transport::{Result, TransportError},
     },
@@ -324,6 +325,10 @@ impl BankClient {
     pub fn new(bank: Bank) -> Self {
         Self::new_shared(&Arc::new(bank))
     }
+
+    pub fn set_sysvar_for_tests<T: Sysvar + SysvarId>(&self, sysvar: &T) {
+        self.bank.set_sysvar_for_tests(sysvar);
+    }
 }
 
 #[cfg(test)]

sdk/program/src/bpf_loader_upgradeable.rs

@@ -52,6 +52,11 @@ pub enum UpgradeableLoaderState {
     },
 }
 impl UpgradeableLoaderState {
+    /// Size of a serialized program account.
+    pub const fn size_of_uninitialized() -> usize {
+        4 // see test_state_size_of_uninitialized
+    }
+
     /// Size of a buffer account's serialized metadata.
     pub const fn size_of_buffer_metadata() -> usize {
         37 // see test_state_size_of_buffer_metadata
@@ -374,6 +379,13 @@ pub fn extend_program(
 mod tests {
     use {super::*, bincode::serialized_size};
 
+    #[test]
+    fn test_state_size_of_uninitialized() {
+        let buffer_state = UpgradeableLoaderState::Uninitialized;
+        let size = serialized_size(&buffer_state).unwrap();
+        assert_eq!(UpgradeableLoaderState::size_of_uninitialized() as u64, size);
+    }
+
     #[test]
     fn test_state_size_of_buffer_metadata() {
         let buffer_state = UpgradeableLoaderState::Buffer {

sdk/src/feature_set.rs

@@ -550,6 +550,10 @@ pub mod enable_alt_bn128_syscall {
     solana_sdk::declare_id!("A16q37opZdQMCbe5qJ6xpBB9usykfv8jZaMkxvZQi4GJ");
 }
 
+pub mod enable_program_redeployment_cooldown {
+    solana_sdk::declare_id!("J4HFT8usBxpcF63y46t1upYobJgChmKyZPm5uTBRg25Z");
+}
+
 lazy_static! {
     /// Map of feature identifiers to user-visible description
     pub static ref FEATURE_NAMES: HashMap<Pubkey, &'static str> = [
@@ -682,6 +686,7 @@ lazy_static! {
         (enable_bpf_loader_set_authority_checked_ix::id(), "enable bpf upgradeable loader SetAuthorityChecked instruction #28424"),
         (cap_transaction_accounts_data_size::id(), "cap transaction accounts data size up to its compute unit limits #27839"),
         (enable_alt_bn128_syscall::id(), "add alt_bn128 syscalls #27961"),
+        (enable_program_redeployment_cooldown::id(), "enable program redeployment cooldown #29135"),
         /*************** ADD NEW FEATURES HERE ***************/
     ]
     .iter()