Skip to content

fix(consensus): two SIP-6 pre-deploy blockers — address case normalization + epoch transition in libp2p sync #763

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
github-actions merged 1 commit into from
Jun 2, 2026
Merged

fix(consensus): two SIP-6 pre-deploy blockers — address case normalization + epoch transition in libp2p sync #763

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
138 changes: 2 additions & 136 deletions bin/sentrix/src/main.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ use libp2p::Multiaddr;
use sentrix::api::events::EventBus;
use sentrix::api::routes::{SharedState, create_router_with_bus};
use sentrix::core::blockchain::{BLOCK_TIME_SECS, Blockchain};
use sentrix::core::transaction::PROTOCOL_TREASURY;
use sentrix::network::libp2p_node::{LibP2pNode, make_multiaddr};
use sentrix::network::node::{DEFAULT_PORT, NodeEvent};
use sentrix::storage::db::Storage;
Expand Down Expand Up @@ -2347,74 +2346,7 @@ async fn cmd_start(
validator_fee,
);

if sentrix::core::epoch::EpochManager::is_epoch_boundary(height) {
tracing::info!("Epoch boundary at height {} — transitioning", height);
let released = bc.stake_registry.process_unbonding(height);
for (delegator, amount) in &released {
// V4 Step 3: post-reward-v2 fork, unbonded
// stake returns from treasury (where it
// was escrowed on Delegate), not a fresh
// mint. Pre-fork path keeps legacy credit
// behaviour for existing chain.db state.
let r = if Blockchain::is_reward_v2_height(height) {
bc.accounts.transfer(
PROTOCOL_TREASURY,
delegator,
*amount,
0,
)
} else {
bc.accounts.credit(delegator, *amount)
};
r.unwrap_or_else(|e| tracing::warn!("unbonding release failed: {}", e));
}
if !released.is_empty() {
tracing::info!("Released {} unbonding entries", released.len());
}

bc.stake_registry.update_active_set();
let active_set = bc.stake_registry.active_set.clone();
let total_staked: u64 = active_set.iter()
.filter_map(|a| bc.stake_registry.get_validator(a))
.map(|v| v.total_stake())
.sum();
bc.epoch_manager.record_block(0);
let finished = bc.epoch_manager.current_epoch.clone();
bc.epoch_manager.history.push(finished);
if bc.epoch_manager.history.len() > bc.epoch_manager.max_history {
bc.epoch_manager.history.remove(0);
}
let next_num = bc.epoch_manager.current_epoch.epoch_number + 1;
let next_start = next_num * sentrix::core::epoch::EPOCH_LENGTH;
bc.epoch_manager.current_epoch = sentrix::core::epoch::EpochInfo {
epoch_number: next_num,
start_height: next_start,
end_height: next_start + sentrix::core::epoch::EPOCH_LENGTH - 1,
validator_set: active_set.clone(),
total_staked,
total_rewards: 0,
total_blocks_produced: 0,
};
// Phase 3 WS: notify sentrix_subscribe(validatorSet)
// — full epoch-advance wire-up. Subscribers see
// the new active set on every epoch boundary,
// not just AddSelfStake re-entry.
if let Some(emitter) = &bc.event_emitter {
emitter.emit_validator_set(next_num, &active_set);
}
tracing::info!("Epoch {} started — {} validators, {} staked",
next_num, active_set.len(), total_staked);

let mut slashing = std::mem::take(&mut bc.slashing);
let slashed = slashing.check_liveness(
&mut bc.stake_registry, &active_set, height,
);
bc.slashing = slashing;
for (val, amt) in &slashed {
tracing::warn!("Slashed {} for {} sentri (downtime)", val, amt);
bc.accounts.burn(*amt);
}
}
bc.run_epoch_bookkeeping(height);

tracing::info!(
"BFT finalized height={} round={}",
Expand Down Expand Up @@ -2963,73 +2895,7 @@ async fn cmd_start(
validator_fee,
);

if sentrix::core::epoch::EpochManager::is_epoch_boundary(height) {
tracing::info!("Epoch boundary at height {} — transitioning", height);
let released = bc.stake_registry.process_unbonding(height);
for (delegator, amount) in &released {
// V4 Step 3 — mirror of the self-produced
// finalize handler above; unbonded stake
// returns from treasury post-fork.
let r = if Blockchain::is_reward_v2_height(height) {
bc.accounts.transfer(
PROTOCOL_TREASURY,
delegator,
*amount,
0,
)
} else {
bc.accounts.credit(delegator, *amount)
};
r.unwrap_or_else(|e| tracing::warn!("unbonding release failed: {}", e));
}
if !released.is_empty() {
tracing::info!("Released {} unbonding entries", released.len());
}

bc.stake_registry.update_active_set();
let active_set = bc.stake_registry.active_set.clone();
let total_staked: u64 = active_set.iter()
.filter_map(|a| bc.stake_registry.get_validator(a))
.map(|v| v.total_stake())
.sum();
bc.epoch_manager.record_block(0);
let finished = bc.epoch_manager.current_epoch.clone();
bc.epoch_manager.history.push(finished);
if bc.epoch_manager.history.len() > bc.epoch_manager.max_history {
bc.epoch_manager.history.remove(0);
}
let next_num = bc.epoch_manager.current_epoch.epoch_number + 1;
let next_start = next_num * sentrix::core::epoch::EPOCH_LENGTH;
bc.epoch_manager.current_epoch = sentrix::core::epoch::EpochInfo {
epoch_number: next_num,
start_height: next_start,
end_height: next_start + sentrix::core::epoch::EPOCH_LENGTH - 1,
validator_set: active_set.clone(),
total_staked,
total_rewards: 0,
total_blocks_produced: 0,
};
// Phase 3 WS: notify sentrix_subscribe(validatorSet) —
// libp2p-applied path mirror of the validator-finalize
// emit above. Both call sites must emit so subscribers
// see the rotation regardless of which path applied
// the boundary block.
if let Some(emitter) = &bc.event_emitter {
emitter.emit_validator_set(next_num, &active_set);
}
tracing::info!("Epoch {} started — {} validators, {} staked",
next_num, active_set.len(), total_staked);

let mut slashing = std::mem::take(&mut bc.slashing);
let slashed = slashing.check_liveness(
&mut bc.stake_registry, &active_set, height,
);
bc.slashing = slashing;
for (val, amt) in &slashed {
tracing::warn!("Slashed {} for {} sentri (downtime)", val, amt);
bc.accounts.burn(*amt);
}
}
bc.run_epoch_bookkeeping(height);

tracing::info!(
"BFT finalized height={} round={}",
Expand Down
76 changes: 76 additions & 0 deletions crates/sentrix-core/src/blockchain.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -519,6 +519,82 @@ impl Blockchain {
self.total_minted
}

/// Run epoch-boundary bookkeeping for the block at `height` if it is
/// an epoch boundary. Encapsulates process_unbonding → unbond releases
/// → update_active_set → epoch rotation → slashing.check_liveness so
/// both the BFT-finalize path (main.rs) and the libp2p sync paths
/// (gossip + GetBlocks catch-up) run identical logic.
///
/// Call AFTER record_block + distribute_reward have already run for
/// this block. Returns without doing anything if the height is not
Comment on lines +528 to +529

@coderabbitai coderabbitai Bot Jun 2, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Remove the second epoch_manager.record_block from the boundary helper.

The new contract says callers reach this helper after record_block has already run, and the PR objective says the libp2p apply paths already did that. Calling self.epoch_manager.record_block(0) again on Line 561 double-counts every boundary block before finished is pushed into history, so epoch block totals drift by one at each rollover.

Suggested fix 
-        self.epoch_manager.record_block(0);
         let finished = self.epoch_manager.current_epoch.clone();
         self.epoch_manager.history.push(finished);

Also applies to: 561-563

🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@crates/sentrix-core/src/blockchain.rs` around lines 528 - 529, The boundary
helper currently calls self.epoch_manager.record_block(0) a second time (the
duplicate call at the boundary helper where `finished` is pushed into history),
which double-counts boundary blocks; remove the redundant
self.epoch_manager.record_block(0) invocation from the boundary helper so the
helper assumes callers have already invoked epoch_manager.record_block, and
update the helper's docstring/comments to state that contract explicitly (no
other logic change required).

/// an epoch boundary.
pub fn run_epoch_bookkeeping(&mut self, height: u64) {
use sentrix_primitives::transaction::PROTOCOL_TREASURY;
use sentrix_staking::epoch::{EpochInfo, EpochManager, EPOCH_LENGTH};

if !EpochManager::is_epoch_boundary(height) {
return;
}

tracing::info!("Epoch boundary at height {} — transitioning", height);
let released = self.stake_registry.process_unbonding(height);
for (delegator, amount) in &released {
let r = if Self::is_reward_v2_height(height) {
self.accounts.transfer(PROTOCOL_TREASURY, delegator, *amount, 0)
} else {
self.accounts.credit(delegator, *amount)
};
r.unwrap_or_else(|e| tracing::warn!("unbonding release failed: {}", e));
}
Comment on lines +540 to +548

@coderabbitai coderabbitai Bot Jun 2, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== process_unbonding definitions and call sites =="
rg -n -C4 '\bfn\s+process_unbonding\b|\bprocess_unbonding\s*\(' --glob '*.rs'

echo
echo "== nearby unbonding mutation logic =="
rg -n -C4 '\bunbond|\brelease|\bretain\b|\bremove\b' --glob '*.rs' | rg 'process_unbonding|unbond'

Repository: sentrix-labs/sentrix

Length of output: 9218

process_unbonding mutates state before payout; swallowed transfer failures can drop releases
StakeRegistry::process_unbonding builds matured_keys and removes matured entries from self.unbonding_queue (unbonding_queue.remove(&key)) before returning released. The caller in crates/sentrix-core/src/blockchain.rs then performs accounts.transfer(...) / accounts.credit(...) but only warns on failure (unwrap_or_else(...warn...)). If those calls fail and the block execution doesn’t roll back the removal, the unbonding entries (and their eventual payouts) are consumed with no retry path.

🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@crates/sentrix-core/src/blockchain.rs` around lines 540 - 548,
process_unbonding currently removes entries from StakeRegistry.unbonding_queue
(StakeRegistry::process_unbonding) before the caller in blockchain.rs performs
payouts via self.accounts.transfer / self.accounts.credit, and transfer failures
are merely warned about, which can permanently drop releases; fix by making the
payout step atomic: either (A) change StakeRegistry::process_unbonding to return
the matured keys and amounts without removing them and only remove entries after
successful transfer/credit, or (B) have process_unbonding keep removals but
return a Result and on any accounts.transfer/accounts.credit error reinsert the
corresponding unbonding entries or return Err to the caller so the block
execution can roll back; ensure you update the blockchain.rs caller to propagate
errors instead of only logging (remove unwrap_or_else warning-only handling) and
reference the same delegator/amount pairs when reinserting or committing
removals.

if !released.is_empty() {
tracing::info!("Released {} unbonding entries", released.len());
}

self.stake_registry.update_active_set();
let active_set = self.stake_registry.active_set.clone();
let total_staked: u64 = active_set
.iter()
.filter_map(|a| self.stake_registry.get_validator(a))
.map(|v| v.total_stake())
.sum();

self.epoch_manager.record_block(0);
let finished = self.epoch_manager.current_epoch.clone();
self.epoch_manager.history.push(finished);
if self.epoch_manager.history.len() > self.epoch_manager.max_history {
self.epoch_manager.history.remove(0);
}
let next_num = self.epoch_manager.current_epoch.epoch_number + 1;
let next_start = next_num * EPOCH_LENGTH;
self.epoch_manager.current_epoch = EpochInfo {
epoch_number: next_num,
start_height: next_start,
end_height: next_start + EPOCH_LENGTH - 1,
validator_set: active_set.clone(),
total_staked,
total_rewards: 0,
total_blocks_produced: 0,
};

if let Some(emitter) = &self.event_emitter {
emitter.emit_validator_set(next_num, &active_set);
}
tracing::info!(
"Epoch {} started — {} validators, {} staked",
next_num,
active_set.len(),
total_staked
);

let mut slashing = std::mem::take(&mut self.slashing);
let slashed = slashing.check_liveness(&mut self.stake_registry, &active_set, height);
self.slashing = slashing;
for (val, amt) in &slashed {
tracing::warn!("Slashed {} for {} sentri (downtime)", val, amt);
self.accounts.burn(*amt);
}
}

// Memory estimate reflects the in-memory window, not the full historical chain
pub fn get_memory_estimate(&self) -> String {
let window_blocks = self.chain.len();
Expand Down
10 changes: 10 additions & 0 deletions crates/sentrix-network/src/libp2p_node.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1132,6 +1132,11 @@ async fn on_swarm_event(
);
// Epoch tracking — mirror main.rs validator-finalize.
chain.epoch_manager.record_block(reward);
// Epoch boundary transition — rotate active set,
// release unbonding, run liveness slashing.
// Previously missing here; libp2p-synced nodes
// diverged from BFT-finalize path at boundaries.
chain.run_epoch_bookkeeping(gossip.block.index);
Comment on lines +1135 to +1139

@coderabbitai coderabbitai Bot Jun 2, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical | ⚡ Quick win

SentrixRequest::NewBlock still misses the same epoch-boundary transition.

This wires run_epoch_bookkeeping into gossip, but the inbound SentrixRequest::NewBlock apply path at Lines 1692-1714 still stops after chain.epoch_manager.record_block(reward). A peer delivering an epoch-boundary block over that request/response route will keep stale active_set / epoch state and can still fork for the same reason this PR is fixing here.

Suggested minimal follow-up 
                         chain.epoch_manager.record_block(reward);
+                        chain.run_epoch_bookkeeping(block_idx);

As per coding guidelines, crates/sentrix-network/**: CONSENSUS-CRITICAL — suggestions only, no destructive rewrites.

🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@crates/sentrix-network/src/libp2p_node.rs` around lines 1135 - 1139, The
NewBlock request handling path must also invoke the epoch-boundary transition;
after the existing call to chain.epoch_manager.record_block(reward) in the
SentrixRequest::NewBlock apply path, call
chain.run_epoch_bookkeeping(gossip.block.index) (same as the gossip path) so the
active_set/unbonding/liveness slashing are updated for epoch-boundary blocks;
locate the SentrixRequest::NewBlock handler and add the run_epoch_bookkeeping
call immediately after chain.epoch_manager.record_block(reward) to keep epoch
state consistent with the gossip branch.

}
let updated =
chain.latest_block().ok().cloned().unwrap_or(gossip.block);
Expand Down Expand Up @@ -2018,6 +2023,11 @@ async fn on_inbound_response(
0,
);
chain.epoch_manager.record_block(reward);
// Epoch boundary transition — rotate active set,
// release unbonding, run liveness slashing.
// Previously missing here; batch-syncing nodes
// diverged from BFT-finalize path at boundaries.
chain.run_epoch_bookkeeping(block.index);
}
// Use H2 (post-add_block state_root hash) — not the raw peer block (PR #78).
let updated = chain
Expand Down
Loading
Loading