refactor: bitcoin rpc porting #6329

fdefelici · 2025-07-24T12:06:13Z

Description

This PR is the first step in refactoring the BitcoinRegtestController to enhance logic correctness, reduce code duplication, increase test coverage. and improve documentation.

It primarily introduces a port of the BitcoinRPCRequest implementation, with the following key changes:

Introduces a RpcTransport layer using the reqwest crate (as used in StacksClient from the stacks-signer package).
Adds a BitcoinRpcClient layer that exposes the relevant RPC APIs.
Aligns the reqwest version used across stacks-node and stacks-signer with the version already resolved in Cargo.lock
Includes a small enhancement to BitcoinCoreController to make it suitable for the new tests added in this PR.

To simplify review, the code introduced here is not yet integrated with the rest of the codebase. Follow-up PRs will handle that integration:

A PR to apply the BitcoinCoreController improvements (currently marked with TODOs).
One PR (or more if get to big) replacing the existing BitcoinRPCRequest with the new BitcoinRpcClient in BitcoinRegtestController.

Applicable issues

fixes Refactor: BitcoinRPCRequest Porting #6250

Additional info (benefits, drawbacks, caveats)

A few design decisions were made with the goal of preserving the current usage of the Bitcoin RPC API. Feedback on the following points would be appreciated:

Limited Rpc API Scope: This implementation includes only the subset of the Bitcoin RPC API currently used in the codebase, both in terms of available methods and supported input/output parameters. The idea is to keep the implementation minimal for now, with the flexibility to expand it as needed in the future.
Nonce Handling in RpcTransport: The original BitcoinRegtestController code uses a fixed nonce value ("stacks") as the id parameter in RPC requests (see code example below). To stay consistent, this PR allows customizable nonces in RpcTransport. However, if this behavior is not strictly required, we could simplify the implementation by generating nonces automatically and internally, avoiding the need to expose this as a configurable option. Would removing nonce configurability be acceptable, or is there a use case for keeping it?

stacks-core/testnet/stacks-node/src/burnchains/bitcoin_regtest_controller.rs

