From 43454610fd76b6189023879b178252b663e69be9 Mon Sep 17 00:00:00 2001
From: zancas <zancas@zingolabs.org>
Date: Tue, 28 Jul 2026 11:53:06 -0700
Subject: [PATCH] refactor!: de-stutter the migration errors and pin the
 NoSyncData contract

This implements the two suggestions from the #2574 review.

MigrationError::MigrationTransactionFailed and
MigrationError::MigrationConfirmationTimeout repeated the enum's name at
every use site; they become TransactionFailed and ConfirmationTimeout.
The display strings are unchanged, so no user-facing text moves. The
rename is breaking on the public MigrationError enum, exactly as the base
PR's rename was.

A unit test pins the contract #2574 gave get_migration_heights: a wallet
that has never synced returns the typed WalletError::NoSyncData from the
wrapper itself, so every migration site inherits the typed refusal from
one place and the promise in the wrapper's doc comment is enforced rather
than asserted.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
---
 zingolib/src/lightclient/error.rs      |  4 ++--
 zingolib/src/lightclient/migrate.rs    |  4 ++--
 zingolib/src/wallet/migration/split.rs | 30 ++++++++++++++++++++++++++
 3 files changed, 34 insertions(+), 4 deletions(-)

diff --git a/zingolib/src/lightclient/error.rs b/zingolib/src/lightclient/error.rs
index 5a33b92f61..2d4dc161db 100644
--- a/zingolib/src/lightclient/error.rs
+++ b/zingolib/src/lightclient/error.rs
@@ -120,10 +120,10 @@ pub enum MigrationError {
     /// A migration transaction (a note split or a part) failed or
     /// disappeared from the wallet.
     #[error("Migration transaction {0} failed or disappeared.")]
-    MigrationTransactionFailed(TxId),
+    TransactionFailed(TxId),
     /// Migration transactions were not confirmed within the polling window.
     #[error("Timed out waiting for migration transactions to confirm.")]
-    MigrationConfirmationTimeout,
+    ConfirmationTimeout,
     /// The scheduled flow was asked to start over a plan that still needs
     /// note splitting, which no scheduled-flow driver executes yet.
     #[error(
diff --git a/zingolib/src/lightclient/migrate.rs b/zingolib/src/lightclient/migrate.rs
index 999e21aa88..b619205275 100644
--- a/zingolib/src/lightclient/migrate.rs
+++ b/zingolib/src/lightclient/migrate.rs
@@ -2178,14 +2178,14 @@ impl LightClient {
                 )
             };
             if let Some(txid) = failed {
-                return Err(MigrationError::MigrationTransactionFailed(txid).into());
+                return Err(MigrationError::TransactionFailed(txid).into());
             }
             if all_confirmed {
                 return Ok(());
             }
             tokio::time::sleep(CONFIRMATION_POLL_INTERVAL).await;
         }
-        Err(MigrationError::MigrationConfirmationTimeout.into())
+        Err(MigrationError::ConfirmationTimeout.into())
     }
 }
 
diff --git a/zingolib/src/wallet/migration/split.rs b/zingolib/src/wallet/migration/split.rs
index c222a39294..696787f8c6 100644
--- a/zingolib/src/wallet/migration/split.rs
+++ b/zingolib/src/wallet/migration/split.rs
@@ -895,6 +895,36 @@ mod tests {
         assert!(params().sweep_min >= MARGINAL_FEE);
     }
 
+    /// The wrapper's typed contract: `get_migration_heights` turns the
+    /// absent-sync case into [`crate::wallet::error::WalletError::NoSyncData`]
+    /// itself, so every migration site inherits the typed refusal from one
+    /// place. A freshly constructed wallet has never synced, which is
+    /// exactly the case the wrapper must type.
+    #[test]
+    fn migration_heights_on_an_unsynced_wallet_are_typed_no_sync_data() {
+        use zingo_common_components::protocol::ActivationHeights;
+
+        use crate::config::WalletConfig;
+        use crate::testutils::default_test_wallet_settings;
+        use crate::wallet::error::WalletError;
+
+        let wallet = crate::wallet::LightWallet::new(
+            ChainType::Regtest(ActivationHeights::default()),
+            WalletConfig::MnemonicPhrase {
+                mnemonic_phrase: zingo_test_vectors::seeds::HOSPITAL_MUSEUM_SEED.to_string(),
+                no_of_accounts: 1.try_into().expect("hard-coded non-zero"),
+                birthday: 1,
+                wallet_settings: default_test_wallet_settings(),
+            },
+        )
+        .expect("a fresh mnemonic wallet constructs");
+
+        assert!(matches!(
+            wallet.get_migration_heights(),
+            Err(WalletError::NoSyncData)
+        ));
+    }
+
     /// The action-count rule the fee model rests on: an Orchard bundle from
     /// NU6.3 disables cross-address transfers, so a spend and an output no
     /// longer share an action. The crates own that rule and the split fee