Differential Review

Analyze differences between two versions of a MultiversX codebase, focusing on security implications of changes, storage layout compatibility, and upgrade safety.

When to Use

• Reviewing pull requests with contract changes

• Auditing upgrade proposals before deployment

• Analyzing differences between deployed code and proposed updates

• Verifying fix implementations don't introduce regressions

1. Upgradeability Checks (MultiversX-Specific)

Storage Layout Compatibility

CRITICAL: Storage layout changes can corrupt existing data.

Struct Field Ordering

// v1 - Original struct #[derive(TopEncode, TopDecode, TypeAbi)] pub struct UserData { pub balance: BigUint, // Offset 0 pub last_claim: u64, // Offset 1 }

// v2 - DANGEROUS: Reordered fields pub struct UserData { pub last_claim: u64, // Now at Offset 0 - BREAKS EXISTING DATA pub balance: BigUint, // Now at Offset 1 - CORRUPTED }

// v2 - SAFE: Append new fields only pub struct UserData { pub balance: BigUint, // Offset 0 - unchanged pub last_claim: u64, // Offset 1 - unchanged pub new_field: bool, // Offset 2 - NEW (safe) }

Storage Mapper Key Changes

// v1 #[storage_mapper("user_balance")] fn user_balance(&self, user: &ManagedAddress) -> SingleValueMapper<BigUint>;

// v2 - DANGEROUS: Changed storage key #[storage_mapper("userBalance")] // Different key = new empty storage! fn user_balance(&self, user: &ManagedAddress) -> SingleValueMapper<BigUint>;

Initialization on Upgrade

CRITICAL: #[init] is NOT called on upgrade. Only #[upgrade] runs.

// v1 - Original contract #[init] fn init(&self) { self.config().set(DefaultConfig::new()); }

// v2 - Added new storage mapper #[storage_mapper("newFeatureEnabled")] fn new_feature_enabled(&self) -> SingleValueMapper<bool>;

// WRONG: Assuming init runs #[init] fn init(&self) { self.config().set(DefaultConfig::new()); self.new_feature_enabled().set(true); // Never runs on upgrade! }

// CORRECT: Initialize in upgrade #[upgrade] fn upgrade(&self) { self.new_feature_enabled().set(true); // Properly initializes }

Breaking Changes Checklist

Change Type Risk Mitigation

Struct field reorder Critical Never reorder, only append

Storage key rename Critical Keep old key, migrate data

New required storage High Initialize in #[upgrade]

Removed endpoint Medium Ensure no external dependencies

Changed endpoint signature Medium Version API or maintain compatibility

New validation rules Medium Consider existing state validity

2. Regression Analysis

New Features Impact

• Do new features break existing invariants?

• Are there new attack vectors introduced?

• Do gas costs change significantly?

Deleted Code Analysis

When code is removed, verify:

• Was this an intentional security fix?

• Was a validation check removed (potential vulnerability)?

• Are there other code paths that depended on this?

// v1 - Had balance check fn withdraw(&self, amount: BigUint) { require!(amount <= self.balance().get(), "Insufficient balance"); // ... withdrawal logic }

// v2 - Check removed - WHY? fn withdraw(&self, amount: BigUint) { // Missing balance check! Was this intentional? // ... withdrawal logic }

Modified Logic Analysis

For changed code, verify:

• Edge cases still handled correctly

• Error messages updated appropriately

• Related code paths updated consistently

3. Review Workflow

Step 1: Generate Clean Diff

Between git tags/commits

git diff v1.0.0..v2.0.0 -- src/

Ignore formatting changes

git diff -w v1.0.0..v2.0.0 -- src/

Focus on specific file

git diff v1.0.0..v2.0.0 -- src/lib.rs

Step 2: Categorize Changes

Create a change inventory:

Change Summary

Storage Changes

• user_data struct: Added reward_multiplier field (SAFE - appended)
• New mapper: feature_flags (VERIFY: initialized in upgrade)

Endpoint Changes

• deposit(): Added token validation (SECURITY FIX)
• withdraw(): Changed gas calculation (VERIFY: no DoS vector)

Removed Code

• legacy_claim(): Removed entire endpoint (VERIFY: no external callers)

New Code

• batch_transfer(): New endpoint (FULL REVIEW NEEDED)

Step 3: Trace Data Flow

For each changed data structure:

• Find all read locations

• Find all write locations

• Verify consistency across changes

Step 4: Verify Test Coverage

Check if new code paths are tested

sc-meta test

Generate test coverage report

cargo tarpaulin --out Html

4. Security-Specific Diff Checks

Access Control Changes

// v1 - Owner only #[only_owner] #[endpoint] fn sensitive_action(&self) { }

// v2 - DANGEROUS: Removed access control #[endpoint] // Now public! Was this intentional? fn sensitive_action(&self) { }

Payment Handling Changes

// v1 - Validated token #[payable("*")] fn deposit(&self) { let payment = self.call_value().single_esdt(); require!(payment.token_identifier == self.accepted_token().get(), "Wrong token"); }

// v2 - DANGEROUS: Removed validation #[payable("*")] fn deposit(&self) { let payment = self.call_value().single_esdt(); // Missing token validation! Accepts any token now }

Arithmetic Changes

// v1 - Safe arithmetic let result = a.checked_add(&b).unwrap_or_else(|| sc_panic!("Overflow"));

// v2 - DANGEROUS: Removed overflow protection let result = a + b; // Can overflow!

5. Deliverable Template

Differential Review Report

Versions Compared: v1.0.0 → v2.0.0 Reviewer: [Name] Date: [Date]

Summary

[One paragraph overview of changes]

Critical Findings

1. [Finding with severity and recommendation]

Storage Compatibility

• No struct field reordering
• New mappers initialized in #[upgrade]
• Storage keys unchanged

Breaking Changes

Recommendations

1. [Specific actionable recommendation]

Common Pitfalls

• Assuming init runs on upgrade: Always check #[upgrade] function

• Missing storage migration: Renamed keys lose existing data

• Removed validations: Could be intentional security fix or accidental vulnerability

• Changed math precision: Can affect existing calculations

• Modified access control: Could expose sensitive functions