Skip to content

add Config.BinaryPath, PATH auto-detect, and Variant() (#97, #98, #99) - #111

Merged
neverDefined merged 2 commits into
mainfrom
feat/phase-6-binary-selection
Apr 26, 2026
Merged

add Config.BinaryPath, PATH auto-detect, and Variant() (#97, #98, #99)#111
neverDefined merged 2 commits into
mainfrom
feat/phase-6-binary-selection

Conversation

@neverDefined

@neverDefined neverDefined commented Apr 26, 2026

Copy link
Copy Markdown
Owner

Summary

  • Config.BinaryPath opt-in override: absolute / relative / bare-name (resolved via PATH).
  • PATH auto-detect when unset: bitcoind-inquisition first, falls back to bitcoind.
  • Variant() getter parses getnetworkinfo.subversion once and caches under a mutex.
  • Embedded bitcoind_manager.sh honors BITCOIND_BIN / BITCOIN_CLI_BIN env vars; defaults to literal names so direct invocation still works.

This is the foundation PR for Phase 6 — subsequent PRs add the deployment registry (PR2), worked example + README (PR3), and the time API (PR4).

Closes #97, #98, #99.

Test plan

  • make ai-check green (fmt + vet + lint + test-race + vuln).
  • New tests pass:
    • Test_Config_BinaryPath_Resolved — explicit BinaryPath = $(which bitcoind) starts/stops cleanly.
    • Test_Config_BinaryPath_Invalid — nonexistent path fails fast at New() with the path in the error.
    • TestRPC_Variant_Returns — running against Core resolves to VariantCore (not Unknown).
    • TestRPC_Variant_Cached — second call returns the same value after the node is stopped (proves caching).
    • TestRPC_Variant_PreStart — pre-Start() call returns errNotConnected.
    • TestRPC_Variant_StringRoundTrip — enum strings stable.
    • Test_Config updated to round-trip BinaryPath through Config().
  • Reviewer-only: confirm against an Inquisition build that Variant() returns VariantInquisition (deferred to [Phase 6.E.4] TestVariantDetection smoke test #109 in PR5; out of scope here).

Notes

  • Additive only — no breaking signatures, per CLAUDE.md invariant Config #5.
  • Variant caching uses sync.Mutex + flag rather than sync.Once to avoid permanently caching a transient RPC error on the first call.
  • bitcoin-cli is resolved as a sibling of bitcoind first, then via PATH — so Inquisition installs that ship paired binaries in their own directory work without further configuration.

🤖 Generated with Claude Code

neverDefined and others added 2 commits April 26, 2026 16:28
Phase 6.A: lay the groundwork for Bitcoin Inquisition support.

Config.BinaryPath lets callers point the harness at any bitcoind binary
without rewriting PATH. When unset, resolveBinary searches PATH for
bitcoind-inquisition first and falls back to bitcoind, so an Inquisition
build alongside Core "just works." Both binaries flow into the embedded
manager script via BITCOIND_BIN / BITCOIN_CLI_BIN env vars; the script
defaults to the literal names so direct invocation still works.

Variant() introspects the running node via getnetworkinfo.subversion,
returning VariantCore or VariantInquisition. The result is cached behind
a mutex so subsequent calls are free; required for SupportsBIP and the
skip-when-missing pattern landing in PR2.

Closes #97, #98, #99.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…rams, skip Core-only tests

End-to-end testing against a real Inquisition 29.2 build surfaced three
defects in the auto-detect path:

1. Inquisition's getnetworkinfo subversion is /Satoshi:29.2.0(inquisition)/
   (lowercase). The case-sensitive Contains check resolved it as Core.
   Extracted parseVariant as a pure helper, made it case-insensitive, and
   added Test_ParseVariant pinning Core/Inquisition strings + casing
   variants so the resolver path is covered without a live binary.

2. Inquisition's -vbparams parser is strict on the 3-field form
   (deployment:start:timeout) where Core 24+ accepts both 3 and 4 fields.
   Emit the 4th field only when MinActivationHeight is non-zero so the
   same Config works against either binary. Test_VBParams_Render covers
   both branches.

3. Inquisition exposes testdummy as a non-BIP9 entry and activates BIP54
   coinbase locktime rules. Three Core-only tests fail under it:
   TestMineUntilActive_Testdummy, TestExampleActivateTestdummy,
   TestRPC_GetBlockTemplate_SubmitBlock. Each now skips on
   VariantInquisition with a comment pointing to PR2's SupportsBIP as
   the eventual replacement for the variant check.

ai-check green against bitcoind-inquisition on PATH. Still additive only;
the 3-field vbparams default keeps the existing Core test suite green
unchanged.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
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.

[Phase 6.A.1] Config.BinaryPath + script env-var indirection

1 participant