From 6a1e34ad66a4ec1d33253560909b435de4d51732 Mon Sep 17 00:00:00 2001 From: yahya <19204398+thep2p@users.noreply.github.com> Date: Sun, 26 Jul 2026 20:09:27 -0400 Subject: [PATCH 1/4] [feat][core] Add try_link atomic slot decision --- src/core/lookup/array_lookup_table.rs | 66 ++++++++++++- src/core/lookup/array_lookup_table_test.rs | 110 ++++++++++++++++++++- src/core/lookup/mod.rs | 46 +++++++++ src/core/mod.rs | 1 + src/core/model/address.rs | 2 +- src/core/model/identity.rs | 2 +- src/node/core_test.rs | 12 ++- 7 files changed, 234 insertions(+), 5 deletions(-) diff --git a/src/core/lookup/array_lookup_table.rs b/src/core/lookup/array_lookup_table.rs index cac79bc..d04664d 100644 --- a/src/core/lookup/array_lookup_table.rs +++ b/src/core/lookup/array_lookup_table.rs @@ -1,4 +1,4 @@ -use crate::core::lookup::{LookupTable, LookupTableLevel}; +use crate::core::lookup::{LinkOutcome, LookupTable, LookupTableLevel}; use crate::core::model; use crate::core::model::direction::Direction; use crate::core::model::identity::Identity; @@ -166,6 +166,70 @@ impl LookupTable for ArrayLookupTable { Ok(entry) } + /// Atomically decides whether `candidate` becomes the neighbor at `(level, direction)`, or + /// whether the existing entry there already sits strictly between this node and `candidate` + /// and the request should be forwarded. Runs entirely under a single `inner.write()` guard — + /// the compare, the decision, and the (conditional) write all happen under one lock + /// acquisition. + /// + /// This method never calls `get_entry`/`update_entry`, whose separately-locked critical + /// sections could not be composed into one atomic decision: two concurrent callers linking + /// the same `(level, direction)` slot could both read the same stale entry under their own + /// `get_entry` call, both independently decide to insert, and both call `update_entry` — + /// the second silently clobbers the first, with no forwarding ever evaluated against the + /// true post-first-write state. + fn try_link( + &self, + level: LookupTableLevel, + direction: Direction, + candidate: Identity, + ) -> anyhow::Result { + if level >= LOOKUP_TABLE_LEVELS { + return Err(anyhow!( + "position is larger than the max lookup table entry number: {}", + level + )); + } + + let mut inner = self.inner.write(); + + let existing = match direction { + Direction::Left => inner.left[level], + Direction::Right => inner.right[level], + }; + + // an existing entry sits strictly between this node and the candidate when, for + // Direction::Right, existing.id() < candidate.id(); for Direction::Left, + // existing.id() > candidate.id() — it is then closer to the candidate's true position + // than this node is, so the slot is left untouched and the decision is to forward. + let outcome = match (existing, direction) { + (Some(existing), Direction::Right) if existing.id() < candidate.id() => { + LinkOutcome::Forward(existing) + } + (Some(existing), Direction::Left) if existing.id() > candidate.id() => { + LinkOutcome::Forward(existing) + } + _ => { + match direction { + Direction::Left => inner.left[level] = Some(candidate), + Direction::Right => inner.right[level] = Some(candidate), + } + LinkOutcome::LinkedDirectly + } + }; + + // Log the try_link decision + tracing::trace!( + "try_link decision at level {} in direction {}: candidate {}, outcome {:?}", + level, + direction, + candidate.id(), + outcome + ); + + Ok(outcome) + } + /// Dynamically compares the lookup table with another for equality. /// This is a deep comparison of the entries in the table. /// Returns true if the entries are equal, false otherwise. diff --git a/src/core/lookup/array_lookup_table_test.rs b/src/core/lookup/array_lookup_table_test.rs index f7e68c2..c15431b 100644 --- a/src/core/lookup/array_lookup_table_test.rs +++ b/src/core/lookup/array_lookup_table_test.rs @@ -3,7 +3,7 @@ mod tests { use crate::core::model::direction::Direction; use crate::core::model::identity::Identity; use crate::core::testutil::fixtures::*; - use crate::core::{model, ArrayLookupTable, LookupTable, LOOKUP_TABLE_LEVELS}; + use crate::core::{model, ArrayLookupTable, LinkOutcome, LookupTable, LOOKUP_TABLE_LEVELS}; use std::collections::HashMap; #[test] @@ -94,6 +94,114 @@ mod tests { assert_eq!(Some(id2), lt.get_entry(0, Direction::Left).unwrap()); } + /// (a) try_link into an empty slot links directly and inserts the candidate, on both sides. + #[test] + fn test_try_link_empty_slot_links_directly() { + let lt = ArrayLookupTable::new(); + let right_candidate = random_identity(); + let left_candidate = random_identity(); + + let outcome = lt.try_link(0, Direction::Right, right_candidate).unwrap(); + assert_eq!(outcome, LinkOutcome::LinkedDirectly); + assert_eq!( + lt.get_entry(0, Direction::Right).unwrap(), + Some(right_candidate) + ); + + let outcome = lt.try_link(0, Direction::Left, left_candidate).unwrap(); + assert_eq!(outcome, LinkOutcome::LinkedDirectly); + assert_eq!( + lt.get_entry(0, Direction::Left).unwrap(), + Some(left_candidate) + ); + } + + /// (b, Right) an existing right neighbor that does NOT sit strictly between self and the + /// candidate (existing.id() > candidate.id()) is overwritten: try_link links directly and + /// get_entry afterward reflects the new candidate, not the old neighbor. + #[test] + fn test_try_link_existing_not_between_overwrites_right() { + let lt = ArrayLookupTable::new(); + let candidate_id = random_identifier(); + let candidate = Identity::new(candidate_id, random_membership_vector(), random_address()); + + let existing_id = random_identifier_greater_than(&candidate_id); + let existing = Identity::new(existing_id, random_membership_vector(), random_address()); + lt.update_entry(existing, 0, Direction::Right).unwrap(); + + let outcome = lt.try_link(0, Direction::Right, candidate).unwrap(); + assert_eq!(outcome, LinkOutcome::LinkedDirectly); + assert_eq!(lt.get_entry(0, Direction::Right).unwrap(), Some(candidate)); + } + + /// (b, Left) an existing left neighbor that does NOT sit strictly between self and the + /// candidate (existing.id() < candidate.id()) is overwritten: try_link links directly and + /// get_entry afterward reflects the new candidate, not the old neighbor. + #[test] + fn test_try_link_existing_not_between_overwrites_left() { + let lt = ArrayLookupTable::new(); + let candidate_id = random_identifier(); + let candidate = Identity::new(candidate_id, random_membership_vector(), random_address()); + + let existing_id = random_identifier_less_than(&candidate_id); + let existing = Identity::new(existing_id, random_membership_vector(), random_address()); + lt.update_entry(existing, 0, Direction::Left).unwrap(); + + let outcome = lt.try_link(0, Direction::Left, candidate).unwrap(); + assert_eq!(outcome, LinkOutcome::LinkedDirectly); + assert_eq!(lt.get_entry(0, Direction::Left).unwrap(), Some(candidate)); + } + + /// (c, Right) an existing right neighbor that sits strictly between self and the candidate + /// (existing.id() < candidate.id()) causes try_link to forward instead of linking: the table + /// is left unchanged, still holding the existing neighbor. + #[test] + fn test_try_link_existing_between_forwards_right() { + let lt = ArrayLookupTable::new(); + let candidate_id = random_identifier(); + let candidate = Identity::new(candidate_id, random_membership_vector(), random_address()); + + let existing_id = random_identifier_less_than(&candidate_id); + let existing = Identity::new(existing_id, random_membership_vector(), random_address()); + lt.update_entry(existing, 0, Direction::Right).unwrap(); + + let outcome = lt.try_link(0, Direction::Right, candidate).unwrap(); + assert_eq!(outcome, LinkOutcome::Forward(existing)); + assert_eq!(lt.get_entry(0, Direction::Right).unwrap(), Some(existing)); + } + + /// (c, Left) an existing left neighbor that sits strictly between self and the candidate + /// (existing.id() > candidate.id()) causes try_link to forward instead of linking: the table + /// is left unchanged, still holding the existing neighbor. + #[test] + fn test_try_link_existing_between_forwards_left() { + let lt = ArrayLookupTable::new(); + let candidate_id = random_identifier(); + let candidate = Identity::new(candidate_id, random_membership_vector(), random_address()); + + let existing_id = random_identifier_greater_than(&candidate_id); + let existing = Identity::new(existing_id, random_membership_vector(), random_address()); + lt.update_entry(existing, 0, Direction::Left).unwrap(); + + let outcome = lt.try_link(0, Direction::Left, candidate).unwrap(); + assert_eq!(outcome, LinkOutcome::Forward(existing)); + assert_eq!(lt.get_entry(0, Direction::Left).unwrap(), Some(existing)); + } + + /// (d) try_link at an out-of-range level returns an error, matching the other lookup-table + /// accessors' bounds-checking behavior. + #[test] + fn test_try_link_out_of_bound_level_errors() { + let lt = ArrayLookupTable::new(); + let candidate = random_identity(); + + let result = lt.try_link(LOOKUP_TABLE_LEVELS, Direction::Right, candidate); + assert!(result.is_err()); + + let result = lt.try_link(LOOKUP_TABLE_LEVELS, Direction::Left, candidate); + assert!(result.is_err()); + } + #[test] /// Test equality of lookup tables. /// The test will create two identical lookup tables and check if they are equal. diff --git a/src/core/lookup/mod.rs b/src/core/lookup/mod.rs index 93bea7f..1a52102 100644 --- a/src/core/lookup/mod.rs +++ b/src/core/lookup/mod.rs @@ -7,6 +7,20 @@ mod array_lookup_table_test; /// LookupTableLevel represents level of a lookup table. entry in the table. pub type LookupTableLevel = usize; +/// outcome of a [`LookupTable::try_link`] compare-then-act decision. +#[derive(Debug, Copy, Clone, PartialEq, Eq)] +pub enum LinkOutcome { + /// the candidate was inserted at the requested `(level, direction)` slot: the slot was + /// empty, or its previous occupant did not sit strictly between this node and the + /// candidate on that side, so the candidate is now the entry there. + LinkedDirectly, + /// the table was left untouched: the carried identity is the existing entry at + /// `(level, direction)`, and it sits strictly between this node and the candidate on that + /// side — it is closer to the candidate's true position, so the link request belongs there + /// instead. + Forward(Identity), +} + /// LookupTable is the core view of Skip Graph node towards the network. pub trait LookupTable: Send + Sync { /// Update the entry at the given level and direction. @@ -29,6 +43,38 @@ pub trait LookupTable: Send + Sync { direction: Direction, ) -> anyhow::Result>; + /// atomically decides whether `candidate` becomes the neighbor at `(level, direction)`, or + /// whether an existing neighbor there already sits strictly between this node and + /// `candidate` on that side and the request should be forwarded to it instead. + /// + /// the decision is atomic: inspecting the current entry and, when accepting, inserting + /// `candidate` happen as one indivisible step with respect to any other concurrent call on + /// the same `(level, direction)` slot — no caller can observe or race a partial decision. + /// + /// `direction` is receiver-owned, never re-interpreted hop-to-hop: it always names this + /// node's own slot (`Direction::Right` this node's own right slot, holding neighbors with + /// larger identifiers; `Direction::Left` its own left slot), never something relative to a + /// caller or hop. an existing entry sits strictly between this node and `candidate` when, + /// for `Direction::Right`, `existing.id() < candidate.id()`; for `Direction::Left`, + /// `existing.id() > candidate.id()`: + /// + /// - if it does: this node's own entry at `(level, direction)` is left unchanged, and + /// [`LinkOutcome::Forward`] carrying that existing neighbor is returned — the caller + /// should retry the link request against that neighbor instead. + /// - otherwise (the slot is empty, or the existing entry does not sit strictly between): + /// `candidate` is inserted into this node's own entry at `(level, direction)`, and + /// [`LinkOutcome::LinkedDirectly`] is returned. + /// + /// # Errors + /// + /// returns an error when `level` is out of bounds. + fn try_link( + &self, + level: LookupTableLevel, + direction: Direction, + candidate: Identity, + ) -> anyhow::Result; + /// Dynamically compares the lookup table with another for equality. fn equal(&self, other: &dyn LookupTable) -> bool; diff --git a/src/core/mod.rs b/src/core/mod.rs index 861606d..97800e0 100644 --- a/src/core/mod.rs +++ b/src/core/mod.rs @@ -7,6 +7,7 @@ pub mod testutil; pub use crate::core::context::IrrevocableContext; pub use crate::core::lookup::array_lookup_table::ArrayLookupTable; pub use crate::core::lookup::array_lookup_table::LOOKUP_TABLE_LEVELS; +pub use crate::core::lookup::LinkOutcome; pub use crate::core::lookup::LookupTable; pub use crate::core::lookup::LookupTableLevel; pub use crate::core::model::address::Address; diff --git a/src/core/model/address.rs b/src/core/model/address.rs index 4b40d42..9f045b5 100644 --- a/src/core/model/address.rs +++ b/src/core/model/address.rs @@ -2,7 +2,7 @@ use fixedstr::{str128, str8}; use std::fmt::Debug; /// Represents a networking address; composed of host + port -#[derive(Copy, Clone, PartialEq)] +#[derive(Copy, Clone, PartialEq, Eq)] pub struct Address { host: str128, // up to 128 bytes (on stack) port: str8, // up to 8 bytes (on stack) diff --git a/src/core/model/identity.rs b/src/core/model/identity.rs index 9622430..b2713e3 100644 --- a/src/core/model/identity.rs +++ b/src/core/model/identity.rs @@ -1,7 +1,7 @@ use crate::core::{Address, Identifier, MembershipVector}; /// Identity is an immutable struct that represents a node's identity in the network (ID, MembershipVector, Address). -#[derive(Copy, Clone, Debug, PartialEq)] +#[derive(Copy, Clone, Debug, PartialEq, Eq)] pub struct Identity { id: Identifier, mem_vec: MembershipVector, diff --git a/src/node/core_test.rs b/src/node/core_test.rs index fa98371..6b87c69 100644 --- a/src/node/core_test.rs +++ b/src/node/core_test.rs @@ -7,7 +7,8 @@ use crate::core::testutil::fixtures::{ span_fixture, }; use crate::core::{ - ArrayLookupTable, IdSearchReq, Identifier, LookupTable, LookupTableLevel, LOOKUP_TABLE_LEVELS, + ArrayLookupTable, IdSearchReq, Identifier, LinkOutcome, LookupTable, LookupTableLevel, + LOOKUP_TABLE_LEVELS, }; use crate::node::core::{BaseCore, Core}; use anyhow::anyhow; @@ -359,6 +360,15 @@ fn test_search_by_id_error_propagation() { Err(anyhow!("simulated lookup table error")) } + fn try_link( + &self, + _: LookupTableLevel, + _: Direction, + _: Identity, + ) -> anyhow::Result { + todo!() + } + fn equal(&self, _: &dyn LookupTable) -> bool { todo!() } From 1edb313651b37ed2dbef071b91f41bdfe0d46f96 Mon Sep 17 00:00:00 2001 From: yahya <19204398+thep2p@users.noreply.github.com> Date: Mon, 27 Jul 2026 14:34:03 -0700 Subject: [PATCH 2/4] [cleanup][docs] Capitalize try_link doc comments --- src/core/lookup/mod.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/core/lookup/mod.rs b/src/core/lookup/mod.rs index 1a52102..c79a30e 100644 --- a/src/core/lookup/mod.rs +++ b/src/core/lookup/mod.rs @@ -7,7 +7,7 @@ mod array_lookup_table_test; /// LookupTableLevel represents level of a lookup table. entry in the table. pub type LookupTableLevel = usize; -/// outcome of a [`LookupTable::try_link`] compare-then-act decision. +/// Outcome of a [`LookupTable::try_link`] compare-then-act decision. #[derive(Debug, Copy, Clone, PartialEq, Eq)] pub enum LinkOutcome { /// the candidate was inserted at the requested `(level, direction)` slot: the slot was @@ -51,7 +51,7 @@ pub trait LookupTable: Send + Sync { /// `candidate` happen as one indivisible step with respect to any other concurrent call on /// the same `(level, direction)` slot — no caller can observe or race a partial decision. /// - /// `direction` is receiver-owned, never re-interpreted hop-to-hop: it always names this + /// The `direction` parameter is receiver-owned, never re-interpreted hop-to-hop: it always names this /// node's own slot (`Direction::Right` this node's own right slot, holding neighbors with /// larger identifiers; `Direction::Left` its own left slot), never something relative to a /// caller or hop. an existing entry sits strictly between this node and `candidate` when, From 80a1ec2b6c0b53bea8e87297ae4c759812779b47 Mon Sep 17 00:00:00 2001 From: yahya <19204398+thep2p@users.noreply.github.com> Date: Tue, 28 Jul 2026 11:11:36 -0700 Subject: [PATCH 3/4] [cleanup][docs] Fix try_link doc grammar and casing --- src/core/lookup/mod.rs | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/src/core/lookup/mod.rs b/src/core/lookup/mod.rs index c79a30e..cfc4076 100644 --- a/src/core/lookup/mod.rs +++ b/src/core/lookup/mod.rs @@ -10,11 +10,11 @@ pub type LookupTableLevel = usize; /// Outcome of a [`LookupTable::try_link`] compare-then-act decision. #[derive(Debug, Copy, Clone, PartialEq, Eq)] pub enum LinkOutcome { - /// the candidate was inserted at the requested `(level, direction)` slot: the slot was + /// The candidate was inserted at the requested `(level, direction)` slot: the slot was /// empty, or its previous occupant did not sit strictly between this node and the /// candidate on that side, so the candidate is now the entry there. LinkedDirectly, - /// the table was left untouched: the carried identity is the existing entry at + /// The table was left untouched: the carried identity is the existing entry at /// `(level, direction)`, and it sits strictly between this node and the candidate on that /// side — it is closer to the candidate's true position, so the link request belongs there /// instead. @@ -43,18 +43,18 @@ pub trait LookupTable: Send + Sync { direction: Direction, ) -> anyhow::Result>; - /// atomically decides whether `candidate` becomes the neighbor at `(level, direction)`, or + /// Atomically decides whether `candidate` becomes the neighbor at `(level, direction)`, or /// whether an existing neighbor there already sits strictly between this node and /// `candidate` on that side and the request should be forwarded to it instead. /// - /// the decision is atomic: inspecting the current entry and, when accepting, inserting + /// The decision is atomic: inspecting the current entry and, when accepting, inserting /// `candidate` happen as one indivisible step with respect to any other concurrent call on /// the same `(level, direction)` slot — no caller can observe or race a partial decision. /// /// The `direction` parameter is receiver-owned, never re-interpreted hop-to-hop: it always names this - /// node's own slot (`Direction::Right` this node's own right slot, holding neighbors with - /// larger identifiers; `Direction::Left` its own left slot), never something relative to a - /// caller or hop. an existing entry sits strictly between this node and `candidate` when, + /// node's own slot (`Direction::Right` is this node's own right slot, holding neighbors with + /// larger identifiers; `Direction::Left` is its own left slot), never something relative to a + /// caller or hop. An existing entry sits strictly between this node and `candidate` when, /// for `Direction::Right`, `existing.id() < candidate.id()`; for `Direction::Left`, /// `existing.id() > candidate.id()`: /// From 731a077645c7461b37fe1efca96913cefa9d4e26 Mon Sep 17 00:00:00 2001 From: yahya <19204398+thep2p@users.noreply.github.com> Date: Wed, 29 Jul 2026 15:45:09 -0700 Subject: [PATCH 4/4] [improve][docs] Document try_link candidate-side precondition --- src/core/lookup/mod.rs | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/src/core/lookup/mod.rs b/src/core/lookup/mod.rs index cfc4076..87bb259 100644 --- a/src/core/lookup/mod.rs +++ b/src/core/lookup/mod.rs @@ -65,6 +65,14 @@ pub trait LookupTable: Send + Sync { /// `candidate` is inserted into this node's own entry at `(level, direction)`, and /// [`LinkOutcome::LinkedDirectly`] is returned. /// + /// # Preconditions + /// + /// The lookup table has no notion of this node's own identifier, so it cannot verify that + /// `candidate` actually belongs on the `direction` side of this node — callers must ensure + /// that before calling. The comparison inside `try_link` only ever weighs the existing entry + /// against `candidate`, never against this node itself, so a violated precondition installs + /// an out-of-order neighbor silently rather than returning an error. + /// /// # Errors /// /// returns an error when `level` is out of bounds.