Skip to content

Update capnp schemas to match Bitcoin Core master (pre-31.x)#9

Merged
Shourya742 merged 8 commits into2140-dev:masterfrom
Sjors:2026/02/before-31
Mar 5, 2026
Merged

Update capnp schemas to match Bitcoin Core master (pre-31.x)#9
Shourya742 merged 8 commits into2140-dev:masterfrom
Sjors:2026/02/before-31

Conversation

@Sjors
Copy link
Collaborator

@Sjors Sjors commented Mar 4, 2026

Contains the latest capnp files in preparation for the v31 release.

I also added basic test coverage for all fields and methods. Added support for the macOS datadir.

Since tests open 8 unix socket connections at once, they actually hit the /*backlog=*/5 limit, so the test contains some retry code.

Finally I added instructions to the README for how to run the integration test.

For ease of review, each commit compiles and passes the tests on its own.

On macOS, Bitcoin Core's default data directory is
~/Library/Application Support/Bitcoin/ rather than ~/.bitcoin/.
Use cfg!(target_os) to select the correct default socket path
for integration tests on each platform.
@Sjors
Copy link
Collaborator Author

Sjors commented Mar 4, 2026

@ryanofsky TIL about ::listen(listen_fd, /*backlog=*/5) in src/ipc/capnp/protocol.cpp. It seems fine and easy to handle, but I wonder if it's really needed?

@Sjors Sjors mentioned this pull request Mar 4, 2026
- Checkout master branch instead of 30.x (needed for 31.x schemas)
- Add -DBUILD_TESTS=OFF to skip building Bitcoin Core tests
- Add -server flag to bitcoin-node for RPC access
- Generate 101 blocks before running tests (mining tests need
  height >= 17 to avoid bad-cb-length from BIP34 height push)
@Sjors Sjors force-pushed the 2026/02/before-31 branch from 2ced1da to 66ccac6 Compare March 4, 2026 20:56
@ryanofsky
Copy link

@ryanofsky TIL about ::listen(listen_fd, /*backlog=*/5) in src/ipc/capnp/protocol.cpp. It seems fine and easy to handle, but I wonder if it's really needed?

Yeah I'm not sure where the number 5 came from, but that it sounds pretty low if we have tests opening 8 connections at once. There shouldn't be much downside to setting a higher limit, could use 16 or 32 or maybe SOMAXCONN

@rustaceanrob
Copy link
Collaborator

rustaceanrob commented Mar 5, 2026

Since tests open 8 unix socket connections at once, they actually hit the /backlog=/5 limit, so the test contains some retry code.

Should we just use cargo test -- --test-threads=1 in CI in that case?

@Sjors
Copy link
Collaborator Author

Sjors commented Mar 5, 2026

@rustaceanrob I kind of like the current workaround, because:

  1. it's easy to forget the --test-threads=1 argument and be confused, who reads a README :-)
  2. the workaround is a useful illustration for other implementers
  3. If Bitcoin Core increases the backlog limit we can drop the workaround, making the improvement visible here

README.md Outdated
### 2. Start bitcoin

