Wipe the secp256k1 scalar used for the [1, N-1] range validation as
soon as DecodeWIF returns, so the decoded private key value does not
linger in this local after use. Addresses review feedback on #2545.
Add SubmitPackage / SubmitPackageAsync / FutureSubmitPackageResult,
wrapping the submitpackage RPC the same way TestMempoolAccept wraps
testmempoolaccept: serialize the topologically-sorted package to hex,
issue the btcjson submitpackage command, and decode the response into
btcjson.SubmitPackageResult (which already maps the raw fields to
higher-level types via its UnmarshalJSON).
This keeps the multi-backend RPC layering intact so callers (e.g.
btcwallet's chain.Interface) can invoke a typed method instead of a
RawRequest. submitpackage is a Bitcoin Core RPC (v24+); btcd has no
server handler for it.
Commit 26124d275 made every peer a sync candidate on regtest and
simnet so that nodes on non-localhost networks (e.g. Docker bridge
networks) can be synced from. Dropping the address requirement was
the intent, but the change also dropped the service-flag requirement,
so light clients became eligible sync peers.
A light client (e.g. neutrino) advertises a recent best height but
can serve neither headers nor blocks. Electing one as the sync peer
stalls the sync until the stall handler disconnects it, and with
other light client connections present the next one is elected and
stalls again, livelocking the sync indefinitely. This surfaced in
neutrino's sync tests, where a btcd simnet node connected to both a
neutrino instance and other btcd nodes never synced.
Keep accepting any peer address on regtest/simnet, but require the
peer to signal SFNodeNetwork or SFNodeNetworkLimited like on any
other network.
DecodeWIF did not validate that the decoded private key falls within the
valid range [1, N-1] for a secp256k1 private key. The raw 32-byte key
material was passed straight to btcec.PrivKeyFromBytes, which reduces the
scalar modulo the group order N and clamps to zero, returning no error.
As a result, a WIF encoding a private key of zero, the group order N, or
any value >= N was silently accepted. For keys >= N this is particularly
dangerous: DecodeWIF returned a private key that differs from the one
actually encoded in the WIF (e.g. a WIF for N+5 decoded to the key 5),
so an application importing such a WIF would obtain a valid-looking but
wrong key pair, with no indication that anything was off. This is also
inconsistent with hdkeychain.NewKeyFromString in the same package, which
already rejects private keys outside [1, N-1].
Validate the range using a ModNScalar (constant time): SetByteSlice
reports an overflow when the value is >= N, and IsZero covers the zero
key. Out-of-range keys now return ErrMalformedPrivateKey.
Add regression test cases (zero, N, and N+5) to TestEncodeDecodeWIF.
Signed-off-by: Lrifton92 <Lrifton92@users.noreply.github.com>
Two pieces of rpctest's global state silently aliased across concurrent
test processes (which is what `go test ./...` does by default, so any
`make unit` that exercises -tags=rpctest hit this):
- btcdExecutablePath compiled to a fixed path /tmp/btcd/rpctest/btcd.
Two `go build` invocations would race on the same file, occasionally
yielding a truncated or stale binary and downstream "tls: certificate
signed by unknown authority" failures when the harness tried to talk
to the resulting node.
- lastPort started at the same defaultNodePort in every process. The
bind-test in NextAvailablePort closes the listener before returning,
so two processes climbing from the same base would frequently hand
out the same port and one harness would die with "connection refused"
when btcd failed to bind.
Suffix the executable with a random uint32 and seed lastPort with a
random offset into a 50k-port window so each process climbs through
its own range.
handleInvMsg early-returned for any inv from a non-syncPeer whenever
sm.current() was false, with the comment that it prevents fetching a
mass of orphans. That guard assumes a syncPeer is already fetching
blocks; when syncPeer is nil, the assumption breaks down and the
early return becomes a deadlock.
The deadlock is reachable whenever two nodes connect at equal heights:
startSync exits without picking a syncPeer (no peer is "higher"), and
nothing later promotes the freshly-mined blocks the peer announces via
inv. The pre-verack disconnect and sync-race regression tests in
integration/sync_race_test.go fail consistently because of this.
Only skip the inv when we actually have a syncPeer. When syncPeer is
nil, fall through and let the normal request path queue the block --
the inv is the only signal that there are blocks to fetch.
StringOrArray.MarshalJSON emits JSON null for a nil slice (see existing
test "nil slice marshals as null" in TestStringOrArrayMarshalJSON), but
UnmarshalJSON did not have a matching case for null and fell to the
default branch, returning "invalid string_or_array value: <nil>". A
round trip of a nil slice therefore failed.
This bit the rpcclient against btcd's own getblockchaininfo, whose
Warnings field is a StringOrArray that the server leaves as a nil slice
when there are no warnings. Every rpctest integration test that touches
GetBlockChainInfo (TestBIP0009, TestBIP0068AndBIP0112Activation,
TestBIP0113Activation, TestPrune) failed to decode the response.
Handle the nil case explicitly so null decodes back to a nil slice, and
add regression cases for "warnings: null" and an omitted warnings field
to TestGetBlockChainInfoWarnings.
p2a_test.go calls btcutil.NewAddressPayToAnchor, but the
NewAddressPayToAnchor constructor lives in the address/v2 module's
address package. Import that package and call it through there so the
integration package builds when -tags=rpctest is set.
`make build`, `make unit-cover`, and `make unit-race` all use plain
`go test` without the rpctest build tag, so anything under
//go:build rpctest -- the entire integration/ package outside of
rpctest/, and parts of rpctest/ itself -- is not exercised by CI.
Bugs that only surface under -tags=rpctest can land on master without
detection.
Add a test-rpctest job that runs `make unit` (which sets
-tags=rpctest) so rpctest-tagged tests are part of every push and PR.
In this commit, we strip all of the local `replace ... => ../...`
directives that were introduced as part of #1825 (the v2 module
restructuring), now that proper tags exist for every freshly carved-out
submodule. Every in-tree go.mod is pinned to the newly published tags:
chainhash/v2.0.0, wire/v2.0.0, chaincfg/v2.0.0, address/v2.0.0,
txscript/v2.0.0, btcutil/v2.0.0, psbt/v2.0.0, and btcec is bumped to
v2.5.0 since it now depends on chainhash/v2 (previously
chaincfg/chainhash).
While here, we also unify the Go toolchain to 1.25 across every
submodule so the workspace resolves a consistent set of language
features.
Finally, we bump the main btcd version to v0.26.0-beta.rc1 in
preparation for the upcoming release candidate.
