Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Enhance Mithril/Cardano node communication #1315

Closed
6 tasks done
jpraynaud opened this issue Oct 20, 2023 · 3 comments · Fixed by #1348, #1403, #1513 or #1541
Closed
6 tasks done

Enhance Mithril/Cardano node communication #1315

jpraynaud opened this issue Oct 20, 2023 · 3 comments · Fixed by #1348, #1403, #1513 or #1541
Labels
idea 💡 Something to discuss and refine optimization 🛠️ Optimization and/or small enhancements

Comments

@jpraynaud
Copy link
Member

jpraynaud commented Oct 20, 2023

Why

Today, the Mithril signer and aggregator communicate with the Cardano node with the Cardano cli which acts as a proxy for providing some of the information essential to running a Mithril network. This communication is not optimal, and we could communicate directly with the Cardano node instead.

What

The Mithril signer and aggregator (will) rely on some information that are provided by the Cardano node :

  • Epoch
  • Epoch Stake Distribution
  • KES Period
  • UTxO of an address (TxDatum in particular)
  • Cardano node version (will)
  • Protocol Parameters (will)
  • ...

The signer and aggregator rely on an a common ChainObserver trait:

pub trait ChainObserver: Sync + Send {
    /// Retrieve the datums associated to and address
    async fn get_current_datums(
        &self,
        address: &ChainAddress,
    ) -> Result<Vec<TxDatum>, ChainObserverError>;

    /// Retrieve the current epoch of the Cardano network
    async fn get_current_epoch(&self) -> Result<Option<Epoch>, ChainObserverError>;

    /// Retrieve the current stake distribution of the Cardano network
    async fn get_current_stake_distribution(
        &self,
    ) -> Result<Option<StakeDistribution>, ChainObserverError>;

    /// Retrieve the KES period of an operational certificate
    async fn get_current_kes_period(
        &self,
        _opcert: &OpCert,
    ) -> Result<Option<KESPeriod>, ChainObserverError> {
        Ok(None)
    }
}

That is currently implemented by the CliObserver.

We would like to have a new implementation CardanoObserver of this trait that:

  • removes the usage of the Cardano cli
  • uses a direct communication with the Cardano node through the socket it exposes and using mini-protocols

A good candidate library to implement this is pallas and pallas-network that already has an implementation of the mini-protocols.

Note:
The signer and aggregator nodes already have a defined configuration parameter with the path of the Cardano node socket cardano_node_socket_path:

How

  • Implement Pallas chain observer skeleton with fallback by default on the Cardano cli chain observer
  • Implement get_current_epoch in Pallas chain observer
  • Implement get_current_datums in Pallas chain observer
  • Implement get_current_stake_distribution in Pallas chain observer
  • Implement get_current_kes_period in Pallas chain observer
  • Get rid of the fallback mechanism once the ChainObserver trait is fully implemented by the Pallas chain observer

Draft of how this could be implemented

  • Evaluate if pallas can give access to the aforementioned information from the Cardano node (all of subset of them)
  • Implement the CardanoObserver for the current version of the ChainObserver trait without relying on the Cardano cli
  • Implement the usage of this new CardanoObserver and activate the dependency with a specific configuration (e.g. by adding a new adapter configuration parameter, a bit like the era_reader_adapter_type in signer and aggregator)
  • If more information than those available in the current version of the ChainObserver trait, add them (with blanket implementation to not break current CliObserver)?
  • Document the modifications in the Mithril User manual

Acceptance Criteria

  • Unit tests (mocks) should be implemented
  • End to end tests should be working consistently
  • All checks and tests should be green in the CI (documentation, linting, ...)
@jpraynaud jpraynaud added optimization 🛠️ Optimization and/or small enhancements to-groom 🤔 Needs grooming labels Oct 20, 2023
@jpraynaud
Copy link
Member Author

Ping @abailly-iohk, @scarmuega, @falcucci

@jpraynaud jpraynaud added the idea 💡 Something to discuss and refine label Oct 20, 2023
@jpraynaud
Copy link
Member Author

@scarmuega, @falcucci, following our discussion, here are some information on the way dependencies of the ChainObserver work.

In order to test the new PallasObserver, you will need to update the dependency injection in the aggregator and signer.

I'd suggest that you make the change in 2 phases:

  • Replace the adapter in the dependency builders at first by hard coding the modifications (you can perfectly modify the aggregator at first, and then the signer in order to make the end to end test debugging easier)
  • Once the end to end tests are working, you can implement a mechanism to select the correct chain observer based on the configuration of the nodes:
    • A similar thing has been done that for the Era ReaderAdapter
    • This will allow to progressively roll-out the update on the different Mithril networks

@robinboening
Copy link

robinboening commented Dec 14, 2023

+1 for working on this issue. I am following this because once the switch is done it allows docker set ups to remove the cardano-cli dependency from the container. This dependency has been annoying me a bit as it currently means you either have to

  1. copy the cardano-cli (+libs) over to your mithril container
  2. or allow the mithril container to execute the cardano node container's cli

Both ways do work, but it'll be much more elegant when the cardano-cli isn't needed anymore.

Keep up the great work! 👏
~Robin

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment