Skip to content

Fix POS round state logic #210

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
sanada08 merged 3 commits into from
Aug 6, 2026
Merged

Fix POS round state logic #210

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
6e6b188
Fix POS round state logic
victor-tucci Aug 3, 2026
8b4f157
Update default seed nodes in net_node.inl
victor-tucci Aug 4, 2026
6ba65cd
Bump project version from 7.0.2 to 7.0.3
victor-tucci Aug 4, 2026
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
2 changes: 1 addition & 1 deletion CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ message(STATUS "CMake version ${CMAKE_VERSION}")
set(CMAKE_OSX_DEPLOYMENT_TARGET 10.15 CACHE STRING "macOS deployment target (Apple clang only)")

project(beldex
VERSION 7.0.2
VERSION 7.0.3
LANGUAGES CXX C)
set(BELDEX_RELEASE_CODENAME "Obscura")

Expand Down
36 changes: 27 additions & 9 deletions src/cryptonote_core/pos.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1044,13 +1044,27 @@ round_state goto_wait_for_next_block_and_clear_round_data(round_context &context
return round_state::wait_for_next_block;
}

// Returns true if the blockchain's top block differs from the tip cached in
// the round context — either because a new block arrived (height changed) or
// because a same-height reorg replaced the top block (hash changed at the
// same height).
static bool chain_top_changed(round_context const &context, cryptonote::Blockchain const &blockchain)
{
uint64_t const chain_height = blockchain.get_current_blockchain_height(true /*lock*/);
if (context.wait_for_next_block.height != chain_height)
return true;
Comment on lines +1054 to +1055

@coderabbitai coderabbitai Bot Aug 4, 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.

📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Add braces around the new conditional branches.

Clang reports readability-braces-around-statements at both locations. Add braces before later edits change the controlled statement unintentionally.

Proposed fix 
-  if (context.wait_for_next_block.height != chain_height)
-    return true;
+  if (context.wait_for_next_block.height != chain_height)
+  {
+    return true;
+  }

-    if (chain_top_changed(context, blockchain))
-      return goto_wait_for_next_block_and_clear_round_data(context);
+    if (chain_top_changed(context, blockchain))
+    {
+      return goto_wait_for_next_block_and_clear_round_data(context);
+    }

Also applies to: 1138-1139

🧰 Tools
🪛 Clang (14.0.6)

[warning] 1054-1054: statement should be inside braces

(readability-braces-around-statements)

🤖 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 `@src/cryptonote_core/pos.cpp` around lines 1054 - 1055, Update the conditional
branches in the affected PoS logic, including the check on
context.wait_for_next_block.height and the corresponding branch near the second
location, to wrap each controlled return statement in braces. Preserve the
existing conditions and return behavior.

Source: Linters/SAST tools

return context.wait_for_next_block.top_hash != blockchain.get_block_id_by_height(chain_height - 1);
}
Comment on lines +1047 to +1057

@coderabbitai coderabbitai Bot Aug 4, 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.

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/usr/bin/env bash
set -euo pipefail

# Inspect the lock lifetime and all chain-tip reads.
rg -n -C 12 \
  'get_current_blockchain_height|get_block_id_by_height|get_tail_id|chain_top_changed|wait_for_next_block' \
  src/cryptonote_core --glob '*.cpp' --glob '*.h'

Repository: Beldex-Coin/beldex

Length of output: 50375

🏁 Script executed:

#!/usr/bin/env bash
set -euo pipefail

# Inspect Blockchain tip read implementations and lock names.
fd -a 'blockchain\.(cpp|h)$' src | sed 's#^\./##' | while read -r f; do
  echo "===== $f ====="
  rg -n -C 8 'get_current_blockchain_height|get_tail_id|get_block_id_by_height' "$f" || true
done

# Focused slice around Blockchain implementation declarations/definitions.
rg -n -C 5 'Blockchain::(get_current_blockchain_height|get_tail_id(?:\b|\(.*\))|get_block_id_by_height)' src/cryptonote_core/blockchain.cpp src/cryptonote_core --glob '*.cpp' --glob '*.h'

Repository: Beldex-Coin/beldex

Length of output: 33330

🏁 Script executed:

#!/usr/bin/env bash
set -euo pipefail

# Focused slice around Blockchain implementation declarations/definitions.
rg -n -C 5 'Blockchain::(get_current_blockchain_height|get_tail_id(?:\b|\(.*\))|get_block_id_by_height)' src/cryptonote_core/blockchain.cpp src/cryptonote_core/blockchain.h

Repository: Beldex-Coin/beldex

Length of output: 3608

Use one locked chain-tip snapshot for POS state updates.

wait_for_next_block() reads the chain height, then chain_top_changed() reads it a second time. get_current_blockchain_height(true) releases the lock, and get_block_id_by_height() also acquires its own lock, so the comparison and the state update can observe different chain tips. Use one atomic snapshot, such as get_tail_id(chain_height_out), and reuse that height/hash for prev_hash, timing setup, and the POS context.

🧰 Tools
🪛 Clang (14.0.6)

[warning] 1051-1051: use a trailing return type for this function

(modernize-use-trailing-return-type)

[warning] 1054-1054: statement should be inside braces

(readability-braces-around-statements)

🤖 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 `@src/cryptonote_core/pos.cpp` around lines 1047 - 1057, The POS wait flow
currently reads the chain tip multiple times, allowing height and hash-dependent
state to come from different snapshots. Update wait_for_next_block() and
chain_top_changed() to obtain one locked chain-tip snapshot via the existing
chain API, then reuse its height and hash for the changed check, prev_hash,
timing setup, and POS round context updates.


