-
Notifications
You must be signed in to change notification settings - Fork 0
feat(temporal): refuse later revisions with earlier system time #122
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
a22945b
5ec989f
b032c85
38542fd
56c57d7
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,17 @@ | ||
| [package] | ||
| name = "revision_order" | ||
| description = "Later document revisions cannot move backward in system time." | ||
| version.workspace = true | ||
| edition.workspace = true | ||
| rust-version.workspace = true | ||
| license.workspace = true | ||
| authors.workspace = true | ||
| repository.workspace = true | ||
| homepage.workspace = true | ||
| readme.workspace = true | ||
| keywords.workspace = true | ||
| categories.workspace = true | ||
| publish = false | ||
|
|
||
| [lints] | ||
| workspace = true |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,48 @@ | ||
| //! Fail-closed revision-order errors. | ||
|
|
||
| use std::fmt; | ||
|
|
||
| /// A fail-closed revision-order error. | ||
| #[derive(Clone, Copy, Debug, Eq, PartialEq)] | ||
| #[non_exhaustive] | ||
| pub enum RevisionOrderError { | ||
| /// A later revision did not have a later system time. | ||
| SystemTimeDidNotIncrease, | ||
| /// A revision number or recovery slice was empty, zero, or mismatched. | ||
| InvalidRevisionPayload, | ||
| } | ||
|
|
||
| impl fmt::Display for RevisionOrderError { | ||
| fn fmt(&self, formatter: &mut fmt::Formatter<'_>) -> fmt::Result { | ||
| let message = match self { | ||
| Self::SystemTimeDidNotIncrease => { | ||
| "later document revisions must have later system time" | ||
| } | ||
| Self::InvalidRevisionPayload => "invalid revision-order payload", | ||
| }; | ||
| formatter.write_str(message) | ||
| } | ||
| } | ||
|
|
||
| impl std::error::Error for RevisionOrderError {} | ||
|
|
||
| #[cfg(test)] | ||
| mod tests { | ||
| use super::RevisionOrderError; | ||
|
|
||
| #[test] | ||
| fn error_messages_are_stable() { | ||
| for (error, message) in [ | ||
| ( | ||
| RevisionOrderError::SystemTimeDidNotIncrease, | ||
| "later document revisions must have later system time", | ||
| ), | ||
| ( | ||
| RevisionOrderError::InvalidRevisionPayload, | ||
| "invalid revision-order payload", | ||
| ), | ||
| ] { | ||
| assert_eq!(error.to_string(), message); | ||
| } | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,21 @@ | ||
| #![forbid(unsafe_code)] | ||
| #![deny(missing_docs)] | ||
| #![allow(clippy::cast_precision_loss)] | ||
| //! Later document revisions cannot move backward in system time. | ||
| //! | ||
| //! A higher revision number is a later assertion about the same document | ||
| //! identity. Its system time must strictly increase (ADR 0002/0013). | ||
|
|
||
| mod error; | ||
| mod revision; | ||
|
|
||
| /// Fail-closed revision-order errors. | ||
| pub use error::RevisionOrderError; | ||
| /// One document revision with a positive revision number and system time. | ||
| pub use revision::DocumentRevision; | ||
| /// Fraction of recovered order flags that match known truth. | ||
| pub use revision::order_recovery_rate; | ||
| /// Refuse a later revision whose system time did not increase. | ||
| pub use revision::refuse_nonincreasing_system_time; | ||
| /// Return whether a later revision has a later system time. | ||
| pub use revision::revisions_are_increasing; |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,138 @@ | ||
| //! Document revisions stamped with system time. | ||
|
|
||
| use crate::RevisionOrderError; | ||
|
|
||
| /// One document revision with a positive revision number and system time. | ||
| #[derive(Clone, Copy, Debug, Eq, PartialEq)] | ||
| pub struct DocumentRevision { | ||
| revision_number: u32, | ||
| system_time_seconds: i64, | ||
| } | ||
|
|
||
| impl DocumentRevision { | ||
| /// Construct a revision whose number is at least one. | ||
| /// | ||
| /// # Errors | ||
| /// | ||
| /// Returns [`RevisionOrderError::InvalidRevisionPayload`] when | ||
| /// `revision_number` is zero. | ||
| pub const fn new( | ||
| revision_number: u32, | ||
| system_time_seconds: i64, | ||
| ) -> Result<Self, RevisionOrderError> { | ||
| if revision_number == 0 { | ||
| return Err(RevisionOrderError::InvalidRevisionPayload); | ||
| } | ||
| Ok(Self { | ||
| revision_number, | ||
| system_time_seconds, | ||
| }) | ||
| } | ||
|
|
||
| /// Positive revision number. | ||
| #[must_use] | ||
| pub const fn revision_number(self) -> u32 { | ||
| self.revision_number | ||
| } | ||
|
|
||
| /// System/record time in seconds. | ||
| #[must_use] | ||
| pub const fn system_time_seconds(self) -> i64 { | ||
| self.system_time_seconds | ||
| } | ||
| } | ||
|
|
||
| /// Return whether `later` has a greater revision number and later system time. | ||
| /// | ||
| /// # Errors | ||
| /// | ||
| /// Returns [`RevisionOrderError::InvalidRevisionPayload`] when `later` is not | ||
| /// a strictly greater revision number than `earlier`. | ||
| pub fn revisions_are_increasing( | ||
| earlier: DocumentRevision, | ||
| later: DocumentRevision, | ||
| ) -> Result<bool, RevisionOrderError> { | ||
| if later.revision_number <= earlier.revision_number { | ||
| return Err(RevisionOrderError::InvalidRevisionPayload); | ||
| } | ||
| Ok(later.system_time_seconds > earlier.system_time_seconds) | ||
| } | ||
|
Comment on lines
+45
to
+59
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 📝 Info: Doc comment on revisions_are_increasing understates behavior The doc for Was this helpful? React with 👍 or 👎 to provide feedback. |
||
|
|
||
| /// Refuse a later revision whose system time did not increase. | ||
| /// | ||
| /// # Errors | ||
| /// | ||
| /// Returns revision-construction errors, or | ||
| /// [`RevisionOrderError::SystemTimeDidNotIncrease`] when the system times | ||
| /// are not strictly increasing. | ||
| pub fn refuse_nonincreasing_system_time( | ||
| earlier: DocumentRevision, | ||
| later: DocumentRevision, | ||
| ) -> Result<(), RevisionOrderError> { | ||
| if revisions_are_increasing(earlier, later)? { | ||
| return Ok(()); | ||
| } | ||
| Err(RevisionOrderError::SystemTimeDidNotIncrease) | ||
| } | ||
|
Comment on lines
+68
to
+76
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 📝 Info: refuse_nonincreasing_system_time returns InvalidRevisionPayload for equal/backward revision numbers
Was this helpful? React with 👍 or 👎 to provide feedback.
Comment on lines
+68
to
+76
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🔍 Error-propagation branch in refuse_nonincreasing_system_time may be untested In Was this helpful? React with 👍 or 👎 to provide feedback. |
||
|
|
||
| /// Fraction of recovered order flags that match known truth. | ||
| /// | ||
| /// # Errors | ||
| /// | ||
| /// Returns [`RevisionOrderError::InvalidRevisionPayload`] when either slice | ||
| /// is empty or the lengths differ. | ||
| pub fn order_recovery_rate(truth: &[bool], decided: &[bool]) -> Result<f64, RevisionOrderError> { | ||
| if truth.is_empty() || truth.len() != decided.len() { | ||
| return Err(RevisionOrderError::InvalidRevisionPayload); | ||
| } | ||
| let mut matches = 0_u32; | ||
| for (truth_flag, decided_flag) in truth.iter().zip(decided) { | ||
| if truth_flag == decided_flag { | ||
| matches += 1; | ||
| } | ||
| } | ||
| Ok(f64::from(matches) / truth.len() as f64) | ||
| } | ||
|
|
||
| #[cfg(test)] | ||
| mod tests { | ||
| use super::{ | ||
| DocumentRevision, order_recovery_rate, refuse_nonincreasing_system_time, | ||
| revisions_are_increasing, | ||
| }; | ||
| use crate::RevisionOrderError; | ||
|
|
||
| #[test] | ||
| fn local_branches_cover_order_and_payloads() { | ||
| let first = DocumentRevision::new(1, 10).expect("first"); | ||
| let second = DocumentRevision::new(2, 20).expect("second"); | ||
| assert_eq!(first.revision_number(), 1); | ||
| assert_eq!(first.system_time_seconds(), 10); | ||
| assert!(revisions_are_increasing(first, second).expect("increasing")); | ||
| refuse_nonincreasing_system_time(first, second).expect("ok"); | ||
| let same_time = DocumentRevision::new(3, 20).expect("same"); | ||
| assert!(!revisions_are_increasing(second, same_time).expect("flat")); | ||
| assert_eq!( | ||
| refuse_nonincreasing_system_time(second, same_time), | ||
| Err(RevisionOrderError::SystemTimeDidNotIncrease) | ||
| ); | ||
| assert_eq!( | ||
| revisions_are_increasing(second, first), | ||
| Err(RevisionOrderError::InvalidRevisionPayload) | ||
| ); | ||
| assert_eq!( | ||
| DocumentRevision::new(0, 1), | ||
| Err(RevisionOrderError::InvalidRevisionPayload) | ||
| ); | ||
| let matched = order_recovery_rate(&[true], &[true]).expect("rate"); | ||
| assert!((matched - 1.0).abs() < f64::EPSILON); | ||
| assert_eq!( | ||
| order_recovery_rate(&[], &[]), | ||
| Err(RevisionOrderError::InvalidRevisionPayload) | ||
| ); | ||
| assert_eq!( | ||
| order_recovery_rate(&[true], &[]), | ||
| Err(RevisionOrderError::InvalidRevisionPayload) | ||
| ); | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,7 @@ | ||
| //! Integration contract for the `revision_order` package identity. | ||
|
|
||
| #[test] | ||
| fn package_identity_is_stable() { | ||
| let observed = std::hint::black_box(env!("CARGO_PKG_NAME")); | ||
| assert_eq!(observed, "revision_order"); | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,77 @@ | ||
| //! Later revisions cannot carry earlier or equal system time. | ||
|
|
||
| use revision_order::{ | ||
| DocumentRevision, RevisionOrderError, order_recovery_rate, refuse_nonincreasing_system_time, | ||
| revisions_are_increasing, | ||
| }; | ||
|
|
||
| fn revision(number: u32, system: i64) -> DocumentRevision { | ||
| DocumentRevision::new(number, system).expect("revision") | ||
| } | ||
|
|
||
| #[test] | ||
| fn later_revisions_cannot_move_backward_in_system_time() { | ||
| let first = revision(1, 10); | ||
| let second = revision(2, 20); | ||
| let backward = revision(3, 15); | ||
| assert!(revisions_are_increasing(first, second).expect("increasing")); | ||
| refuse_nonincreasing_system_time(first, second).expect("ok"); | ||
| assert!(!revisions_are_increasing(second, backward).expect("backward")); | ||
| assert_eq!( | ||
| refuse_nonincreasing_system_time(second, backward), | ||
| Err(RevisionOrderError::SystemTimeDidNotIncrease) | ||
| ); | ||
| assert_eq!( | ||
| refuse_nonincreasing_system_time(second, revision(4, 20)), | ||
| Err(RevisionOrderError::SystemTimeDidNotIncrease) | ||
| ); | ||
| } | ||
|
|
||
| #[test] | ||
| fn recovered_order_flags_match_known_truth_better_than_accepting_all() { | ||
| let pairs = [ | ||
| (revision(1, 10), revision(2, 20)), | ||
| (revision(2, 20), revision(3, 15)), | ||
| (revision(3, 30), revision(4, 40)), | ||
| ]; | ||
| let truth = [true, false, true]; | ||
| let recovered = [ | ||
| revisions_are_increasing(pairs[0].0, pairs[0].1).expect("p0"), | ||
| revisions_are_increasing(pairs[1].0, pairs[1].1).expect("p1"), | ||
| revisions_are_increasing(pairs[2].0, pairs[2].1).expect("p2"), | ||
| ]; | ||
| let collapsed = [true, true, true]; | ||
| let recovered_rate = order_recovery_rate(&truth, &recovered).expect("recovered"); | ||
| let collapsed_rate = order_recovery_rate(&truth, &collapsed).expect("collapsed"); | ||
| let expected = { | ||
| let mut matches = 0_u32; | ||
| for (truth_flag, decided_flag) in truth.iter().zip(recovered.iter()) { | ||
| if truth_flag == decided_flag { | ||
| matches += 1; | ||
| } | ||
| } | ||
| f64::from(matches) / f64::from(u32::try_from(truth.len()).expect("len")) | ||
| }; | ||
| assert!((recovered_rate - expected).abs() < f64::EPSILON); | ||
| assert!(recovered_rate > collapsed_rate); | ||
| } | ||
|
|
||
| #[test] | ||
| fn empty_or_invalid_revision_payloads_fail_closed() { | ||
| assert_eq!( | ||
| DocumentRevision::new(0, 10), | ||
| Err(RevisionOrderError::InvalidRevisionPayload) | ||
| ); | ||
| assert_eq!( | ||
| order_recovery_rate(&[], &[]), | ||
| Err(RevisionOrderError::InvalidRevisionPayload) | ||
| ); | ||
| assert_eq!( | ||
| order_recovery_rate(&[true], &[]), | ||
| Err(RevisionOrderError::InvalidRevisionPayload) | ||
| ); | ||
| assert_eq!( | ||
| order_recovery_rate(&[true, false], &[true]), | ||
| Err(RevisionOrderError::InvalidRevisionPayload) | ||
| ); | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
📝 Info: System-time gate is consistent with bitemporal revision semantics
The gate refuses a later revision unless its
system_time_secondsstrictly increases (revision.rs,55-58). This is transaction/system-time ordering only and does not constrain event/valid time, so it remains consistent with AGENTS.md #5 (revision edges may point to the past in event time). The strict>comparison also correctly rejects equal system times, matching the PR's stated 'earlier or equal' refusal. No bug; noting because the distinction between system time and event time is the crux of correctness here.Was this helpful? React with 👍 or 👎 to provide feedback.