fix(merge): route identity-tagged single-value channels through conflict detection#520
Draft
spreston8 wants to merge 13 commits into
Draft
fix(merge): route identity-tagged single-value channels through conflict detection#520spreston8 wants to merge 13 commits into
spreston8 wants to merge 13 commits into
Conversation
Co-Authored-By: Claude <noreply@anthropic.com>
Co-Authored-By: Claude <noreply@anthropic.com>
Co-Authored-By: Claude <noreply@anthropic.com>
…ict detection Channels whose identity matches a registered mergeable tag are bound by the single-value contract regardless of end-of-deploy value shape. When the value isn't a single Int, the channel previously dropped out of number_channels_data and the merger fell through to the multiset combine path, silently unioning distinct sibling writes. Carry the identity-tagged set alongside number_channels_data through EventLogIndex and the deploy/replay pipeline. Add a new conflicts() check that flags two-branch writes on contract-bound channels lacking a commutative representation so they are resolved through the rejected- deploy buffer flow rather than the multiset path. Co-Authored-By: Claude <noreply@anthropic.com>
Add docs/casper/STATE_MERGING.md covering the two-layer mergeability model (CSP multiset vs Number-channel single-value contract), the four conflicts() checks with predicates, the end-to-end merger pipeline (EventLogIndex → conflicts → resolve → compute_merged_state → compute_trie_actions), the rejected-deploy buffer recovery flow, and the MergeableChsForDeploy carrier. Fix CONSENSUS_PROTOCOL.md §6 step 3 — enumerate all conflict checks (same-I/O-event races, potential cross-branch COMMs, base-join touches, identity-tagged non-commutative pending writes, same user deploy ID) instead of only naming the same-deploy-id pass. Expand casper/README.md §Mergeable Channels to name the single-value contract, distinguish commutative vs non-commutative representations for tagged channels, and reference the new identity_tagged_channels field. Expand rspace/README.md §Merger with the new struct names and a link to STATE_MERGING.md. Co-Authored-By: Claude <noreply@anthropic.com>
Document and pin the structural safety of the asymmetric case (one branch's produce mergeable, the other's not) at conflicts() check #4. calculate_number_channel_merge always emits exactly one datum from the commutative diff, so the single-value invariant is preserved regardless of what a non-mergeable branch adds to ChannelChange::added. The asymmetric case therefore does not require conflict-detection intervention. Adds a code comment to that effect next to check #4 and a companion negative test asserting conflicts() does not flag the asymmetric case. Co-Authored-By: Claude <noreply@anthropic.com>
Production uses compute_conflict_map_event_indexed (inverted-index O(n) pass) to build the pairwise conflict map, not conflicts() (O(n²) pairwise). Check #4 added in the previous commit only lived in conflicts(), so it was structurally invisible to production — the unit-level test passed but CI continued to wedge. Adds an inverted index for non-mergeable pending produces on identity-tagged channels, populated in the same single pass as unconsumed_produces_by_channel, and emits pair conflicts for channels with 2+ contributing branches. New test exercises the event-indexed function on the same input shape conflicts() handles, pinning the production code path. Co-Authored-By: Claude <noreply@anthropic.com>
Six observation points emit ERROR-level [MULTI-DATUM-ORIGIN] events when a tagged channel has >1 Datum: process_deploy (play), play_system_deploy_internal (play), get_number_channel sanitize fallback, convert_number_channels_to_diff strict-fail, and the replay-side equivalents in replay_runtime. Each event logs path, kind, channel hash, datum count, merge type, and per-Datum produce-hash to correlate back to the originating deploy via event logs. The empirical bridge test exercises bridge.rho (the exact contract test_shard_degradation deploys) against genesis-derived TreeHashMaps with multi-deploy-per-block + closeBlock + custom high-balance genesis. Across 100 iterations / 25,700 tagged channel observations / 400 bridge deploys it produces zero multi-Datum — single-runtime reproduction is exhausted. Instrumentation is purely additive; existing behavior unchanged. Removal: grep -rn '\[MULTI-DATUM-ORIGIN\]\|multi_datum_origin'. Co-Authored-By: Claude <noreply@anthropic.com>
CI's casper unit-test job uses -D warnings. The three imports (block_dag_storage_from_dyn, KeyValueBlockStore, HistoryReader) were carried over from the earlier sequential-bridge test draft and aren't reached after the rewrite. Production binary build was unaffected; instrumentation captured the wedge correctly. Co-Authored-By: Claude <noreply@anthropic.com>
Adds per-produce / per-consume diagnostics on identity-tagged channels in
the rholang interpreter. Closes the forensic loop left by the existing
post-eval MULTI-DATUM-ORIGIN events: those observed multi-Datum AFTER it
landed; these new events fire AT the moment of write with the
produced_hash that identifies which Datum was just produced, so operators
can correlate orphan datum sources back to their originating produce.
reduce.rs::produce_inner — after space.produce on a tagged channel:
DEBUG (normal) / WARN (datum_count > 1) at target f1r3fly.merge.tagged_op
fields: op, channel_short, channel_full, merge_type, persistent, path,
produced_hash, datum_count, datum_sources (WARN only)
reduce.rs::consume_inner — analogous for each tagged consume source.
#[tracing::instrument] spans on process_deploy, play_system_deploy_internal,
run_user_deploy, replay_system_deploy_internal — carry deploy_sig + path or
kind + system_deploy_type, so child events emitted during eval inherit the
deploy correlation context.
node/src/main.rs::init_json_logging — flip with_current_span(false) to
true so span fields appear on nested event JSON. Was previously discarded.
CI workflow integration-test step gets RUST_LOG="info,f1r3fly.merge=debug":
baseline info preserves all lifecycle anchors and avoids third-party crate
noise; prefix directive raises the new tagged_op events to DEBUG and keeps
existing TRACE-level tag_check events filtered out.
Co-Authored-By: Claude <noreply@anthropic.com>
…ests Add structured logging across every stage of the multi-parent merge pipeline so the next CI run can reconstruct the orphan datum's origin: - get_number_channels_data: per-deploy commutative + identity_tagged summary - save/get_mergeable_data: store-roundtrip preservation of identity_tagged - block_index::create_event_log_index: per-deploy entry shape - EventLogIndex::new + ::combine: full set sizes + MergeType mismatches - DeployChainIndex::new: per-chain multi-added detection - compute_conflict_map_event_indexed: per-check pair counts + CHECK-4-CANDIDATE - resolve_conflicts: branch grouping + conflict_map non-empty entries - compute_merged_state: per-item state_changes + post-combine multi-added - StateChange::combine + ChannelChange::combine: per-channel overlap detection - compute_trie_actions: OR-fold vs multiset dispatch decision per channel - make_trie_action: WARN when writing >1 datum to a channel (smoking gun) - apply_trie_actions: LMDB write breakdown with multi-write WARN All emit under f1r3fly.merge.* targets so existing CI RUST_LOG covers them. Add direct-merge regression test variants in merge_bitmask_or_tagged_channel_spec covering linear-consume, peek-then-consume, persistent-produce, and produce-no-consume patterns. Variants C and D reproduce multi-Datum-with- rejection: PR #520 check #4 fires correctly but pre-existing datums survive. Co-Authored-By: Claude <noreply@anthropic.com>
dag_merger.rs::compute_branch_derived had a filter that excluded any DeployChainIndex containing a system deploy (closeBlock/slash/heartbeat) from `combined_event_log`. The filter was correct for `user_deploy_ids` (prevents every two blocks being marked as mutual conflicts via their shared closeBlock marker) but incorrect for `combined_event_log`. The event log feeds compute_conflict_map_event_indexed's inverted indexes. Excluding system-deploy-containing chains blinds the conflict checks (#1-#4) — they see empty event logs, emit zero pair candidates, and conflict_set_merger accepts every branch. The merger then multiset- combines state_changes from those "phantom" chains, dropping multi-Datum onto tagged channels via make_trie_action's multiset_diff fallthrough. In production, user deploys typically merge into one chain with the block's closeBlock via shared state dependency (gas accounting touches shared system contracts). The filter swallows that entire chain. Confirmed via CI run 25973566430 readonly@22:07:53.281546Z: identity_tagged_non_mergeable_pending_channels: 0 but state_changes had multi-element added on channel 17bf47748d73a5fa driving make_trie_action to write 2 datums. Lift BranchDerived + compute_branch_derived out of merge() to module level (pub) for direct unit testing. Add dag_merger_branch_derived_spec regression tests covering: chain with user+system deploys must include its event log; chain with only user deploys must work; all-system chain's event log must still contribute. Also add variants E (multi-block via compute_state), F (pre-state orphan propagation), G (peek+produce-no-consume bootstrap), H (parallel-produce bootstrap) to merge_bitmask_or_tagged_channel_spec as regression guards documenting the multi-Datum class. Co-Authored-By: Claude <noreply@anthropic.com>
The prior commit (bf3d274) removed the chain-level system-deploy filter on combined_event_log but didn't compensate for the events it exposed. CI run 25974799710 showed the consequence: 700+ produces_consumed events from closeBlock's PoS state mutex (runMVar on the shared stateCh) fired spurious check #1 races across sibling closeBlocks. Validators diverged on rejection sets → BondsCacheMismatch, KvStoreError "Missing mergeable entry", LFB stalls, consensus breakage. This commit replaces the chain-level filter with per-deploy commutativity classification, threaded through EventLogIndex::new at construction time: - closeBlock + heartbeat: is_commutative_system_deploy=true → all produces/consumes marked mergeable, so check #1's both_mergeable override skips them. - slash: is_commutative_system_deploy=false → channel-based filter (same as user deploys). Conservative; slash semantics being reworked separately. - user deploys: is_commutative_system_deploy=false → channel-based filter (unchanged behavior). With per-event classification at construction time, the chain-level filter in dag_merger::compute_branch_derived can be dropped safely. User-deploy events on tagged non-commutative channels remain non- mergeable → check #4 fires on the wedge surface → rejection. Validates against CSV pipeline spec coverage (28 cell tests across the diagonal X-cells + closeBlock-shape variants). All 5 previously-ignored wedge reproducers now pass: - 4! × 4! same V₀ tagged non-commutative + closeBlock chain - ! X × ! X tagged non-commutative + closeBlock chain - ! 4 × ! 4 same-for tagged non-commutative + closeBlock chain - !! 4 × !! 4 same-for tagged non-commutative + closeBlock chain - C ! × C ! same-produce tagged non-commutative + closeBlock chain Plus the 2 previously-ignored dag_merger_branch_derived tests asserting combined_event_log includes chains with system deploys. Regression check (release mode): 52 merging integration + 101 casper unit + 34 rspace++ merger unit + 68 batch2 multi-validator tests — 0 failures. The naive filter-removal regression mode from bf3d274 does not recur. Schema unchanged: MergeableChsForDeploy and mergeable_store wire format identical; is_commutative_system_deploy is derived from SystemDeployData at create_event_log_index time, not persisted. See docs/casper/STATE_MERGING.md for the two-layer mergeability model this fix preserves. Co-Authored-By: Claude <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Channels whose identity matches a registered mergeable tag are bound by the single-value contract regardless of end-of-deploy value shape. When the value isn't a single Int (e.g. a non-numeric write to a tag-prefixed channel), the channel previously dropped out of
number_channels_dataand the merger fell through to the multiset combine path, silently unioning distinct sibling writes — corrupting a state that downstream readers expect to hold one value.This PR carries the identity-tagged set alongside
number_channels_datathroughEventLogIndexand the deploy/replay pipeline, and adds a newconflicts()check that flags two-branch writes on contract-bound channels lacking a commutative representation. Such cases are now resolved through the rejected-deploy buffer flow rather than the multiset path.Key changes
MergeableChsForDeploy(rspace++/src/rspace/merger/merging_logic.rs) — new carrier type bundlingcommutative: NumberChannelsEndValandidentity_tagged: HashableSet<Blake2b256Hash>. Threaded through the runtime, replay, runtime_manager, block_index pipeline.EventLogIndex.identity_tagged_channels— new field, populated from the runtime path, read byconflicts().conflicts()check Integrate Rancher into reproducible development environment for Kubernetes cluster provisioning and management #4 — flags channels where both branches leave a pending non-produces_mergeableproduce on the same identity-tagged channel.MergeTypeunchanged — closed sum of commutative strategies; no new variant added.ChannelChange::combineunchanged — CSP-level multiset semantics preserved; the contract is now closed upstream inconflicts().Background
The mechanism: at the seam between CSP-level mergeability (multiset, encoded in
ChannelChange::combine) and the higher-level Number-channel single-value contract (encoded inmergeable_tags), contract membership was decided per-deploy by end-of-deploy value shape. Channels whose identity matched a tag but whose value was non-Int (e.g. a TreeHashMap leaf holding a Map) silently fell out of the contract for that deploy. When multiple sibling branches each landed such a write, the merger combined them via the multiset path, producing a multi-datum post-state on a channel the runtime reads as single-valued. The next propose attempt would read that state viaconvert_to_read_number, raiseNumber channel ... has N pre-state values; single-value invariant violated, and wedge.The fix decouples contract membership from value shape: the identity-tagged set is always populated from the runtime's
mergeable_tagsfilter and carried through to conflict detection, so two-branch writes on contract-bound channels are flagged regardless of whether either branch produced a commutative-mergeable end value.Co-Authored-By: Claude noreply@anthropic.com