Redesign mithril client api #1332

Alenar · 2023-11-08T16:48:34Z

Content

This PR add the mithril-client library crate, see #1311 for details.

Still todo:

Polish/refine/complete the developer doc on website

Pre-submit checklist

Branch
- Tests are provided (if possible)
- Crates versions are updated (if relevant)
- Commit sequence broadly makes sense
- Key commits have useful messages
PR
- No clippy warnings in the CI
- Self-reviewed the diff
- Useful pull request description
- Reviewer requested
Documentation
- Update README file (if relevant)
- Update documentation website (if relevant)
- Add dev blog post (if relevant)

Issue(s)

Relates to #1311

github-actions · 2023-11-08T16:59:16Z

Test Results

    3 files ±  0   29 suites +12 6m 31s ⏱️ -43s
732 tests +17 732 ✔️ +17 0 💤 ±0 0 ❌ ±0
840 runs +51 840 ✔️ +51 0 💤 ±0 0 ❌ ±0

Results for commit 76347c5. ± Comparison against base commit b0177e5.

♻️ This comment has been updated with latest results.

mithril-client/src/certificate_client.rs

Alenar · 2023-11-08T17:28:04Z

mithril-client/src/message.rs

+    /// If not set a default implementation will be used.
+    pub fn with_immutable_digester(
+        mut self,
+        immutable_digester: Arc<dyn ImmutableDigester>,


Should we re-export ImmutableDigester and it's error chain ?

mithril-client/src/mithril_stake_distribution_client.rs

mithril-client/src/type_alias.rs

mithril-client/src/snapshot_downloader.rs

mithril-client/src/snapshot_client.rs

mithril-client/examples/snapshot_list_get_show_download_verify.rs

mithril-client/tests/snapshot_list_get_show_download_verify.rs

jpraynaud

LGTM 👍

Nice work guys 💪

The majority of the comments are related to fixing typos.

.github/workflows/ci.yml

docs/website/root/manual/developer-docs/nodes/mithril-client-library.md

mithril-client-cli/Cargo.toml

mithril-client/examples/snapshot_list_get_show_download_verify.rs

mithril-client/src/feedback.rs

.github/workflows/ci.yml

jpraynaud · 2023-11-10T09:16:01Z

@scarmuega, @falcucci this is the PR for the new mithril-client library. It should be merged very shortly.

We also intend to publish it on crates.io in the comping weeks 👍

scarmuega · 2023-11-10T09:19:22Z

amazing!

jpraynaud

Few more comments 👼

docs/website/root/manual/developer-docs/nodes/mithril-client-library.md

mithril-client/src/lib.rs

docs/website/root/manual/developer-docs/nodes/mithril-client-library.md

…ient This allow to reduce the number of re-export of `mithril-common` needed since we can now use a `CerticateMessage` as our main certicate type instead of using the `mithril_common::entities::Certificate` full type. Meaning that we don't have to re-export crypto helpers or types anymore ! To do so we define a new `CertificateVerifier` trait that only have a method that validate a chain and implement it with a new `MithrilCertificateVerifier` struct that use internally the verifier from `mithril-common`. As with the `InternalCertificateRetriever` this add some complexity to this specific part but the trade-off for it are worth it (as said before: no crypto type re-export leading to far less mithril-common dependency on the api level).

Using `pub use` instead of `pub type` lead to a better rust-doc since the re-exported type are displayed as 'internal' type instead of alias. + Move all alias to a dedicated mod to cleanup the `lib.rs`.

This simplify the import of those types that are likely the one that will be the most used by third parties. Co-authored-by: Damien Lachaume <dlachaumepalo@users.noreply.github.com>

Fix typos and move `certificate().verify_chain` before `snapshot().download_unpack` in integration test, doc and example.

… 'mithril-client' to 'mithril-client-cli'

- Fix outdated `utils` module documentation. - Add more details on a hack comment. - rename `aggregator_url` to `aggregator_endpoint`. - rename `SnapshotDownloadComplete` mithril event to `SnapshotDownloadCompleted`. - Add a Todo on the verify chain to specify that we want to merge most of its code in the `mithril-common` codebase.

Since the cli won't be used anymore to define the client library.

Typos in dev documentation and lib.rs

Only use the features that we needs

By running 'nix flake update' command.

…/1311/redesign_mithril_client_api Redesign mithril client api

Alenar requested review from jpraynaud, ghubertpalo and dlachaume November 8, 2023 16:48

Alenar temporarily deployed to testing-preview November 8, 2023 17:03 — with GitHub Actions Inactive

Alenar commented Nov 8, 2023

View reviewed changes

mithril-client/src/certificate_client.rs Show resolved Hide resolved

Alenar force-pushed the ensemble/1311/redesign_mithril_client_api branch from 4fd9587 to a1f5ca9 Compare November 8, 2023 17:15

Alenar commented Nov 8, 2023

View reviewed changes

mithril-client/src/mithril_stake_distribution_client.rs Outdated Show resolved Hide resolved

dlachaume reviewed Nov 8, 2023

View reviewed changes

Alenar force-pushed the ensemble/1311/redesign_mithril_client_api branch from a1f5ca9 to 96628b3 Compare November 8, 2023 17:33

Alenar temporarily deployed to testing-preview November 8, 2023 17:47 — with GitHub Actions Inactive

dlachaume reviewed Nov 9, 2023

View reviewed changes

mithril-client/examples/snapshot_list_get_show_download_verify.rs Show resolved Hide resolved

dlachaume reviewed Nov 9, 2023

View reviewed changes

mithril-client/tests/snapshot_list_get_show_download_verify.rs Show resolved Hide resolved

dlachaume temporarily deployed to testing-preview November 9, 2023 13:59 — with GitHub Actions Inactive

jpraynaud approved these changes Nov 9, 2023

View reviewed changes

dlachaume force-pushed the ensemble/1311/redesign_mithril_client_api branch 2 times, most recently from 342795d to 27fc71f Compare November 9, 2023 16:36

dlachaume temporarily deployed to testing-preview November 9, 2023 16:46 — with GitHub Actions Inactive

Alenar temporarily deployed to testing-preview November 9, 2023 17:17 — with GitHub Actions Inactive

ghubertpalo approved these changes Nov 10, 2023

View reviewed changes

jpraynaud reviewed Nov 10, 2023

View reviewed changes

dlachaume temporarily deployed to testing-preview November 10, 2023 11:29 — with GitHub Actions Inactive

jpraynaud reviewed Nov 10, 2023

View reviewed changes

docs/website/root/manual/developer-docs/nodes/mithril-client-library.md Show resolved Hide resolved

docs/website/root/manual/developer-docs/nodes/mithril-client-library.md Show resolved Hide resolved

Alenar force-pushed the ensemble/1311/redesign_mithril_client_api branch from c3c9d98 to 862eca1 Compare November 10, 2023 14:47

Alenar temporarily deployed to testing-preview November 10, 2023 14:59 — with GitHub Actions Inactive

Alenar force-pushed the ensemble/1311/redesign_mithril_client_api branch from 862eca1 to 863b4c1 Compare November 13, 2023 10:37

Alenar had a problem deploying to testing-preview November 13, 2023 10:47 — with GitHub Actions Failure

Alenar and others added 24 commits November 13, 2023 12:47

Remove unneeded common re-export and rework the those who are left

7e959cc

Using `pub use` instead of `pub type` lead to a better rust-doc since the re-exported type are displayed as 'internal' type instead of alias. + Move all alias to a dedicated mod to cleanup the `lib.rs`.

Export content of the client & message submodules in the lib

b50b344

This simplify the import of those types that are likely the one that will be the most used by third parties. Co-authored-by: Damien Lachaume <dlachaumepalo@users.noreply.github.com>

Rust-doc for feedback submodule

1587b81

Add rust-doc to missing modules + some harmonization

02727a7

Add example that list, get, download, validate snapshot

40ee582

Enhance example by using an Indicatif powered feedback receiver

76597aa

Adapt CI jobs to the new mithril-client library

717c97d

Adjustements after review

c04bd7a

Fix typos and move `certificate().verify_chain` before `snapshot().download_unpack` in integration test, doc and example.

Fix rust doc url in developer docs and update missing replacements of…

a82986f

… 'mithril-client' to 'mithril-client-cli'

Add synthax highlighting for rust and toml in docusaurus config

636690e

Update mithril-client library developer doc

0cfca39

Fix most typos raised in the PR review

7166920

Remove build-binary feature from mithril-client-cli

dda5462

Since the cli won't be used anymore to define the client library.

Add example that list, get, and validate mithril stake distribution

86d7577

Add examples build in to the test job of the CI

1b450ab

Fix PR review comments

b5bdb83

Typos in dev documentation and lib.rs

Simplify mithril-client dependency to tokio

89ef912

Only use the features that we needs

Update crates versions

d35906f

Add mithril-client to flake.nix

53a0dc7

Upgrade nix flake dependencies

64150ca

By running 'nix flake update' command.

Add mithril-client main Makefile components

3148bf4

Add simple Makefile & Readme to the new mithril-client crate

76347c5

Alenar force-pushed the ensemble/1311/redesign_mithril_client_api branch from f6f73d3 to 76347c5 Compare November 13, 2023 11:55

Alenar temporarily deployed to testing-preview November 13, 2023 12:04 — with GitHub Actions Inactive

Alenar merged commit 78981d2 into main Nov 13, 2023
26 checks passed

Alenar deleted the ensemble/1311/redesign_mithril_client_api branch November 13, 2023 12:07

falcucci pushed a commit to falcucci/mithril that referenced this pull request Nov 28, 2023

Merge pull request input-output-hk#1332 from input-output-hk/ensemble…

039cb40

…/1311/redesign_mithril_client_api Redesign mithril client api

falcucci pushed a commit to falcucci/mithril that referenced this pull request Nov 28, 2023

Merge pull request input-output-hk#1332 from input-output-hk/ensemble…

f575513

…/1311/redesign_mithril_client_api Redesign mithril client api

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Redesign mithril client api #1332

Redesign mithril client api #1332

Alenar commented Nov 8, 2023 •

edited

Loading

github-actions bot commented Nov 8, 2023 •

edited

Loading

Alenar Nov 8, 2023

jpraynaud left a comment

jpraynaud commented Nov 10, 2023

scarmuega commented Nov 10, 2023

jpraynaud left a comment

Redesign mithril client api #1332

Redesign mithril client api #1332

Conversation

Alenar commented Nov 8, 2023 • edited Loading

Content

Pre-submit checklist

Issue(s)

github-actions bot commented Nov 8, 2023 • edited Loading

Test Results

Alenar Nov 8, 2023

Choose a reason for hiding this comment

jpraynaud left a comment

Choose a reason for hiding this comment

jpraynaud commented Nov 10, 2023

scarmuega commented Nov 10, 2023

jpraynaud left a comment

Choose a reason for hiding this comment

Alenar commented Nov 8, 2023 •

edited

Loading

github-actions bot commented Nov 8, 2023 •

edited

Loading