Add real cross-implementation diff and genuine migration verification (#226) (#745)

Issue #226 asked for differential testing between contract
implementations, but the existing framework (commit 0169685) only
diffed hello-world against a second instance of itself, despite its
own doc comment claiming to compare "hello-world vs lending". This
strengthens both real gaps found on review:

- Add a ContractAdapter trait and a genuine LendingAdapter (wrapping
  the separate `lending` contract crate) in diff_harness.rs, and a
  new hello_world_vs_lending_test.rs that runs the same deposit/
  zero-amount/position-reflects-balance checks across both
  implementations. Borrow/repay/withdraw are intentionally NOT
  cross-diffed — hello-world borrows against existing collateral,
  while lending's borrow atomically deposits new collateral and
  borrows in one call, requiring collateral_amount > 0 every time.
  Forcing a 1:1 comparison there would misrepresent what's tested;
  documented under "Known Structural Differences".

- Replace the placebo migration test (which only re-created a client
  handle to an untouched contract) with a real one: lending's own
  migration_verification_test.rs now drives the actual UpgradeManager
  governance lifecycle (propose -> approve -> queue timelock ->
  advance ledger -> execute, and separately execute -> rollback) and
  asserts a live lending position is untouched by it. Neither
  hello-world nor lending actually swaps WASM code anywhere in this
  repo (UpgradeManager only tracks an approved hash/version in
  storage), so a true WASM-swap migration test isn't feasible without
  a separate compiled .wasm artifact and build step -- documented as
  a known limitation rather than faked.

- Fix an unrelated but blocking bug found while touching this crate:
  hello-world/src/tests/mod.rs declared four prop-test modules
  (prop_arithmetic_test, prop_interest_test, prop_liquidation_test,
  prop_deposit_test) whose source files were never created for this
  crate -- a hard compile error blocking the entire crate, including
  the differential tests. Removed the phantom declarations (the real
  equivalents already exist in the `lending` crate under different
  names).

- Wire the new tests into the CI differential-test step.

Note: this environment has no Rust/cargo toolchain, so none of this
could be compiled or run locally -- reviewed carefully against
existing, working test code in both crates instead. CI will be the
first real compile.

Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>

Author: soyaya

.github/workflows/ci-cd.yml

@@ -100,7 +100,9 @@ jobs:
         run: |
           cd stellar-lend
           cargo test --package hello-world --lib tests::differential_test -- --nocapture 2>&1 | tee differential-test-report.txt
+          cargo test --package hello-world --lib tests::hello_world_vs_lending_test -- --nocapture 2>&1 | tee -a differential-test-report.txt
           cargo test --package hello-world --lib tests::migration_verification_test -- --nocapture 2>&1 | tee -a differential-test-report.txt
+          cargo test --package stellarlend-lending --lib migration_verification_test -- --nocapture 2>&1 | tee -a differential-test-report.txt
           grep "test result:" differential-test-report.txt
 
       - name: Upload differential test report

stellar-lend/contracts/hello-world/Cargo.toml

@@ -19,3 +19,4 @@ stellar-macros = { version = "0.6.0" }
 
 [dev-dependencies]
 soroban-sdk = { workspace = true, features = ["testutils"] }
+stellarlend-lending = { path = "../lending" }

stellar-lend/contracts/hello-world/DIFFERENTIAL_TEST_REPORT.md

@@ -2,28 +2,36 @@
 
 ## What This Tests
 
-Differential testing runs the **same inputs against two independent contract instances** and asserts their outputs are identical. This catches subtle behavioral regressions that unit tests miss — especially after upgrades or refactors.
+Differential testing runs the **same inputs against two independent contract implementations** and asserts their outputs are identical. This catches subtle behavioral regressions that unit tests miss — especially after upgrades or refactors.
+
+Two flavors are covered:
+
+1. **Same implementation, two instances** (`differential_test.rs`) — regression guard: two fresh `hello-world` instances must always agree.
+2. **Genuinely different implementations** (`hello_world_vs_lending_test.rs`) — compares `hello-world` against the separate `lending` contract crate via a shared `ContractAdapter` trait.
 
 ## Files
 
 | File | Purpose |
 |---|---|
-| `src/tests/diff_harness.rs` | `HwAdapter`, `PositionSnapshot`, `DivergenceReport` — core harness |
-| `src/tests/differential_test.rs` | Property comparison tests (deposit, borrow, repay, zero-amount, sequential) |
-| `src/tests/migration_verification_test.rs` | Storage layout survives upgrade (collateral, debt, admin, multi-user) |
+| `src/tests/diff_harness.rs` | `HwAdapter`, `LendingAdapter`, `ContractAdapter` trait, `PositionSnapshot`, `DivergenceReport` — core harness |
+| `src/tests/differential_test.rs` | Same-implementation property comparison tests (deposit, borrow, repay, zero-amount, sequential) |
+| `src/tests/hello_world_vs_lending_test.rs` | Cross-implementation comparison: `hello-world` vs `lending` (deposit only — see below) |
+| `src/tests/migration_verification_test.rs` | hello-world: storage read-back sanity across a re-created client handle (weak — see "Known Structural Differences") |
+| `../lending/src/migration_verification_test.rs` | lending: drives the *real* `UpgradeManager` governance lifecycle (propose → approve → queue timelock → execute/rollback) and confirms it doesn't disturb lending's own application state |
 
 ## Running Locally
 
 ```bash
 cd stellar-lend
-# All differential tests
 cargo test --package hello-world --lib tests::differential_test -- --nocapture
+cargo test --package hello-world --lib tests::hello_world_vs_lending_test -- --nocapture
 cargo test --package hello-world --lib tests::migration_verification_test -- --nocapture
+cargo test --package stellarlend-lending --lib migration_verification_test -- --nocapture
 ```
 
 ## How Divergences Are Reported
 
-If two instances return different results for the same input, the test panics with:
+If two instances/implementations return different results for the same input, the test panics with:
 
 ```
 [DIVERGENCE] deposit: v1=Ok(true) v2=Err(())
@@ -36,13 +44,30 @@ If two instances return different results for the same input, the test panics wi
 |---|---|
 | Non-deterministic behavior | Ledger timestamp pinned via `env.ledger().set_timestamp()` before each test |
 | State-dependent outputs | Tests run full sequences: deposit → borrow → repay → check position |
-| Zero-amount inputs | Explicit test asserting both instances reject consistently |
-| Storage layout across upgrades | `migration_verification_test.rs` reads raw storage keys via `env.as_contract()` |
+| Zero-amount inputs | Explicit tests asserting both same-implementation instances *and* both cross-implementation contracts reject consistently |
+| Storage layout across upgrades | `migration_verification_test.rs` (hello-world) reads raw storage keys via `env.as_contract()`; `migration_verification_test.rs` (lending) drives the real `UpgradeManager` state machine including its 48h timelock |
 | Multiple users | Multi-user migration test with 5 users and distinct amounts |
+| Performance differences | Not covered — no benchmark comparison between implementations yet |
+
+## Known Structural Differences (why some operations aren't cross-implementation-diffed)
+
+`hello-world` and `lending` have genuinely different domain models, discovered while building the cross-implementation harness:
+
+- **Borrow**: `hello-world.borrow_asset` borrows against previously-deposited collateral. `lending.borrow` is atomic — it deposits new collateral *and* borrows in the same call, and rejects `collateral_amount <= 0`. There is no way to call it "borrow only, against existing collateral" the way hello-world does, so a literal same-inputs comparison would require synthetically inventing a matching action for one side, which would test the harness's own workaround rather than the contracts. **Not compared.**
+- **Asset model**: `hello-world` takes `asset: Option<Address>` (single/native asset). `lending` requires `asset: Address` on every call (multi-asset). The `ContractAdapter` trait picks one fixed `Address` per adapter instance to keep `deposit` comparable.
+- **Position shape**: `hello-world::Position { collateral, debt }` vs `lending::UserPositionSummary { collateral_balance, debt_balance, collateral_value, debt_value, health_factor }`. Only the two directly-equivalent raw balance fields are compared; value/health-factor fields depend on an oracle neither adapter configures.
+
+If `lending`'s API changes to make borrow/repay/withdraw genuinely comparable (e.g. a non-atomic borrow-against-existing-collateral entry point is added), extend `ContractAdapter` and `hello_world_vs_lending_test.rs` accordingly.
+
+## Known Limitation: No Real WASM-Swap Migration Test
+
+Neither `hello-world` nor `lending` currently exposes a real "upgrade this contract's code" entry point — `UpgradeManager` (`common/src/upgrade.rs`, used by `lending`/`amm`/`bridge`) only tracks an *approved* WASM hash + version in storage; it never calls Soroban's `env.deployer().update_current_contract_wasm(..)`. Genuinely testing storage-layout survival across a real code upgrade would require compiling and checking in a separate `.wasm` artifact for an "old" version and loading it via `env.register_contract_wasm(..)` — a build-pipeline addition, not a test-code change, and out of scope here. `lending/src/migration_verification_test.rs` instead verifies the real, available claim: driving the actual governance lifecycle to completion (and to rollback) does not disturb a live position. hello-world's own `migration_verification_test.rs` is weaker (it only re-creates a client handle to an untouched contract) because hello-world doesn't integrate `UpgradeManager` at all.
+
+Separately, `scripts/migration-simulator` and `environments/migration-sandbox` already provide real migration dry-run tooling against a forked network — that's operational tooling, not part of this Rust unit-test suite.
 
 ## CI Integration
 
-Differential tests run as a dedicated CI step in `.github/workflows/ci-cd.yml` and upload `differential-test-report.txt` as an artifact on every push/PR. A failure here means a behavioral regression was introduced.
+Differential and migration-verification tests (both crates) run as a dedicated CI step in `.github/workflows/ci-cd.yml` and upload `differential-test-report.txt` as an artifact on every push/PR. A failure here means a behavioral regression was introduced.
 
 ## Known Acceptable Divergences
 

stellar-lend/contracts/hello-world/src/tests/diff_harness.rs

@@ -11,6 +11,7 @@
 use soroban_sdk::{testutils::Address as _, Address, Env};
 
 use crate::{HelloContract, HelloContractClient};
+use stellarlend_lending::{LendingContract, LendingContractClient};
 
 // ── Shared position snapshot ──────────────────────────────────────────────
 
@@ -106,3 +107,77 @@ impl<'a> HwAdapter<'a> {
         }
     }
 }
+
+// ── Cross-implementation adapter trait ────────────────────────────────────
+//
+// Lets a single set of property tests run against genuinely different
+// contract implementations (not just two instances of the same one).
+// Only `deposit`/`get_position` are part of this trait: hello-world and
+// lending have incompatible domain models for borrow (hello-world borrows
+// against previously-deposited collateral; lending's `borrow` atomically
+// deposits new collateral *and* borrows in the same call, and always
+// requires a positive `collateral_amount`), so forcing a 1:1 comparison
+// there would require synthetic workarounds that misrepresent what's being
+// tested. See "Known Structural Differences" in DIFFERENTIAL_TEST_REPORT.md.
+pub trait ContractAdapter {
+    fn deposit(&self, user: &Address, amount: i128) -> Result<i128, ()>;
+    fn get_position(&self, user: &Address) -> PositionSnapshot;
+}
+
+impl<'a> ContractAdapter for HwAdapter<'a> {
+    fn deposit(&self, user: &Address, amount: i128) -> Result<i128, ()> {
+        HwAdapter::deposit(self, user, amount)
+    }
+
+    fn get_position(&self, user: &Address) -> PositionSnapshot {
+        HwAdapter::get_position(self, user)
+    }
+}
+
+// ── lending adapter — a genuinely separate contract implementation ───────
+
+pub struct LendingAdapter<'a> {
+    pub client: LendingContractClient<'a>,
+    pub env: &'a Env,
+    pub admin: Address,
+    pub asset: Address,
+}
+
+impl<'a> LendingAdapter<'a> {
+    pub fn new(env: &'a Env) -> Self {
+        let contract_id = env.register(LendingContract, ());
+        let client = LendingContractClient::new(env, &contract_id);
+        let admin = Address::generate(env);
+        let asset = Address::generate(env);
+        // debt_ceiling / min_borrow_amount are unused by the deposit-only
+        // comparison but required by `initialize`.
+        client.initialize(&admin, &1_000_000_000_000, &1);
+        Self { client, env, admin, asset }
+    }
+
+    pub fn deposit(&self, user: &Address, amount: i128) -> Result<i128, ()> {
+        self.client
+            .try_deposit_collateral(user, &self.asset, &amount)
+            .map_err(|_| ())
+            .and_then(|r| r.map_err(|_| ()))
+            .map(|_| self.get_position(user).collateral)
+    }
+
+    pub fn get_position(&self, user: &Address) -> PositionSnapshot {
+        let pos = self.client.get_user_position(user);
+        PositionSnapshot {
+            collateral: pos.collateral_balance,
+            debt: pos.debt_balance,
+        }
+    }
+}
+
+impl<'a> ContractAdapter for LendingAdapter<'a> {
+    fn deposit(&self, user: &Address, amount: i128) -> Result<i128, ()> {
+        LendingAdapter::deposit(self, user, amount)
+    }
+
+    fn get_position(&self, user: &Address) -> PositionSnapshot {
+        LendingAdapter::get_position(self, user)
+    }
+}

stellar-lend/contracts/hello-world/src/tests/hello_world_vs_lending_test.rs

@@ -0,0 +1,110 @@
+//! Differential tests comparing two *genuinely different* contract
+//! implementations: `hello-world` (this crate) vs the separate `lending`
+//! crate (`stellarlend_lending::LendingContract`).
+//!
+//! This is the part of issue #226 ("Different contract implementations —
+//! e.g. upgraded vs original") that `differential_test.rs` does not cover:
+//! that file only diffs hello-world against a second instance of itself.
+//!
+//! Scope: only `deposit` + the resulting position are compared. See
+//! "Known Structural Differences" in DIFFERENTIAL_TEST_REPORT.md for why
+//! borrow/repay/withdraw are excluded — the two contracts' domain models
+//! for those operations are not comparable without misrepresenting what's
+//! being tested.
+
+#![cfg(test)]
+
+use soroban_sdk::{testutils::Address as _, Address, Env};
+
+use super::diff_harness::{ContractAdapter, DivergenceReport, HwAdapter, LendingAdapter};
+
+fn make_env() -> Env {
+    let env = Env::default();
+    env.mock_all_auths();
+    env.ledger().set_timestamp(1_000_000);
+    env
+}
+
+fn diff_deposit_cross(
+    a: &dyn ContractAdapter,
+    b: &dyn ContractAdapter,
+    user_a: &Address,
+    user_b: &Address,
+    amount: i128,
+    reports: &mut Vec<DivergenceReport>,
+) {
+    let r1 = a.deposit(user_a, amount).map(|v| v > 0);
+    let r2 = b.deposit(user_b, amount).map(|v| v > 0);
+    if r1 != r2 {
+        reports.push(DivergenceReport::new("cross_deposit", r1, r2));
+    }
+}
+
+fn assert_no_divergences(reports: &[DivergenceReport]) {
+    if !reports.is_empty() {
+        let msgs: Vec<String> = reports
+            .iter()
+            .map(|r| format!("[DIVERGENCE] {}: v1={} v2={}", r.operation, r.v1, r.v2))
+            .collect();
+        panic!("Divergences detected:\n{}", msgs.join("\n"));
+    }
+}
+
+/// A successful deposit must be accepted by both implementations.
+#[test]
+fn test_cross_impl_deposit_accepted_consistently() {
+    let env = make_env();
+    let hw = HwAdapter::new(&env);
+    let lending = LendingAdapter::new(&env);
+    let user_a = Address::generate(&env);
+    let user_b = Address::generate(&env);
+    let mut reports = Vec::new();
+
+    diff_deposit_cross(&hw, &lending, &user_a, &user_b, 1_000_000, &mut reports);
+    assert_no_divergences(&reports);
+}
+
+/// A zero-amount deposit must be rejected by both implementations.
+#[test]
+fn test_cross_impl_zero_deposit_rejected_consistently() {
+    let env = make_env();
+    let hw = HwAdapter::new(&env);
+    let lending = LendingAdapter::new(&env);
+    let user_a = Address::generate(&env);
+    let user_b = Address::generate(&env);
+    let mut reports = Vec::new();
+
+    diff_deposit_cross(&hw, &lending, &user_a, &user_b, 0, &mut reports);
+    assert_no_divergences(&reports);
+}
+
+/// After a successful deposit, both implementations must report non-zero
+/// collateral for that user (exact scale/units are implementation-specific,
+/// so only the "collateral was recorded" property is compared).
+#[test]
+fn test_cross_impl_deposit_reflected_in_position() {
+    let env = make_env();
+    let hw = HwAdapter::new(&env);
+    let lending = LendingAdapter::new(&env);
+    let user_a = Address::generate(&env);
+    let user_b = Address::generate(&env);
+
+    hw.deposit(&user_a, 2_000_000).expect("hello-world deposit should succeed");
+    lending
+        .deposit(&user_b, 2_000_000)
+        .expect("lending deposit should succeed");
+
+    let pos_a = hw.get_position(&user_a);
+    let pos_b = lending.get_position(&user_b);
+
+    assert!(pos_a.collateral > 0, "hello-world collateral must be recorded");
+    assert!(pos_b.collateral > 0, "lending collateral must be recorded");
+    assert_eq!(
+        pos_a.debt, 0,
+        "hello-world debt must remain zero after a deposit-only flow"
+    );
+    assert_eq!(
+        pos_b.debt, 0,
+        "lending debt must remain zero after a deposit-only flow"
+    );
+}

stellar-lend/contracts/hello-world/src/tests/mod.rs

@@ -1,5 +1,6 @@
 pub mod diff_harness;
 pub mod differential_test;
+pub mod hello_world_vs_lending_test;
 pub mod migration_verification_test;
 pub mod access_control_regression_test;
 pub mod admin_test;
@@ -54,9 +55,13 @@ pub mod test_utils;
 pub mod amm_compound_test;
 
 // Property-based tests (proptest)
-pub mod prop_arithmetic_test;
-pub mod prop_interest_test;
-pub mod prop_liquidation_test;
+//
+// prop_arithmetic_test, prop_interest_test, prop_liquidation_test, and
+// prop_deposit_test were declared here by a prior refactor but their source
+// files were never added for this crate (the equivalent property tests live
+// in the separate `lending` crate as borrow_prop_test.rs, interest_rate_prop_test.rs,
+// invariant_prop_test.rs, and deposit_prop_test.rs). Declaring a `mod` for a
+// nonexistent file is a hard compile error, so the phantom declarations are
+// removed rather than left broken.
 pub mod prop_fees_test;
 pub mod prop_supply_cap_test;
-mod prop_deposit_test;

stellar-lend/contracts/lending/src/lib.rs

@@ -111,6 +111,8 @@ mod insurance_test;
 #[cfg(test)]
 mod math_safety_test;
 #[cfg(test)]
+mod migration_verification_test;
+#[cfg(test)]
 mod pause_test;
 #[cfg(test)]
 mod reentrancy_fuzz_test;