chore: check for state slot at commit time (#60)

* good-progress-it-compiles

* more-cleanups

* getting-there

* found-a-few-edge-cases

* saving-progress

* some-cleanups

* rust-everything-seems-to-work

* integration-test-typescript-fix

* nit-comment

* more-cleanups

* fees-naming-clarifications

* todos

* comments-nits

* slot-check

* done

* fix: integration tests

* chore: format

---------

Co-authored-by: Gabriele Picco <piccogabriele@gmail.com>

Author: crypto-vincent

src/error.rs

@@ -29,8 +29,10 @@ pub enum DlpError {
     InvalidWhitelistProgramConfig = 10,
     #[error("Account already undelegated")]
     AlreadyUndelegated = 11,
+    #[error("Committed state slot is outdated")]
+    OutdatedSlot = 12,
     #[error("Computation overflow detected")]
-    Overflow = 12,
+    Overflow = 13,
 }
 
 impl From<DlpError> for ProgramError {

src/processor/commit_state.rs

@@ -16,7 +16,7 @@ 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, msg, pubkey::Pubkey};
+use solana_program::{account_info::AccountInfo, entrypoint::ProgramResult, pubkey::Pubkey};
 
 /// Commit a new state of a delegated Pda
 ///
@@ -50,6 +50,26 @@ pub fn process_commit_state(
     load_initialized_validator_fees_vault(validator, validator_fees_vault, false)?;
     load_program(system_program, system_program::id())?;
 
+    // Read delegation metadata
+    let mut delegation_metadata_data = delegation_metadata_account.try_borrow_mut_data()?;
+    let mut delegation_metadata =
+        DelegationMetadata::try_from_bytes_with_discriminator(&delegation_metadata_data)?;
+
+    // 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())?;
+
     // Load delegation record
     let delegation_record_data = delegation_record_account.try_borrow_data()?;
     let delegation_record =
@@ -85,7 +105,6 @@ pub fn process_commit_state(
         let program_config_data = program_config_account.try_borrow_data()?;
         let program_config =
             ProgramConfig::try_from_bytes_with_discriminator(&program_config_data)?;
-        msg!("Program Config: {:?}", program_config);
         if !program_config.approved_validators.contains(validator.key) {
             return Err(DlpError::InvalidWhitelistProgramConfig.into());
         }
@@ -137,20 +156,6 @@ pub fn process_commit_state(
     let mut commit_record_data = commit_record_account.try_borrow_mut_data()?;
     commit_record.to_bytes_with_discriminator(&mut commit_record_data)?;
 
-    // Read undelegation metadata
-    let mut delegation_metadata_data = delegation_metadata_account.try_borrow_mut_data()?;
-    let mut delegation_metadata =
-        DelegationMetadata::try_from_bytes_with_discriminator(&delegation_metadata_data)?;
-
-    // Once the account is marked as undelegatable, any subsequent commit should fail
-    if delegation_metadata.is_undelegatable {
-        return Err(DlpError::AlreadyUndelegated.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())?;
-
     // Copy the new state to the initialized PDA
     let mut commit_state_data = commit_state_account.try_borrow_mut_data()?;
     (*commit_state_data).copy_from_slice(commit_state_bytes);

src/processor/finalize.rs

@@ -19,7 +19,6 @@ use solana_program::{
 /// 3. Close the state diff account
 /// 4. Close the commit state record
 ///
-///
 /// Accounts expected: Authority Record, Buffer PDA, Delegated PDA
 pub fn process_finalize(
     _program_id: &Pubkey,
@@ -46,64 +45,57 @@ pub fn process_finalize(
     let mut delegation_metadata =
         DelegationMetadata::try_from_bytes_with_discriminator(&delegation_metadata_data)?;
 
-    // Load committed state
+    // Load delegation record
+    let mut delegation_record_data = delegation_record_account.try_borrow_mut_data()?;
+    let delegation_record =
+        DelegationRecord::try_from_bytes_with_discriminator_mut(&mut delegation_record_data)?;
+
+    // Load commit record
     let commit_record_data = commit_record_account.try_borrow_data()?;
     let commit_record = CommitRecord::try_from_bytes_with_discriminator(&commit_record_data)?;
 
-    // If the commit slot is greater than the last update slot, we verify and finalize the state
-    // If slot is equal or less, we simply close the commitment accounts
-    // TODO - This could probably be done in the commit_state IX
-    // TODO - since allowing commiting an obsolete state is counter-productive in the first place?
-    if commit_record.slot > delegation_metadata.last_update_external_slot {
-        // Load delegation record
-        let mut delegation_record_data = delegation_record_account.try_borrow_mut_data()?;
-        let delegation_record =
-            DelegationRecord::try_from_bytes_with_discriminator_mut(&mut delegation_record_data)?;
-
-        // 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,
-        )?;
-
-        // Check that the commit record is the right now
-        if !commit_record.account.eq(delegated_account.key) {
-            return Err(DlpError::InvalidDelegatedAccount.into());
-        }
-        if !commit_record.identity.eq(validator.key) {
-            return Err(DlpError::InvalidReimbursementAccount.into());
-        }
-
-        // Settle accounts lamports
-        settle_lamports_balance(
-            delegated_account,
-            commit_state_account,
-            validator_fees_vault,
-            delegation_record.lamports,
-            commit_record.lamports,
-        )?;
-
-        // Copying the new state to the delegated account
-        let commit_state_data = commit_state_account.try_borrow_data()?;
-        delegated_account.realloc(commit_state_data.len(), false)?;
-        let mut delegated_account_data = delegated_account.try_borrow_mut_data()?;
-        (*delegated_account_data).copy_from_slice(&commit_state_data);
-
-        delegation_metadata.last_update_external_slot = commit_record.slot;
-        delegation_record.lamports = delegated_account.lamports();
-        delegation_metadata.to_bytes_with_discriminator(&mut delegation_metadata_data.as_mut())?;
-
-        // Dropping references
-        drop(commit_state_data);
-        drop(delegated_account_data);
+    verify_state(
+        validator,
+        delegation_record,
+        commit_record,
+        commit_state_account,
+    )?;
+
+    // Check that the commit record is the right one
+    if !commit_record.account.eq(delegated_account.key) {
+        return Err(DlpError::InvalidDelegatedAccount.into());
     }
+    if !commit_record.identity.eq(validator.key) {
+        return Err(DlpError::InvalidReimbursementAccount.into());
+    }
+
+    // Settle accounts lamports
+    settle_lamports_balance(
+        delegated_account,
+        commit_state_account,
+        validator_fees_vault,
+        delegation_record.lamports,
+        commit_record.lamports,
+    )?;
+
+    // Update the delegation metadata
+    delegation_metadata.last_update_external_slot = commit_record.slot;
+    delegation_metadata.to_bytes_with_discriminator(&mut delegation_metadata_data.as_mut())?;
+
+    // Update the delegation record
+    delegation_record.lamports = delegated_account.lamports();
+
+    // Load commit state
+    let commit_state_data = commit_state_account.try_borrow_data()?;
+
+    // Copying the new commit state to the delegated account
+    delegated_account.realloc(commit_state_data.len(), false)?;
+    let mut delegated_account_data = delegated_account.try_borrow_mut_data()?;
+    (*delegated_account_data).copy_from_slice(&commit_state_data);
 
     // Drop remaining reference before closing accounts
     drop(commit_record_data);
-    drop(delegation_metadata_data);
+    drop(commit_state_data);
 
     // Closing accounts
     close_pda(commit_state_account, validator)?;