Lines 2512 to 2519 in a5587d1

    
           pub fn get_raw_transaction(config: &Config, txid: &Txid) -> RPCResult<String> { 
        
               debug!("Get raw transaction {txid}"); 
        
               let payload = BitcoinRPCRequest { 
        
                   method: "getrawtransaction".to_string(), 
        
                   params: vec![format!("{txid}").into()], 
        
                   id: "stacks".to_string(), 
        
                   jsonrpc: "2.0".to_string(), 
        
               };

Checklist

Test coverage for new or modified code paths
Changelog is updated
Required documentation changes (e.g., docs/rpc/openapi.yaml and rpc-endpoints.md for v2 endpoints, event-dispatcher.md for new events)
New clarity functions have corresponding PR in clarity-benchmarking repo
New integration test(s) added to bitcoin-tests.yml

…rk#6250

…etwork#6250

…to refactor/bitcoin-rpc

…cks-network#6250

…ks-network#6250

stacks-node/src/tests/bitcoin_regtest.rs

stacks-node/src/burnchains/rpc/rpc_transport/mod.rs

stacks-network#6250

fdefelici · 2025-08-14T14:09:11Z

PR updated with the merge of #6366 (related to BitcoinAddress regtest HRP management).

obycode

LGTM

Jiloc

LGTM!

codecov · 2025-08-14T16:11:33Z

Codecov Report

❌ Patch coverage is 57.92929% with 833 lines in your changes missing coverage. Please review.
✅ Project coverage is 58.23%. Comparing base (495fe91) to head (cff3c9b).
⚠️ Report is 70 commits behind head on develop.

Files with missing lines	Patch %	Lines
...ode/src/burnchains/rpc/bitcoin_rpc_client/tests.rs	50.76%	352 Missing ⚠️
stacks-node/src/tests/bitcoin_rpc_integrations.rs	38.17%	332 Missing ⚠️
...cks-node/src/burnchains/rpc/rpc_transport/tests.rs	64.10%	70 Missing ⚠️
...-node/src/burnchains/rpc/bitcoin_rpc_client/mod.rs	89.20%	27 Missing ⚠️
stackslib/src/chainstate/stacks/mod.rs	43.24%	21 Missing ⚠️
...rc/burnchains/rpc/bitcoin_rpc_client/test_utils.rs	87.50%	13 Missing ⚠️
stacks-node/src/tests/bitcoin_regtest.rs	88.70%	7 Missing ⚠️
...tacks-node/src/burnchains/rpc/rpc_transport/mod.rs	91.30%	6 Missing ⚠️
...ommon/src/deps_common/bitcoin/network/serialize.rs	54.54%	5 Missing ⚠️

❌ Your project status has failed because the head coverage (58.23%) is below the target coverage (80.00%). You can increase the head coverage or adjust the target coverage.

Additional details and impacted files

@@             Coverage Diff             @@
##           develop    #6329      +/-   ##
===========================================
+ Coverage    49.06%   58.23%   +9.17%     
===========================================
  Files          552      558       +6     
  Lines       352098   354077    +1979     
===========================================
+ Hits        172759   206199   +33440     
+ Misses      179339   147878   -31461

Files with missing lines	Coverage Δ
stacks-node/src/burnchains/mod.rs	`62.50% <ø> (ø)`
stacks-node/src/tests/mod.rs	`45.17% <ø> (ø)`
stackslib/src/burnchains/bitcoin/address.rs	`54.02% <ø> (+5.36%)`	⬆️
...ommon/src/deps_common/bitcoin/network/serialize.rs	`50.96% <54.54%> (+3.08%)`	⬆️
...tacks-node/src/burnchains/rpc/rpc_transport/mod.rs	`91.30% <91.30%> (ø)`
stacks-node/src/tests/bitcoin_regtest.rs	`89.09% <88.70%> (+69.09%)`	⬆️
...rc/burnchains/rpc/bitcoin_rpc_client/test_utils.rs	`87.50% <87.50%> (ø)`
stackslib/src/chainstate/stacks/mod.rs	`73.51% <43.24%> (+9.81%)`	⬆️
...-node/src/burnchains/rpc/bitcoin_rpc_client/mod.rs	`89.20% <89.20%> (ø)`
...cks-node/src/burnchains/rpc/rpc_transport/tests.rs	`64.10% <64.10%> (ø)`
... and 2 more

... and 400 files with indirect coverage changes

Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 495fe91...cff3c9b. Read the comment docs.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

github-actions · 2025-08-22T00:20:01Z

This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

fdefelici added 27 commits July 8, 2025 10:01

refactor: create BitcoinRpcClient, stacks-network#6250

bca8621

refactor: rpc list_unspent, stacks-network#6250

85f3fe0

test: add list_unspent mock test, stacks-network#6250

b61355a

refactor: extract rcp_transport module, stacks-network#6250

69cdc3e

refactor: reviewed rpc endpoint, stacks-network#6250

1121105

refactor: add import_descriptor and get_descriptor_info, stacks-netwo…

4ce8085

…rk#6250

refactor: add stop rpc, stacks-network#6250

b855f25

test: add rpc integration tests, stacks-network#6250

6164471

test: add integration tests, stacks-network#6250

fed6853

test: enhance bitcoin core controller to support custom arg, stacks-n…

71e1c90

…etwork#6250

test: add get_transaction inte test, stacks-network#6250

27dd024

fix: merge conflict with develop

6e824a4

test: improve descriptor tests, stacks-network#6250

6d2d27c

Merge branch 'develop' of https://github.com/fdefelici/stacks-core in…

5a0ea11

…to refactor/bitcoin-rpc

chore: add docs for testing api, stacks-network#6250

48400c9

chore: add copyright, stacks-network#6250

d71dcd4

feat: complete rpc_transport implementation, stacks-network#6250

14f8792

chore: improve rpc api and docs, stacks-network#6250

4bb1962

chore: add docs, stacks-network#6250

85ca874

test: add test for bitcoin rpc auth, stacks-network#6250

f284412

feat: add invalidate_block rpc, stacks-network#6250

6d8a634

feat: complete implementation and docs, stacks-network#6250

523667a

chore: cleaning, stacks-network#6250

8d6fa55

Merge branch 'develop' into refactor/bitcoin-rpc

99c60dc

chore: make BitcoinCoreController::config private and update doc, sta…

a7d3907

…cks-network#6250

chore: reorganize in rpc folder, stacks-network#6250

cc3a1db

test: configure integration tests with ignore and BITCOIND_TEST, stac…

a29a8ac

…ks-network#6250

fdefelici self-assigned this Jul 24, 2025

chore: revert ParsedUTXO mod, stacks-network#6250

32f901e

fdefelici marked this pull request as ready for review July 24, 2025 14:28

Jiloc reviewed Aug 13, 2025

View reviewed changes

stacks-node/src/tests/bitcoin_regtest.rs Show resolved Hide resolved

stacks-node/src/burnchains/rpc/rpc_transport/mod.rs Outdated Show resolved Hide resolved

stacks-node/src/burnchains/rpc/rpc_transport/mod.rs Outdated Show resolved Hide resolved

fdefelici added 6 commits August 13, 2025 16:15

crc: rpc error tag in doc, stacks-network#6250

62260f6

crc: rpc transport tag in doc, stacks-network#6250

98a66f4

crc: BitcoinRcpClient::new comment, stacks-network#6250

284577e

crc: list_unspent availability doc, stacks-network#6250

062f83a

crc: get_descriptor_info doc nit, stacks-network#6250

46ce0e4

crc: import_descriptors doc nit, stacks-network#6250

ec2db90

fdefelici dismissed stale reviews from jcnelson and obycode via ec2db90 August 14, 2025 10:21

fdefelici added 4 commits August 14, 2025 12:28

crc: import_descriptors response doc nit, stacks-network#6250

2a95955

Merge branch 'develop' into refactor/bitcoin-rpc

db9e9b9

refactor: using new BitcoinAddress::from_string supporting regtest hrp,

2fd677b

stacks-network#6250

refactor: remove duplication related to BitcoinAddress deserialization,

cff3c9b

stacks-network#6250

fdefelici requested review from Jiloc, obycode and jcnelson August 14, 2025 14:09

obycode approved these changes Aug 14, 2025

View reviewed changes

Jiloc approved these changes Aug 14, 2025

View reviewed changes

fdefelici enabled auto-merge August 14, 2025 15:42

fdefelici added this pull request to the merge queue Aug 14, 2025

Merged via the queue into stacks-network:develop with commit 8715d36 Aug 14, 2025
3 of 4 checks passed

fdefelici deleted the refactor/bitcoin-rpc branch August 14, 2025 16:10

github-project-automation bot moved this from Status: In Review to Status: ✅ Done in Stacks Core Eng Aug 14, 2025

fdefelici mentioned this pull request Aug 18, 2025

refactor: BitcoinCoreController rcp rollout + housekeeping #6385

Merged

5 tasks

github-actions bot added the locked label Aug 22, 2025

github-actions bot locked as resolved and limited conversation to collaborators Aug 22, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

refactor: bitcoin rpc porting #6329

refactor: bitcoin rpc porting #6329

Uh oh!

fdefelici commented Jul 24, 2025 •

edited

Loading

Uh oh!

Uh oh!

Uh oh!

Uh oh!

fdefelici commented Aug 14, 2025

Uh oh!

obycode left a comment

Uh oh!

Jiloc left a comment

Uh oh!

Uh oh!

codecov bot commented Aug 14, 2025

Uh oh!

github-actions bot commented Aug 22, 2025

Uh oh!

Uh oh!

	pub fn get_raw_transaction(config: &Config, txid: &Txid) -> RPCResult<String> {
	debug!("Get raw transaction {txid}");
	let payload = BitcoinRPCRequest {
	method: "getrawtransaction".to_string(),
	params: vec![format!("{txid}").into()],
	id: "stacks".to_string(),
	jsonrpc: "2.0".to_string(),
	};

refactor: bitcoin rpc porting #6329

refactor: bitcoin rpc porting #6329

Uh oh!

Conversation

fdefelici commented Jul 24, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Applicable issues

Additional info (benefits, drawbacks, caveats)

Checklist

Uh oh!

Uh oh!

Uh oh!

Uh oh!

fdefelici commented Aug 14, 2025

Uh oh!

obycode left a comment

Choose a reason for hiding this comment

Uh oh!

Jiloc left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

codecov bot commented Aug 14, 2025

Codecov Report

Uh oh!

github-actions bot commented Aug 22, 2025

Uh oh!

Uh oh!

fdefelici commented Jul 24, 2025 •

edited

Loading