Skip to content

Conversation

@Sosthene00
Copy link
Collaborator

Use a workspace structure with a minimalist spdk-core and then one crate for a backend/architecture

@pythcoiner
Copy link
Contributor

In spdk-core we can build w/ both default async + sync features, maybe adding something like this in spdk-core/src/lib.rs could make it clear that it's not appropriate:

#[cfg(all(feature = "async", feature = "sync"))]
compile_error!("Cannot use both sync & async features together");

@Sosthene00
Copy link
Collaborator Author

In spdk-core we can build w/ both default async + sync features, maybe adding something like this in spdk-core/src/lib.rs could make it clear that it's not appropriate:

#[cfg(all(feature = "async", feature = "sync"))]
compile_error!("Cannot use both sync & async features together");

Ok with this, being explicit about those 2 features simultaneously doesn't make sense, but for now it's actually the default to pull both sync and async code, I though the overhead wasn't big and didn't want to start restricting too much before I was clear. Feature sync eliminate all async code though

@pythcoiner
Copy link
Contributor

clippy yell a lot on my side, feel free to cherry-pick in 53bf2aa

@pythcoiner
Copy link
Contributor

I have try to implem a dummy updater here

I got weird build error that seems related to async-trait:

Checking spdk-core v0.1.0 (/home/pyth/rust/spdk/spdk-core)
error[E0195]: lifetime parameters or bounds on method `record_scan_progress` do not match the trait declaration
  --> spdk-core/src/updater/dummy_updater.rs:52:34
   |
