Skip to content

Rename VNode storage model to Entry and harden coverage #607

Description

@rings-auto-reviewer

Motivation

The current VNode / VirtualNode storage model is misleading. In Chord literature, a virtual node usually means an additional logical position for a physical node, often used for balancing. In this codebase, VirtualNode is not a routing node and does not participate in topology as a node. It is a stored resource record with a ring key, payload, kind, operation algebra, cache behavior, and successor-sync behavior.

Current evidence:

  • crates/core/src/dht/vnode.rs defines VNodeType, VNodeOperation, and VirtualNode as the storage record model.
  • crates/core/src/dht/types.rs documents VirtualNode as a resource wrapper, while still calling it a logical node.
  • crates/core/src/dht/chord.rs exposes storage actions such as SomeVNode, FindVNodeForOperate, and SyncVNodeWithSuccessor.
  • crates/core/src/message/types.rs exposes protocol messages such as SearchVNode, FoundVNode, OperateVNode, and SyncVNodeWithSuccessor.
  • crates/node/src exposes the same naming through RPC, provider, processor, and prelude APIs.

This creates a model mismatch: the storage object is named as a node even though its semantics are an entry in the Chord-backed storage layer. It also makes review of Correct Chord / topology behavior harder because node membership and stored entries are not separated at the language level.

The test coverage for this area is also too thin. The current tests cover a few point cases, but they do not witness the full entry algebra, error boundaries, storage protocol paths, or async DHT schedule behavior.

Proposed Solution

Rename the storage model from VNode / VirtualNode to Entry and migrate the public protocol names consistently.

Suggested naming:

  • VirtualNode -> Entry
  • VNodeType -> EntryKind
  • VNodeOperation -> EntryOperation
  • VNodeStorage -> EntryStorage
  • vnode_lookup -> entry_lookup
  • vnode_operate -> entry_operate
  • sync_vnode_with_successor -> sync_entries_with_successor
  • local_cache_put/get parameter names should use entry, not vnode
  • SearchVNode -> SearchEntry
  • FoundVNode -> FoundEntry
  • OperateVNode -> OperateEntry
  • SyncVNodeWithSuccessor -> SyncEntriesWithSuccessor
  • PeerRingAction::SomeVNode -> PeerRingAction::SomeEntry
  • PeerRingRemoteAction::FindVNodeForOperate -> FindEntryForOperate

If these names are public API, keep deprecated compatibility aliases for one release where practical, but internal implementation should use the new names.

Also consider whether the Entry field currently named did should be documented as an entry ring key or renamed in a separate step. The important boundary is that an entry key and a node identity have the same representation but different semantics.

Required Tests

Pure entry model tests:

  • overwrite_replaces_data_for_same_data_entry
  • overwrite_rejects_non_data_entry
  • overwrite_rejects_kind_mismatch
  • overwrite_rejects_key_mismatch
  • extend_appends_data_for_same_entry
  • extend_trims_oldest_items_at_max_len
  • extend_rejects_non_data_entry
  • touch_moves_existing_items_to_end_once
  • touch_trims_oldest_non_touched_items_at_max_len
  • join_subring_adds_member_to_subring_entry
  • join_subring_rejects_non_subring_entry
  • operation_default_entry_matches_operation_kind
  • message_payload_entry_key_targets_successor_of_signer
  • affine_preserves_payload_and_kind_while_rotating_keys

Storage protocol tests:

  • local entry lookup returns SomeEntry without remote action.
  • local miss returns a remote search action for the responsible successor.
  • found entry is written to local cache.
  • local entry operation writes through storage.
  • remote entry operation returns FindEntryForOperate.
  • successor sync sends only entries that no longer belong to the current successor interval.

Handler / async tests:

  • SearchEntry handler returns cached/local entry when present.
  • SearchEntry handler forwards when the responsible node is remote.
  • FoundEntry handler caches every returned entry.
  • OperateEntry handler executes the operation at the responsible node.
  • SyncEntriesWithSuccessor handler stores all incoming entries without leaking unrelated side effects.
  • full DHT async schedule includes entry lookup, operation, cache fill, and successor sync under message reordering.

Test quality requirements:

  • no unwrap, expect, panic!, todo!, unimplemented!, or unchecked indexing in production paths.
  • test names should state the proposition they witness.
  • protocol/topology tests should exercise async schedule behavior, not only linear happy paths.
  • comments should name invariants where Rust types do not fully express them.

Alternatives

DhtRecord or DhtEntry would also be semantically accurate, but this module is already inside the DHT implementation. Entry is shorter and avoids repeating the module context.

PhantomNode is not recommended. It still says Node, so it keeps the topology/routing ambiguity. It also suggests Rust PhantomData-style marker semantics, while this object is runtime data with payload, operations, storage, and sync behavior.

Acceptance Criteria

  • New code uses Entry terminology internally for the storage model.
  • Public APIs are migrated or have a documented deprecation path.
  • Chord node membership and storage entry semantics are visibly separate in names and docs.
  • Entry algebra has unit/table tests for success and failure cases.
  • Entry storage protocol has integration tests for local, remote, cache, operation, and successor-sync paths.
  • Full async DHT schedule coverage includes entry behavior.
  • Nightly rustfmt and Rust clippy checks pass under the repo's normal gates.

Metadata

Metadata

Assignees

No one assigned

    Labels

    enhancementNew feature or requestrustPull requests that update rust code

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions