From 0bbb12a011432766175c339734c6ec819ec203d5 Mon Sep 17 00:00:00 2001
From: Edoscoba <obisimon86@gmail.com>
Date: Wed, 25 Feb 2026 21:31:47 +0100
Subject: [PATCH] perf(storage): pack Vault struct fields (Issue 48)

---
 SPEC.md                                 | 27 +++++++++++++++++++------
 contracts/vesting_contracts/src/lib.rs  | 26 +++++++++++-------------
 contracts/vesting_contracts/src/test.rs |  1 +
 3 files changed, 34 insertions(+), 20 deletions(-)

diff --git a/SPEC.md b/SPEC.md
index 4e9db4b..732c91a 100644
--- a/SPEC.md
+++ b/SPEC.md
@@ -203,15 +203,30 @@ All values stored in `instance` storage.
 
 ```rust
 pub struct Vault {
-    pub owner: Address,          // Current beneficiary
-    pub total_amount: i128,      // Total tokens in this vault
-    pub released_amount: i128,   // Tokens already claimed or revoked
-    pub start_time: u64,         // Vesting start (unix timestamp)
-    pub end_time: u64,           // Vesting end (unix timestamp)
-    pub is_initialized: bool,    // Lazy init flag
+    // i128 (largest)
+    pub total_amount: i128, // = initial_deposit_shares
+    pub released_amount: i128,
+    pub keeper_fee: i128,
+    pub staked_amount: i128,
+
+    // 8-byte values
+    pub owner: Address,
+    pub delegate: Option<Address>,
+    pub title: String,
+    pub start_time: u64,
+    pub end_time: u64,
+    pub creation_time: u64,
+    pub step_duration: u64,
+
+    // bools (smallest)
+    pub is_initialized: bool,
+    pub is_irrevocable: bool,
+    pub is_transferable: bool,
 }
 ```
 
+> **Soroban serialization note:** `#[contracttype]` structs are serialized as an ordered tuple (field order matters). Reordering fields changes the on-ledger schema and requires a migration strategy if any `Vault` entries already exist. Storage serialization has no alignment padding; this change primarily reduces Rust in-memory padding. For upgrade-safe evolution, prefer explicit versioning (e.g., `VaultV1`/`VaultV2`) over reordering existing fields.
+
 > **Note for auditors:** The `VestingContract` does not compute a vested amount internally. It tracks `total_amount` and `released_amount` only. The actual time-based vesting calculation — and any enforcement of `start_time`/`end_time` at claim time — is **not present** in `claim_tokens()`. Any caller can claim any unreleased amount regardless of the current time. This is a significant design note detailed further in [Known Limitations](#known-limitations--auditor-notes).
 
 ### Vault Lifecycle
diff --git a/contracts/vesting_contracts/src/lib.rs b/contracts/vesting_contracts/src/lib.rs
index 941b701..40c1945 100644
--- a/contracts/vesting_contracts/src/lib.rs
+++ b/contracts/vesting_contracts/src/lib.rs
@@ -53,28 +53,26 @@ pub enum DataKey {
 
 #[contracttype]
 #[derive(Clone)]
+// NOTE: `#[contracttype]` structs serialize by field order (tuple-style).
+// Reordering fields is a storage schema change; only do this pre-deploy or with migration.
 pub struct Vault {
-    pub owner: Address,
-    pub delegate: Option<Address>,
     pub total_amount: i128, // = initial_deposit_shares
     pub released_amount: i128,
-    pub start_time: u64,
-    pub end_time: u64,
     pub keeper_fee: i128, // Fee paid to anyone who triggers auto_claim
+    pub staked_amount: i128, // Amount currently staked in external contract
+
+    pub owner: Address,
+    pub delegate: Option<Address>,
     pub title: String, // Short human-readable title (max 32 chars)
+
+    pub start_time: u64,
+    pub end_time: u64,
+    pub creation_time: u64, // Timestamp of creation for clawback grace period
+    pub step_duration: u64, // Duration of each vesting step in seconds (0 = linear)
+
     pub is_initialized: bool, // Lazy initialization flag
     pub is_irrevocable: bool, // Security flag to prevent admin withdrawal
-    pub creation_time: u64, // Timestamp of creation for clawback grace period
     pub is_transferable: bool, // Can the beneficiary transfer this vault?
-    pub step_duration: u64, // Duration of each vesting step in seconds (0 = linear)
-    pub staked_amount: i128, // Amount currently staked in external contract
-    pub keeper_fee: i128,
-    pub is_initialized: bool,
-    pub is_irrevocable: bool,
-    pub creation_time: u64,
-    pub is_transferable: bool,
-    pub step_duration: u64,
-    pub staked_amount: i128,
 }
 
 #[contracttype]
diff --git a/contracts/vesting_contracts/src/test.rs b/contracts/vesting_contracts/src/test.rs
index 9d4c70c..0c85209 100644
--- a/contracts/vesting_contracts/src/test.rs
+++ b/contracts/vesting_contracts/src/test.rs
@@ -641,6 +641,7 @@ impl MockStakingContract {
     pub fn stake(env: Env, vault_id: u64, amount: i128, _validator: Address) {
         env.events().publish((Symbol::new(&env, "stake"), vault_id), amount);
     }
+}
 
     pub fn unstake(env: Env, vault_id: u64, amount: i128) {
         env.events()