Browse Source

Fix - Loader-v3 to v4 migration of closed programs (#6106)

* Rekeys the feature enable_loader_v4.

* Have closed programs be deleted by the rent collector instead of assigning them to the system program.

* Allows uninitialized program accounts to be closed.
Alexander Meißner 6 months ago
parent
commit
64e19be684
3 changed files with 25 additions and 21 deletions
  1. 1 1
      feature-set/src/lib.rs
  2. 14 9
      programs/bpf_loader/src/lib.rs
  3. 10 11
      programs/loader-v4/src/lib.rs

+ 1 - 1
feature-set/src/lib.rs

@@ -819,7 +819,7 @@ pub mod remaining_compute_units_syscall_enabled {
 }
 }
 
 
 pub mod enable_loader_v4 {
 pub mod enable_loader_v4 {
-    solana_pubkey::declare_id!("G8yMNsNUd4p3VB22ycrPEB1qRgepCFeFpAqD2Lr66s36");
+    solana_pubkey::declare_id!("2aQJYqER2aKyb3cZw22v4SL2xMX7vwXBRWfvS4pTrtED");
 }
 }
 
 
 pub mod require_rent_exempt_split_destination {
 pub mod require_rent_exempt_split_destination {

+ 14 - 9
programs/bpf_loader/src/lib.rs

@@ -39,7 +39,6 @@ use {
     },
     },
     solana_sdk_ids::{
     solana_sdk_ids::{
         bpf_loader, bpf_loader_deprecated, bpf_loader_upgradeable, loader_v4, native_loader,
         bpf_loader, bpf_loader_deprecated, bpf_loader_upgradeable, loader_v4, native_loader,
-        system_program,
     },
     },
     solana_system_interface::{instruction as system_instruction, MAX_PERMITTED_DATA_LENGTH},
     solana_system_interface::{instruction as system_instruction, MAX_PERMITTED_DATA_LENGTH},
     solana_transaction_context::{IndexOfAccount, InstructionContext, TransactionContext},
     solana_transaction_context::{IndexOfAccount, InstructionContext, TransactionContext},
@@ -1400,11 +1399,7 @@ fn process_loader_upgradeable_instruction(
             }
             }
             program.set_data_from_slice(&[])?;
             program.set_data_from_slice(&[])?;
             program.checked_add_lamports(programdata_funds)?;
             program.checked_add_lamports(programdata_funds)?;
-            if program_len == 0 {
-                program.set_owner(&system_program::id().to_bytes())?;
-            } else {
-                program.set_owner(&loader_v4::id().to_bytes())?;
-            }
+            program.set_owner(&loader_v4::id().to_bytes())?;
             drop(program);
             drop(program);
 
 
             let mut programdata =
             let mut programdata =
@@ -1412,7 +1407,18 @@ fn process_loader_upgradeable_instruction(
             programdata.set_lamports(0)?;
             programdata.set_lamports(0)?;
             drop(programdata);
             drop(programdata);
 
 
-            if program_len > 0 {
+            if program_len == 0 {
+                invoke_context
+                    .program_cache_for_tx_batch
+                    .store_modified_entry(
+                        program_address,
+                        Arc::new(ProgramCacheEntry::new_tombstone(
+                            clock_slot,
+                            ProgramCacheEntryOwner::LoaderV4,
+                            ProgramCacheEntryType::Closed,
+                        )),
+                    );
+            } else {
                 invoke_context.native_invoke(
                 invoke_context.native_invoke(
                     solana_loader_v4_interface::instruction::set_program_length(
                     solana_loader_v4_interface::instruction::set_program_length(
                         &program_address,
                         &program_address,
@@ -1474,7 +1480,6 @@ fn process_loader_upgradeable_instruction(
             let mut programdata =
             let mut programdata =
                 instruction_context.try_borrow_instruction_account(transaction_context, 0)?;
                 instruction_context.try_borrow_instruction_account(transaction_context, 0)?;
             programdata.set_data_from_slice(&[])?;
             programdata.set_data_from_slice(&[])?;
-            programdata.set_owner(&system_program::id().to_bytes())?;
             drop(programdata);
             drop(programdata);
 
 
             ic_logger_msg!(log_collector, "Migrated program {:?}", &program_address);
             ic_logger_msg!(log_collector, "Migrated program {:?}", &program_address);
@@ -1813,7 +1818,7 @@ mod tests {
         },
         },
         solana_pubkey::Pubkey,
         solana_pubkey::Pubkey,
         solana_rent::Rent,
         solana_rent::Rent,
-        solana_sdk_ids::sysvar,
+        solana_sdk_ids::{system_program, sysvar},
         solana_svm_feature_set::SVMFeatureSet,
         solana_svm_feature_set::SVMFeatureSet,
         std::{fs::File, io::Read, ops::Range, sync::atomic::AtomicU64},
         std::{fs::File, io::Read, ops::Range, sync::atomic::AtomicU64},
     };
     };

+ 10 - 11
programs/loader-v4/src/lib.rs

@@ -189,8 +189,7 @@ fn process_instruction_set_program_length(
     let authority_address = instruction_context
     let authority_address = instruction_context
         .get_index_of_instruction_account_in_transaction(1)
         .get_index_of_instruction_account_in_transaction(1)
         .and_then(|index| transaction_context.get_key_of_account_at_index(index))?;
         .and_then(|index| transaction_context.get_key_of_account_at_index(index))?;
-    let is_initialization =
-        new_size > 0 && program.get_data().len() < LoaderV4State::program_data_offset();
+    let is_initialization = program.get_data().len() < LoaderV4State::program_data_offset();
     if is_initialization {
     if is_initialization {
         if !loader_v4::check_id(program.get_owner()) {
         if !loader_v4::check_id(program.get_owner()) {
             ic_logger_msg!(log_collector, "Program not owned by loader");
             ic_logger_msg!(log_collector, "Program not owned by loader");
@@ -1127,6 +1126,15 @@ mod tests {
             ),
             ),
         );
         );
 
 
+        // Close uninitialized program account
+        process_instruction(
+            vec![],
+            &bincode::serialize(&LoaderV4Instruction::SetProgramLength { new_size: 0 }).unwrap(),
+            transaction_accounts.clone(),
+            &[(3, false, true), (1, true, false), (2, true, true)],
+            Ok(()),
+        );
+
         // Error: Program not owned by loader
         // Error: Program not owned by loader
         process_instruction(
         process_instruction(
             vec![],
             vec![],
@@ -1163,15 +1171,6 @@ mod tests {
             Err(InstructionError::MissingRequiredSignature),
             Err(InstructionError::MissingRequiredSignature),
         );
         );
 
 
-        // Error: Program is and stays uninitialized
-        process_instruction(
-            vec![],
-            &bincode::serialize(&LoaderV4Instruction::SetProgramLength { new_size: 0 }).unwrap(),
-            transaction_accounts.clone(),
-            &[(3, false, true), (1, true, false), (2, true, true)],
-            Err(InstructionError::AccountDataTooSmall),
-        );
-
         // Error: Program is not retracted
         // Error: Program is not retracted
         process_instruction(
         process_instruction(
             vec![],
             vec![],