Skip to content

ci: prepare wallet-capable integration test environment#14

Merged
rustaceanrob merged 3 commits into2140-dev:masterfrom
Sjors:2026/03/mempool
Mar 11, 2026
Merged

ci: prepare wallet-capable integration test environment#14
rustaceanrob merged 3 commits into2140-dev:masterfrom
Sjors:2026/03/mempool

Conversation

@Sjors
Copy link
Collaborator

@Sjors Sjors commented Mar 6, 2026

This PR prepares for #11 which needs a mempool.

Cargo.toml Outdated
capnpc = "0.25.0"

[dev-dependencies]
bitcoin = "0.32"
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Not sure if this is a good crate, @Shourya742?

Copy link
Collaborator

Choose a reason for hiding this comment

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

This will be very painful to upgrade to 0.33.0 as the crate has changed substantially. I recommend using bitcoin_primitives if possible.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I switched to the 0.33 beta tag of bitcoin-primitives.

@Sjors Sjors marked this pull request as draft March 6, 2026 15:08
@Sjors
Copy link
Collaborator Author

Sjors commented Mar 6, 2026

In addition to picking a different crate, I also need to add some trivial test that uses the new methods. Marking draft.

@rustaceanrob
Copy link
Collaborator

Needs rebase

@Sjors Sjors force-pushed the 2026/03/mempool branch from a688b69 to 18ae0ba Compare March 10, 2026 14:18
@Sjors Sjors marked this pull request as ready for review March 10, 2026 14:29
@rustaceanrob rustaceanrob mentioned this pull request Mar 10, 2026
@rustaceanrob
Copy link
Collaborator

Needs rebase

@Sjors Sjors force-pushed the 2026/03/mempool branch from 18ae0ba to d263ae4 Compare March 10, 2026 15:46
@Sjors
Copy link
Collaborator Author

Sjors commented Mar 10, 2026

Rebased after #16.

.collect()
}

fn display_hash_to_internal_bytes(hex: &str) -> Vec<u8> {
Copy link
Collaborator

Choose a reason for hiding this comment

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

internals has a BlockHash type that should deal with the display vs. internal nonsense, but this is fine for now.


fn bitcoin_rpc_owned(wallet: Option<&str>, args: &[String]) -> Result<String, String> {
let mut command = Command::new(bitcoin_bin());
command.arg("rpc").arg("-regtest").arg("-rpcwait");
Copy link
Collaborator

@rustaceanrob rustaceanrob Mar 11, 2026

Choose a reason for hiding this comment

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

Same -chain=regtest thing here. Hit a panic in when testing.

@rustaceanrob
Copy link
Collaborator

failures:

---- wallet_helpers_create_mempool_transaction stdout ----

thread 'wallet_helpers_create_mempool_transaction' (844621) panicked at tests/util/bitcoin_core.rs:179:37:
failed to create self-transfer in ipc-test: bitcoin rpc command failed: error code: -6
error message:
Insufficient funds
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

Having an insufficient funds error on my end. Is the README missing a block generation step?

@Sjors
Copy link
Collaborator Author

Sjors commented Mar 11, 2026

Having an insufficient funds error on my end. Is the README missing a block generation step?

No, the test framework should add more funds. Let me check that.

@Sjors Sjors force-pushed the 2026/03/mempool branch from d263ae4 to f3f7319 Compare March 11, 2026 12:04
@Sjors
Copy link
Collaborator Author

Sjors commented Mar 11, 2026

  • I removed some pre-existing duplication in a new commit 20c2062
  • I lowered the self transfers to 0.01 BTC. There's an initial balance check / topup for 1 BTC.
  • making better use of internals like BlockHash
  • added test/util/bitcoin_core_wallet.rs for all the wallet stuff
  • the new test checks that the mempool size increased (this adds serde-json)
  • simplified the self-transfer helper to just return bitcoin_primitives::Transaction

Also rebased (though the README change didn't cause a conflict).

@rustaceanrob
Copy link
Collaborator

Still getting insufficient funds on my end, but I am on a recent commit of bitcoin/bitcoin and CI passes.

ACK f3f7319

@Sjors
Copy link
Collaborator Author

Sjors commented Mar 11, 2026

Ok, we'll triage that some other time if others run into it.

@rustaceanrob rustaceanrob merged commit cd9b07b into 2140-dev:master Mar 11, 2026
1 check passed
This was referenced Mar 11, 2026
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