```sh
./build/bin/bitcoin node -regtest -ipcbind=unix -server -debug=ipc -daemon
Copy link
Collaborator

Choose a reason for hiding this comment

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

nit: -regtest is stricter than -chain=regtest, whereby a default chain set in bitcoin.conf will not allow the first to run, whereas the latter will run.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Fixed

@rustaceanrob
Copy link
Collaborator

Tested ACK 66ccac6

Followed the README and all tests pass. LGTM

@Sjors
Copy link
Collaborator Author

Sjors commented Mar 5, 2026

I just realized some of these tests are racy, so I'll push another commit to mark those as serial / parallel.
https://crates.io/crates/serial_test

Sjors added 4 commits March 5, 2026 11:14
Factor bootstrapping logic (RPC system spawn, Init construct, thread
creation) into a reusable async helper. This simplifies writing new
integration tests that connect to a running Bitcoin Core IPC socket.

Also drops the now-unnecessary `mut` from `let rpc_system`
in the integration test.
Sync capnp proto files with Bitcoin Core master (e09b81638ba1).

bitcoin/bitcoin#33819 (mining: getCoinbase() returns struct instead of raw tx)
  mining.capnp:
    - BlockTemplate.getCoinbaseTx now returns CoinbaseTx struct (was Data)
    - Added CoinbaseTx struct with fields: tx, txFee, totalFee,
      blockReservedWeight, coinbaseOutputMaxAdditionalSigops, defaultSignetChallenge,
      defaultWitnessCommitment
    - Added constants: maxMoney, maxDouble, defaultBlockReservedWeight,
      defaultCoinbaseOutputMaxAdditionalSigops
    - Added default values to all struct fields (BlockTemplateOptions,
      Amount, CoinbaseTx)

bitcoin/bitcoin#34568 (mining: Break compatibility with existing IPC mining clients)
  init.capnp:
    - makeMining renumbered from @2 to @3
    - Added deprecated makeMiningOld2 @2 placeholder (Cap'n Proto
      requires sequential ordinals, so this cannot be removed)
  mining.capnp:
    - Mining: removed getCoinbaseCommitment and getWitnessCommitmentIndex
    - Mining: added interrupt @6
    - BlockTemplate: methods renumbered (@5 through @9)

bitcoin/bitcoin#34184 (mining: add cooldown to createNewBlock() immediately after IBD)
  mining.capnp:
    - Mining.createNewBlock: added context @3, cooldown @4 params
    - Mining.checkBlock: added context @2 param
    - Mining.createNewBlock: timeout now has default value (60000000)

Also adds make_mining_old2_rejected integration test verifying that the
server returns an error for the deprecated method.
The IPC socket's listen backlog can reject connections when all
tests connect concurrently.

In Bitcoin Core src/ipc/capnp/protocol.cpp:
::listen(listen_fd, /*backlog=*/5)

Add retry logic with short backoff to connect_unix_stream() so tests
run reliably without --test-threads=1.
@Sjors Sjors force-pushed the 2026/02/before-31 branch from 66ccac6 to 7ed2d0d Compare March 5, 2026 15:04
@Sjors
Copy link
Collaborator Author

Sjors commented Mar 5, 2026

Added 7ed2d0d to mark the tests serial / parallel.

Sjors added 2 commits March 5, 2026 15:38
Add tests that call every method in the Mining and BlockTemplate
interfaces and inspect all returned properties. This ensures the capnp
schema is correctly encoded and methods don't crash.

Split into focused test functions:
  mining_constants: maxMoney, maxDouble, defaultBlockReservedWeight,
    defaultCoinbaseOutputMaxAdditionalSigops
  mining_basic_queries: isTestChain, isInitialBlockDownload, getTip
  mining_wait_tip_changed: waitTipChanged with short timeout
  mining_block_template_inspection: createNewBlock + getBlockHeader,
    getBlock, getTxFees, getTxSigops, getCoinbaseTx (all CoinbaseTx
    fields), getCoinbaseMerklePath
  mining_block_template_lifecycle: waitNext, interruptWait,
    submitSolution (garbage), destroy
  mining_check_block_and_interrupt: checkBlock (garbage), interrupt

The node must have height >= 17 before running (createNewBlock
fails with bad-cb-length when the BIP34 height push is < 2 bytes).
Run only shared-state mining tests under serial execution and mark other
integration tests as parallel.

Keep serialization for tests that can observe global tip changes or trigger
interrupt behavior, while allowing unrelated tests to run concurrently.
@Sjors Sjors force-pushed the 2026/02/before-31 branch from 7ed2d0d to 65c0443 Compare March 5, 2026 18:49
@Sjors
Copy link
Collaborator Author

Sjors commented Mar 5, 2026

While working on adding support for bitcoin/bitcoin#34644 I noticed the checkBlock() test triggers a disconnect (actually a crash because that PR hasn't been rebased on the latest multiprocess fixes) instead of regular failure. Made it a bit safer.

This was referenced Mar 5, 2026
Copy link
Collaborator

@Shourya742 Shourya742 left a comment

Choose a reason for hiding this comment

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

ACK 65c0443

@Shourya742 Shourya742 merged commit ad27ffc into 2140-dev:master Mar 5, 2026
1 check passed
@Sjors Sjors deleted the 2026/02/before-31 branch March 6, 2026 11:12
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.

5 participants