Fix floresta's re-exports#1176
Conversation
luisschwab
added
the
code quality
|
|
jaoleal
left a comment
jaoleal
left a comment
There was a problem hiding this comment.
Nice, some notes and
the second commit, chore(floresta): clean up the floresta crate tittle is ambiguous, the description is good but i suggest a more precise title for the commit: chore: granular feature-aware re-exports for the floresta crate
| mempool = ["dep:floresta-mempool"] | ||
|
|
||
| # floresta-metrics | ||
| metrics = ["dep:floresta-metrics"] |
There was a problem hiding this comment.
I think that switching the third commit and the second commit of places may reduce the diff
| metrics = { workspace = true, optional = true } | ||
|
|
||
| [dev-dependencies] | ||
| bitcoin = { workspace = true, features = ["serde"] } |
There was a problem hiding this comment.
For this commit, shouldn't you also remove it from here? Otherwise why did you do this?
There was a problem hiding this comment.
The issue is that it's not enough to just enable the serde dependency, we also need to enable bitcoin's serde. I caught this when testing floresta's 400K feature combinations. We weren't checking this edge case before with current test matrices.
There was a problem hiding this comment.
But test-utils is pulling bitcoin/serde. Will test-utils be used for something that's not floresta-chain's tests?
There was a problem hiding this comment.
Maybe this should live in floresta-common instead. Follow-up.
| "chain-bitcoinkernel", | ||
| "chain-flat-chainstore", |
There was a problem hiding this comment.
Thinking out loud again: floresta-chain-persintance and floresta-consensus 👀
* Add missing crate re-exports
* Re-export crates granularly using features:
- Every re-exported crate is locked behind a feature,
and every crate's feature is also locked behind a feature.
This makes it so that consumers have to be explicit about
what they want to import.
* Clean up the `Cargo.toml`
* Add a minimal README
* Modify `feature_matrix.sh` so that `cargo-hack`
wont test every feature combination on the
`floresta` crate (486,160 combinations). Instead,
it will only test the `full` feature.
Move `metrics` into `crates/floresta-metrics` such that libfloresta consumers can export metrics without being locked to `floresta-node`.
41216f1 to
beb6ecd
Compare
4 participants
Closes #1157
This PR fixes re-exports on the
florestameta-crate.It adds missing crate re-exports, implements a more granular crate export via feature flags, and moves
metricsundercrates/. I've also added a minimal README for theflorestacrate, which can be extended later.Re-exports
We were missing re-exports for a few crates, namely:
floresta-compact-filtersfloresta-domainfloresta-nodefloresta-rpcSelecting features from individual re-exported crates was also not possible. I've devised a more granular feature-based selection for that. Every crate is locked behind a feature, and every crate's feature is also locked behind a feature. For example: if I want to select
floresta-chainwith theflat-chainstorefeature, I can either importflorestawithchainandchain-flat-chainstore, or simplychain-flat-chainstore.Since
cargo-hackwould try to test every feature combination (~400K), I've made it only test thefullfeature.cargo-rbmtwill make this look cleaner.I've also updated
floresta's examples so that they use the crates it re-exports (e.g.use floresta::chaininstead ofuse floresta_chain).Metrics
florestaconsumers could not use the metrics exporters implemented here, since that was conditional on usingfloresta-node. Now,metricsis moved undercrates/floresta-metricsand re-exported onfloresta.