Address review comments

Author: PabloMansanet

chains/solana/contracts/programs/ccip-offramp/src/context.rs

@@ -665,6 +665,13 @@ pub struct CloseExecutionReportBufferContext<'info> {
     )]
     pub execution_report_buffer: Account<'info, ExecutionReportBuffer>,
 
+    #[account(
+        seeds = [seed::CONFIG],
+        bump,
+        constraint = valid_version(config.load()?.version, MAX_CONFIG_V) @ CcipOfframpError::InvalidVersion,
+    )]
+    pub config: AccountLoader<'info, Config>,
+
     #[account(mut)]
     pub authority: Signer<'info>,
     pub system_program: Program<'info, System>,

chains/solana/contracts/programs/ccip-offramp/src/instructions/v1/buffering.rs

@@ -25,7 +25,7 @@ impl Buffering for ExecutionReportBuffer {
     }
 
     fn is_complete(&self) -> bool {
-        self.is_initialized() && self.filled_chunks() == self.total_chunks
+        self.is_initialized() && self.filled_chunks() == self.num_chunks
     }
 
     fn bytes(&self) -> Result<&[u8]> {
@@ -62,6 +62,11 @@ impl Buffering for ExecutionReportBuffer {
             report_length as usize,
             CcipOfframpError::ExecutionReportBufferInvalidLength
         );
+        require_eq!(
+            self.num_chunks,
+            num_chunks,
+            CcipOfframpError::ExecutionReportBufferInvalidChunkNumber
+        );
 
         let chunk_mask = 1u64 << chunk_index;
         require!(
@@ -79,13 +84,13 @@ impl Buffering for ExecutionReportBuffer {
             // Only the terminator (last chunk) can be smaller than the others.
             require_eq!(
                 chunk_index,
-                self.total_chunks - 1,
+                self.num_chunks - 1,
                 CcipOfframpError::ExecutionReportBufferInvalidChunkSize
             );
         }
 
         require_gt!(
-            self.total_chunks,
+            self.num_chunks,
             chunk_index,
             CcipOfframpError::ExecutionReportBufferInvalidChunkIndex
         );
@@ -116,7 +121,7 @@ impl ExecutionReportBuffer {
             CcipOfframpError::ExecutionReportBufferInvalidLength
         );
         require!(
-            total_chunks < 64 && total_chunks > 0,
+            total_chunks <= 64 && total_chunks > 0,
             CcipOfframpError::ExecutionReportBufferInvalidChunkSize
         );
 
@@ -136,7 +141,7 @@ impl ExecutionReportBuffer {
             CcipOfframpError::ExecutionReportBufferInvalidLength,
         );
         self.chunk_length = global_chunk_length;
-        self.total_chunks = total_chunks;
+        self.num_chunks = total_chunks;
         self.data.resize(report_length as usize, 0);
 
         Ok(())
@@ -177,7 +182,7 @@ mod tests {
         ExecutionReportBuffer {
             data: vec![],
             chunk_bitmap: 0,
-            total_chunks: 0,
+            num_chunks: 0,
             chunk_length: 0,
             version: 0,
         }

chains/solana/contracts/programs/ccip-offramp/src/instructions/v1/execute.rs

@@ -432,7 +432,7 @@ fn internal_execute<'info>(
 pub struct ExecuteReportContextRemainingAccountsLayout<'a> {
     pub messaging_accounts: Option<ExecuteContextRemainingMessagingAccounts<'a>>,
     pub token_accounts_per_token: Vec<ExecuteContextRemainingTokenAccounts<'a>>,
-    pub buffering_accounts: Option<ExecuteContextRemainingBufferingAccounts<'a>>,
+    pub buffering_account: Option<ExecuteContextRemainingBufferingAccount<'a>>,
 }
 
 pub struct ExecuteContextRemainingMessagingAccounts<'a> {
@@ -446,7 +446,7 @@ pub struct ExecuteContextRemainingTokenAccounts<'a> {
     pub accounts: &'a [AccountInfo<'a>],
 }
 
-pub struct ExecuteContextRemainingBufferingAccounts<'a> {
+pub struct ExecuteContextRemainingBufferingAccount<'a> {
     pub execution_buffer_account: &'a AccountInfo<'a>,
 }
 
