Skip to content

rpc: expose signet challenge in chain info - #2538

Open
ziggie1984 wants to merge 3 commits into
btcsuite:masterfrom
ziggie1984:expose-signet-challenge-rpc
Open

rpc: expose signet challenge in chain info#2538
ziggie1984 wants to merge 3 commits into
btcsuite:masterfrom
ziggie1984:expose-signet-challenge-rpc

Conversation

@ziggie1984

Copy link
Copy Markdown
Contributor

Summary

  • retain the signet challenge in chaincfg custom signet params
  • expose signet_challenge from getblockchaininfo and getmininginfo
  • add a temporary local chaincfg/v2 replace for root-module builds until a tagged/pseudo module version is used

Tests

  • go test ./... in chaincfg
  • go test ./... in btcjson
  • go test . in the root btcd module

@Lrifton92 Lrifton92 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Nice, clean additive change — the defensive copy in CustomSignetParams plus the TestSigNetChallenge case that mutates the caller slice after the fact is exactly the right guard, and routing both SigNetParams and custom signets through the same constructor means the default and custom networks both expose the challenge without a second code path. The omitempty keeps non-signet responses unchanged. Two things worth clarifying before this can land:

1. The replace github.com/btcsuite/btcd/chaincfg/v2 => ./chaincfg (you already flagged it as a TODO). Beyond not being releasable as-is, a replace in the root go.mod isn't honored by downstream consumers — btcwallet/lnd importing chaincfg/v2 v2.0.0 would still get the version without SignetChallenge. So this effectively depends on a tagged/pseudo chaincfg/v2 landing first. What's the intended sequencing — cut the chaincfg/v2 tag with the new field, then drop the replace here?

2. Duplicating signet_challenge into getmininginfo. getblockchaininfo is the natural home for it. Is exposing it on getmininginfo as well intentional? I couldn't find a Bitcoin Core precedent for the field on getmininginfo (didn't verify Core's exact schema, so worth a second look) — if there isn't one, keeping it to getblockchaininfo avoids a btcd-specific divergence.

Everything else (field ordering, help text) reads fine to me.

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