From 5911ce2f6a4be56fb93b0503e51413e0ba8abf57 Mon Sep 17 00:00:00 2001 From: lightsing Date: Thu, 19 Mar 2026 14:55:19 +0800 Subject: [PATCH 1/3] fix retain, replace by filter --- crates/cli/src/commands/purge.rs | 39 ++-- crates/core/src/codec/v1/digest.rs | 2 +- crates/core/src/codec/v1/timestamp.rs | 308 +++++++++++--------------- packages/sdk-rs/src/purge.rs | 157 ++++++------- 4 files changed, 208 insertions(+), 298 deletions(-) diff --git a/crates/cli/src/commands/purge.rs b/crates/cli/src/commands/purge.rs index 3c66bcc..a0a9122 100644 --- a/crates/cli/src/commands/purge.rs +++ b/crates/cli/src/commands/purge.rs @@ -1,7 +1,10 @@ use clap::Args; use std::{collections::HashSet, path::PathBuf}; use tracing::{error, info, warn}; -use uts_core::codec::{Decode, Encode, VersionedProof, v1::DetachedTimestamp}; +use uts_core::codec::{ + Decode, Encode, VersionedProof, + v1::{Attestation, DetachedTimestamp, PendingAttestation}, +}; use uts_sdk::Sdk; #[derive(Debug, Args)] @@ -26,9 +29,13 @@ impl Purge { async fn purge_one(&self, path: &PathBuf) -> eyre::Result<()> { let file = tokio::fs::read(path).await?; - let mut proof = VersionedProof::::decode(&mut &*file)?; + let proof = VersionedProof::::decode(&mut &*file)?; - let pending = Sdk::list_pending(&proof); + let pending = proof + .attestations() + .filter(|att| att.tag == PendingAttestation::TAG) + .map(|att| PendingAttestation::from_raw(att).map(|p| p.uri)) + .collect::, _>>()?; if pending.is_empty() { info!( "[{}] no pending attestations found, skipping", @@ -48,7 +55,7 @@ impl Purge { let uris_to_purge = if self.yes { // Purge all when --yes flag is used - None + pending.into_iter().collect() } else { // Interactive selection print!("Enter numbers to purge (comma-separated), 'all', or 'none' to skip: "); @@ -64,7 +71,7 @@ impl Purge { } if input.eq_ignore_ascii_case("all") { - None + pending.into_iter().collect() } else { let mut selected = HashSet::new(); for part in input.split(',') { @@ -82,32 +89,28 @@ impl Purge { info!("[{}] no valid selections, skipping", path.display()); return Ok(()); } - Some(selected) + selected } }; - let result = Sdk::purge_pending_by_uris(&mut proof, uris_to_purge.as_ref()); + let Some(result) = Sdk::filter_pending_by_uris(&proof, |uri| uris_to_purge.contains(uri)) + else { + error!("won't purge [{}], results in empty proof", path.display()); + return Ok(()); + }; if result.purged.is_empty() { info!("[{}] nothing to purge", path.display()); return Ok(()); } - if !result.has_remaining { - warn!( - "[{}] purging would leave no attestations in the file, skipping", - path.display() - ); - return Ok(()); - } - let mut buf = Vec::new(); - proof.encode(&mut buf)?; + VersionedProof::new(result.new_stamp).encode(&mut buf)?; tokio::fs::write(path, buf).await?; info!( - "[{}] purged {} pending attestation(s)", + "purged {} pending attestation(s) from [{}] ", + result.purged.len(), path.display(), - result.purged.len() ); Ok(()) } diff --git a/crates/core/src/codec/v1/digest.rs b/crates/core/src/codec/v1/digest.rs index eeab95b..4c4f0d7 100644 --- a/crates/core/src/codec/v1/digest.rs +++ b/crates/core/src/codec/v1/digest.rs @@ -11,7 +11,7 @@ use core::fmt; use digest::{Output, typenum::Unsigned}; /// Header describing the digest that anchors a timestamp. -#[derive(Clone, Default, PartialEq, Eq, PartialOrd, Ord, Hash)] +#[derive(Copy, Clone, Default, PartialEq, Eq, PartialOrd, Ord, Hash)] #[cfg_attr( feature = "serde", serde_with::serde_as, diff --git a/crates/core/src/codec/v1/timestamp.rs b/crates/core/src/codec/v1/timestamp.rs index 02472a3..756a957 100644 --- a/crates/core/src/codec/v1/timestamp.rs +++ b/crates/core/src/codec/v1/timestamp.rs @@ -10,7 +10,7 @@ use crate::{ }; use allocator_api2::SliceExt; use core::fmt::Debug; -use std::sync::OnceLock; +use std::{ops::Not, sync::OnceLock}; pub(crate) mod builder; mod decode; @@ -156,153 +156,23 @@ impl Timestamp { AttestationIter { stack: vec![self] } } - /// Iterates over all pending attestation steps in this timestamp. + /// Iterates over all attestation steps in this timestamp. /// /// # Note /// - /// This iterator will yield `Timestamp` instead of `RawAttestation`. + /// This iterator will yield `Timestamp` instead of `RawAttestation` #[inline] - pub fn pending_attestations_mut(&mut self) -> PendingAttestationIterMut<'_, A> { - PendingAttestationIterMut { stack: vec![self] } - } - - /// Retains only the attestations for which the predicate returns `true`, - /// removing all others from this timestamp tree. - /// - /// The predicate receives each [`RawAttestation`] and should return `true` - /// to keep the attestation or `false` to remove it. This is analogous to - /// [`Vec::retain`] but operates on the attestation leaves of the timestamp - /// tree. - /// - /// Returns `Some(count)` where `count` is the number of attestations removed, - /// or `None` if the entire timestamp would be empty after filtering (all - /// attestations were removed). - /// - /// When a FORK node is left with only one remaining branch after filtering, - /// it is collapsed into that branch to maintain the invariant that FORKs - /// have at least two children. - pub fn retain_attestations(&mut self, mut f: F) -> Option - where - F: FnMut(&RawAttestation) -> bool, - { - self.retain_attestations_inner(&mut f) - } - - fn retain_attestations_inner(&mut self, f: &mut F) -> Option - where - F: FnMut(&RawAttestation) -> bool, - { - // Phase 1: recursively filter children and compute result - let (removed_count, should_collapse) = match self { - Timestamp::Attestation(attestation) => { - return if f(attestation) { Some(0) } else { None }; - } - Timestamp::Step(step) if step.op == OpCode::FORK => { - let mut removed = 0usize; - step.next - .retain_mut(|child| match child.retain_attestations_inner(f) { - None => { - removed += 1; - false - } - Some(count) => { - removed += count; - true - } - }); - - if step.next.is_empty() { - return None; - } - - (removed, step.next.len() == 1) - } - Timestamp::Step(step) => { - debug_assert!(step.next.len() == 1, "non-FORK must have exactly one child"); - return step.next[0].retain_attestations_inner(f); - } - }; - - // Phase 2: collapse single-branch FORK - if should_collapse && let Timestamp::Step(step) = self { - let remaining = step.next.pop().unwrap(); - *self = remaining; - } - - Some(removed_count) - } - - /// Retains only the attestations for which the predicate returns `true`, - /// removing all others from this timestamp tree. - /// - /// Unlike [`retain_attestations`](Self::retain_attestations), this variant - /// provides mutable access to each [`RawAttestation`] in the predicate, - /// allowing in-place modification of attestations during filtering. - /// - /// Returns `Some(count)` where `count` is the number of attestations removed, - /// or `None` if the entire timestamp would be empty after filtering. - /// - /// When a FORK node is left with only one remaining branch after filtering, - /// it is collapsed into that branch to maintain the invariant that FORKs - /// have at least two children. - pub fn retain_attestations_mut(&mut self, mut f: F) -> Option - where - F: FnMut(&mut RawAttestation) -> bool, - { - self.retain_attestations_mut_inner(&mut f) + pub fn attestations_mut(&mut self) -> AttestationIterMut<'_, A> { + AttestationIterMut { stack: vec![self] } } - fn retain_attestations_mut_inner(&mut self, f: &mut F) -> Option - where - F: FnMut(&mut RawAttestation) -> bool, - { - let (removed_count, should_collapse) = match self { - Timestamp::Attestation(attestation) => { - return if f(attestation) { Some(0) } else { None }; - } - Timestamp::Step(step) if step.op == OpCode::FORK => { - let mut removed = 0usize; - step.next - .retain_mut(|child| match child.retain_attestations_mut_inner(f) { - None => { - removed += 1; - false - } - Some(count) => { - removed += count; - true - } - }); - - if step.next.is_empty() { - return None; - } - - (removed, step.next.len() == 1) - } - Timestamp::Step(step) => { - debug_assert!(step.next.len() == 1, "non-FORK must have exactly one child"); - return step.next[0].retain_attestations_mut_inner(f); - } - }; - - if should_collapse && let Timestamp::Step(step) = self { - let remaining = step.next.pop().unwrap(); - *self = remaining; - } - - Some(removed_count) - } - - /// Purges all pending attestations from this timestamp tree. - /// - /// This is a convenience wrapper around [`retain_attestations`](Self::retain_attestations) - /// that removes all attestations tagged as pending. + /// Iterates over all pending attestation steps in this timestamp. /// - /// Returns `Some(count)` where `count` is the number of pending attestations removed, - /// or `None` if the entire timestamp consists only of pending attestations. - pub fn purge_pending(&mut self) -> Option { - self.retain_attestations(|att| att.tag != PendingAttestation::TAG) + /// This is a shorthand by calling `attestations_mut` + #[inline] + pub fn pending_attestations_mut(&mut self) -> impl Iterator> + '_ { + self.attestations_mut() + .filter(|ts| ts.as_attestation().expect("infallible").tag == PendingAttestation::TAG) } } @@ -402,6 +272,72 @@ impl Timestamp { next: timestamps, })) } + + /// Make a copy of the timestamp, by filtering the attestations for which the predicate + /// returns `true`. + /// + /// The predicate receives each [`RawAttestation`] and should return `false` to filter the + /// attestation or `true` to keep it. + /// + /// Returns `Some(timestamp)` where `timestamp` is the modified copy of the original one, + /// or `None` if the entire timestamp would be empty after filtering. + /// + /// When a FORK node is left with only one remaining branch after filtering, + /// it is collapsed into that branch to maintain the invariant that FORKs + /// have at least two children. + pub fn filter_attestations(&self, mut f: F) -> Option> + where + F: FnMut(&RawAttestation) -> bool, + { + let mut this = self.clone(); + + // This only be called when it's in root level + if let Timestamp::Attestation(attestation) = &this { + return if f(attestation) { Some(this) } else { None }; + } + + this.filter_attestations_inner(&mut f).not().then_some(this) + } + + /// Returns `true` if this branch should be filtered + fn filter_attestations_inner(&mut self, f: &mut F) -> bool + where + F: FnMut(&RawAttestation) -> bool, + { + match self { + Timestamp::Step(step) if step.op == OpCode::FORK => { + step.next + .retain_mut(|child| !child.filter_attestations_inner(f)); + if step.next.is_empty() { + return true; + } + + // collapse + if step.next.len() == 1 { + let remaining = step.next.pop().unwrap(); + *self = remaining; + } + false + } + Timestamp::Step(step) => { + debug_assert!(step.next.len() == 1, "non-FORK must have exactly one child"); + step.next[0].filter_attestations_inner(f) + } + + Timestamp::Attestation(attestation) => f(attestation), + } + } + + /// Purges all pending attestations from this timestamp tree. + /// + /// This is a convenience wrapper around [`retain_attestations`](Self::filter_attestations) + /// that removes all attestations tagged as pending. + /// + /// Returns `Some(count)` where `count` is the number of pending attestations removed, + /// or `None` if the entire timestamp consists only of pending attestations. + pub fn purge_pending(&self) -> Option> { + self.filter_attestations(|att| att.tag == PendingAttestation::TAG) + } } impl MayHaveInput for Timestamp { @@ -471,11 +407,11 @@ impl<'a, A: Allocator> Iterator for AttestationIter<'a, A> { } } -pub struct PendingAttestationIterMut<'a, A: Allocator> { +pub struct AttestationIterMut<'a, A: Allocator> { stack: Vec<&'a mut Timestamp>, } -impl<'a, A: Allocator> Iterator for PendingAttestationIterMut<'a, A> { +impl<'a, A: Allocator> Iterator for AttestationIterMut<'a, A> { type Item = &'a mut Timestamp; fn next(&mut self) -> Option { @@ -486,11 +422,7 @@ impl<'a, A: Allocator> Iterator for PendingAttestationIterMut<'a, A> { self.stack.push(next); } } - Timestamp::Attestation(attestation) => { - if attestation.tag == PendingAttestation::TAG { - return Some(ts); - } - } + Timestamp::Attestation(_) => return Some(ts), } } None @@ -522,7 +454,7 @@ mod tests { #[test] fn purge_pending_single_pending() { - let mut ts = make_pending("https://example.com"); + let ts = make_pending("https://example.com"); assert!( ts.purge_pending().is_none(), "all-pending should return None" @@ -531,8 +463,8 @@ mod tests { #[test] fn purge_pending_single_confirmed() { - let mut ts = make_bitcoin(100); - assert_eq!(ts.purge_pending(), Some(0)); + let ts = make_bitcoin(100); + assert_eq!(ts, ts.purge_pending().unwrap()); } #[test] @@ -540,13 +472,12 @@ mod tests { // FORK with one pending and one confirmed branch let pending = make_pending("https://example.com"); let confirmed = make_bitcoin(100); - let mut ts = Timestamp::merge(alloc_vec![pending, confirmed]); + let ts = Timestamp::merge(alloc_vec![pending, confirmed]); - let result = ts.purge_pending(); - assert_eq!(result, Some(1)); + let result = ts.purge_pending().unwrap(); // After purge, the FORK should be collapsed since only 1 branch remains assert!( - !matches!(ts, Timestamp::Step(ref s) if s.op == OpCode::FORK), + !matches!(result, Timestamp::Step(ref s) if s.op == OpCode::FORK), "FORK with 1 branch should be collapsed" ); } @@ -555,7 +486,7 @@ mod tests { fn purge_pending_fork_all_pending() { let p1 = make_pending("https://a.example.com"); let p2 = make_pending("https://b.example.com"); - let mut ts = Timestamp::merge(alloc_vec![p1, p2]); + let ts = Timestamp::merge(alloc_vec![p1, p2]); assert!(ts.purge_pending().is_none()); } @@ -564,11 +495,8 @@ mod tests { fn purge_pending_fork_all_confirmed() { let c1 = make_bitcoin(100); let c2 = make_bitcoin(200); - let mut ts = Timestamp::merge(alloc_vec![c1, c2]); - - assert_eq!(ts.purge_pending(), Some(0)); - // FORK should remain since both branches are kept - assert!(matches!(ts, Timestamp::Step(ref s) if s.op == OpCode::FORK)); + let ts = Timestamp::merge(alloc_vec![c1, c2]); + assert_eq!(ts, ts.purge_pending().unwrap()); } #[test] @@ -578,12 +506,25 @@ mod tests { let inner_confirmed = make_bitcoin(100); let inner_fork = Timestamp::merge(alloc_vec![inner_pending, inner_confirmed]); let outer_confirmed = make_bitcoin(200); - let mut ts = Timestamp::merge(alloc_vec![inner_fork, outer_confirmed]); + let ts = Timestamp::merge(alloc_vec![inner_fork, outer_confirmed]); - let result = ts.purge_pending(); - assert_eq!(result, Some(1)); + let result = ts.purge_pending().unwrap(); // Outer FORK remains (2 branches), inner FORK collapsed - assert!(matches!(ts, Timestamp::Step(ref s) if s.op == OpCode::FORK)); + assert!(matches!(result, Timestamp::Step(ref s) if s.op == OpCode::FORK)); + } + + #[test] + fn purge_attestations_complex() { + let pending1 = make_pending("https://example1.com"); + let pending2 = make_pending("https://example2.com"); + let mut ts = Timestamp::builder(); + ts.keccak256(); + let ts = ts.concat(Timestamp::merge(alloc_vec![pending1, pending2])); + let confirmed = make_bitcoin(100); + let ts = Timestamp::merge(alloc_vec![ts, confirmed]); + + let result = ts.purge_pending().unwrap(); + assert!(matches!(result, Timestamp::Attestation(a) if a.tag == BitcoinAttestation::TAG)) } #[test] @@ -592,32 +533,30 @@ mod tests { let p1 = make_pending("https://a.example.com"); let p2 = make_pending("https://b.example.com"); let confirmed = make_bitcoin(100); - let mut ts = Timestamp::merge(alloc_vec![p1, p2, confirmed]); + let ts = Timestamp::merge(alloc_vec![p1, p2, confirmed]); // Retain confirmed + second pending, removing first pending - let result = ts.retain_attestations(|att| { - if att.tag != PendingAttestation::TAG { - return true; - } - let p = PendingAttestation::from_raw(att).unwrap(); - p.uri != "https://a.example.com" - }); - assert_eq!(result, Some(1)); + let result = ts + .filter_attestations(|att| { + if att.tag != PendingAttestation::TAG { + return false; + } + let p = PendingAttestation::from_raw(att).unwrap(); + p.uri == "https://a.example.com" + }) + .unwrap(); // FORK should remain since 2 branches are still present (p2 + confirmed) - assert!(matches!(ts, Timestamp::Step(ref s) if s.op == OpCode::FORK)); + assert!(matches!(result, Timestamp::Step(ref s) if s.op == OpCode::FORK)); } #[test] fn retain_attestations_keep_all() { let p1 = make_pending("https://a.example.com"); let confirmed = make_bitcoin(100); - let mut ts = Timestamp::merge(alloc_vec![p1, confirmed]); + let ts = Timestamp::merge(alloc_vec![p1, confirmed]); // Retain everything - let result = ts.retain_attestations(|_| true); - assert_eq!(result, Some(0)); - // FORK should remain unchanged - assert!(matches!(ts, Timestamp::Step(ref s) if s.op == OpCode::FORK)); + assert_eq!(ts, ts.filter_attestations(|_| false).unwrap()); } #[test] @@ -625,12 +564,13 @@ mod tests { // Test removing confirmed attestations (not just pending) let pending = make_pending("https://example.com"); let confirmed = make_bitcoin(100); - let mut ts = Timestamp::merge(alloc_vec![pending, confirmed]); + let ts = Timestamp::merge(alloc_vec![pending, confirmed]); // Remove bitcoin attestations, keep pending - let result = ts.retain_attestations(|att| att.tag == PendingAttestation::TAG); - assert_eq!(result, Some(1)); + let result = ts + .filter_attestations(|att| att.tag != PendingAttestation::TAG) + .unwrap(); // FORK collapsed since only 1 branch remains - assert!(!matches!(ts, Timestamp::Step(ref s) if s.op == OpCode::FORK)); + assert!(!matches!(result, Timestamp::Step(ref s) if s.op == OpCode::FORK)); } } diff --git a/packages/sdk-rs/src/purge.rs b/packages/sdk-rs/src/purge.rs index 6f99a61..fdd194b 100644 --- a/packages/sdk-rs/src/purge.rs +++ b/packages/sdk-rs/src/purge.rs @@ -1,114 +1,81 @@ -use std::collections::HashSet; -use tracing::info; +use crate::Sdk; use uts_core::{ - alloc::Allocator, + alloc::{Allocator, vec::Vec}, codec::v1::{Attestation, DetachedTimestamp, PendingAttestation}, }; -use crate::Sdk; - -/// Result of a purge operation on a detached timestamp. +/// Represents the result of filtering pending attestations from a detached timestamp. #[derive(Debug)] -pub struct PurgeResult { - /// URIs of the pending attestations that were purged. - pub purged: Vec, - /// Whether the timestamp still has any attestations remaining. - pub has_remaining: bool, +pub struct PurgeResult { + /// URIs of the pending attestations that were excluded (purged) during the filtering process. + pub purged: Vec, + /// A new detached timestamp instance containing only the retained attestations. + pub new_stamp: DetachedTimestamp, } impl Sdk { - /// Lists all pending attestation URIs in the given detached timestamp. - pub fn list_pending(stamp: &DetachedTimestamp) -> Vec { - stamp - .attestations() - .filter_map(|att| { - PendingAttestation::from_raw(att) - .ok() - .map(|p| p.uri.to_string()) - }) - .collect() - } - - /// Purges all pending attestations from the given detached timestamp. + /// Filters out all pending attestations from the given detached timestamp. + /// + /// This method creates a new `DetachedTimestamp` excluding all entries tagged as `Pending`. + /// The original `stamp` remains unchanged. /// - /// Returns a [`PurgeResult`] containing the URIs of purged attestations - /// and whether the timestamp still has remaining (non-pending) attestations. + /// # Returns /// - /// If all attestations were pending, the timestamp becomes invalid and - /// `has_remaining` will be `false` — callers should handle this case - /// (e.g., by not writing the file). - pub fn purge_pending(stamp: &mut DetachedTimestamp) -> PurgeResult { - Self::purge_pending_by_uris(stamp, None) + /// Returns `Some(PurgeResult)` if results in a valid timestamp, otherwise returns `None`. + pub fn filter_pending( + stamp: &DetachedTimestamp, + ) -> Option> { + Self::filter_pending_by_uris(stamp, |_| true) } - /// Purges selected pending attestations from the given detached timestamp. + /// Filters pending attestations from the given detached timestamp based on a predicate. /// - /// If `uris_to_purge` is `None`, all pending attestations are purged. - /// If `uris_to_purge` is `Some(set)`, only pending attestations whose URI - /// is in the set are purged. + /// This function iterates over the attestations in `stamp`. For each attestation tagged as + /// `Pending`, it attempts to decode the URI and applies the `predicate`. + /// - If the predicate returns `true`, the attestation is excluded from the new timestamp. + /// - If the predicate returns `false`, or if the attestation is not pending, it is retained. /// - /// This is implemented using [`Timestamp::retain_attestations`] under the hood. + /// # Arguments /// - /// Returns a [`PurgeResult`] containing the URIs of purged attestations - /// and whether the timestamp still has remaining (non-pending) attestations. - pub fn purge_pending_by_uris( - stamp: &mut DetachedTimestamp, - uris_to_purge: Option<&HashSet>, - ) -> PurgeResult { - let pending_uris = Self::list_pending(stamp); - - if pending_uris.is_empty() { - info!("no pending attestations found"); - return PurgeResult { - purged: Vec::new(), - has_remaining: true, - }; - } - - let purged_uris: Vec = match &uris_to_purge { - Some(set) => pending_uris - .iter() - .filter(|u| set.contains(*u)) - .cloned() - .collect(), - None => pending_uris, - }; - - if purged_uris.is_empty() { - info!("no matching pending attestations to purge"); - return PurgeResult { - purged: Vec::new(), - has_remaining: true, - }; - } + /// * `stamp` - A reference to the source `DetachedTimestamp`. + /// * `predicate` - A closure that determines whether a specific pending attestation URI + /// should be excluded. Returns `true` to exclude, `false` to retain. + /// + /// # Note + /// + /// - Non-pending attestations are always retained. + /// - Malformed `PendingAttestation` entries (those that fail to decode) are safely retained + /// in the new timestamp to prevent data loss. + /// + /// # Returns + /// + /// Returns `Some(PurgeResult)` if results in a valid timestamp, otherwise returns `None`. + pub fn filter_pending_by_uris( + stamp: &DetachedTimestamp, + mut predicate: F, + ) -> Option> + where + F: FnMut(&str) -> bool, + { + let mut purged = Vec::new_in(stamp.allocator().clone()); - let result = stamp.retain_attestations(|att| { - if att.tag != PendingAttestation::TAG { - return true; // keep non-pending attestations - } - match &uris_to_purge { - None => false, // purge all pending - Some(set) => { - let uri = PendingAttestation::from_raw(att) - .map(|p| p.uri.to_string()) - .unwrap_or_default(); - !set.contains(&uri) // keep if NOT in the purge set + stamp + .filter_attestations(|att| { + if att.tag != PendingAttestation::TAG { + return false; // keep non-pending attestations } - } - }); - - if let Some(purged) = result { - info!("purged {purged} pending attestation(s)"); - PurgeResult { - purged: purged_uris, - has_remaining: true, - } - } else { - info!("all attestations were pending, timestamp is now empty"); - PurgeResult { - purged: purged_uris, - has_remaining: false, - } - } + let Ok(uri) = PendingAttestation::from_raw(att).map(|p| p.uri) else { + return true; + }; + let result = predicate(&uri); + if result { + purged.push(uri.to_string()) + } + result + }) + .map(|new_stamp| { + let new_stamp = DetachedTimestamp::::from_parts(*stamp.header(), new_stamp); + PurgeResult { purged, new_stamp } + }) } } From 79c7bb962713e0e0faa76133ca2dccf1d11529ec Mon Sep 17 00:00:00 2001 From: lightsing Date: Fri, 20 Mar 2026 14:49:45 +0800 Subject: [PATCH 2/3] apply review --- crates/cli/src/commands/purge.rs | 26 ++++++++++++++++++++------ crates/core/src/codec/v1/timestamp.rs | 12 +++++------- packages/sdk-rs/src/purge.rs | 2 +- 3 files changed, 26 insertions(+), 14 deletions(-) diff --git a/crates/cli/src/commands/purge.rs b/crates/cli/src/commands/purge.rs index a0a9122..a439a04 100644 --- a/crates/cli/src/commands/purge.rs +++ b/crates/cli/src/commands/purge.rs @@ -1,9 +1,12 @@ use clap::Args; use std::{collections::HashSet, path::PathBuf}; use tracing::{error, info, warn}; -use uts_core::codec::{ - Decode, Encode, VersionedProof, - v1::{Attestation, DetachedTimestamp, PendingAttestation}, +use uts_core::{ + codec::{ + Decode, Encode, VersionedProof, + v1::{Attestation, DetachedTimestamp, PendingAttestation}, + }, + utils::Hexed, }; use uts_sdk::Sdk; @@ -34,8 +37,19 @@ impl Purge { let pending = proof .attestations() .filter(|att| att.tag == PendingAttestation::TAG) - .map(|att| PendingAttestation::from_raw(att).map(|p| p.uri)) - .collect::, _>>()?; + .flat_map(|att| { + PendingAttestation::from_raw(att) + .map(|p| p.uri) + .inspect_err(|e| { + warn!( + "[{path}] skipped malformed PendingAttestation (value = {data}), error: {e}", + path = path.display(), + data = Hexed(&att.data) + ) + }) + .ok() + }) + .collect::>(); if pending.is_empty() { info!( "[{}] no pending attestations found, skipping", @@ -108,7 +122,7 @@ impl Purge { VersionedProof::new(result.new_stamp).encode(&mut buf)?; tokio::fs::write(path, buf).await?; info!( - "purged {} pending attestation(s) from [{}] ", + "purged {} pending attestation(s) from [{}]", result.purged.len(), path.display(), ); diff --git a/crates/core/src/codec/v1/timestamp.rs b/crates/core/src/codec/v1/timestamp.rs index 756a957..0603261 100644 --- a/crates/core/src/codec/v1/timestamp.rs +++ b/crates/core/src/codec/v1/timestamp.rs @@ -276,9 +276,6 @@ impl Timestamp { /// Make a copy of the timestamp, by filtering the attestations for which the predicate /// returns `true`. /// - /// The predicate receives each [`RawAttestation`] and should return `false` to filter the - /// attestation or `true` to keep it. - /// /// Returns `Some(timestamp)` where `timestamp` is the modified copy of the original one, /// or `None` if the entire timestamp would be empty after filtering. /// @@ -293,7 +290,7 @@ impl Timestamp { // This only be called when it's in root level if let Timestamp::Attestation(attestation) = &this { - return if f(attestation) { Some(this) } else { None }; + return if f(attestation) { None } else { Some(this) }; } this.filter_attestations_inner(&mut f).not().then_some(this) @@ -330,11 +327,12 @@ impl Timestamp { /// Purges all pending attestations from this timestamp tree. /// - /// This is a convenience wrapper around [`retain_attestations`](Self::filter_attestations) + /// This is a convenience wrapper around [`filter_attestations`](Self::filter_attestations) /// that removes all attestations tagged as pending. /// - /// Returns `Some(count)` where `count` is the number of pending attestations removed, - /// or `None` if the entire timestamp consists only of pending attestations. + /// Returns `Some(filtered)` where `filtered` is a copy of this timestamp tree with all + /// pending attestations removed, or `None` if removing pending attestations would leave + /// the timestamp tree empty. pub fn purge_pending(&self) -> Option> { self.filter_attestations(|att| att.tag == PendingAttestation::TAG) } diff --git a/packages/sdk-rs/src/purge.rs b/packages/sdk-rs/src/purge.rs index fdd194b..4d92886 100644 --- a/packages/sdk-rs/src/purge.rs +++ b/packages/sdk-rs/src/purge.rs @@ -65,7 +65,7 @@ impl Sdk { return false; // keep non-pending attestations } let Ok(uri) = PendingAttestation::from_raw(att).map(|p| p.uri) else { - return true; + return false; // keep malformed pending attestations to avoid data loss }; let result = predicate(&uri); if result { From 4940f0c32ada4c8e912cffdadf26038888faafeb Mon Sep 17 00:00:00 2001 From: lightsing Date: Fri, 20 Mar 2026 15:02:10 +0800 Subject: [PATCH 3/3] apply review --- crates/cli/src/commands/purge.rs | 11 +++++++++-- crates/core/src/codec/v1/timestamp.rs | 2 +- packages/sdk-rs/src/purge.rs | 16 +++++++++++++--- 3 files changed, 23 insertions(+), 6 deletions(-) diff --git a/crates/cli/src/commands/purge.rs b/crates/cli/src/commands/purge.rs index a439a04..2935ec2 100644 --- a/crates/cli/src/commands/purge.rs +++ b/crates/cli/src/commands/purge.rs @@ -18,6 +18,10 @@ pub struct Purge { /// Skip the interactive confirmation prompt and purge all pending attestations. #[arg(short = 'y', long = "yes", default_value_t = false)] yes: bool, + /// Purge malformed pending attestations that fail to decode. By default, these are retained + /// to avoid data loss, but enabling this flag will attempt to purge them as well. + #[arg(long = "purge-malformed", default_value_t = false)] + purge_malformed: bool, } impl Purge { @@ -107,8 +111,11 @@ impl Purge { } }; - let Some(result) = Sdk::filter_pending_by_uris(&proof, |uri| uris_to_purge.contains(uri)) - else { + let Some(result) = Sdk::filter_pending_by_uris( + &proof, + |uri| uris_to_purge.contains(uri), + self.purge_malformed, + ) else { error!("won't purge [{}], results in empty proof", path.display()); return Ok(()); }; diff --git a/crates/core/src/codec/v1/timestamp.rs b/crates/core/src/codec/v1/timestamp.rs index 0603261..19db5a4 100644 --- a/crates/core/src/codec/v1/timestamp.rs +++ b/crates/core/src/codec/v1/timestamp.rs @@ -288,7 +288,7 @@ impl Timestamp { { let mut this = self.clone(); - // This only be called when it's in root level + // This can only be called at the root level if let Timestamp::Attestation(attestation) = &this { return if f(attestation) { None } else { Some(this) }; } diff --git a/packages/sdk-rs/src/purge.rs b/packages/sdk-rs/src/purge.rs index 4d92886..8a607cf 100644 --- a/packages/sdk-rs/src/purge.rs +++ b/packages/sdk-rs/src/purge.rs @@ -19,13 +19,20 @@ impl Sdk { /// This method creates a new `DetachedTimestamp` excluding all entries tagged as `Pending`. /// The original `stamp` remains unchanged. /// + /// # Arguments + /// + /// * `stamp` - A reference to the source `DetachedTimestamp`. + /// * `purge_malformed` - A boolean flag indicating whether malformed `PendingAttestation` entries + /// should be purged (`true`) or retained (`false`) in the new timestamp. + /// /// # Returns /// /// Returns `Some(PurgeResult)` if results in a valid timestamp, otherwise returns `None`. pub fn filter_pending( stamp: &DetachedTimestamp, + purge_malformed: bool, ) -> Option> { - Self::filter_pending_by_uris(stamp, |_| true) + Self::filter_pending_by_uris(stamp, |_| true, purge_malformed) } /// Filters pending attestations from the given detached timestamp based on a predicate. @@ -40,6 +47,8 @@ impl Sdk { /// * `stamp` - A reference to the source `DetachedTimestamp`. /// * `predicate` - A closure that determines whether a specific pending attestation URI /// should be excluded. Returns `true` to exclude, `false` to retain. + /// * `purge_malformed` - A boolean flag indicating whether malformed `PendingAttestation` entries + /// should be purged (`true`) or retained (`false`) in the new timestamp. /// /// # Note /// @@ -49,10 +58,11 @@ impl Sdk { /// /// # Returns /// - /// Returns `Some(PurgeResult)` if results in a valid timestamp, otherwise returns `None`. + /// Returns `Some(PurgeResult)` if it results in a valid timestamp, otherwise returns `None`. pub fn filter_pending_by_uris( stamp: &DetachedTimestamp, mut predicate: F, + purge_malformed: bool, ) -> Option> where F: FnMut(&str) -> bool, @@ -65,7 +75,7 @@ impl Sdk { return false; // keep non-pending attestations } let Ok(uri) = PendingAttestation::from_raw(att).map(|p| p.uri) else { - return false; // keep malformed pending attestations to avoid data loss + return purge_malformed; }; let result = predicate(&uri); if result {