round_state wait_for_next_block(uint64_t hf17_height, round_context &context, cryptonote::Blockchain const &blockchain)
{
//
// NOTE: If already processing POS for height, wait for next height
// NOTE: If already processing POS for height, wait for next height. The top
// hash is compared too (not just the height) so that a same-height reorg
// re-arms the round context against the new tip
//
uint64_t chain_height = blockchain.get_current_blockchain_height(true /*lock*/);
if (context.wait_for_next_block.height == chain_height)
if (!chain_top_changed(context, blockchain))
{
for (static uint64_t last_height = 0; last_height != chain_height; last_height = chain_height)
MDEBUG(log_prefix(context) << "Network is currently producing block " << chain_height << ", waiting until next block");
Expand Down Expand Up @@ -1119,9 +1133,9 @@ round_state prepare_for_round(round_context &context, master_nodes::master_node_
return goto_wait_for_next_block_and_clear_round_data(context);
}

// Also check if the blockchain has changed, in which case we stop and
// restart POS stages.
if (context.wait_for_next_block.height != blockchain.get_current_blockchain_height(true /*lock*/))
// Also check if the blockchain has changed (new block, or same-height
// reorg), in which case we stop and restart POS stages.
if (chain_top_changed(context, blockchain))
return goto_wait_for_next_block_and_clear_round_data(context);

// 'queue_for_next_round' is set when an intermediate POS stage has failed
Expand Down Expand Up @@ -1161,6 +1175,11 @@ round_state prepare_for_round(round_context &context, master_nodes::master_node_
}

std::vector<crypto::hash> const entropy = master_nodes::get_POS_entropy_for_next_block(blockchain.get_db(), context.wait_for_next_block.top_hash, context.prepare_for_round.round);
if (entropy.empty())
{
MDEBUG(log_prefix(context) << "Unable to prepare POS quorum for height " << context.wait_for_next_block.height << " because quorum entropy is unavailable; waiting for the next block");
return goto_wait_for_next_block_and_clear_round_data(context);
}
auto const active_node_list = blockchain.get_master_node_list().active_master_nodes_infos();
auto hf_version = blockchain.get_network_version();
crypto::public_key const &block_leader = blockchain.get_master_node_list().get_block_leader().key;
Expand Down Expand Up @@ -1212,10 +1231,9 @@ round_state prepare_for_round(round_context &context, master_nodes::master_node_

round_state wait_for_round(round_context &context, cryptonote::Blockchain const &blockchain)
{
const auto curr_height = blockchain.get_current_blockchain_height(true /*lock*/);
if (context.wait_for_next_block.height != curr_height)
if (chain_top_changed(context, blockchain))
{
MTRACE(log_prefix(context) << "Block height changed whilst waiting for round " << +context.prepare_for_round.round << ", restarting POS stages");
MTRACE(log_prefix(context) << "Chain tip changed whilst waiting for round " << +context.prepare_for_round.round << ", restarting POS stages");
return goto_wait_for_next_block_and_clear_round_data(context);
}

Expand All @@ -1231,7 +1249,7 @@ round_state wait_for_round(round_context &context, cryptonote::Blockchain const
// For testing purposes: we apply possible random non-response and random delays to half of all
// blocks; we go in batches of 10: 10 maybe-faulty blocks followed by 10 well-behaved blocks.
// (Faulty blocks have an odd second-last height digit).
if (curr_height % 20 >= 10)
if (context.wait_for_next_block.height % 20 >= 10)
{
size_t faulty_chance = tools::uniform_distribution_portable(tools::rng, 100);
if (faulty_chance < 10)
Expand Down
6 changes: 3 additions & 3 deletions src/p2p/net_node.inl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -593,11 +593,11 @@ namespace nodetool
}
else
{
full_addrs.insert("seed1.beldex.io:19090");
full_addrs.insert("seed1.rpcnode.stream:19090");
full_addrs.insert("seed2.rpcnode.stream:19090");
full_addrs.insert("seed3.beldex.io:19090");
full_addrs.insert("seed3.rpcnode.stream:19090");
full_addrs.insert("seed4.rpcnode.stream:19090");
full_addrs.insert("seed5.beldex.io:19090");
full_addrs.insert("seed5.rpcnode.stream:19090");
Comment on lines +596 to +600

@coderabbitai coderabbitai Bot Aug 4, 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.

🩺 Stability & Availability | 🔵 Trivial

Verify bootstrap redundancy before release.

All five MAINNET seed entries now use rpcnode.stream. A DNS or provider outage for this domain can remove every default bootstrap endpoint. The fallback path also reuses the same seed list at Line 1463-1466. Confirm that seed1.rpcnode.stream, seed3.rpcnode.stream, and seed5.rpcnode.stream resolve to the intended MAINNET P2P services on port 19090. Confirm that the seed infrastructure has independent failure domains.

🤖 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 `@src/p2p/net_node.inl` around lines 596 - 600, Verify that
seed1.rpcnode.stream, seed3.rpcnode.stream, and seed5.rpcnode.stream resolve to
the intended MAINNET P2P services on port 19090, and confirm these endpoints use
independent DNS/provider failure domains. Apply any required corrections to the
default seed entries and the fallback seed list so bootstrap redundancy is
preserved.

}
return full_addrs;
}
Expand Down
Loading