feat: return Ok() on valid commit/finalize without failing (#61)

* feat: return Ok() on valid commit/finalize without failing

* chore: add magicblock prefix to the crate

Author: GabrielePicco

Cargo.lock

@@ -1,7 +1,7 @@
 [package]
-name = "delegation-program"
+name = "magicblock-delegation-program"
 description = "Delegation program for the Ephemeral Rollups"
-version = "0.1.3"
+version = "1.0.0"
 authors = ["Magicblock Labs <dev@magicblock.gg>"]
 edition = "2021"
 license = "MIT"
@@ -37,4 +37,4 @@ rand = "0.8.5"
 solana-program-test = "1.18.26"
 solana-sdk = "1.18.26"
 tokio = { version = "1.40.0", features = ["full"] }
-delegation-program = { path = ".", features = ["unit_test_config"] }
+magicblock-delegation-program = { path = ".", features = ["unit_test_config"] }

Cargo.toml

@@ -2,7 +2,7 @@ use solana_program::pubkey;
 use solana_program::pubkey::Pubkey;
 
 /// The delegation session fees (extracted in percentage from the delegation PDAs rent on closure).
-pub const RENT_FEES_PERCENTAGE: u8 = 30;
+pub const RENT_FEES_PERCENTAGE: u8 = 10;
 
 /// The fees extracted from the validator earnings (extracted in percentage from the validator fees claims).
 pub const PROTOCOL_FEES_PERCENTAGE: u8 = 10;

src/consts.rs

@@ -65,7 +65,7 @@ pub fn process_instruction(
             processor::process_init_validator_fees_vault(program_id, accounts, data)?
         }
         discriminator::DlpDiscriminator::InitFeesVault => {
-            processor::process_init_fees_vault(program_id, accounts, data)?
+            processor::process_init_protocol_fees_vault(program_id, accounts, data)?
         }
         discriminator::DlpDiscriminator::ValidatorClaimFees => {
             processor::process_validator_claim_fees(program_id, accounts, data)?

src/lib.rs

@@ -6,7 +6,6 @@ use crate::processor::utils::loaders::{
     load_signer, load_uninitialized_pda,
 };
 use crate::processor::utils::pda::create_pda;
-use crate::processor::utils::verify::verify_state;
 use crate::state::{CommitRecord, DelegationMetadata, DelegationRecord, ProgramConfig};
 use crate::{
     commit_record_seeds_from_delegated_account, commit_state_seeds_from_delegated_account,
@@ -15,8 +14,8 @@ use borsh::BorshDeserialize;
 use solana_program::program::invoke;
 use solana_program::program_error::ProgramError;
 use solana_program::system_instruction::transfer;
-use solana_program::system_program;
 use solana_program::{account_info::AccountInfo, entrypoint::ProgramResult, pubkey::Pubkey};
+use solana_program::{msg, system_program};
 
 /// Commit a new state of a delegated Pda
 ///
@@ -55,17 +54,24 @@ pub fn process_commit_state(
     let mut delegation_metadata =
         DelegationMetadata::try_from_bytes_with_discriminator(&delegation_metadata_data)?;
 
+    // If the commit slot is greater than the last update slot, we can proceed.
+    // If the slot is equal or less, we simply do not commit.
+    // Since commit instructions are typically bundled, we return without error
+    // so that correct commits are executed.
+    if commit_record_slot <= delegation_metadata.last_update_external_slot {
+        msg!(
+            "Slot {} is outdated, previous slot is {}. Skipping commit",
+            commit_record_slot,
+            delegation_metadata.last_update_external_slot
+        );
+        return Ok(());
+    }
+
     // Once the account is marked as undelegatable, any subsequent commit should fail
     if delegation_metadata.is_undelegatable {
         return Err(DlpError::AlreadyUndelegated.into());
     }
 
-    // If the commit slot is greater than the last update slot, we can proceed
-    // If slot is equal or less, we simply do not commit
-    if commit_record_slot <= delegation_metadata.last_update_external_slot {
-        return Err(DlpError::OutdatedSlot.into());
-    }
-
     // Update delegation metadata undelegation flag
     delegation_metadata.is_undelegatable = args.allow_undelegation;
     delegation_metadata.to_bytes_with_discriminator(&mut delegation_metadata_data.as_mut())?;
@@ -160,14 +166,7 @@ pub fn process_commit_state(
     let mut commit_state_data = commit_state_account.try_borrow_mut_data()?;
     (*commit_state_data).copy_from_slice(commit_state_bytes);
 
-    // TODO - We'll need to implement state validation
-    // TODO - fix: this logic should probably be done in either commit_state OR finalize IX, not both
-    verify_state(
-        validator,
-        delegation_record,
-        &commit_record,
-        commit_state_account,
-    )?;
+    // TODO - Add additional validation for the commitment, e.g. sufficient validator stake
 
     Ok(())
 }

src/processor/commit_state.rs

@@ -1,6 +1,6 @@
 use crate::error::DlpError;
 use crate::processor::utils::loaders::{
-    load_initialized_commit_record, load_initialized_commit_state,
+    is_uninitialized_account, load_initialized_commit_record, load_initialized_commit_state,
     load_initialized_delegation_metadata, load_initialized_delegation_record,
     load_initialized_validator_fees_vault, load_owned_pda, load_program, load_signer,
 };
@@ -9,7 +9,7 @@ use crate::processor::utils::verify::verify_state;
 use crate::state::{CommitRecord, DelegationMetadata, DelegationRecord};
 use solana_program::program_error::ProgramError;
 use solana_program::{
-    account_info::AccountInfo, entrypoint::ProgramResult, pubkey::Pubkey, system_program,
+    account_info::AccountInfo, entrypoint::ProgramResult, msg, pubkey::Pubkey, system_program,
 };
 
 /// Finalize a committed state, after validation, to a delegated account
@@ -33,12 +33,27 @@ pub fn process_finalize(
 
     load_signer(validator)?;
     load_owned_pda(delegated_account, &crate::id())?;
-    load_initialized_commit_state(delegated_account, commit_state_account, true)?;
-    load_initialized_commit_record(delegated_account, commit_record_account, true)?;
     load_initialized_delegation_record(delegated_account, delegation_record_account, true)?;
     load_initialized_delegation_metadata(delegated_account, delegation_metadata_account, true)?;
     load_initialized_validator_fees_vault(validator, validator_fees_vault, true)?;
     load_program(system_program, system_program::id())?;
+    let load_cs = load_initialized_commit_state(delegated_account, commit_state_account, true);
+    let load_cr = load_initialized_commit_record(delegated_account, commit_record_account, true);
+
+    // Since finalize instructions are typically bundled, we return without error
+    // if there is nothing to be finalized, so that correct finalizes are executed
+    if let (Err(ProgramError::InvalidAccountOwner), Err(ProgramError::InvalidAccountOwner)) =
+        (&load_cs, &load_cr)
+    {
+        if is_uninitialized_account(commit_state_account)
+            && is_uninitialized_account(commit_record_account)
+        {
+            msg!("No state to be finalized. Skipping finalize.");
+            return Ok(());
+        }
+    }
+    load_cs?;
+    load_cr?;
 
     // Load delegation metadata
     let mut delegation_metadata_data = delegation_metadata_account.try_borrow_mut_data()?;

src/processor/finalize.rs

@@ -9,28 +9,26 @@ use crate::processor::utils::pda::create_pda;
 
 /// Initialize the global fees vault
 ///
-/// 1. Create the fees vault PDA
-pub fn process_init_fees_vault(
+/// 1. Create the protocol fees vault PDA
+pub fn process_init_protocol_fees_vault(
     _program_id: &Pubkey,
     accounts: &[AccountInfo],
     _data: &[u8],
 ) -> ProgramResult {
-    // TODO - nit: for naming convention this vault (account+IX) could be named the "protocol_fees_vault"
-
     // Load Accounts
-    let [payer, fees_vault, system_program] = accounts else {
+    let [payer, protocol_fees_vault, system_program] = accounts else {
         return Err(ProgramError::NotEnoughAccountKeys);
     };
 
     load_signer(payer)?;
     load_program(system_program, system_program::id())?;
 
     let bump_fees_vault =
-        load_uninitialized_pda(fees_vault, fees_vault_seeds!(), &crate::id(), true)?;
+        load_uninitialized_pda(protocol_fees_vault, fees_vault_seeds!(), &crate::id(), true)?;
 
     // Create the fees vault account
     create_pda(
-        fees_vault,
+        protocol_fees_vault,
         &crate::id(),
         8,
         fees_vault_seeds!(),

src/processor/init_fees_vault.rs

@@ -275,18 +275,16 @@ fn process_delegation_cleanup<'a, 'info>(
     fees_vault: &'a AccountInfo<'info>,
     validator_fees_vault: &'a AccountInfo<'info>,
 ) -> ProgramResult {
-    // TODO - fix: there is a double-fee being applied because validator_claim_fees also applies protocol level fees
-    // TODO - fix: we should probably only let the validator apply fees there? or not apply extra fees during validator_fees_claim
     close_pda_with_fees(
         delegation_record_account,
         rent_reimbursement,
-        &[fees_vault, validator_fees_vault],
+        &[validator_fees_vault, fees_vault],
         RENT_FEES_PERCENTAGE,
     )?;
     close_pda_with_fees(
         delegation_metadata_account,
         rent_reimbursement,
-        &[fees_vault, validator_fees_vault],
+        &[validator_fees_vault, fees_vault],
         RENT_FEES_PERCENTAGE,
     )?;
     Ok(())

src/processor/undelegate.rs

@@ -100,6 +100,13 @@ pub fn load_initialized_pda(
     Ok(pda.1)
 }
 
+/// Returns true if the account is uninitialized based on the following conditions:
+/// - Owner is the system program.
+/// - Data is empty.
+pub fn is_uninitialized_account(info: &AccountInfo) -> bool {
+    info.owner.eq(&system_program::id()) && info.data_is_empty()
+}
+
 /// Errors if:
 /// - Owner is not the system program.
 /// - Data is not empty.

src/processor/utils/loaders.rs

@@ -127,6 +127,7 @@ pub(crate) fn close_pda_with_fees<'a, 'info>(
     let total_lamports = target_account.lamports();
     let mut remaining_amount = total_lamports;
 
+    // TODO - simplify fees charging logic
     for fee_address in fees_addresses.iter() {
         let fee_amount = (remaining_amount as u128)
             .checked_mul(fee_percentage as u128)