electrsd: include in workspace and CI#570
electrsd: include in workspace and CI#570satsfy wants to merge 12 commits intorust-bitcoin:masterfrom
Conversation
|
One thing I thought to include is a default electrsd version, like bitcoind in Edit: I suppose we need the default so tests work with an explicit version. |
2a42a61 to
4c939ef
Compare
|
Latest force push improves the legibility of the solution, sets a default electrs version, reworked the version feature graph so the |
af412f2 to
6e540e4
Compare
|
Needs rebase mate. Out of interest what made you go with |
6e540e4 to
8343421
Compare
|
Great point, I made a mistake regarding EDIT: The problem with v0.10.6 in CI was caused by using the legacy |
8343421 to
12645ba
Compare
|
On the latest rebase:
|
eca4af4 to
70b269f
Compare
|
Can we separate the fmt changes from actual code changes in c61ea18 |
70b269f to
28022c5
Compare
|
Latest rebase: |
93b3716 to
3866f93
Compare
When running `--all-features`, both `electrs_0_8_10` and `esplora_a33e97e1` features are enabled. This PR handles such case by enabling the legacy cookie handling when `esplora_a33e97e1` is enabled. The `legacy` flag is semantically meaningless, therefore it was dropped.
`std` was included in bitreq for the download `get()` function.
It tested an unsuported version 0.11.x.
3866f93 to
cd0ddfe
Compare
|
Latest rebase updates lockfiles because of upstream changes to fix ci |
| #[cfg(not(feature = "electrs_0_8_10"))] | ||
| #[cfg(any(feature = "electrs_0_9_1", not(feature = "electrs_0_8_10")))] |
There was a problem hiding this comment.
Can you explain what you want to achieve with this feature gate please?
There was a problem hiding this comment.
E.g "I want this code block to run for these versions: a, b, c"
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
That's a good solution, I implemented it on the latest force push.
|
No need for the last patch mate, we have #571 ready to release that change. I was planning on merging this after that release. |
cd0ddfe to
d4e07c0
Compare
|
Rebased removing |
0fa0e28 to
5fc3c64
Compare
|
Latest force push implements configuration of version checks in the build file. |
Mirrors bitcoind solution. It also removes default electrs download for non-esplora paths.
Closes #552
electrsdin the workspace so it is built, linted, and tested alongside the other crates, mirroring thebitcoindcrate pattern.cargo fmtand clippy fixes in electrsd./contrib/update-lock-files.shelectrs_0_10_6the default version feature. Version features are now chained like bitcoind as a version cascade:electrs_0_10_6 -> 0_9_11 -> 0_9_1 -> 0_8_10 -> esplora.--all-featuresresolves to 0.10.6.versions.rswith legible constantsUSE_LEGACY_COOKIE(replacing oldlegacyfeature),USE_JSONRPC_IMPORT,USE_VERBOSE_ARG.--no-default-featuresso it does not accidentally combine with the default electrs version.no-downloadjob now enablesbitcoind_downloadbecause it still needs a bitcoind binary while using the system-providedELECTRS_EXEC.23_2feature name in ee9443c.RUST_LOG=debugon electrsd jobs.Tests now include electrsd. Check by running:
cargo test --all-featuresI've had various versions problems. Ran a CI matrix of every bitcoin version and electrds in Ubuntu 24.04 to understand what would work with what.