@@ -505,10 +505,10 @@ impl<'a> ExecuteReportContextRemainingAccountsLayout<'a> {
 
         // First, if the report is buffered, it means the last account is the buffering account.
         // We can simply remove it from consideration from now on.
-        let (remaining_accounts_without_buffering, buffering_accounts) = if report_is_buffered {
+        let (remaining_accounts_without_buffering, buffering_account) = if report_is_buffered {
             (
                 &remaining_accounts[0..(remaining_accounts.len() - 1)],
-                Some(ExecuteContextRemainingBufferingAccounts {
+                Some(ExecuteContextRemainingBufferingAccount {
                     execution_buffer_account: remaining_accounts
                         .last()
                         .ok_or(CcipOfframpError::ExecutionReportUnavailable)?,
@@ -603,7 +603,7 @@ impl<'a> ExecuteReportContextRemainingAccountsLayout<'a> {
         Ok(Self {
             messaging_accounts,
             token_accounts_per_token,
-            buffering_accounts,
+            buffering_account,
         })
     }
 }

chains/solana/contracts/programs/ccip-offramp/src/lib.rs

@@ -745,6 +745,8 @@ pub enum CcipOfframpError {
     ExecutionReportBufferInvalidLength,
     #[msg("Chunk lies outside the execution report buffer")]
     ExecutionReportBufferInvalidChunkIndex,
+    #[msg("Total number of chunks is not consistent")]
+    ExecutionReportBufferInvalidChunkNumber,
     #[msg("Chunk size is too small")]
     ExecutionReportBufferChunkSizeTooSmall,
     #[msg("Invalid chunk size")]

chains/solana/contracts/programs/ccip-offramp/src/state.rs

@@ -105,7 +105,7 @@ pub struct OnRampAddress {
 pub struct ExecutionReportBuffer {
     pub version: u8,
     pub chunk_bitmap: u64,
-    pub total_chunks: u8,
+    pub num_chunks: u8,
     pub chunk_length: u32,
     #[max_len(0)]
     pub data: Vec<u8>,

chains/solana/contracts/target/idl/ccip_offramp.json

@@ -1134,6 +1134,11 @@
           "isMut": true,
           "isSigner": false
         },
+        {
+          "name": "config",
+          "isMut": false,
+          "isSigner": false
+        },
         {
           "name": "authority",
           "isMut": true,
@@ -1328,7 +1333,7 @@
             "type": "u64"
           },
           {
-            "name": "totalChunks",
+            "name": "numChunks",
             "type": "u8"
           },
           {
@@ -2533,21 +2538,26 @@
     },
     {
       "code": 9061,
+      "name": "ExecutionReportBufferInvalidChunkNumber",
+      "msg": "Total number of chunks is not consistent"
+    },
+    {
+      "code": 9062,
       "name": "ExecutionReportBufferChunkSizeTooSmall",
       "msg": "Chunk size is too small"
     },
     {
-      "code": 9062,
+      "code": 9063,
       "name": "ExecutionReportBufferInvalidChunkSize",
       "msg": "Invalid chunk size"
     },
     {
-      "code": 9063,
+      "code": 9064,
       "name": "ExecutionReportBufferIncomplete",
       "msg": "Execution report buffer is not complete: chunks are missing"
     },
     {
-      "code": 9064,
+      "code": 9065,
       "name": "ExecutionReportUnavailable",
       "msg": "Execution report wasn't provided either directly or via buffer"
     }

chains/solana/contracts/target/types/ccip_offramp.ts

@@ -1134,6 +1134,11 @@ export type CcipOfframp = {
           "isMut": true,
           "isSigner": false
         },
+        {
+          "name": "config",
+          "isMut": false,
+          "isSigner": false
+        },
         {
           "name": "authority",
           "isMut": true,
@@ -1328,7 +1333,7 @@ export type CcipOfframp = {
             "type": "u64"
           },
           {
-            "name": "totalChunks",
+            "name": "numChunks",
             "type": "u8"
           },
           {
@@ -2533,21 +2538,26 @@ export type CcipOfframp = {
     },
     {
       "code": 9061,
+      "name": "ExecutionReportBufferInvalidChunkNumber",
+      "msg": "Total number of chunks is not consistent"
+    },
+    {
+      "code": 9062,
       "name": "ExecutionReportBufferChunkSizeTooSmall",
       "msg": "Chunk size is too small"
     },
     {
-      "code": 9062,
+      "code": 9063,
       "name": "ExecutionReportBufferInvalidChunkSize",
       "msg": "Invalid chunk size"
     },
     {
-      "code": 9063,
+      "code": 9064,
       "name": "ExecutionReportBufferIncomplete",
       "msg": "Execution report buffer is not complete: chunks are missing"
     },
     {
-      "code": 9064,
+      "code": 9065,
       "name": "ExecutionReportUnavailable",
       "msg": "Execution report wasn't provided either directly or via buffer"
     }
@@ -3690,6 +3700,11 @@ export const IDL: CcipOfframp = {
           "isMut": true,
           "isSigner": false
         },
+        {
+          "name": "config",
+          "isMut": false,
+          "isSigner": false
+        },
         {
           "name": "authority",
           "isMut": true,
@@ -3884,7 +3899,7 @@ export const IDL: CcipOfframp = {
             "type": "u64"
           },
           {
-            "name": "totalChunks",
+            "name": "numChunks",
             "type": "u8"
           },
           {
@@ -5089,21 +5104,26 @@ export const IDL: CcipOfframp = {
     },
     {
       "code": 9061,
+      "name": "ExecutionReportBufferInvalidChunkNumber",
+      "msg": "Total number of chunks is not consistent"
+    },
+    {
+      "code": 9062,
       "name": "ExecutionReportBufferChunkSizeTooSmall",
       "msg": "Chunk size is too small"
     },
     {
-      "code": 9062,
+      "code": 9063,
       "name": "ExecutionReportBufferInvalidChunkSize",
       "msg": "Invalid chunk size"
     },
     {
-      "code": 9063,
+      "code": 9064,
       "name": "ExecutionReportBufferIncomplete",
       "msg": "Execution report buffer is not complete: chunks are missing"
     },
     {
-      "code": 9064,
+      "code": 9065,
       "name": "ExecutionReportUnavailable",
       "msg": "Execution report wasn't provided either directly or via buffer"
     }

chains/solana/contracts/tests/ccip/ccip_router_test.go

@@ -6,7 +6,6 @@ import (
 	"encoding/hex"
 	"fmt"
 	"maps"
-	"math"
 	"math/big"
 	"sort"
 	"testing"
@@ -9571,10 +9570,6 @@ func TestCCIPRouter(t *testing.T) {
 				ix = sendChunk(t, 0, chunkSize, 4, data, bufferID, bufferPDA, transmitter)
 				result = testutils.SendAndConfirm(ctx, t, solanaGoClient, []solana.Instruction{ix}, transmitter, config.DefaultCommitment)
 				totalSolSpentInFees += result.Meta.Fee
-				// Checking again after the terminator shuffling
-				ix = sendChunk(t, 3, terminatorSize, 4, data, bufferID, bufferPDA, transmitter)
-				result = testutils.SendAndFailWith(ctx, t, solanaGoClient, []solana.Instruction{ix}, transmitter, config.DefaultCommitment, []string{"Error Code: ExecutionReportBufferAlreadyContainsChunk"})
-				totalSolSpentInFees += result.Meta.Fee
 				ix = sendChunk(t, 1, terminatorSize, 4, data, bufferID, bufferPDA, transmitter)
 				result = testutils.SendAndFailWith(ctx, t, solanaGoClient, []solana.Instruction{ix}, transmitter, config.DefaultCommitment, []string{"Error Code: ExecutionReportBufferInvalidChunkSize"})
 				totalSolSpentInFees += result.Meta.Fee
@@ -9592,19 +9587,12 @@ func TestCCIPRouter(t *testing.T) {
 				rent := (initialLamports - finalLamportsBeforeClose) - totalSolSpentInFees
 
 				// We now close the buffer to recover the rent
-				closeIx, err := ccip_offramp.NewCloseExecutionReportBufferInstruction(bufferID[:], bufferPDA, transmitter.PublicKey(), solana.SystemProgramID).ValidateAndBuild()
+				closeIx, err := ccip_offramp.NewCloseExecutionReportBufferInstruction(bufferID[:], bufferPDA, config.OfframpConfigPDA, transmitter.PublicKey(), solana.SystemProgramID).ValidateAndBuild()
 				require.NoError(t, err)
 				result = testutils.SendAndConfirm(ctx, t, solanaGoClient, []solana.Instruction{closeIx}, transmitter, config.DefaultCommitment)
 				finalLamportsAfterClose := getLamports(transmitter.PublicKey())
 				closeDiff := finalLamportsAfterClose - finalLamportsBeforeClose
-
-				within10PercentOf := func(a int, b int) bool {
-					diff := math.Abs(float64(a - b))
-					return diff <= 0.1*math.Max(float64(a), float64(b))
-				}
-
-				// TODO work out exact math: the rent is being recovered but there's some slight drift in the fees.
-				require.True(t, within10PercentOf(int(rent), int(closeDiff-result.Meta.Fee)))
+				require.Equal(t, rent-result.Meta.Fee, closeDiff)
 			})
 
 			t.Run("When executing a report with oversized data, it fails, but it succeeds when buffered", func(t *testing.T) {
@@ -9794,7 +9782,7 @@ func TestCCIPRouter(t *testing.T) {
 				require.Equal(t, commitReport.MerkleRoot.MaxSeqNr, rootAccount.MaxMsgNr)
 			})
 
-			t.Run("Direct buffering execution, happy path", func(t *testing.T) {
+			t.Run("Buffered execution, happy path", func(t *testing.T) {
 				transmitter := getTransmitter()
 
 				msgAccounts := []solana.PublicKey{config.CcipLogicReceiver, config.ReceiverExternalExecutionConfigPDA, config.ReceiverTargetAccountPDA, solana.SystemProgramID}