Skip to content

refactor: separate network and consensus logic 1/N [instantsend] #6959

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
merged 5 commits into from
Nov 18, 2025
Merged

refactor: separate network and consensus logic 1/N [instantsend] #6959

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
466e2a2
feat: introduce new bridge interface NetHandler to separate network l…
knst Nov 5, 2025
90d7879
refactor: use NetHandler class to separate instantsend logic and netw…
knst Nov 11, 2025
3c26523
fix: update log records from CInstantSendManager for NetInstantSend i…
knst Nov 11, 2025
a26a974
perf: std::move a shared_ptr with islock in ProcessMessage
knst Nov 15, 2025
d4d64f6
fix: don't remove handlers too early
knst Nov 17, 2025
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: 2 additions & 0 deletions src/Makefile.am
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -252,6 +252,7 @@ BITCOIN_CORE_H = \
instantsend/db.h \
instantsend/instantsend.h \
instantsend/lock.h \
instantsend/net_instantsend.h \
instantsend/signing.h \
kernel/coinstats.h \
key.h \
Expand Down Expand Up @@ -523,6 +524,7 @@ libbitcoin_node_a_SOURCES = \
instantsend/db.cpp \
instantsend/instantsend.cpp \
instantsend/lock.cpp \
instantsend/net_instantsend.cpp \
instantsend/signing.cpp \
kernel/coinstats.cpp \
llmq/blockprocessor.cpp \
Expand Down
8 changes: 8 additions & 0 deletions src/init.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,7 @@
#include <flat-database.h>
#include <governance/governance.h>
#include <instantsend/instantsend.h>
#include <instantsend/net_instantsend.h>
#include <llmq/context.h>
#include <llmq/dkgsessionmgr.h>
#include <llmq/options.h>
Expand Down Expand Up @@ -250,6 +251,9 @@ void Interrupt(NodeContext& node)
if (node.active_ctx) {
node.active_ctx->Interrupt();
}
if (node.peerman) {
node.peerman->InterruptHandlers();
}
if (node.llmq_ctx) {
node.llmq_ctx->Interrupt();
}
Expand Down Expand Up @@ -285,7 +289,9 @@ void PrepareShutdown(NodeContext& node)
StopREST();
StopRPC();
StopHTTPServer();

if (node.active_ctx) node.active_ctx->Stop();
if (node.peerman) node.peerman->StopHandlers();
if (node.llmq_ctx) node.llmq_ctx->Stop();

for (const auto& client : node.chain_clients) {
Expand Down Expand Up @@ -2193,6 +2199,7 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info)
g_active_notification_interface = std::make_unique<ActiveNotificationInterface>(*node.active_ctx, *node.mn_activeman);
RegisterValidationInterface(g_active_notification_interface.get());
}
node.peerman->AddExtraHandler(std::make_unique<NetInstantSend>(node.peerman.get(), *node.llmq_ctx->isman, *node.llmq_ctx->qman, chainman.ActiveChainstate()));
Copy link

@coderabbitai coderabbitai bot Nov 17, 2025

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

🧩 Analysis chain

Verify llmq_ctx initialization before dereferencing.

The code dereferences node.llmq_ctx->isman and node.llmq_ctx->qman without checking if llmq_ctx is null. While the pattern throughout this section suggests llmq_ctx is expected to be initialized by this point, consider adding an assertion to make the assumption explicit:

assert(node.llmq_ctx);
node.peerman->AddExtraHandler(std::make_unique<NetInstantSend>(node.peerman.get(), *node.llmq_ctx->isman, *node.llmq_ctx->qman, chainman.ActiveChainstate()));

This would make the code's expectations clear and provide better diagnostics if the assumption is violated.

Based on learnings

🏁 Script executed:

#!/bin/bash
# Description: Verify llmq_ctx initialization before line 2200 in init.cpp
# Expected: Find where llmq_ctx is initialized or asserted before use

# Search for llmq_ctx initialization/creation in the initialization flow
rg -n -B5 -A2 'llmq_ctx\s*=' src/init.cpp

# Search for assertions on llmq_ctx
rg -n 'Assert.*llmq_ctx|assert.*llmq_ctx' src/init.cpp

# Check how llmq_ctx is used elsewhere for patterns
rg -n -C3 'node\.llmq_ctx->' src/init.cpp | head -50

Length of output: 2417

Looking at the script output, I need to understand the Node structure definition and verify where llmq_ctx is initialized before it's dereferenced. Let me search for more context.

🏁 Script executed:

#!/bin/bash
# Find Node structure definition to see how llmq_ctx is declared
fd -e h -e hpp | xargs rg -l 'struct Node|class Node' | head -5

# Search for where llmq_ctx is assigned in init.cpp
rg -n 'node\.llmq_ctx\s*=' src/init.cpp

# Find assertions or checks at start of InitializeNodeContext function
rg -n -A10 'void InitializeNodeContext' src/init.cpp | head -30

Length of output: 107

🏁 Script executed:

#!/bin/bash
# Get context around line 2100 to understand the function structure before line 2172
rg -n -B50 'node\.cj_walletman = CJWalletManager' src/init.cpp | tail -60

Length of output: 2444

🏁 Script executed:

#!/bin/bash
# Find Node structure definition
fd 'node\.h' | xargs cat 2>/dev/null | grep -A50 'struct Node\|llmq_ctx' | head -80

Length of output: 1995

🏁 Script executed:

#!/bin/bash
# Check context around line 2100-2200 to see if there's an early guard or assertion
rg -n -B80 'node\.cj_walletman = CJWalletManager::make' src/init.cpp | head -100

Length of output: 4889

🏁 Script executed:

#!/bin/bash
# Look for the start of InitializeNodeContext function to understand preconditions
rg -n 'void InitializeNodeContext\(' src/init.cpp -A5

Length of output: 0

🏁 Script executed:

#!/bin/bash
# Find the actual Node struct definition
rg -n 'struct NodeContext\s*{' src/node/context.h -A100 | head -120

Length of output: 107

🏁 Script executed:

#!/bin/bash
# Look for where llmq_ctx is created/initialized in init.cpp
rg -n 'llmq_ctx.*make_unique|llmq_ctx.*std::' src/init.cpp

Length of output: 165

🏁 Script executed:

#!/bin/bash
# Check what happens before line 2164 to see initialization flow
rg -n -B100 'ChainstateManager& chainman = \*Assert' src/init.cpp | grep -A100 'Step 4\|Step 5\|Step 6' | head -80

Length of output: 0

🏁 Script executed:

#!/bin/bash
# Find Node struct with simpler pattern
grep -n 'struct Node' src/node/context.h | head -5

# Look for llmq_ctx member declaration
grep -n 'llmq_ctx' src/node/context.h

# Find the function containing line 2200 and its start
rg -n '^bool AppInitializeNodeContext' src/init.cpp

Length of output: 69

🏁 Script executed:

#!/bin/bash
# Look for early checks before line 2172 in the function
sed -n '1,2172p' src/init.cpp | tail -100 | head -60

Length of output: 3909

🏁 Script executed:

#!/bin/bash
# Look for where llmq_ctx is initialized/created prior to this function
rg -n 'node.llmq_ctx = ' src/init.cpp

Length of output: 0

🏁 Script executed:

#!/bin/bash
# Find which function contains line 2200
sed -n '1900,2200p' src/init.cpp | grep -n 'bool \|void ' | tail -3

Length of output: 189

🏁 Script executed:

#!/bin/bash
# Look for LLMQContext creation/initialization
rg -n 'LLMQContext\(' src/ -g '*.cpp' -g '*.h'

Length of output: 4254

🏁 Script executed:

#!/bin/bash
# Check if there's any Assert on llmq_ctx before line 2172
rg -n -B200 'CJWalletManager::make.*llmq_ctx' src/init.cpp | grep -E 'Assert.*llmq_ctx|assert.*llmq_ctx|llmq_ctx.*Start|Step [0-9]'

Length of output: 0

🏁 Script executed:

#!/bin/bash
# Find the function containing line 2200 by searching backwards from that line
sed -n '1,2200p' src/init.cpp | grep -n '^bool \|^void ' | tail -1

Length of output: 85

🏁 Script executed:

#!/bin/bash
# Look for where LLMQContext is instantiated/created in init.cpp
rg -n 'make_unique<LLMQContext>|std::make_unique.*LLMQContext' src/init.cpp

Length of output: 0

🏁 Script executed:

#!/bin/bash
# Check LoadChainstate signature to see if llmq_ctx is a parameter
rg -n 'ChainstateLoadingError LoadChainstate' src/init.cpp -A5

Length of output: 0

🏁 Script executed:

#!/bin/bash
# Look for any setup/creation of llmq_ctx before the main initialization function
rg -n 'llmq_ctx' src/init.cpp | head -20

Length of output: 1240

🏁 Script executed:

#!/bin/bash
# Search within AppInitMain function (lines 1485-2200+) for llmq_ctx setup
sed -n '1485,2010p' src/init.cpp | grep -n 'llmq_ctx'

Length of output: 64

🏁 Script executed:

#!/bin/bash
# Look for where NodeContext is populated or llmq_ctx is created outside init.cpp
rg -n 'node\.llmq_ctx.*=' src/ -g '*.cpp' | grep -v test | grep -v rpc

Length of output: 241

🏁 Script executed:

#!/bin/bash
# Check if llmq_ctx is created in AppInitializeNodeContext or similar
rg -n 'AppInitialize' src/init.cpp | head -10

Length of output: 0

🏁 Script executed:

#!/bin/bash
# Look for Assert calls with llmq_ctx anywhere in init.cpp
rg -n 'Assert.*llmq_ctx' src/init.cpp

Length of output: 79

Add explicit assertion for llmq_ctx to document precondition.

The code at line 2200 dereferences node.llmq_ctx->isman and node.llmq_ctx->qman without checking if llmq_ctx is null. While the initialization flow ensures llmq_ctx is created before AppInitMain is called and remains non-null throughout this phase (evidenced by similar dereferences at lines 2172, 2186, 2195, 2295 without checks), there is no explicit assertion to document this precondition.

Adding an assertion makes the assumption clear and provides better diagnostics:

assert(node.llmq_ctx);
node.peerman->AddExtraHandler(std::make_unique<NetInstantSend>(node.peerman.get(), *node.llmq_ctx->isman, *node.llmq_ctx->qman, chainman.ActiveChainstate()));

This assertion should be placed before the first dereference of llmq_ctx members in AppInitMain (e.g., before line 2172).

🤖 Prompt for AI Agents 
In src/init.cpp around lines 2172–2200, the code dereferences node.llmq_ctx
(e.g., node.llmq_ctx->isman and ->qman) without an explicit check; add an
assertion documenting the precondition by inserting assert(node.llmq_ctx);
before the first dereference in AppInitMain (before line 2172) so subsequent
uses (including the call at line 2200) are guarded by a clear diagnostic.


// ********************************************************* Step 7d: Setup other Dash services

Expand Down Expand Up @@ -2288,6 +2295,7 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info)
// ********************************************************* Step 10a: schedule Dash-specific tasks

node.llmq_ctx->Start(*node.peerman);
node.peerman->StartHandlers();
if (node.active_ctx) node.active_ctx->Start(*node.connman, *node.peerman);

node.scheduler->scheduleEvery(std::bind(&CNetFulfilledRequestManager::DoMaintenance, std::ref(*node.netfulfilledman)), std::chrono::minutes{1});
Expand Down
Loading
Loading