electrsd: include in workspace and CI by satsfy · Pull Request #570 · rust-bitcoin/corepc

satsfy · 2026-04-24T05:48:33Z

Closes #552

This PR includes electrsd in corepc workspace. With this change, now test, lints and formats also consider electrsd.

In order for this inclusion to fit in workspace:

The cargo --all-features test faced several version conflicts becasue these tests enable all electrsd feature flags at the same time. The solution developed was to create a new test-only feature flag called all_features_test that is gets also enabled by default for finer version control, such that only the latest electrsd version runs.
Version compiler constant is gated through the new flag, making only the latest electrsd version activated.
All version-specific call sites have handle the new flag.
Fixes formatting and linting errors as well as adding appropriate rbmt lint configurations for electrsd.
Updates corresponding lock files for electrsd dependencies.

All of the commits are ordered to the same end, enabling CI to pass and tests to run.

satsfy · 2026-04-24T20:37:34Z

One thing I thought to include is a default electrsd version, like bitcoind in 0_17_2, I suppose either 0_8_10 or 0_10_6. I have not included a default because it required including std (for bitreq's get() to download), and I'm not sure if it's this PR's scope.

Edit: I suppose we need the default so tests work with an explicit version.

satsfy · 2026-04-25T04:09:35Z

Latest force push improves the legibility of the solution, sets a default electrs version, reworked the version feature graph so the cfg cascade in versions.rs is unambiguous. Replaced the legacy feature with versions::USE_LEGACY_COOKIE so --all-features tests pass.

tcharding · 2026-04-28T01:10:26Z

Needs rebase mate. Out of interest what made you go with electrs_0_8_10 instead of electrs_0_10_6?

satsfy · 2026-04-28T03:03:25Z

Great point, I made a mistake regarding 0_8_10. I had matched bitcoind pattern, but accidentally reversed the order of the versions. The highest version in bitcoind chain is the lowest, meanwhile I put the highest for electrsd. Latest rebase reverses that order.

EDIT: The problem with v0.10.6 in CI was caused by using the legacy --jsonrpc-import arg after 0.8.10. That nuance was lost on me because execution would fail on my Ubuntu 22.04 for requiring GLIBC 2.38 to test 0.10.6 properly.

satsfy · 2026-04-28T19:42:37Z

On the latest rebase:

New electrs feature version cascade: electrs_0_10_6 -> 0_9_11 -> 0_9_1 -> 0_8_10 -> esplora.
Centralized the version-specific behavior in versions.rs with legible constants USE_LEGACY_COOKIE, USE_JSONRPC_IMPORT, USE_VERBOSE_ARG.
Esplora is tested with --no-default-features so it does not accidentally combine with the default electrs version.
no-download job now enables bitcoind_download because it still needs a bitcoind binary while using the system-provided ELECTRS_EXEC.
Corrected the bitcoind 23_2 feature name in ee9443c (is this correct? Should I have done it in a separate PR)?
Optional change that I think should be merged: make electrsd CI coverage easier to debug by putting RUST_LOG=debug on electrsd jobs.

tcharding · 2026-04-29T07:08:54Z

Can we separate the fmt changes from actual code changes in c61ea18

satsfy · 2026-04-29T19:29:21Z

Latest rebase:

Split rustfmt cleanup (17866ee), lint error fixes (33158b2) and new rbmt lint configuration (9143712).
Fix Cargo.toml link typo (28022c5)
Simplifies version conditions:
- Using any in #[cfg(any(feature = "electrs_0_9_1", not(feature = "electrs_0_8_10")))]
- or crate::versions::IS_ELECTRS_0_8_10.

satsfy · 2026-04-29T21:42:47Z

Latest rebase updates lockfiles because of upstream changes to fix ci

tcharding · 2026-04-30T01:13:01Z

-    #[cfg(not(feature = "electrs_0_8_10"))]
+    #[cfg(any(feature = "electrs_0_9_1", not(feature = "electrs_0_8_10")))]


Can you explain what you want to achieve with this feature gate please?

E.g "I want this code block to run for these versions: a, b, c"

So bitcoind feature gates, which served as a model for making electrsd features, are organized into a chain:

30_2 = ["30_0"] 30_0 = ["29_0"] ... 0_18_1 = ["0_17_2"] 0_17_2 = []

In bitcoind's file client_versions.rs:48, you can find a similar example of this gate:

#[cfg(all(feature = "0_21_2", not(feature = "22_1")))] pub use corepc_client::{client_sync::v21::*, types::v21 as vtype};

So the pattern is all(X, not(Y)).
Now imagine I want the negation of a particular version. Say, I need not(version X).
I would express that in feature gates as not(all(X, not(Y)))
But you mentioned above its hard to understand what that means and I agree.
So I did not(all(X, not(Y))) = any(not(X), Y) = any(Y, not(X)) (De Morgan's laws) such as in:

#[cfg(any(feature = "electrs_0_9_1", not(feature = "electrs_0_8_10")))]

Would you prefer if I experimented with some less complex ways of solving the problem? I suppose I'd need to break away from the bitcoind solution and design a new feature flag solution. We do things this way to prevent users from enabling 2 flags at the same time, and the program not knowing which to use.

The alternative would be writing an "O(n²)" solution where every flag is checked against every other flag to ensure the same functionality. On a new electrsd version N, the versions file would be incremented in at least N lines of code.

Could this check be done in one place, e.g. build.rs and a new feature created like electrs_0_8_10_only then all of the any not feature gates changes can be feature = "electrs_0_8_10_only" etc. The new const bools can also then be removed.

That's a good solution, I implemented it on the latest force push.

tcharding · 2026-04-30T01:19:45Z

No need for the last patch mate, we have #571 ready to release that change. I was planning on merging this after that release.

tcharding · 2026-05-06T02:38:04Z

-        let mut cookie_value;
-        #[cfg(feature = "legacy")]
-        {
+        let cookie_value = if cfg!(feature = "esplora_a33e97e1_only") {


Cool, this is clear! Way better than not(electrs_0_8_10).

tcharding · 2026-05-06T02:43:43Z

Sorry for the iterations, this feature gating and weird feature setup (caused by the repo not you) is hard to get right. I'm going to need to come back to this with fresh eyes.

tcharding · 2026-05-08T04:49:19Z

-electrs_0_10_6 = ["download"]
-
 bitcoind_download = ["bitcoind/download", "download"]
+default = ["electrs_0_10_6", "download"]


Why is default to enable download? We don't do that in bitcoind, there was some reason we stopped doing so but I forget right now.

I'll remove it. I left it in because a few weeks ago, the download came by default for every feature flag of elecstrd, and some other PR removed, but I had left it on by default.

tcharding · 2026-05-08T04:54:51Z

-      - run: cd electrsd && cargo test --features ${{ matrix.features }}
+      - run: cd electrsd && RUST_LOG=debug cargo test --features ${{ matrix.features }}


Why do we need this when its set above? Does RUST_LOG: debug not work?

But in test-electrs-no-download its not set. Can we pick one way and do it uniform?

Oh my bad, I had missed it. Will put it on env for every electrsd job.

tcharding · 2026-05-08T04:56:51Z

+  'cfg(feature, values("electrs_0_9_11_only", "electrs_0_9_1_only", "electrs_0_8_10_only", "esplora_a33e97e1_only"))',
+  'cfg(esplora_a33e97e1)',


We shouldn't need the last one because its a feature, right?

Actually we don't need any of these do we because of the loop in build.rs?

+ for v in versions { + println!("cargo:rustc-check-cfg=cfg({}_only)", v); + }

I think I had put it because of some cargo check or lint warning, but I can't reproduce it anymore, so I'm dropping the change.

tcharding · 2026-05-08T05:00:37Z

+        let env_name = format!("CARGO_FEATURE_{}", v.to_uppercase());
+        if std::env::var_os(&env_name).is_some() {
+            // Emits cfg for the highest enabled version only and then returns.
+            println!("cargo:rustc-cfg={}_only", v);


Suggested change

println!("cargo:rustc-cfg={}_only", v);

println!("cargo:rustc-cfg={}_and_below", v);

I think I finally get it. The 'only' was throwing me off before big time.

Sorry, but the _only indeed means only a specific version. The latest change should clear up the confusion, because this older version was not using the only properly. It was an error.

tcharding · 2026-05-08T05:01:06Z

Sorry got to run half way through review.

tcharding · 2026-05-08T05:01:17Z

Wrong button.

satsfy · 2026-05-08T22:17:28Z

Force-pushed. Changes:

Rebase with master.
Update lockfiles to fix CI errors.
In versions.rs, replace _only cfg aliases with chained not(...) guards . It was a mistake to make that change because the solution would not compile for electrs versions below 0.10.6. I could not use the _only suffixed flags they are not present at build time, causing a compilation error. But if you do add to Cargo.toml (which I tried), they break -all-features tests because all the _only flags are included when only one of them should.
Move RUST_LOG=debug out of inline invocation and into the job env block.
Add --no-default-features back to cargo test runs to prevent the default features from leaking into other versions in CI test.
Drop download from the default feature set.
Revert cfg!(feature = "esplora_a33e97e1_only") back to cfg!(esplora_a33e97e1_only).

tcharding · 2026-05-11T05:18:43Z

+#[cfg(all(
+    not(feature = "electrs_0_8_10"),
+    not(feature = "electrs_0_9_1"),
+    not(feature = "electrs_0_9_11"),
+    not(feature = "electrs_0_10_6"),
+    not(feature = "esplora_a33e97e1"),
+))]


Suggested change

#[cfg(all(

not(feature = "electrs_0_8_10"),

not(feature = "electrs_0_9_1"),

not(feature = "electrs_0_9_11"),

not(feature = "electrs_0_10_6"),

not(feature = "esplora_a33e97e1"),

))]