52 |       async fn record_scan_progress(
   |                                    ^ lifetimes do not match method in trait
   |
  ::: spdk-core/src/updater/updater.rs:43:1
   |
43 |   #[async_trait::async_trait]
   |   --------------------------- this bound might be missing in the impl
...
51 |       async fn record_scan_progress(
   |  ______________-
52 | |         &mut self,
53 | |         start: Height,
54 | |         current: Height,
55 | |         end: Height,
56 | |     ) -> Result<()>;
   | |_____- lifetimes in impl do not match this method in trait

error[E0195]: lifetime parameters or bounds on method `record_block_outputs` do not match the trait declaration
  --> spdk-core/src/updater/dummy_updater.rs:61:34
   |
61 |       async fn record_block_outputs(
   |                                    ^ lifetimes do not match method in trait
   |
  ::: spdk-core/src/updater/updater.rs:43:1
   |
43 |   #[async_trait::async_trait]
   |   --------------------------- this bound might be missing in the impl
...
64 |       async fn record_block_outputs(
   |  ______________-
65 | |         &mut self,
66 | |         height: Height,
67 | |         blkhash: BlockHash,
68 | |         found_outputs: HashMap<OutPoint, OwnedOutput>,
69 | |     ) -> Result<()>;
   | |_____- lifetimes in impl do not match this method in trait

error[E0195]: lifetime parameters or bounds on method `record_block_inputs` do not match the trait declaration
  --> spdk-core/src/updater/dummy_updater.rs:70:33
   |
70 |       async fn record_block_inputs(
   |                                   ^ lifetimes do not match method in trait
   |
  ::: spdk-core/src/updater/updater.rs:43:1
   |
43 |   #[async_trait::async_trait]
   |   --------------------------- this bound might be missing in the impl
...
77 |       async fn record_block_inputs(
   |  ______________-
78 | |         &mut self,
79 | |         blkheight: Height,
80 | |         blkhash: BlockHash,
81 | |         found_inputs: HashSet<OutPoint>,
82 | |     ) -> Result<()>;
   | |_____- lifetimes in impl do not match this method in trait

error[E0195]: lifetime parameters or bounds on method `save_to_persistent_storage` do not match the trait declaration
  --> spdk-core/src/updater/dummy_updater.rs:79:40
   |
79 |     async fn save_to_persistent_storage(&mut self) -> anyhow::Result<()> {
   |                                        ^ lifetimes do not match method in trait
   |
  ::: spdk-core/src/updater/updater.rs:43:1
   |
43 | #[async_trait::async_trait]
   | --------------------------- this bound might be missing in the impl
...
85 |     async fn save_to_persistent_storage(&mut self) -> Result<()>;
   |              ------------------------------------- lifetimes in impl do not match this method in trait

For more information about this error, try `rustc --explain E0195`.
error: could not compile `spdk-core` (lib) due to 4 previous errors

@pythcoiner
Copy link
Contributor

In spdk-core we can build w/ both default async + sync features, maybe adding something like this in spdk-core/src/lib.rs could make it clear that it's not appropriate:

#[cfg(all(feature = "async", feature = "sync"))]
compile_error!("Cannot use both sync & async features together");

Ok with this, being explicit about those 2 features simultaneously doesn't make sense, but for now it's actually the default to pull both sync and async code, I though the overhead wasn't big and didn't want to start restricting too much before I was clear. Feature sync eliminate all async code though

I just figure out that this create a new issue: we cannot run a bare cargo clippy or cargo build on the workspace if one crate want to use the sync feature, as cargo seems willing to do an "union" off all features and build spdk-core only once...

Comment on lines +39 to +56
fn get_block_data_for_range(
&self,
range: RangeInclusive<u32>,
dust_limit: Amount,
with_cutthrough: bool,
) -> BlockDataIterator {
let client = self.client.clone();

Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
fn get_block_data_for_range(
&self,
range: RangeInclusive<u32>,
dust_limit: Amount,
with_cutthrough: bool,
) -> BlockDataIterator {
let client = self.client.clone();
fn get_block_data_for_range(
&self,
mut range: RangeInclusive<u32>,
dust_limit: Amount,
with_cutthrough: bool,
) -> BlockDataIterator {
let client = self.client.clone();
// blindbit will return an error 500 for genesis block
if *range.start() == 0 {
range = RangeInclusive::new(1, *range.end());
}

@cygnet3
Copy link
Owner

cygnet3 commented Nov 29, 2025

Since we removed parallel block scanning we should revert the change of SpScanner to a trait too

@Sosthene00
Copy link
Collaborator Author

Since we removed parallel block scanning we should revert the change of SpScanner to a trait too

Should we? I don't think it hurts and downstream may want to implement some or all methods differently. We may want to clean it up and propose more default implem maybe

@Sosthene00
Copy link
Collaborator Author

I have try to implem a dummy updater here

I got weird build error that seems related to async-trait:

Checking spdk-core v0.1.0 (/home/pyth/rust/spdk/spdk-core)
error[E0195]: lifetime parameters or bounds on method `record_scan_progress` do not match the trait declaration
  --> spdk-core/src/updater/dummy_updater.rs:52:34
   |
52 |       async fn record_scan_progress(
   |                                    ^ lifetimes do not match method in trait
   |
  ::: spdk-core/src/updater/updater.rs:43:1
   |
43 |   #[async_trait::async_trait]
   |   --------------------------- this bound might be missing in the impl
...
51 |       async fn record_scan_progress(
   |  ______________-
52 | |         &mut self,
53 | |         start: Height,
54 | |         current: Height,
55 | |         end: Height,
56 | |     ) -> Result<()>;
   | |_____- lifetimes in impl do not match this method in trait

error[E0195]: lifetime parameters or bounds on method `record_block_outputs` do not match the trait declaration
  --> spdk-core/src/updater/dummy_updater.rs:61:34
   |
61 |       async fn record_block_outputs(
   |                                    ^ lifetimes do not match method in trait
   |
  ::: spdk-core/src/updater/updater.rs:43:1
   |
43 |   #[async_trait::async_trait]
   |   --------------------------- this bound might be missing in the impl
...
64 |       async fn record_block_outputs(
   |  ______________-
65 | |         &mut self,
66 | |         height: Height,
67 | |         blkhash: BlockHash,
68 | |         found_outputs: HashMap<OutPoint, OwnedOutput>,
69 | |     ) -> Result<()>;
   | |_____- lifetimes in impl do not match this method in trait

error[E0195]: lifetime parameters or bounds on method `record_block_inputs` do not match the trait declaration
  --> spdk-core/src/updater/dummy_updater.rs:70:33
   |
70 |       async fn record_block_inputs(
   |                                   ^ lifetimes do not match method in trait
   |
  ::: spdk-core/src/updater/updater.rs:43:1
   |
43 |   #[async_trait::async_trait]
   |   --------------------------- this bound might be missing in the impl
...
77 |       async fn record_block_inputs(
   |  ______________-
78 | |         &mut self,
79 | |         blkheight: Height,
80 | |         blkhash: BlockHash,
81 | |         found_inputs: HashSet<OutPoint>,
82 | |     ) -> Result<()>;
   | |_____- lifetimes in impl do not match this method in trait

error[E0195]: lifetime parameters or bounds on method `save_to_persistent_storage` do not match the trait declaration
  --> spdk-core/src/updater/dummy_updater.rs:79:40
   |
79 |     async fn save_to_persistent_storage(&mut self) -> anyhow::Result<()> {
   |                                        ^ lifetimes do not match method in trait
   |
  ::: spdk-core/src/updater/updater.rs:43:1
   |
43 | #[async_trait::async_trait]
   | --------------------------- this bound might be missing in the impl
...
85 |     async fn save_to_persistent_storage(&mut self) -> Result<()>;
   |              ------------------------------------- lifetimes in impl do not match this method in trait

For more information about this error, try `rustc --explain E0195`.
error: could not compile `spdk-core` (lib) due to 4 previous errors

Just seen this, is it still happening?

@cygnet3
Copy link
Owner

cygnet3 commented Nov 30, 2025

Since we removed parallel block scanning we should revert the change of SpScanner to a trait too

Should we? I don't think it hurts and downstream may want to implement some or all methods differently. We may want to clean it up and propose more default implem maybe

I think making SpScanner a trait it is too much flexibility without a clear benefit. The SpScanner is essentially the main struct of SPDK. The SpScanner takes a data source (that implements ChainBackend trait) and provides scanning results to some sink (that implements Updater trait). Consumers of SPDK have some degree of flexibility by using different chain backends / updaters, since they might e.g. use a different way of storing results. But if we want to provide a different way of scanning, I think it makes more sense to just make different scanner structs, e.g. ParallelBlockScanner or something. I don't think there is a need to have all these structs implement an overarching Scanner trait.

@pythcoiner
Copy link
Contributor

Just seen this, is it still happening?

yes

@Sosthene00
Copy link
Collaborator Author

I have try to implem a dummy updater here

I got weird build error that seems related to async-trait:

Checking spdk-core v0.1.0 (/home/pyth/rust/spdk/spdk-core)
error[E0195]: lifetime parameters or bounds on method `record_scan_progress` do not match the trait declaration
  --> spdk-core/src/updater/dummy_updater.rs:52:34
   |
52 |       async fn record_scan_progress(
   |                                    ^ lifetimes do not match method in trait
   |
  ::: spdk-core/src/updater/updater.rs:43:1
   |
43 |   #[async_trait::async_trait]
   |   --------------------------- this bound might be missing in the impl
...
51 |       async fn record_scan_progress(
   |  ______________-
52 | |         &mut self,
53 | |         start: Height,
54 | |         current: Height,
55 | |         end: Height,
56 | |     ) -> Result<()>;
   | |_____- lifetimes in impl do not match this method in trait

error[E0195]: lifetime parameters or bounds on method `record_block_outputs` do not match the trait declaration
  --> spdk-core/src/updater/dummy_updater.rs:61:34
   |
61 |       async fn record_block_outputs(
   |                                    ^ lifetimes do not match method in trait
   |
  ::: spdk-core/src/updater/updater.rs:43:1
   |
43 |   #[async_trait::async_trait]
   |   --------------------------- this bound might be missing in the impl
...
64 |       async fn record_block_outputs(
   |  ______________-
65 | |         &mut self,
66 | |         height: Height,
67 | |         blkhash: BlockHash,
68 | |         found_outputs: HashMap<OutPoint, OwnedOutput>,
69 | |     ) -> Result<()>;
   | |_____- lifetimes in impl do not match this method in trait

error[E0195]: lifetime parameters or bounds on method `record_block_inputs` do not match the trait declaration
  --> spdk-core/src/updater/dummy_updater.rs:70:33
   |
70 |       async fn record_block_inputs(
   |                                   ^ lifetimes do not match method in trait
   |
  ::: spdk-core/src/updater/updater.rs:43:1
   |
43 |   #[async_trait::async_trait]
   |   --------------------------- this bound might be missing in the impl
...
77 |       async fn record_block_inputs(
   |  ______________-
78 | |         &mut self,
79 | |         blkheight: Height,
80 | |         blkhash: BlockHash,
81 | |         found_inputs: HashSet<OutPoint>,
82 | |     ) -> Result<()>;
   | |_____- lifetimes in impl do not match this method in trait

error[E0195]: lifetime parameters or bounds on method `save_to_persistent_storage` do not match the trait declaration
  --> spdk-core/src/updater/dummy_updater.rs:79:40
   |
79 |     async fn save_to_persistent_storage(&mut self) -> anyhow::Result<()> {
   |                                        ^ lifetimes do not match method in trait
   |
  ::: spdk-core/src/updater/updater.rs:43:1
   |
43 | #[async_trait::async_trait]
   | --------------------------- this bound might be missing in the impl
...
85 |     async fn save_to_persistent_storage(&mut self) -> Result<()>;
   |              ------------------------------------- lifetimes in impl do not match this method in trait

For more information about this error, try `rustc --explain E0195`.
error: could not compile `spdk-core` (lib) due to 4 previous errors

I think I addressed that in my last commit, let me know if you see that error again

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.

4 participants