Skip to content

Conversation

@pwojcikdev
Copy link
Contributor

Should fix:

[ RUN      ] node.bidirectional_tcp
Assertion `wallets.reps ().voting > 0` failed
/workspace/nano/node/request_aggregator.cpp:90 [bool nano::request_aggregator::request(const request_type&, const std::shared_ptr<nano::transport::channel>&)]'
 0# nano::generate_stacktrace[abi:cxx11]() at /workspace/nano/lib/stacktrace.cpp:13
 1# assert_internal(char const*, char const*, char const*, unsigned int, bool, std::basic_string_view<char, std::char_traits<char> >) at /workspace/nano/lib/assert.cpp:28
 2# nano::request_aggregator::request(std::vector<std::pair<nano::block_hash, nano::root>, std::allocator<std::pair<nano::block_hash, nano::root> > > const&, std::shared_ptr<nano::transport::channel> const&) at /workspace/nano/node/request_aggregator.cpp:90
 3# (anonymous namespace)::process_visitor::confirm_req(nano::messages::confirm_req const&) at /workspace/nano/node/message_processor.cpp:215
 4# nano::messages::confirm_req::visit(nano::messages::message_visitor&) const at /workspace/nano/messages/confirm.cpp:56
 5# nano::message_processor::process(nano::messages::message const&, std::shared_ptr<nano::transport::channel> const&) at /workspace/nano/node/message_processor.cpp:298
 6# nano::message_processor::run_batch(std::unique_lock<nano::mutex>&) at /workspace/nano/node/message_processor.cpp:151
 7# nano::message_processor::run() at /workspace/nano/node/message_processor.cpp:125
 8# nano::message_processor::start()::{lambda()#1}::operator()() const at /workspace/nano/node/message_processor.cpp:60
 9# void std::__invoke_impl<void, nano::message_processor::start()::{lambda()#1}>(std::__invoke_other, nano::message_processor::start()::{lambda()#1}&&) at /usr/include/c++/11/bits/invoke.h:61
10# std::__invoke_result<nano::message_processor::start()::{lambda()#1}>::type std::__invoke<nano::message_processor::start()::{lambda()#1}>(nano::message_processor::start()::{lambda()#1}&&) at /usr/include/c++/11/bits/invoke.h:97
11# void std::thread::_Invoker<std::tuple<nano::message_processor::start()::{lambda()#1}> >::_M_invoke<0ul>(std::_Index_tuple<0ul>) at /usr/include/c++/11/bits/std_thread.h:259
12# std::thread::_Invoker<std::tuple<nano::message_processor::start()::{lambda()#1}> >::operator()() at /usr/include/c++/11/bits/std_thread.h:266
13# std::thread::_State_impl<std::thread::_Invoker<std::tuple<nano::message_processor::start()::{lambda()#1}> > >::_M_run() at /usr/include/c++/11/bits/std_thread.h:211
14# execute_native_thread_routine in ./core_test
15# start_thread at ./nptl/pthread_create.c:442
16# 0x00007F25621238C0 at ../sysdeps/unix/sysv/linux/x86_64/clone3.S:83

../ci/tests/run-tests.sh: line 52:    20 Aborted                 (core dumped) "${executable}" "$@"

@gr0vity-dev-bot
Copy link

Test Results for Commit 5f3bc7d

Pull Request 5019: Results
Overall Status:

Test Case Results

  • 5n4pr_conf_change_dependant: PASS (Duration: 144s)
  • 5n4pr_conf_change_independant: PASS (Duration: 142s)
  • 5n4pr_conf_send_dependant: PASS (Duration: 133s)
  • 5n4pr_conf_send_independant: PASS (Duration: 121s)
  • 5n4pr_rocks_10k_bintree: PASS (Duration: 113s)
  • 5n4pr_rocks_10k_change: PASS (Duration: 140s)

Last updated: 2026-01-30 02:22:48 UTC

@pwojcikdev pwojcikdev force-pushed the request-aggregator-disable-when-no-reps branch from 5f3bc7d to e96a053 Compare January 30, 2026 11:37
@pwojcikdev pwojcikdev requested a review from Copilot January 30, 2026 12:45
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR fixes a race condition that caused assertion failures when wallets.reps().voting was checked during request processing. The assertion was racy because the wallet state could change concurrently with message processing, leading to intermittent test failures.

Changes:

  • Removed the racy debug_assert(wallets.reps().voting > 0) from the request method
  • Added a runtime check using threads.empty() to reject requests when the aggregator is disabled
  • Added node_config dependency to check enable_voting during startup
  • Added test coverage for the disabled voting scenario

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.

File Description
nano/node/request_aggregator.hpp Adds node_config as a dependency to access voting configuration
nano/node/request_aggregator.cpp Removes racy assertion, adds early return in start() when voting is disabled, and checks threads.empty() in request() to reject requests when disabled
nano/core_test/request_aggregator.cpp Adds test to verify that requests are rejected when voting is disabled

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@pwojcikdev pwojcikdev merged commit 2232aaf into nanocurrency:develop Jan 30, 2026
32 of 34 checks passed
@pwojcikdev pwojcikdev deleted the request-aggregator-disable-when-no-reps branch January 30, 2026 13:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants