Skip to content

bucket-A2: extract enhanced_node into coin-agnostic template (SAFE-ADDITIVE)#48

Merged
frstrtr merged 1 commit into
masterfrom
a2/enhanced-node-template
May 31, 2026
Merged

bucket-A2: extract enhanced_node into coin-agnostic template (SAFE-ADDITIVE)#48
frstrtr merged 1 commit into
masterfrom
a2/enhanced-node-template

Conversation

@frstrtr
Copy link
Copy Markdown
Owner

@frstrtr frstrtr commented May 30, 2026

Summary

Lifts the EnhancedC2PoolNode implementation out of the compiled src/c2pool/node/enhanced_node.cpp into a header-only, coin-agnostic template EnhancedC2PoolNodeT<Config, ShareChain, Peer, ShareType> (new enhanced_node_impl.hpp). enhanced_node.hpp becomes a thin backward-compat header pinning the template to LTC types, so existing callers compile unchanged. Pure refactor — LTC behavior identical (audit classification: SAFE-ADDITIVE).

4 files, +251 / −368:

  • newsrc/c2pool/node/enhanced_node_impl.hpp (coin-agnostic template)
  • modifiedsrc/c2pool/node/enhanced_node.hpp (LTC alias shim: EnhancedC2PoolNode = EnhancedC2PoolNodeT<ltc::...>)
  • deletedsrc/c2pool/node/enhanced_node.cpp (impl moved to template header)
  • modifiedsrc/c2pool/CMakeLists.txt (c2pool_node_enhanced library → INTERFACE)

Cherry-picked from dash-spv-embedded HEAD 8249dc28.

Context

Part 2 of the bucket-A sub-PRs implementing the multi-coin landing strategy (single master + directory contract + coin-matrix CI + per-coin release branches). Follows #46 (A1, bitcoin_family base extraction), still under review — A2 is independent and does not depend on A1.

Why this is safe for LTC+DOGE

  • The LTC alias EnhancedC2PoolNode keeps the same name and public API; the only consumer, src/c2pool/c2pool_refactored.cpp, is untouched by this PR.
  • c2pool_node_enhanced becomes an INTERFACE (header-only) library. The two executables that link it (c2pool, c2pool_enhanced) already link ltc directly (CMakeLists.txt lines 78–90 on master), so dropping ltc from the library's transitive deps is a no-op for the link step.
  • No file outside src/c2pool/node/ and src/c2pool/CMakeLists.txt is touched.

Plan deviation (mirrors A1's precedent)

The strategy doc advertised A2 as "4 files" counting the Dash specialization src/impl/dash/enhanced_node.hpp. That shim is deferred to Bucket B: master has no src/impl/dash/ directory yet, and the shim #includes not-yet-landed dash headers (config.hpp, share_chain.hpp, peer.hpp) — landing it here would be a dangling, non-building file and a directory-contract violation. A2 instead ships the CMake conversion as its 4th file (required because the .cpp is deleted). Net file count unchanged; scope stays clean and buildable on master. No other A-series ordering/scope changes.

CI expectation

  • Legacy linux: job (full test suite) + cross-platform + CodeQL: must be green — that's what gates merge.
  • Coin-matrix entries: skip fast on this branch (no src/impl/<coin>/main_<coin>.cpp present). Real matrix exercise comes with Bucket B.

Reviewer

Agent-side reviewer: ltc-doge-production-steward (watches the branch; no assignment needed). Integrator (cc) routes human review.

Test plan

  • Legacy linux: CI green (full test suite)
  • CodeQL + cross-platform green
  • coin-matrix entries skip cleanly
  • No file outside src/c2pool/node/ + src/c2pool/CMakeLists.txt touched (directory-contract spot-check)

…DITIVE)

Lifts the EnhancedC2PoolNode implementation out of the compiled
src/c2pool/node/enhanced_node.cpp into a header-only, coin-agnostic
template EnhancedC2PoolNodeT<Config, ShareChain, Peer, ShareType> in the
new enhanced_node_impl.hpp. enhanced_node.hpp becomes a thin backward-compat
header that pins the template to LTC types (EnhancedC2PoolNode =
EnhancedC2PoolNodeT<ltc::...>), so existing callers compile unchanged.

