From 772efe1d3a55ce3fe433639e441d425c83756054 Mon Sep 17 00:00:00 2001 From: Sagar Patil Date: Thu, 28 Aug 2025 16:32:05 -0700 Subject: [PATCH 1/3] Fixes #2154 support single change format in restore metadata parsing Handle cases where restore operation returns only one change instead of expected two changes, preventing TxSorobanInvalid errors. --- .../src/commands/contract/restore.rs | 42 ++++++++++++++++--- 1 file changed, 37 insertions(+), 5 deletions(-) diff --git a/cmd/soroban-cli/src/commands/contract/restore.rs b/cmd/soroban-cli/src/commands/contract/restore.rs index 3ee48d43bc..e47a73bcc2 100644 --- a/cmd/soroban-cli/src/commands/contract/restore.rs +++ b/cmd/soroban-cli/src/commands/contract/restore.rs @@ -239,9 +239,40 @@ impl NetworkRunnable for Cmd { } fn parse_changes(changes: &[LedgerEntryChange]) -> Option { - match (&changes[0], &changes[1]) { - ( - LedgerEntryChange::State(_), + match changes.len() { + // Handle case with 2 changes (original expected format) + 2 => match (&changes[0], &changes[1]) { + ( + LedgerEntryChange::State(_), + LedgerEntryChange::Restored(LedgerEntry { + data: + LedgerEntryData::Ttl(TtlEntry { + live_until_ledger_seq, + .. + }), + .. + }) + | LedgerEntryChange::Updated(LedgerEntry { + data: + LedgerEntryData::Ttl(TtlEntry { + live_until_ledger_seq, + .. + }), + .. + }) + | LedgerEntryChange::Created(LedgerEntry { + data: + LedgerEntryData::Ttl(TtlEntry { + live_until_ledger_seq, + .. + }), + .. + }), + ) => Some(*live_until_ledger_seq), + _ => None, + }, + // Handle case with 1 change (single "Restored" type change) + 1 => match &changes[0] { LedgerEntryChange::Restored(LedgerEntry { data: LedgerEntryData::Ttl(TtlEntry { @@ -265,8 +296,9 @@ fn parse_changes(changes: &[LedgerEntryChange]) -> Option { .. }), .. - }), - ) => Some(*live_until_ledger_seq), + }) => Some(*live_until_ledger_seq), + _ => None, + }, _ => None, } } From b865bcb45393c6af5f68bfa1fc91848dc70a1fc7 Mon Sep 17 00:00:00 2001 From: Sagar Patil Date: Fri, 29 Aug 2025 10:55:38 -0700 Subject: [PATCH 2/3] Add tests for the changes --- .../src/commands/contract/restore.rs | 260 ++++++++++++++++++ 1 file changed, 260 insertions(+) diff --git a/cmd/soroban-cli/src/commands/contract/restore.rs b/cmd/soroban-cli/src/commands/contract/restore.rs index e47a73bcc2..8f2e9d998c 100644 --- a/cmd/soroban-cli/src/commands/contract/restore.rs +++ b/cmd/soroban-cli/src/commands/contract/restore.rs @@ -302,3 +302,263 @@ fn parse_changes(changes: &[LedgerEntryChange]) -> Option { _ => None, } } + +#[cfg(test)] +mod tests { + use super::*; + use crate::xdr::{ + LedgerEntry, LedgerEntryChange, LedgerEntryData, TtlEntry, Hash, + }; + + #[test] + fn test_parse_changes_two_changes_restored() { + // Test the original expected format with 2 changes + let ttl_entry = TtlEntry { + live_until_ledger_seq: 12345, + key_hash: Hash([0; 32]), + }; + + let changes = vec![ + LedgerEntryChange::State(LedgerEntry { + data: LedgerEntryData::Ttl(ttl_entry.clone()), + last_modified_ledger_seq: 0, + ext: crate::xdr::LedgerEntryExt::V0, + }), + LedgerEntryChange::Restored(LedgerEntry { + data: LedgerEntryData::Ttl(ttl_entry), + last_modified_ledger_seq: 0, + ext: crate::xdr::LedgerEntryExt::V0, + }), + ]; + + let result = parse_changes(&changes); + assert_eq!(result, Some(12345)); + } + + #[test] + fn test_parse_changes_two_changes_updated() { + // Test the original expected format with 2 changes, but second change is Updated + let ttl_entry = TtlEntry { + live_until_ledger_seq: 67890, + key_hash: Hash([0; 32]), + }; + + let changes = vec![ + LedgerEntryChange::State(LedgerEntry { + data: LedgerEntryData::Ttl(ttl_entry.clone()), + last_modified_ledger_seq: 0, + ext: crate::xdr::LedgerEntryExt::V0, + }), + LedgerEntryChange::Updated(LedgerEntry { + data: LedgerEntryData::Ttl(ttl_entry), + last_modified_ledger_seq: 0, + ext: crate::xdr::LedgerEntryExt::V0, + }), + ]; + + let result = parse_changes(&changes); + assert_eq!(result, Some(67890)); + } + + #[test] + fn test_parse_changes_two_changes_created() { + // Test the original expected format with 2 changes, but second change is Created + let ttl_entry = TtlEntry { + live_until_ledger_seq: 11111, + key_hash: Hash([0; 32]), + }; + + let changes = vec![ + LedgerEntryChange::State(LedgerEntry { + data: LedgerEntryData::Ttl(ttl_entry.clone()), + last_modified_ledger_seq: 0, + ext: crate::xdr::LedgerEntryExt::V0, + }), + LedgerEntryChange::Created(LedgerEntry { + data: LedgerEntryData::Ttl(ttl_entry), + last_modified_ledger_seq: 0, + ext: crate::xdr::LedgerEntryExt::V0, + }), + ]; + + let result = parse_changes(&changes); + assert_eq!(result, Some(11111)); + } + + #[test] + fn test_parse_changes_single_change_restored() { + // Test the new single change format with Restored type + let ttl_entry = TtlEntry { + live_until_ledger_seq: 22222, + key_hash: Hash([0; 32]), + }; + + let changes = vec![ + LedgerEntryChange::Restored(LedgerEntry { + data: LedgerEntryData::Ttl(ttl_entry), + last_modified_ledger_seq: 0, + ext: crate::xdr::LedgerEntryExt::V0, + }), + ]; + + let result = parse_changes(&changes); + assert_eq!(result, Some(22222)); + } + + #[test] + fn test_parse_changes_single_change_updated() { + // Test the new single change format with Updated type + let ttl_entry = TtlEntry { + live_until_ledger_seq: 33333, + key_hash: Hash([0; 32]), + }; + + let changes = vec![ + LedgerEntryChange::Updated(LedgerEntry { + data: LedgerEntryData::Ttl(ttl_entry), + last_modified_ledger_seq: 0, + ext: crate::xdr::LedgerEntryExt::V0, + }), + ]; + + let result = parse_changes(&changes); + assert_eq!(result, Some(33333)); + } + + #[test] + fn test_parse_changes_single_change_created() { + // Test the new single change format with Created type + let ttl_entry = TtlEntry { + live_until_ledger_seq: 44444, + key_hash: Hash([0; 32]), + }; + + let changes = vec![ + LedgerEntryChange::Created(LedgerEntry { + data: LedgerEntryData::Ttl(ttl_entry), + last_modified_ledger_seq: 0, + ext: crate::xdr::LedgerEntryExt::V0, + }), + ]; + + let result = parse_changes(&changes); + assert_eq!(result, Some(44444)); + } + + #[test] + fn test_parse_changes_invalid_two_changes() { + // Test invalid 2-change format (first change is not State) + let ttl_entry = TtlEntry { + live_until_ledger_seq: 55555, + key_hash: Hash([0; 32]), + }; + + let changes = vec![ + LedgerEntryChange::Restored(LedgerEntry { + data: LedgerEntryData::Ttl(ttl_entry.clone()), + last_modified_ledger_seq: 0, + ext: crate::xdr::LedgerEntryExt::V0, + }), + LedgerEntryChange::Restored(LedgerEntry { + data: LedgerEntryData::Ttl(ttl_entry), + last_modified_ledger_seq: 0, + ext: crate::xdr::LedgerEntryExt::V0, + }), + ]; + + let result = parse_changes(&changes); + assert_eq!(result, None); + } + + #[test] + fn test_parse_changes_invalid_single_change() { + // Test invalid single change format (not TTL data) + let changes = vec![ + LedgerEntryChange::Restored(LedgerEntry { + data: LedgerEntryData::Account(crate::xdr::AccountEntry { + account_id: crate::xdr::AccountId(crate::xdr::PublicKey::PublicKeyTypeEd25519( + crate::xdr::Uint256([0; 32]) + )), + balance: 0, + seq_num: SequenceNumber(0), + num_sub_entries: 0, + inflation_dest: None, + flags: 0, + home_domain: crate::xdr::String32::default(), + thresholds: crate::xdr::Thresholds::default(), + signers: crate::xdr::VecM::default(), + ext: crate::xdr::AccountEntryExt::V0, + }), + last_modified_ledger_seq: 0, + ext: crate::xdr::LedgerEntryExt::V0, + }), + ]; + + let result = parse_changes(&changes); + assert_eq!(result, None); + } + + #[test] + fn test_parse_changes_empty_changes() { + // Test empty changes array + let changes = vec![]; + + let result = parse_changes(&changes); + assert_eq!(result, None); + } + + #[test] + fn test_parse_changes_three_changes() { + // Test with 3 changes (should return None) + let ttl_entry = TtlEntry { + live_until_ledger_seq: 66666, + key_hash: Hash([0; 32]), + }; + + let changes = vec![ + LedgerEntryChange::State(LedgerEntry { + data: LedgerEntryData::Ttl(ttl_entry.clone()), + last_modified_ledger_seq: 0, + ext: crate::xdr::LedgerEntryExt::V0, + }), + LedgerEntryChange::Restored(LedgerEntry { + data: LedgerEntryData::Ttl(ttl_entry.clone()), + last_modified_ledger_seq: 0, + ext: crate::xdr::LedgerEntryExt::V0, + }), + LedgerEntryChange::Updated(LedgerEntry { + data: LedgerEntryData::Ttl(ttl_entry), + last_modified_ledger_seq: 0, + ext: crate::xdr::LedgerEntryExt::V0, + }), + ]; + + let result = parse_changes(&changes); + assert_eq!(result, None); + } + + #[test] + fn test_parse_changes_mixed_invalid_types() { + // Test with mixed valid and invalid change types + let ttl_entry = TtlEntry { + live_until_ledger_seq: 77777, + key_hash: Hash([0; 32]), + }; + + let changes = vec![ + LedgerEntryChange::State(LedgerEntry { + data: LedgerEntryData::Ttl(ttl_entry.clone()), + last_modified_ledger_seq: 0, + ext: crate::xdr::LedgerEntryExt::V0, + }), + LedgerEntryChange::State(LedgerEntry { + data: LedgerEntryData::Ttl(ttl_entry), + last_modified_ledger_seq: 0, + ext: crate::xdr::LedgerEntryExt::V0, + }), + ]; + + let result = parse_changes(&changes); + assert_eq!(result, None); + } +} From 9999d9f8485f614c2ef6e9479399f2489a426aed Mon Sep 17 00:00:00 2001 From: Sagar Patil Date: Mon, 1 Sep 2025 13:39:03 -0700 Subject: [PATCH 3/3] fix formatting --- .../src/commands/contract/restore.rs | 116 ++++++++---------- 1 file changed, 53 insertions(+), 63 deletions(-) diff --git a/cmd/soroban-cli/src/commands/contract/restore.rs b/cmd/soroban-cli/src/commands/contract/restore.rs index 8f2e9d998c..adee7600e6 100644 --- a/cmd/soroban-cli/src/commands/contract/restore.rs +++ b/cmd/soroban-cli/src/commands/contract/restore.rs @@ -306,9 +306,7 @@ fn parse_changes(changes: &[LedgerEntryChange]) -> Option { #[cfg(test)] mod tests { use super::*; - use crate::xdr::{ - LedgerEntry, LedgerEntryChange, LedgerEntryData, TtlEntry, Hash, - }; + use crate::xdr::{Hash, LedgerEntry, LedgerEntryChange, LedgerEntryData, TtlEntry}; #[test] fn test_parse_changes_two_changes_restored() { @@ -317,7 +315,7 @@ mod tests { live_until_ledger_seq: 12345, key_hash: Hash([0; 32]), }; - + let changes = vec![ LedgerEntryChange::State(LedgerEntry { data: LedgerEntryData::Ttl(ttl_entry.clone()), @@ -330,7 +328,7 @@ mod tests { ext: crate::xdr::LedgerEntryExt::V0, }), ]; - + let result = parse_changes(&changes); assert_eq!(result, Some(12345)); } @@ -342,7 +340,7 @@ mod tests { live_until_ledger_seq: 67890, key_hash: Hash([0; 32]), }; - + let changes = vec![ LedgerEntryChange::State(LedgerEntry { data: LedgerEntryData::Ttl(ttl_entry.clone()), @@ -355,7 +353,7 @@ mod tests { ext: crate::xdr::LedgerEntryExt::V0, }), ]; - + let result = parse_changes(&changes); assert_eq!(result, Some(67890)); } @@ -367,7 +365,7 @@ mod tests { live_until_ledger_seq: 11111, key_hash: Hash([0; 32]), }; - + let changes = vec![ LedgerEntryChange::State(LedgerEntry { data: LedgerEntryData::Ttl(ttl_entry.clone()), @@ -380,7 +378,7 @@ mod tests { ext: crate::xdr::LedgerEntryExt::V0, }), ]; - + let result = parse_changes(&changes); assert_eq!(result, Some(11111)); } @@ -392,15 +390,13 @@ mod tests { live_until_ledger_seq: 22222, key_hash: Hash([0; 32]), }; - - let changes = vec![ - LedgerEntryChange::Restored(LedgerEntry { - data: LedgerEntryData::Ttl(ttl_entry), - last_modified_ledger_seq: 0, - ext: crate::xdr::LedgerEntryExt::V0, - }), - ]; - + + let changes = vec![LedgerEntryChange::Restored(LedgerEntry { + data: LedgerEntryData::Ttl(ttl_entry), + last_modified_ledger_seq: 0, + ext: crate::xdr::LedgerEntryExt::V0, + })]; + let result = parse_changes(&changes); assert_eq!(result, Some(22222)); } @@ -412,15 +408,13 @@ mod tests { live_until_ledger_seq: 33333, key_hash: Hash([0; 32]), }; - - let changes = vec![ - LedgerEntryChange::Updated(LedgerEntry { - data: LedgerEntryData::Ttl(ttl_entry), - last_modified_ledger_seq: 0, - ext: crate::xdr::LedgerEntryExt::V0, - }), - ]; - + + let changes = vec![LedgerEntryChange::Updated(LedgerEntry { + data: LedgerEntryData::Ttl(ttl_entry), + last_modified_ledger_seq: 0, + ext: crate::xdr::LedgerEntryExt::V0, + })]; + let result = parse_changes(&changes); assert_eq!(result, Some(33333)); } @@ -432,15 +426,13 @@ mod tests { live_until_ledger_seq: 44444, key_hash: Hash([0; 32]), }; - - let changes = vec![ - LedgerEntryChange::Created(LedgerEntry { - data: LedgerEntryData::Ttl(ttl_entry), - last_modified_ledger_seq: 0, - ext: crate::xdr::LedgerEntryExt::V0, - }), - ]; - + + let changes = vec![LedgerEntryChange::Created(LedgerEntry { + data: LedgerEntryData::Ttl(ttl_entry), + last_modified_ledger_seq: 0, + ext: crate::xdr::LedgerEntryExt::V0, + })]; + let result = parse_changes(&changes); assert_eq!(result, Some(44444)); } @@ -452,7 +444,7 @@ mod tests { live_until_ledger_seq: 55555, key_hash: Hash([0; 32]), }; - + let changes = vec![ LedgerEntryChange::Restored(LedgerEntry { data: LedgerEntryData::Ttl(ttl_entry.clone()), @@ -465,7 +457,7 @@ mod tests { ext: crate::xdr::LedgerEntryExt::V0, }), ]; - + let result = parse_changes(&changes); assert_eq!(result, None); } @@ -473,27 +465,25 @@ mod tests { #[test] fn test_parse_changes_invalid_single_change() { // Test invalid single change format (not TTL data) - let changes = vec![ - LedgerEntryChange::Restored(LedgerEntry { - data: LedgerEntryData::Account(crate::xdr::AccountEntry { - account_id: crate::xdr::AccountId(crate::xdr::PublicKey::PublicKeyTypeEd25519( - crate::xdr::Uint256([0; 32]) - )), - balance: 0, - seq_num: SequenceNumber(0), - num_sub_entries: 0, - inflation_dest: None, - flags: 0, - home_domain: crate::xdr::String32::default(), - thresholds: crate::xdr::Thresholds::default(), - signers: crate::xdr::VecM::default(), - ext: crate::xdr::AccountEntryExt::V0, - }), - last_modified_ledger_seq: 0, - ext: crate::xdr::LedgerEntryExt::V0, + let changes = vec![LedgerEntryChange::Restored(LedgerEntry { + data: LedgerEntryData::Account(crate::xdr::AccountEntry { + account_id: crate::xdr::AccountId(crate::xdr::PublicKey::PublicKeyTypeEd25519( + crate::xdr::Uint256([0; 32]), + )), + balance: 0, + seq_num: SequenceNumber(0), + num_sub_entries: 0, + inflation_dest: None, + flags: 0, + home_domain: crate::xdr::String32::default(), + thresholds: crate::xdr::Thresholds::default(), + signers: crate::xdr::VecM::default(), + ext: crate::xdr::AccountEntryExt::V0, }), - ]; - + last_modified_ledger_seq: 0, + ext: crate::xdr::LedgerEntryExt::V0, + })]; + let result = parse_changes(&changes); assert_eq!(result, None); } @@ -502,7 +492,7 @@ mod tests { fn test_parse_changes_empty_changes() { // Test empty changes array let changes = vec![]; - + let result = parse_changes(&changes); assert_eq!(result, None); } @@ -514,7 +504,7 @@ mod tests { live_until_ledger_seq: 66666, key_hash: Hash([0; 32]), }; - + let changes = vec![ LedgerEntryChange::State(LedgerEntry { data: LedgerEntryData::Ttl(ttl_entry.clone()), @@ -532,7 +522,7 @@ mod tests { ext: crate::xdr::LedgerEntryExt::V0, }), ]; - + let result = parse_changes(&changes); assert_eq!(result, None); } @@ -544,7 +534,7 @@ mod tests { live_until_ledger_seq: 77777, key_hash: Hash([0; 32]), }; - + let changes = vec![ LedgerEntryChange::State(LedgerEntry { data: LedgerEntryData::Ttl(ttl_entry.clone()), @@ -557,7 +547,7 @@ mod tests { ext: crate::xdr::LedgerEntryExt::V0, }), ]; - + let result = parse_changes(&changes); assert_eq!(result, None); }