From ff9e7c244c270853a8a8f58ee950859866358e1d Mon Sep 17 00:00:00 2001 From: tv Date: Sun, 8 Nov 2026 18:20:13 +0000 Subject: remake: aborts written as the user who asks; no abort by time Review 2, N3: 1150 let any member write an abort decision once a remake had expired a grace period ago, judged by the change's own time, the writer's word. A member could race the coordinator's legitimate commit with an abort dated as it liked; both taken, Automerge shows one, and nodes that acted on the commit split from those that didn't. The two writers of such aborts are operator requests whose endpoints already need write on _continuum/remakes: aborting an expired remake (DELETE /admin/remakes/{id}) and superseding one (POST /admin/remakes?force=supersede). Both now commit as the user who asks (the proposal and its vote with them, for a supersede), which the rules take as one who may write the decision. The clause judging an abort by its time goes, and with it the time the rules carried. An expired remake is still aborted through any member, by an operator. --- src/remake/api.rs | 50 +++++++++++++++++++++++++++++++++++++++++--------- src/remake/proposal.rs | 6 +++++- 2 files changed, 46 insertions(+), 10 deletions(-) (limited to 'src/remake') diff --git a/src/remake/api.rs b/src/remake/api.rs index 7aeb675..d3df908 100644 --- a/src/remake/api.rs +++ b/src/remake/api.rs @@ -302,11 +302,12 @@ async fn wait_until_final(host: &H, id: &str) -> Option { /// `POST /admin/remakes`. pub(crate) async fn start( State(host): State, + axum::Extension(user): axum::Extension, Query(query): Query, headers: HeaderMap, body: Bytes, ) -> Response { - start_kind(host, query, &headers, body).await + start_kind(host, query, &headers, body, Some(user.0)).await } async fn start_kind( @@ -314,6 +315,7 @@ async fn start_kind( query: StartQuery, headers: &HeaderMap, body: Bytes, + user: Option, ) -> Response { if !host.is_node_ready().await { return error_response(StatusCode::SERVICE_UNAVAILABLE, "node is not ready", None); @@ -515,6 +517,7 @@ async fn start_kind( Supersede { force: force.clone(), active_peers: active_peers(&host).await, + by: user, }, now, grace, @@ -631,16 +634,18 @@ pub(crate) async fn get( /// `DELETE /admin/remakes/{id}`. pub(crate) async fn abort( State(host): State, + axum::Extension(user): axum::Extension, Path(id): Path, Query(query): Query, ) -> Response { - abort_by_id(host, id, query).await + abort_by_id(host, id, query, Some(user.0)).await } /// `DELETE /admin/remakes`: aborts whichever remake of the local epoch is still /// undecided (proposed or expired), the oldest if there are several. pub(crate) async fn abort_current( State(host): State, + axum::Extension(user): axum::Extension, Query(query): Query, ) -> Response { let local = EpochRecord::from(host.current_epoch()); @@ -657,7 +662,7 @@ pub(crate) async fn abort_current( .await; match open { - Some(id) => abort_by_id(host, id, query).await, + Some(id) => abort_by_id(host, id, query, Some(user.0)).await, None => error_response( StatusCode::NOT_FOUND, "no remake of the local epoch is in progress", @@ -666,7 +671,12 @@ pub(crate) async fn abort_current( } } -async fn abort_by_id(host: H, id: String, query: AbortQuery) -> Response { +async fn abort_by_id( + host: H, + id: String, + query: AbortQuery, + user: Option, +) -> Response { let force = match Force::parse(query.force.as_deref(), ABORT_FORCE) { Ok(force) => force, Err(reason) => return error_response(StatusCode::BAD_REQUEST, reason, None), @@ -678,7 +688,7 @@ async fn abort_by_id(host: H, id: String, query: AbortQuery) -> Respons match current.phase { Phase::Proposed => {} - Phase::Expired => return abort_expired(&host, current, &force).await, + Phase::Expired => return abort_expired(&host, current, &force, user).await, Phase::Committed => { return error_response( StatusCode::CONFLICT, @@ -750,7 +760,12 @@ async fn abort_by_id(host: H, id: String, query: AbortQuery) -> Respons /// Aborts expired remake `current` from this node: its coordinator is gone, and can't /// commit it any more (`coordinator`), unless `abort_obstacle` says otherwise. Answers /// with the final report. -async fn abort_expired(host: &H, current: Report, force: &Force) -> Response { +async fn abort_expired( + host: &H, + current: Report, + force: &Force, + user: Option, +) -> Response { // Whether it expired is judged by this node's clock: one far ahead would abort a // remake its coordinator may still commit (a contested remake). if let Some(refused) = clock_obstacle(host, force, "whether the remake expired").await { @@ -785,8 +800,10 @@ async fn abort_expired(host: &H, current: Report, force: &Force) -> Res decided_at: host.now_millis(), }; let clock = host.clock(); + // As the user who asks, who may write remakes (`api::access`): the rules take a + // decision but the coordinator's from one who may write it. let written = host - .commit_doc(move |doc| -> Result { + .commit_doc_for(user, move |doc| -> Result { let now = clock.now_millis(); let expired = records::load(doc, &record_id).is_some_and(|record| { record.phase(now, grace) == Phase::Expired && now >= abortable_at(&record, grace) @@ -906,7 +923,8 @@ pub(crate) mod testing { CONTENT_TYPE, axum::http::HeaderValue::from_static("application/json"), ); - into_json(start_kind(host, query, &headers, Bytes::from(body.to_string())).await).await + into_json(start_kind(host, query, &headers, Bytes::from(body.to_string()), None).await) + .await } /// `DELETE /admin/remakes/{id}` with `?force=`. @@ -919,7 +937,21 @@ pub(crate) mod testing { wait: false, force: force.map(str::to_owned), }; - into_json(abort_by_id(host, id.to_owned(), query).await).await + into_json(abort_by_id(host, id.to_owned(), query, None).await).await + } + + /// `DELETE /admin/remakes/{id}` with `?force=`, asked by `user`. + pub(crate) async fn abort_as( + host: H, + id: &str, + force: Option<&str>, + user: &str, + ) -> (StatusCode, JsonValue) { + let query = AbortQuery { + wait: false, + force: force.map(str::to_owned), + }; + into_json(abort_by_id(host, id.to_owned(), query, Some(user.to_owned())).await).await } /// `GET /admin/remakes/{id}`: the report, if the remake exists here. diff --git a/src/remake/proposal.rs b/src/remake/proposal.rs index 7695b4c..f8b3b0e 100644 --- a/src/remake/proposal.rs +++ b/src/remake/proposal.rs @@ -86,6 +86,9 @@ pub(super) struct ExpiredRemake { pub(super) struct Supersede { pub(super) force: Force, pub(super) active_peers: HashSet, + /// Who asks (the API's user): the proposal, and the decisions aborting the remakes + /// it supersedes, are written as theirs, as the rules judge them (`access::rules`). + pub(super) by: Option, } #[derive(Debug, thiserror::Error)] @@ -126,8 +129,9 @@ pub(super) async fn propose( let me = host.node_id(); let started = host.remake().started_at().plus_ms(grace); let base_at_commit = kind::by_name(&proposal.kind).is_some_and(|kind| kind.base_at_commit()); + let by = supersede.by.clone(); let record = host - .commit_doc(move |doc| -> Result { + .commit_doc_for(by, move |doc| -> Result { let mut all = records::load_all(doc); if let Some((open, _)) = all.iter().find(|(_, record)| { record.proposal.from_epoch == proposal.from_epoch -- cgit v1.3.1