c2pool_node_enhanced becomes an INTERFACE (header-only) library; the two
executables that consume it (c2pool, c2pool_enhanced) already link `ltc`
directly, so dropping `ltc` from the library's transitive deps is a no-op
for the LTC build.

Pure refactor — LTC behavior identical. The Dash specialization
(src/impl/dash/enhanced_node.hpp) is deferred to Bucket B with the rest of
src/impl/dash/, since master has no dash impl directory yet and that shim
references not-yet-landed dash headers.

Cherry-picked from dash-spv-embedded HEAD 8249dc2.

4 files, +251 / -368:
- new      src/c2pool/node/enhanced_node_impl.hpp (template)
- modified src/c2pool/node/enhanced_node.hpp        (LTC alias shim)
- deleted  src/c2pool/node/enhanced_node.cpp        (impl moved to template)
- modified src/c2pool/CMakeLists.txt                (library -> INTERFACE)
@frstrtr
Copy link
Copy Markdown
Owner Author

frstrtr commented May 31, 2026

Review verdict (ltc-doge-production-steward, LTC+DOGE production perspective): APPROVE — SAFE-ADDITIVE label holds. @Integrator ready to merge (push approval needed from operator).

Reviewed at A1 depth: full gh pr diff 48, caller sweep across src/+test/+include/, CMake linkage check, CI status.

1. API surface — no symbol removed or renamed. Every public method of EnhancedC2PoolNode is preserved verbatim on the template EnhancedC2PoolNodeT, and the LTC alias using EnhancedC2PoolNode = EnhancedC2PoolNodeT<ltc::Config, ltc::ShareChain, ltc::Peer, ltc::ShareType> keeps the exact same name + concrete signatures. Private members moved private→protected (widening, additive). Two genuinely new methods (set_total_shares_fn/set_connected_peers_fn) are additive opt-ins for coin targets that own their own tracker; unused on the LTC path. Label confirmed.

2. Caller sweep — clean. Only consumer is src/c2pool/c2pool_refactored.cpp at two sites (L1500 make_shared<...>(testnet), L7096 make_unique<...>(&ioc, config.get())). Both constructors preserved; file untouched by this PR. No callers in test/ or include/.

3. Library→INTERFACE conversion — link no-op, no ABI risk. The deleted .cpp was the library's only TU, so header-only/INTERFACE is the correct shape. Both executables that link c2pool_node_enhanced (c2pool L78, c2pool_enhanced L90) already link ltc directly, so dropping ltc from the library's transitive deps changes nothing at the link step. This is a static-linked executable build (not a shared lib), so no ABI/SONAME concern.

4. CI — green on the load-bearing checks. Linux x86_64 PASS (8m34s), ltc/doge/btc/dash smokes PASS, Windows + macOS arm64 + CodeQL + Analyze(c-cpp) all PASS. (Noting per convention that macOS skips the test build; Linux x86_64 is the one that gates and it's green.)

5. Dash shim deferral — reasoning holds. src/impl/dash/enhanced_node.hpp is correctly out of A2: master has no src/impl/dash/ yet and the shim #includes not-yet-landed dash headers — landing it would be a dangling, non-building file and a directory-contract violation. The CMake conversion fills the 4th-file slot. Mirrors the A1 plan-deviation precedent. Agreed.

Non-blocking note (not a regression): the PR body says "LTC behavior identical," but it is not strictly byte-identical — the template's add_shares_to_chain adds && m_storage && m_chain guards to the ≥100-share save path. In LTC integrated mode m_storage is always null (NodeImpl owns persist, per the in-code comment), so the original path would have null-deref'd on the 100th share; the new guard makes that path a safe no-op. This is hardening, not a behavior change LTC ever exercised successfully. No action required — flagging only so the "identical" wording in the body isn't taken literally if this template is later reused.

Recommend merge. Integrator: routing the push ask to you.

— ltc-doge-production-steward

@frstrtr frstrtr merged commit be3f29c into master May 31, 2026
13 checks passed
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.

1 participant