#[cfg(not(feature = "esplora_a33e97e1"))]

Recently worked out this can be simplified in bitcoind too. This works because of the cascading features. Same change in this file at the bottom.

There was another gate I thought was wrong then I realised the same, really the feature means esplora_a33e97e1 and later versions

Cascading features were removed entirely in the latest version.

tcharding · 2026-05-11T05:19:24Z

-const VERSION: &str = "v0.10.6";
+#[cfg(all(feature = "electrs_0_9_1", not(feature = "electrs_0_9_11")))]
+const VERSION: &str = "v0.9.1";
+#[cfg(all(feature = "electrs_0_8_10", not(feature = "electrs_0_9_1")))]


Why not use the _only flags?

I agree, this was one of the reasons to have the new feature.

I have removed these flags. They were too complex. What I did instead if to gate with.

#[cfg(all(feature = "electrs_0_8_10", not(feature = "all_features_test")))]

I think this is reasonable because it reads very clear meaning.

tcharding · 2026-05-11T05:20:00Z

Reviewed: 285011c

jamillambert

I am not sure the removal of the legacy feature is a good idea. This could break downstream for no real benefit.

Other than that and the few features gates that should be _only the rest looks good.

Reviewed 285011c

satsfy · 2026-05-13T22:54:48Z

The latest force push reworks the solution a lot, after some suggestions:

