Make corepc-node optional in payjoin-test-utils#1424
Draft
DanGould wants to merge 1 commit intopayjoin:masterfrom
Draft
Make corepc-node optional in payjoin-test-utils#1424DanGould wants to merge 1 commit intopayjoin:masterfrom
DanGould wants to merge 1 commit intopayjoin:masterfrom
Conversation
Collaborator
Pull Request Test Coverage Report for Build 23306722576Details
💛 - Coveralls |
spacebear21
reviewed
Mar 19, 2026
payjoin-test-utils/src/lib.rs
Outdated
Comment on lines
+22
to
+29
| #[cfg(feature = "bitcoind")] | ||
| use bitcoin::Amount; | ||
| #[cfg(feature = "bitcoind")] | ||
| pub use corepc_node; | ||
| #[cfg(feature = "bitcoind")] | ||
| use corepc_node::AddressType; | ||
| #[cfg(feature = "bitcoind")] | ||
| use tracing::Level; |
Collaborator
There was a problem hiding this comment.
nit: to avoid this kind of confusing feature gating we should move bitcoind to its own module and feature gate the whole thing once
Contributor
Author
There was a problem hiding this comment.
o yeah I was only concerned about the gates being clean in a group and forgot about that option. good call.
spacebear21
requested changes
Mar 19, 2026
Collaborator
spacebear21
left a comment
spacebear21
left a comment
There was a problem hiding this comment.
concept ACK with one structural change request
DanGould
force-pushed
the
test-utils-no-bitcoind
branch
from
2046195 to
9d2151a
Compare
Test constants (ORIGINAL_PSBT, EXAMPLE_URL, etc.) and the TestServices infrastructure don't require bitcoind. Only init_bitcoind* functions need the corepc-node crate, which downloads a ~40MB binary at build time. Move bitcoind-dependent functions to a `bitcoind` module, gated behind a `bitcoind` feature (default = on). Re-export from crate root for backwards compatibility. Downstream consumers can now import payjoin-test-utils for constants without triggering the bitcoind download.
DanGould
force-pushed
the
test-utils-no-bitcoind
branch
from
9d2151a to
87d590c
Compare
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
3 participants
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.
Test constants (ORIGINAL_PSBT, EXAMPLE_URL, etc.) and the TestServices infrastructure don't require bitcoind. Only init_bitcoind* functions need the corepc-node crate, which downloads a ~40MB binary at build time.
Gate corepc-node behind a
bitcoindfeature (default = on) so downstream consumers can import payjoin-test-utils for constants without triggering the bitcoind download. This unblocks nix environments and CI setups that provide their own bitcoind via BITCOIND_EXE.The split lets FFI bindings import test constants (originalPsbt(), exampleUrl()) without downloading bitcoind — so dart test, python
-m pytest, npm test work in any environment, not just ones with network access. No new bindings needed — the constants are already
exported through payjoin-ffi's test_utils.rs; the only blocker was corepc-node's build.rs firing unconditionally.
This was claude baby. I needed this for an agent in nix with no network.
Pull Request Checklist
Please confirm the following before requesting review:
AI
in the body of this PR.