manuelmauro / algonaut

A rusty sdk for Algorand.
MIT License
64 stars 37 forks source link

[feature request] split `algod`, `kmd`, and `indexer` into features #125

Open skylerqpq opened 2 years ago

skylerqpq commented 2 years ago

Is your feature request related to a problem? Please describe. I use algonaut in a project, but specifically only use algod, I would like to not need to compile kmd and indexer related code. This isn't a huge deal for the most part, it's just that all code has to be compiled.

Describe the solution you'd like add the following features to the algonaut_model, algonaut_client and algonaut: "kmd", "indexer", "algod", (and add them to default features if that is desirable, presumably it is).

#[cfg(feature = "<appropriate feature>")] for kmd, indexer, and algod modules across the aforementioned crates

Sadly this is a breaking change (for default-features = false users at the least) so it needs to be accompanied by a minor version bump (0.x -> 0.y)

Describe alternatives you've considered

  1. Keep with the status quo.
  2. Only add some of those features but not others
  3. Add no-* features. This is considered non-idiomatic, but has the advantage of being non-breaking.

Additional context N/A

ivnsch commented 2 years ago

Interesting. I'd estimate that most people don't use at least kmd, so being able to leave services out for sure has a point. A more radical (but cleaner IMO) approach would be to put them in separate projects.

But also, how justified is this? Given incremental compilation and that these services are pretty lean code wise it seems a rather small optimization. The macro resolution might be a bit costly, but still?

AlterionX commented 2 years ago

It's has been a bit but agreed on the cleaner version, where the modules get split up based on each service instead of by model/client/etc. My reasoning being that the algod client can change independently of indexer & kmd.

The justification is kinda dependent on the context by which this is used. Technically, if it's delivered often enough, even 1% could be significant and algonaut's a library, so... That said, it'd probably be better to at least check the compilation sizes before saying whether it should or shouldn't happen. Might be more effective looking into how well we can trim down dependencies via features though.

ivnsch commented 2 years ago

Yeah, the binary sizes (and possibly compilation times) would help us prioritizing this.

Noting that we've the Algorand test suite, which depends on all 3 projects (I personally question the need for this, but we can't realistically change them), and which we are executing as part of the Github CI. I'm not sure about the exact flow, but it should continue to be executed with the CI of all the projects.

Edit: For the tests CI, we probably should configure each project to filter features/tags relevant to them. For cross-project testing (which in most/all cases means using KMD to sign in unrelated tests), we'd put the tests in the project of what's being tested primarily and add the auxiliary components as dev dependencies.

ivnsch commented 2 years ago

This issue has gained more relevance with the addition of the ABI and Atomic Transaction Composer, which are entirely optional. I'm not sure why (haven't had time to investigate yet), it increased my WASM (optimized) build size almost 2x (to ~1.8mb). I'm not using any of these features.

manuelmauro commented 2 years ago

This solution would be ideal also in case we would like to add some higher level features to algonaut. In particular, I was thinking of crates for ARC3 (NFTs) or DID.