Skip to content
Snippets Groups Projects

Fix #262 distance oracle pool index

Fix #262 distance oracle pool index
4 unresolved threads
Merged Benjamin Gallois requested to merge fix-262 into master
4 unresolved threads
  • Fixed #262 (closed): Switched to continuous indexing for naming distance evaluation files to prevent old files from being picked up by the inherent, avoiding unexpected behavior.
  • Added crate-level documentation to provide an overview of the distance oracle's purpose and usage.
  • Added a landing page for documentation duniter/index.html.
  • Added a prefix check to prevent mismatches between the oracle and runtime versions, avoiding the Smith being penalized for a negligence error.
  • Added log for distance-oracle started from the main binary.
  • Refactored run and run_and_save oracle main functions:
    • Split into three thematic functions for better modularity:
      1. Checks if there is work to do and ensures files can be saved successfully.
      2. Handles the distance evaluation logic.
      3. Saves the computed distance to a file and cleans up outdated files.
Edited by Benjamin Gallois

Merge request reports

Loading
Loading

Activity

Filter activity
  • Approvals
  • Assignees & reviewers
  • Comments (from bots)
  • Comments (from users)
  • Commits & branches
  • Edits
  • Labels
  • Lock status
  • Mentions
  • Merge request status
  • Tracking
27 use api::{AccountId, IdtyIndex};
59 use api::{AccountId, EvaluationPool, IdtyIndex, H256};
28 60
29 61 use codec::Encode;
30 62 use fnv::{FnvHashMap, FnvHashSet};
31 use log::{debug, error, info};
63 use log::{debug, info, warn};
32 64 use rayon::iter::{IntoParallelRefIterator, ParallelIterator};
33 65 use std::{io::Write, path::PathBuf};
34 66
35 // TODO select metadata file using features
67 /// The file version must match the version used by the inherent data provider.
68 /// This ensures that the smith avoids accidentally submitting invalid data
69 /// in case there are changes in logic between the runtime and the oracle,
70 /// thereby preventing potential penalties.
71 const VERSION_PREFIX: &str = "001-";
  • The version prefix is also in client/distance/src/lib.rs, we should not forget to update it everywhere at the same time

  • Yes, that’s the point. The prefix will only be updated when there is a breaking change between the runtime and the oracle, so the user will be notified in the log in case of a mismatch and will not be penalized.

  • The TODO was about how to handle the different runtimes between gdev/gtest/g1 (in the same repo). I don't know if the version prefix solves this problem, it depends on the workflow adopted for runtime upgrades through the different chains. But OK to remove the TODO, as tests will break if something goes wrong then, so it can't be forgotten.

  • Ok, I overlooked this. To do so, we should have 3 metadata files and then use conditional compiling. I expect the metadata to work mostly interchangeably because we are only querying storage with the same types, but I'm not very well documented on the metadata part...

  • Please register or sign in to reply
  • 165 .read_dir()
    166 .unwrap_or_else(|e| {
    167 panic!(
    168 "Cannot read distance evaluation result directory `{:?}`: {:?}",
    169 settings.evaluation_result_dir, e
    170 )
    171 })
    172 .flatten()
    173 .filter_map(|entry| {
    174 entry
    175 .file_name()
    176 .into_string()
    177 .ok()
    178 .and_then(|name| {
    179 name.split('-')
    180 .last()?
  • 1 # Distance oracle
    • instead of deleting the file we should link to the auto-generated documentation so that people who explore the repo find their way to the explanations

      we can expect it to be on ipns://doc.duniter.org/ even the publishing process is currently manual.

    • It should be good now. Each principal directory (node, distance-oracle, pallets, and runtime) should have a README file that references the autogenerated documentation. From the documentation, contributors should be able to navigate the project.

    • Please register or sign in to reply
  • I'm requesting a review from @tuxmain and will keep looking at the documentation part before approving

  • Hugo Trentesaux requested review from @tuxmain and removed review request for @HugoTrentesaux

    requested review from @tuxmain and removed review request for @HugoTrentesaux

  • 32 //! ## How it works
    33 //! - The **Distance Pallet** adds an evaluation request at period `i` in the runtime.
    34 //! - The **Distance Oracle** evaluates this request at period `i + 1`, computes the necessary results and stores them on disk.
    35 //! - The **Inherent Data Provider** reads this evaluation result from disk at period `i + 2` and provides it to the runtime to perform the required operations.
    36 //!
    37 //! ## Usage
    38 //!
    39 //! ### Docker Integration
    40 //!
    41 //! To run the Distance Oracle, use the provided Docker setup. Refer to the [docker-compose.yml](../docker-compose.yml) file for an example configuration.
    42 //!
    43 //! Example Output:
    44 //! ```text
    45 //! 2023-12-09T14:45:05.942Z INFO [distance_oracle] Nothing to do: Pool does not exist
    46 //! Waiting 1800 seconds before next execution...
    47 //! ```
    • Comment on lines +37 to +47

      The functional documentation makes sense here but the smith documentation is already in /docs. If we place partial documentations at various places it will be harder to read and to update.

    • True, we have to define a documentation strategy. We can discuss it in #233 (closed) or in a new issue, but I think the closer it is to the code the better.

      It does not prevent from having more complete and less up to date documentation on the Duniter website for example.

    • Please register or sign in to reply
  • added 1 commit

    Compare with previous version

  • added 1 commit

    Compare with previous version

  • added 1 commit

    • fc1c7a6d - reference rust doc in readme

    Compare with previous version

  • added 1 commit

    • fef8ead1 - reference rust doc in readme

    Compare with previous version

  • added 1 commit

    • 95169c88 - reference rust doc in readme

    Compare with previous version

  • Pascal Engélibert approved this merge request

    approved this merge request

  • added 1 commit

    Compare with previous version

  • mentioned in commit ebc4730c

  • Please register or sign in to reply
    Loading