From b95b688a362142243df96b88ff8789c8512a1d14 Mon Sep 17 00:00:00 2001 From: elizabethengelman <4752801+elizabethengelman@users.noreply.github.com> Date: Wed, 10 Sep 2025 11:33:13 -0400 Subject: [PATCH 1/5] Add another parse_changes case to match --- cmd/soroban-cli/src/commands/contract/restore.rs | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/cmd/soroban-cli/src/commands/contract/restore.rs b/cmd/soroban-cli/src/commands/contract/restore.rs index 12bef7ad57..2a9441f0a4 100644 --- a/cmd/soroban-cli/src/commands/contract/restore.rs +++ b/cmd/soroban-cli/src/commands/contract/restore.rs @@ -269,7 +269,17 @@ fn parse_changes(changes: &[LedgerEntryChange]) -> Option { .. }), ) => Some(*live_until_ledger_seq), - _ => None, + ( LedgerEntryChange::Restored(LedgerEntry { + data: + LedgerEntryData::Ttl(TtlEntry { + live_until_ledger_seq, + .. + }), + .. + }), LedgerEntryChange::Restored(_)) => Some(*live_until_ledger_seq), + _ => { + None + }, }, // Handle case with 1 change (single "Restored" type change) 1 => match &changes[0] { From afd7339044eb50956d9743de009559b2ba82aa4f Mon Sep 17 00:00:00 2001 From: elizabethengelman <4752801+elizabethengelman@users.noreply.github.com> Date: Wed, 10 Sep 2025 12:13:37 -0400 Subject: [PATCH 2/5] Temporarily comment out test_parse_changes_invalid_two_changes --- .../src/commands/contract/restore.rs | 48 +++++++++---------- 1 file changed, 24 insertions(+), 24 deletions(-) diff --git a/cmd/soroban-cli/src/commands/contract/restore.rs b/cmd/soroban-cli/src/commands/contract/restore.rs index 2a9441f0a4..944db8f1c3 100644 --- a/cmd/soroban-cli/src/commands/contract/restore.rs +++ b/cmd/soroban-cli/src/commands/contract/restore.rs @@ -447,30 +447,30 @@ mod tests { 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_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() { From cf5b7a8287a59669549fba64a9328ad9ccd43232 Mon Sep 17 00:00:00 2001 From: elizabethengelman <4752801+elizabethengelman@users.noreply.github.com> Date: Thu, 11 Sep 2025 11:00:06 -0400 Subject: [PATCH 3/5] Refactor parse_changes in restore --- .../src/commands/contract/restore.rs | 179 ++++++++---------- 1 file changed, 81 insertions(+), 98 deletions(-) diff --git a/cmd/soroban-cli/src/commands/contract/restore.rs b/cmd/soroban-cli/src/commands/contract/restore.rs index 944db8f1c3..6fa9ef6ab6 100644 --- a/cmd/soroban-cli/src/commands/contract/restore.rs +++ b/cmd/soroban-cli/src/commands/contract/restore.rs @@ -239,51 +239,22 @@ impl NetworkRunnable for Cmd { } fn parse_changes(changes: &[LedgerEntryChange]) -> Option { - 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), - ( LedgerEntryChange::Restored(LedgerEntry { - data: - LedgerEntryData::Ttl(TtlEntry { - live_until_ledger_seq, - .. - }), - .. - }), LedgerEntryChange::Restored(_)) => Some(*live_until_ledger_seq), - _ => { - None - }, - }, - // Handle case with 1 change (single "Restored" type change) - 1 => match &changes[0] { - LedgerEntryChange::Restored(LedgerEntry { + if changes.len() == 3 { + return None; + } + + changes + .iter() + .filter_map(|change| match change { + LedgerEntryChange::State(LedgerEntry { + data: + LedgerEntryData::Ttl(TtlEntry { + live_until_ledger_seq, + .. + }), + .. + }) + | LedgerEntryChange::Restored(LedgerEntry { data: LedgerEntryData::Ttl(TtlEntry { live_until_ledger_seq, @@ -308,15 +279,17 @@ fn parse_changes(changes: &[LedgerEntryChange]) -> Option { .. }) => Some(*live_until_ledger_seq), _ => None, - }, - _ => None, - } + }).max() } #[cfg(test)] mod tests { use super::*; - use crate::xdr::{Hash, LedgerEntry, LedgerEntryChange, LedgerEntryData, TtlEntry}; + use crate::xdr::{ + ContractDataDurability::Persistent, ContractDataEntry, ContractId, Hash, LedgerEntry, + LedgerEntryChange, LedgerEntryData, ScAddress, ScSymbol, ScVal, SequenceNumber, StringM, + TtlEntry, + }; #[test] fn test_parse_changes_two_changes_restored() { @@ -343,6 +316,39 @@ mod tests { assert_eq!(result, Some(12345)); } + #[test] + fn test_parse_two_changes_that_had_expired() { + let ttl_entry = TtlEntry { + live_until_ledger_seq: 55555, + key_hash: Hash([0; 32]), + }; + + let counter = "COUNTER".parse::>().unwrap().into(); + let contract_data_entry = ContractDataEntry { + ext: ExtensionPoint::default(), + contract: ScAddress::Contract(ContractId(Hash([0; 32]))), + key: ScVal::Symbol(ScSymbol(counter)), + durability: Persistent, + val: ScVal::U32(1), + }; + + let changes = vec![ + LedgerEntryChange::Restored(LedgerEntry { + data: LedgerEntryData::Ttl(ttl_entry.clone()), + last_modified_ledger_seq: 37429, + ext: crate::xdr::LedgerEntryExt::V0, + }), + LedgerEntryChange::Restored(LedgerEntry { + data: LedgerEntryData::ContractData(contract_data_entry.clone()), + last_modified_ledger_seq: 37429, + ext: crate::xdr::LedgerEntryExt::V0, + }), + ]; + + let result = parse_changes(&changes); + assert_eq!(result, Some(55555)); + } + #[test] fn test_parse_changes_two_changes_updated() { // Test the original expected format with 2 changes, but second change is Updated @@ -447,30 +453,32 @@ mod tests { 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_two_changes() { + // Test invalid 2-change format (not TTL data) + let not_ttl_change = 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![not_ttl_change.clone(), not_ttl_change]; + let result = parse_changes(&changes); + assert_eq!(result, None); + } #[test] fn test_parse_changes_invalid_single_change() { @@ -536,29 +544,4 @@ mod tests { 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 8a64ec1ca01c0b81462780d0891f679aeb05b1d0 Mon Sep 17 00:00:00 2001 From: elizabethengelman <4752801+elizabethengelman@users.noreply.github.com> Date: Thu, 11 Sep 2025 11:31:37 -0400 Subject: [PATCH 4/5] Fmt & clippy --- cmd/soroban-cli/src/commands/contract/restore.rs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/cmd/soroban-cli/src/commands/contract/restore.rs b/cmd/soroban-cli/src/commands/contract/restore.rs index 6fa9ef6ab6..91d91f7c48 100644 --- a/cmd/soroban-cli/src/commands/contract/restore.rs +++ b/cmd/soroban-cli/src/commands/contract/restore.rs @@ -279,7 +279,8 @@ fn parse_changes(changes: &[LedgerEntryChange]) -> Option { .. }) => Some(*live_until_ledger_seq), _ => None, - }).max() + }) + .max() } #[cfg(test)] @@ -323,7 +324,7 @@ mod tests { key_hash: Hash([0; 32]), }; - let counter = "COUNTER".parse::>().unwrap().into(); + let counter = "COUNTER".parse::>().unwrap(); let contract_data_entry = ContractDataEntry { ext: ExtensionPoint::default(), contract: ScAddress::Contract(ContractId(Hash([0; 32]))), From 7bd5c862ae29be7de9f244480fdd8d993674db8f Mon Sep 17 00:00:00 2001 From: elizabethengelman <4752801+elizabethengelman@users.noreply.github.com> Date: Thu, 11 Sep 2025 14:46:28 -0400 Subject: [PATCH 5/5] Address PR feedback --- .../src/commands/contract/restore.rs | 44 +------------------ 1 file changed, 1 insertion(+), 43 deletions(-) diff --git a/cmd/soroban-cli/src/commands/contract/restore.rs b/cmd/soroban-cli/src/commands/contract/restore.rs index 91d91f7c48..93c598ad6b 100644 --- a/cmd/soroban-cli/src/commands/contract/restore.rs +++ b/cmd/soroban-cli/src/commands/contract/restore.rs @@ -239,22 +239,10 @@ impl NetworkRunnable for Cmd { } fn parse_changes(changes: &[LedgerEntryChange]) -> Option { - if changes.len() == 3 { - return None; - } - changes .iter() .filter_map(|change| match change { - LedgerEntryChange::State(LedgerEntry { - data: - LedgerEntryData::Ttl(TtlEntry { - live_until_ledger_seq, - .. - }), - .. - }) - | LedgerEntryChange::Restored(LedgerEntry { + LedgerEntryChange::Restored(LedgerEntry { data: LedgerEntryData::Ttl(TtlEntry { live_until_ledger_seq, @@ -515,34 +503,4 @@ mod tests { 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); - } }