Repository navigation
Merge btcd v0.27.0 into lbcd - #10
Open
bitcoinbrisbane wants to merge 163 commits into
Open
bitcoinbrisbane wants to merge 163 commits into
bitcoinbrisbane wants to merge 163 commits into
Conversation
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>
In this commit, we vendor the Android networking shim that backs InterfaceAddrs on android into the btcutil module, rather than importing it from github.com/kcalvinalvin/anet. The shim works around golang/go#40569, where net.InterfaceAddrs fails with "route ip+net: netlinkrib: permission denied" on Android 11 and later because of the tightened restrictions on NETLINK sockets. btcutil only ever calls anet.InterfaceAddrs, wired in from net_android.go under the android build tag. The motivation is dependency hygiene. The previous import pointed at a fork hosted under an individual contributor's account, which is not a home we want a release dependency to live in. By copying the source into internal/anet we drop the external module entirely: the package is now unimportable from outside btcutil, and go.mod no longer references the fork. The source originates from github.com/wlynxg/anet, and its BSD 3-Clause LICENSE travels with it. Unlike upstream, this copy does not reach into the standard library's net package via //go:linkname; it carries its own ipv6ZoneCache, so no -ldflags "-checklinkname=0" linker flag is needed to build it. When golang/go#40569 is resolved upstream, both this package and the net_android.go shim can be removed.
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 btcsuite#2545.
The websocket dialer had no handshake timeout, so a server that accepted the TCP connection but never completed the opening handshake hung the dial forever. gorilla/websocket also reads up to 1 KiB of the response body when the handshake fails, so a server that sends an error status but stalls the body hangs it too, where btcsuite/websocket returned right after the headers. Bound the dial and the handshake by the same one-minute timeout used for HTTP POST requests.
The /ws endpoint upgrades the connection before the client has authenticated, and only expects the authenticate command afterwards.
Set a write deadline for every message. It is a generous five minutes, so a slow link can still receive a large reply.
With --proxy and --torisolation, Tor stream isolation was only enabled when --proxyuser or --proxypass was also set, so a plain --proxy=127.0.0.1:9050 --torisolation silently ran without it. The credentials were only meant to decide whether to warn that they get overridden, as they did when isolation was added in 2015; the two conditions were merged in a 2016 refactor. Enable isolation whenever --torisolation is set, except in bridge mode, where --onion is also set and gets the isolation instead.
btcsuite/go-socks, a fork of samuel/go-socks is outdated. Replace it with a maintained package golang.org/x/net/proxy. Add internal/socks, which keeps the API btcd used on top of x/net/proxy, and the behavior it relied on: - The connection reports the requested destination as its remote address. outboundPeerConnected whitelists peers by it, so with a plain x/net/proxy connection, which reports the proxy's address, a whitelisted local Tor proxy would have whitelisted every peer reached through it. - Its local address is the address the proxy bound, now parsed correctly. - Tor stream isolation uses new random credentials for every connection. - Hostnames, including onion addresses, are passed to the proxy unresolved. - The timeout covers the SOCKS handshake too. rpcclient now dials its websocket through the proxy with the dialer's context, so the handshake timeout also covers the proxy. Verified end to end with two simnet nodes connected through a forwarding SOCKS5 proxy: the handshake completes, getpeerinfo reports the peer and the proxy's bound address, the peer connection uses random isolation credentials, and btcctl and an rpcclient websocket client reach the RPC server through the proxy.
When knownInventory moved from btcd's own inventory map to the generic dcrd/lru cache in 875b51c (2020), the *wire.InvVect pointers were passed as the keys. The old map was keyed by the dereferenced value, but the generic cache compares the pointers, so inventory from one message never matched the same inventory from another. Every check of whether a peer already knows some inventory came out false, and btcd announced inventory back to the peers that had just announced it. Store and look up the values again, through a knowsInventory helper.
github.com/decred/dcrd/lru is deprecated in favor of github.com/decred/dcrd/container/lru, which provides typed generic caches. Use a Set for the known inventory, sent nonces, known addresses and v1 downgrade addresses, and a Map for the per-source v2 handshake limiters, which removes a type assertion. Lookups keep their semantics: Set.Contains and Map.Get mark the item as most recently used, like Cache.Contains and KVCache.Lookup did.
v2transport still required btcec/v2 v2.4.0, which pulled in the old chaincfg/chainhash v1 module, and a 2017 snapshot of btclog. Require btcec/v2 v2.5.0 and btclog v1.0.0, the versions the rest of the repo uses, which also replaces chaincfg/chainhash with chainhash/v2.
The unit, unit-race and unit-cover targets ran the tests of every sub-module except v2transport, so its tests never ran in CI. Add it to all three and upload its coverage like the others.
Each go.mod requires the published versions of the other modules in this repo, so CI only tests a change to a sub-module against the code that uses it once the sub-module has been tagged and the requirements bumped. Add a job that puts all modules into a go.work workspace, so each one builds against the in-tree code of the others, and runs the build and the tests, including the rpctest integration tests.
In this commit, we read from the client before waiting for the server to finish handling the connection. Waiting first let the authentication timeout close an unread pipe and hide a reply queued for an unauthenticated malformed request. With both pre-authentication guards removed, the old test still passed. The updated helper catches the unexpected replies for both single and batch requests.
multi: update dependencies and fix RPC, Tor, and cross-build failures
Update the vectors under data/ to the current ones in bitcoin/bips
(bip-0327/vectors at 927b6de9, last changed in 1c6ac0c4):
- nonce_gen: rand_ is now 0F..0F, and the expected secnonce and
pubnonce are given separately.
- sign_verify: new valid cases with an empty and a 38-byte message,
a longer invalid pubnonce, and new signatures in the verify cases
after the "Fix the four test vectors" change.
- key_sort: one more key.
- sig_agg: the error case now names its contributor.
Test changes:
- The nonce generation test reads rand_ from the vector and checks
the pubnonce as well as the secnonce.
- The verify-fail test ignored signer_index and always checked the
signature against the first signer, so the "Wrong signer" case did
not fail with the corrected signature. It now uses the nonce and
key at signer_index.
- The unused memsetRandReader is removed.
- The two valid cases with a message that is not 32 bytes are skipped,
as Sign takes a 32-byte message.
DecodeAddress treats the text before the last '1' as a bech32 prefix. A simnet base58 address such as "Sb1..." starts with "Sb", which matches the simnet prefix "sb" when case is ignored, so it was decoded only as bech32 and rejected with "string not all lowercase or all uppercase". When bech32 decoding fails, try base58 and return the address if that succeeds. If it also fails, return the bech32 error as before. Strings that decode as bech32 keep their errors, including unsupported witness versions and program lengths. Fixes btcsuite#1798.
btcec/schnorr/musig2: refresh BIP 327 test vectors
In this commit, we upgrade CI, Docker builds and release binaries to Go 1.27.3. This release adds the crypto/x509 mitigation for CVE-2026-111943, which distrusts affected intermediate CA keys. We also move every module language version to Go 1.26.10, the latest release on the previous supported branch.
In this commit, we spell out the patched Go point-release floor for each supported branch. The go directive can require a minimum version, but it cannot prevent builds with Go 1.27.0 through 1.27.2.
build: upgrade Go toolchain to 1.27.3
Check witness and non-witness output descriptions for consistency before using their value in fee calculation. Reuse the existing output lookup, bounds check and comparison helper, and propagate conflicts as errors. Cover matching descriptions, conflicting amounts and scripts, and an invalid output index through the public fee API.
Reject a zero final private scalar or a public point at infinity in both child derivation methods. Return the existing ErrInvalidChild so callers can skip the index without receiving an unusable child key. Preserve the historical non-standard derivation behavior otherwise. Factor the child HMAC boundary so deterministic tests can force final cancellation in both private and public paths of each method. Retain the existing derivation vectors for compatibility coverage.
…ture-validation txscript: align signature opcode validation
address: fall back to base58 after bech32 fails
multi: reject inconsistent UTXOs and invalid keys
In this commit, we set btcd and btcctl to the next minor release version and drop the beta suffix.
In this commit, we pin the newly published wire, btcec, address, txscript, and btcutil patch releases in the root module. This makes a normal btcd build include the fixes already merged into the repository, without relying on the in-tree module workspace. We also tidy the checksums and drop the obsolete Android anet dependency.
In this commit, we update the release instructions to prepare notes for the GitHub release instead of requiring a CHANGES commit. The release notes still list merged PRs and contributors, and we publish them after pushing the signed tag.
build: prepare v0.27.0 release version
Conflicts: Dockerfile (kept lbcd names/ports, took btcd's --platform flag and build changes) and wire/common.go (took btcd's version; lbcd's only change there was removing the free list, which left Borrow/Return callers undefined). Also keeps the two lbcd-specific wire fixes: MaxBlockHeaderPayload counts the ClaimTrie hash (112 bytes), and TestBlockHeaderWire sets ClaimTrie[0]. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
README: keep lbcd's rewrite, update the minimum Go version to btcd's 1.26.10. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
lbcd master is based on btcd v0.26.0 and differs from it by only 22 files. This merges
btcsuite/btcdmaster (v0.27.0, 161 commits) into lbcd as a real merge commit, so btcd history is preserved.Conflicts (2)
Dockerfile: kept lbcd's names and ports, took btcd's--platformflag and build changes.wire/common.go: took btcd's version. lbcd's only change there was removing the binary free list, which leftBorrow/Returncallers undefined and broke the build on master.lbcd-specific wire fixes kept
MaxBlockHeaderPayloadcounts the ClaimTrie hash (112-byte header).TestBlockHeaderWiresetsClaimTrie[0] = 0x33like its sibling test.These two overlap with #8, which also fixes the
wirebuild. Merge either one first; the other will need a trivial rebase.Review notes
This brings in everything btcd shipped between v0.26 and v0.27, including p2p and consensus-adjacent changes, so it needs a real review. I have not checked LBRY-specific behaviour (claimtrie header, network params) beyond the existing unit tests.
Testing
go build ./...,go vet ./...cd wire && go test ./...rpctest) and race jobs: left to CI🤖 Generated with Claude Code