Kixunil / tonic_lnd

Rust implementation of LND RPC client using async GRPC library `tonic`
31 stars 44 forks source link

Explicit selection of RPCs using cargo features #29

Closed okjodom closed 12 months ago

okjodom commented 1 year ago

Allow explicit selection of RPCs using caargo features, for slimmer dependency and faster compilation. If no feature is selected, all are included by default

  1. Makes Lightning RPC optionally available under lightningrpc feature gate.

    Usage: cargo run --features=lightningrpc --example getinfo <address> <tls.cert> <file.macaroon>

    1. Makes Wallet RPC optionally available under walletrpc feature gate.

      Usage: cargo run --features=walletrpc --example <example> <address> <tls.cert> <file.macaroon>

  2. Makes Version RPC optionally available under versionrpc feature gate.

    Usage: cargo run --features=versionrpc --example getversion <address> <tls.cert> <file.macaroon>

  3. Make Sign and Peers RPCs available under signrpc and peersrpc feature gates respectively

closes #26

Kixunil commented 1 year ago

Why is it behind feature gate and not unconditional? Does it significantly increase compile time?

okjodom commented 1 year ago

Why is it behind feature gate and not unconditional? Does it significantly increase compile time?

I haven't benchmarked compile times. I'm using the feature gates to allow very selective dependency on the RPCs. Specifically, I have a use case that needs LND routerrpc and never any of the other rpcs that can be supported

Kixunil commented 1 year ago

Would be cool to benchmark it a bit. If it doesn't increase compile time by let's say more than 5 seconds on some reasonable machine I probably wouldn't bother much with gates. But I don't mind them too much either.

okjodom commented 1 year ago

Would be cool to benchmark it a bit. If it doesn't increase compile time by let's say more than 5 seconds on some reasonable machine I probably wouldn't bother much with gates. But I don't mind them too much either.

Ack. Posting benchmarks below. I still would want gates so I I can pick this feature and shake out the rest of the features. If routerrpc were unconditional, and we end up with some other unconditional feature, that also came along for the ride, I would be keen on avoiding the bloat.

PS: We could pick what features to make unconditional by observing what features are depended on most often by other features in the feature table

/* Cargo.toml */

[features]
lightningrpc = []
signrpc = []
walletrpc = ["signrpc"]
routerrpc = ["lightningrpc"]
all = ["lightningrpc", "walletrpc", "routerrpc"]

I suspect lightningrpc might end up being such a feature

okjodom commented 1 year ago

Would be cool to benchmark it a bit. If it doesn't increase compile time by let's say more than 5 seconds on some reasonable machine I probably wouldn't bother much with gates. But I don't mind them too much either.

Ack. Posting benchmarks below.

Effect of feature gates on compile times.
Benchmark compilation for `track_payment` example.

1. All features on by default : No feature gates applied

Fresh compile: 9.49s

                       | dev [unoptimized + debuginfo] target(s) |    AVG
- Clean compile        |    2.86s, 2.66s, 2.63s, 2.61s, 2.80s    |  ~ 2.71s
- Recompile (baseline) |    0.06s, 0.08s, 0.08s, 0.08s, 0.07s    |  ~ 0.07s

2. All features on by default : Feature gate `all` applied on compile

Fresh compile: 4.67s

                       | dev [unoptimized + debuginfo] target(s) |    AVG
- Clean compile        |    2.65s, 2.61s, 2.60s, 2.83s, 2.57s    |  ~ 2.65s
- Recompile (baseline) |    0.08s, 0.07s, 0.06s, 0.08s, 0.06s    |    0.07s

3. All features off by default : Only `routerrpc` feature is applied on compile 

Fresh compile: 4.13s

                       | dev [unoptimized + debuginfo] target(s) |    AVG
- Clean compile        |    2.51s, 2.55s, 2.66s, 2.52s, 2.52s    |  ~ 2.55s
- Recompile (baseline) |    0.07s, 0.08s, 0.07s, 0.08s, 0.08s    |  ~ 0.08s

------------------------------------------------------------------------------

Notes:

Fresh compile - Do a compilation after making a code change, for example, after deleting / reintroducing feature gate markers
Clean compile - Do a compilation after deleting the assets produced in target dir, but without code changes
Re compile    - Do a compilation immediately after clean compile. There are assets cached in the target dir, and is no code change to compile

Benchmarks on:
- 32 GiB RAM
- Intel® Core™ i7-9750H CPU @ 2.60GHz × 12 Processor
- Ubuntu 22.04.1
- rustc 1.65.0 (897e37553 2022-11-02)

@Kixunil , I collected the above benchmarks. IMO, this looks much in favour of feature gates, especially as more features are added to support the full LND RPC API

Kixunil commented 1 year ago

Cool, thank! It's funny that it really is those 5 sec. But the relative factor is ~2 which is a lot. I agree with having the gates.

okjodom commented 1 year ago

@Kixunil, I aggregated a bunch of my long running contributions under this PR. Primarily, it adds feature gates and adds Router RPC.

Figured I could make all features available by default to avoid a breaking change for existing dependents in the wild. Dependents can now opt-out of features by applying explicit selections of rpcs they need

okjodom commented 12 months ago

These changes are included in fedimint fork of this library. Closing PR as stale.