Revert almost all flag changes and simplify the PRs diff. Cascading features were entirely removed.
Tighten scope, removing some unnecessary changes (the 0.8.10 unrelated fix and CI debug logs) to other PRs.
Removed the build-time-created flags.
This is the simple solution I should have done from the start: add a all_features_test flag that gets enabled only when cargo test --all-features gets run. The user is never supposed to use that flag. Feature gating became more complex but much more readable.

NOTE: cannot move lint for format changes to other PRs because workspace now exercises it, causing CI failures. It is an inseparable part of the PR.

I am not sure the removal of the legacy feature is a good idea

I'm new-ish to OSS, and didn't realize how this would affect downstream. The latest push adds it back and keeps the flag situation intact for downstream.

Introducing electrsd in workspace cause --all-features tests to enable all electrsd feature flags, this commit creates a new test-only feature that is only enabled on these particular tests. Version compiler constant gets appropriately gates through the new flag so only the latest electsd version is activated. It introduces flag to all version specific call sites.

satsfy · 2026-05-13T23:00:28Z

Latest push simple rebase + lockfile update.

tcharding · 2026-05-14T03:58:30Z

Looking good. To review I decided to just hack around and see what feel out. Want to look at the last commit on this branch and pull out any changes that you like?

https://github.com/tcharding/corepc/tree/push-zmqllslturol

satsfy requested review from RCasatta, jamillambert and tcharding as code owners April 24, 2026 05:48

jamillambert reviewed Apr 24, 2026

View reviewed changes

Comment thread electrsd/src/versions.rs Outdated

jamillambert reviewed Apr 24, 2026

View reviewed changes

Comment thread electrsd/src/lib.rs Outdated

satsfy force-pushed the include-electrsd branch from 2a42a61 to 4c939ef Compare April 25, 2026 04:01

