-
-
Notifications
You must be signed in to change notification settings - Fork 106
Description
Problem
Recent bugs (#2010, #1996) revealed that contract operations (PUT/UPDATE) can fail to cache state locally in certain code paths, causing peers to serve stale data. These bugs were hard to catch because:
- Symptoms were timing-dependent and inconsistent
- Multiple interacting bugs masked each other
- No runtime checks verified expected post-conditions
Proposal
Add debug_assert! checks at critical points in operation flows to verify invariants that should always hold. These assertions:
- Fail fast during development/testing when invariants are violated
- Have zero runtime cost in release builds
- Document expected behavior directly in code
- Make violations immediately visible rather than manifesting as subtle bugs later
Suggested Assertions
1. After PUT/UPDATE Initiation
Location: crates/core/src/operations/put.rs after request_put(), update.rs after request_update()
Invariant: "After initiating a PUT/UPDATE, the local peer MUST have the state cached"
#[cfg(debug_assertions)]
{
// Verify the initiating peer has the state cached locally
let local_state = op_manager.ring.get_contract_state(&key).await;
debug_assert!(
local_state.is_some(),
"Invariant violation: initiating peer must cache state after PUT/UPDATE. \
key={}, tx={}",
key, id
);
}2. After Successful State Merge
Location: Test contract and real contracts in update_state()
Invariant: "State merges are idempotent - merging identical states should not change version"
#[cfg(debug_assertions)]
{
debug_assert!(
!state_changed || new_version > old_version,
"Invariant violation: version must increment only when state changes. \
state_changed={}, old_version={}, new_version={}",
state_changed, old_version, new_version
);
}3. After Broadcasting to Network
Location: crates/core/src/operations/put.rs and update.rs after broadcast
Invariant: "Contract must be in seed list after successful PUT"
#[cfg(debug_assertions)]
{
debug_assert!(
op_manager.ring.is_seeding_contract(&key),
"Invariant violation: contract must be in seed list after successful PUT. \
key={}, tx={}",
key, id
);
}4. Version Monotonicity
Location: Contract update_state() implementations
Invariant: "Version numbers must be monotonically increasing"
#[cfg(debug_assertions)]
{
let old_version = extract_version(&old_state);
let new_version = extract_version(&new_state);
debug_assert!(
new_version >= old_version,
"Invariant violation: version must not decrease. \
old={}, new={}",
old_version, new_version
);
}Implementation Notes
- Use
debug_assert!(notassert!) so assertions are compiled out in release builds - Include helpful context in assertion messages (transaction IDs, keys, versions)
- Focus on post-conditions that should always hold after operations complete
- Consider adding a
#[cfg(debug_assertions)]helper module for complex invariant checks
Benefits
- Catches bugs early - Violations fail immediately during testing rather than causing subtle downstream issues
- Self-documenting - Assertions serve as machine-checked documentation of expected behavior
- Zero cost in production - Compiled out in release builds
- Easier debugging - Clear assertion messages pinpoint exactly what went wrong
Related Issues
- fix: client-initiated PUT requests not cached locally when forwarded to target peer #2010 - Client-initiated PUT not cached locally
- fix: persist contract state after PUT merge in upsert_contract_state #1996 - Incoming PUT not cached locally
[AI-assisted debugging and comment]
Metadata
Metadata
Assignees
Labels
Type
Projects
Status