satsfy force-pushed the include-electrsd branch 2 times, most recently from af412f2 to 6e540e4 Compare April 25, 2026 04:23

satsfy requested a review from jamillambert April 25, 2026 17:49

satsfy force-pushed the include-electrsd branch from 6e540e4 to 8343421 Compare April 28, 2026 03:02

satsfy force-pushed the include-electrsd branch from 8343421 to 12645ba Compare April 28, 2026 19:35

satsfy force-pushed the include-electrsd branch 2 times, most recently from eca4af4 to 70b269f Compare April 28, 2026 20:29

tcharding reviewed Apr 29, 2026

View reviewed changes

Comment thread electrsd/src/ext.rs Outdated

satsfy force-pushed the include-electrsd branch from 70b269f to 28022c5 Compare April 29, 2026 19:22

satsfy force-pushed the include-electrsd branch 5 times, most recently from 3866f93 to cd0ddfe Compare April 29, 2026 21:26

tcharding reviewed Apr 30, 2026

View reviewed changes

tcharding reviewed May 6, 2026

View reviewed changes

tcharding reviewed May 8, 2026

View reviewed changes

tcharding closed this May 8, 2026

tcharding reopened this May 8, 2026

satsfy force-pushed the include-electrsd branch from 8d274a5 to 285011c Compare May 8, 2026 22:17

tcharding reviewed May 11, 2026

View reviewed changes

jamillambert mentioned this pull request May 12, 2026

Do a bunch of releases #581

Closed

jamillambert reviewed May 12, 2026

View reviewed changes

satsfy force-pushed the include-electrsd branch 2 times, most recently from 04ae05b to fded3bb Compare May 13, 2026 22:46

satsfy added 7 commits May 13, 2026 19:58

chore: update lock files for electrsd inclusion

b9e70b2

electrsd: apply clippy lint error fix

ed853c2

electrsd: apply rustfmt cleanup

ed5dc81

electrsd: add rbmt lint configuration

4484b66

ci: skip electrsd downloads select test features

9cd16d6

Include electrsd in workspace

0f4c5fc

satsfy force-pushed the include-electrsd branch from fded3bb to 0f4c5fc Compare May 13, 2026 22:59

		#[cfg(not(feature = "electrs_0_8_10"))]
		#[cfg(any(feature = "electrs_0_9_1", not(feature = "electrs_0_8_10")))]

		- run: cd electrsd && cargo test --features ${{ matrix.features }}
		- run: cd electrsd && RUST_LOG=debug cargo test --features ${{ matrix.features }}

		'cfg(feature, values("electrs_0_9_11_only", "electrs_0_9_1_only", "electrs_0_8_10_only", "esplora_a33e97e1_only"))',
		'cfg(esplora_a33e97e1)',

	println!("cargo:rustc-cfg={}_only", v);
	println!("cargo:rustc-cfg={}_and_below", v);

Conversation

satsfy commented Apr 24, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

satsfy commented Apr 24, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

satsfy commented Apr 25, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

tcharding commented Apr 28, 2026

Uh oh!

satsfy commented Apr 28, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

satsfy commented Apr 28, 2026

Uh oh!

tcharding commented Apr 29, 2026

Uh oh!

Uh oh!

satsfy commented Apr 29, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

satsfy commented Apr 29, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

tcharding Apr 30, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tcharding Apr 30, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

satsfy Apr 30, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tcharding commented Apr 30, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tcharding commented May 6, 2026

Uh oh!

tcharding May 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tcharding May 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

satsfy May 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tcharding commented May 8, 2026

Uh oh!

satsfy commented Apr 24, 2026 •

edited

Loading

satsfy commented Apr 24, 2026 •

edited

Loading

satsfy commented Apr 25, 2026 •

edited

Loading

satsfy commented Apr 28, 2026 •

edited

Loading

satsfy commented Apr 29, 2026 •

edited

Loading

satsfy commented Apr 29, 2026 •

edited

Loading

tcharding Apr 30, 2026 •

edited

Loading

tcharding Apr 30, 2026 •

edited

Loading

satsfy Apr 30, 2026 •

edited

Loading

tcharding May 8, 2026 •

edited

Loading

tcharding May 8, 2026 •

edited

Loading

satsfy May 8, 2026 •

edited

Loading

satsfy commented May 8, 2026 •

edited

Loading

satsfy commented May 13, 2026 •

edited

Loading

tcharding commented May 14, 2026 •

edited

Loading