Repository: input-output-hk/mithril (rust)

Issue:
Stake distribution discrepancy
## Problem

We observe some signers stop being able to participate to multisignatures with an error message `invlid signature`. We discovered that the chain observer returned a different stake distribution during an epoch depending the time it was queried. This led to broken certificate chain due to no signature issued during some epochS.
We found it the CLI tool could return the stake distribution used to perform epoch retribution but it has to be called with each `pool_id`.

Here is a plan to tackle this problem:

 - [x] hardcode the stake distribution with the same value for every signer. This will stabilize the certificate chain in a short term.
 - [x] change the `ChainObserver` so it queries the CLI tool to get the stake distribution per Epoch.

## Points of attention 

 * is the stake distribution information the same as the one we retrieve today? If not, this could lead to a stake distribution corruption of the stake stored by the aggregator for future signatures. 

Pull request description:
Fix Stake Distribution retrieval
## Content
This PR includes a fix on the Stake Distribution retrieval. It makes use `cardano-cli query stake-snapshot` to retrieve an accurate stake distribution for each available stake pool. This allows to retrieve the `Stake Distribution` at the correct `Epoch` (previous)

## Pre-submit checklist

- Branch
  - [x] Tests are provided (if possible)
  - [x] Commit sequence broadly makes sense
  - [x] Key commits have useful messages
- PR
  - [x] No clippy warnings in the CI
  - [x] Self-reviewed the diff
  - [x] Useful pull request description
  - [x] Reviewer requested
- Documentation
  - [x] Update README file (not needed)
  - [x] Update documentation website (not needed)
  - [x] Add dev blog post

## Comments
Once this PR is merged, the SPO will need to recompile their node and signing should work as expected after `2` epochs

## Issue
Closes #497 


Patch:
diff --git a/docs/blog/2022-09-13-stake-distribution-retrieval.md b/docs/blog/2022-09-13-stake-distribution-retrieval.md
new file mode 100644
index 000000000..f50b812c7
--- /dev/null
+++ b/docs/blog/2022-09-13-stake-distribution-retrieval.md
@@ -0,0 +1,27 @@
+---
+title: Stake Distribution retrieval fixed
+authors:
+- name: Mithril Team
+tags: [stake-distribution, certificate]
+---
+
+### The way the Mithril nodes retrieve the Stake Distribution is changing
+
+**PR**: `Fix Stake Distribution retrieval` [#499](https://github.com/input-output-hk/mithril/pull/499)
+
+**Issue**: `Stake distribution discrepancy` [#497](https://github.com/input-output-hk/mithril/issues/497)
+
+We have noticed that the way the Mithril nodes computed the `Stake Distribution` was erroneous: the epoch that was used to make the computation was the **current epoch** instead of the **previous epoch**. This has lead to some de-synchronization between the Signers and the hosted GCP Aggregator for a few epochs.
+
+Indeed, the `Stake Distribution` retrieved from the Cardano node depended on the time at which it was done: the nodes where having differents values that prevented them from being able to work together to produce valid multi-signatures. The problem is related to the epoch that is used (**current epoch**) to make the computation of the `Stake Distribution` when the `cardano-cli query stake-distribution` command is ran, whereas the Mithril protocol needs to work with the **previous epoch**.
+
+A workaround is being implemented in this fix that will compute differently the `Stake Distribution` to target the **previous epoch**. To do so, the Stake value that is now retrieved sequentially for each pool available in the `cardano-cli query stake-distribution` by using the command `cardano-cli query stake-snapshot --stake-pool-id **pool-id*`. This guarantees that the `Stake Distribution` is computed deterministically on all nodes of the Mithril Network.
+
+We will continue our efforts to enhance the way the `Stake Distribution` is retrieved in the future, and so that it works smoothly on the `mainnet` (where the numbers of pools is bigger `~3,000` vs `~100` on the `preview` network).
+
+The SPOs need to recompile their Signer node in order to compute correctly the `Stake Distributions` on their node (as in this [guide](https://mithril.network/doc/manual/getting-started/run-signer-node)).
+It should then take up to `2` epochs before they are able to successfully register their individual signatures with the Aggregator.
+
+More information about the `Certificate Chain` and the epochs retrieval requirements is available [here](https://mithril.network/doc/mithril/mithril-protocol/certificates).
+
+Feel free to reach out to us on the [Discord channel](https://discord.gg/5kaErDKDRq) for questions and/or help.
diff --git a/docs/root/manual/getting-started/bootstrap-cardano-node.md b/docs/root/manual/getting-started/bootstrap-cardano-node.md
index d9bfb7f75..b450d4198 100644
--- a/docs/root/manual/getting-started/bootstrap-cardano-node.md
+++ b/docs/root/manual/getting-started/bootstrap-cardano-node.md
@@ -125,9 +125,6 @@ NETWORK=testnet
 # Aggregator API endpoint URL
 AGGREGATOR_ENDPOINT=https://aggregator.api.mithril.network/aggregator
 
-# Genesis verification key
-GENESIS_VERIFICATION_KEY=$(wget -q -O - https://raw.githubusercontent.com/input-output-hk/mithril/main/TEST_ONLY_genesis.vkey)
-
 # Digest of the latest produced snapshot for convenience of the demo
 # You can also modify this variable and set it to the value of the digest of a snapshot that you can retrieve at step 2
 SNAPSHOT_DIGEST=$(curl -s $AGGREGATOR_ENDPOINT/snapshots | jq -r '.[0].digest')
@@ -205,7 +202,7 @@ to /home/mithril/data/testnet/cd587611b5ff2445c714bef083d9455ed3e42e9304ae0ad38b
 Verify the Certificate of the snapshot and unpack its content in order to feed the Cardano node database
 
 ```bash
-GENESIS_VERIFICATION_KEY=$GENESIS_VERIFICATION_KEY NETWORK=$NETWORK AGGREGATOR_ENDPOINT=$AGGREGATOR_ENDPOINT ./mithril-client restore $SNAPSHOT_DIGEST
+NETWORK=$NETWORK AGGREGATOR_ENDPOINT=$AGGREGATOR_ENDPOINT ./mithril-client restore $SNAPSHOT_DIGEST
 ```
 
 You will see that the snapshot archive is unpacked and that the associated certificate is valid
diff --git a/mithril-common/src/chain_observer/cli_observer.rs b/mithril-common/src/chain_observer/cli_observer.rs
index af51dfd2a..d8b900345 100644
--- a/mithril-common/src/chain_observer/cli_observer.rs
+++ b/mithril-common/src/chain_observer/cli_observer.rs
@@ -12,6 +12,10 @@ use crate::CardanoNetwork;
 #[async_trait]
 pub trait CliRunner {
     async fn launch_stake_distribution(&self) -> Result<String, Box<dyn Error + Sync + Send>>;
+    async fn launch_stake_snapshot(
+        &self,
+        stake_pool_id: &str,
+    ) -> Result<String, Box<dyn Error + Sync + Send>>;
     async fn launch_epoch(&self) -> Result<String, Box<dyn Error + Sync + Send>>;
 }
 
@@ -41,6 +45,18 @@ impl CardanoCliRunner {
         command
     }
 
+    fn command_for_stake_snapshot(&self, stake_pool_id: &str) -> Command {
+        let mut command = self.get_command();
+        command
+            .arg("query")
+            .arg("stake-snapshot")
+            .arg("--stake-pool-id")
+            .arg(stake_pool_id);
+        self.post_config_command(&mut command);
+
+        command
+    }
+
     fn command_for_epoch(&self) -> Command {
         let mut command = self.get_command();
         command.arg("query").arg("tip");
@@ -93,6 +109,29 @@ impl CliRunner for CardanoCliRunner {
         }
     }
 
+    async fn launch_stake_snapshot(
+        &self,
+        stake_pool_id: &str,
+    ) -> Result<String, Box<dyn Error + Sync + Send>> {
+        let output = self
+            .command_for_stake_snapshot(stake_pool_id)
+            .output()
+            .await?;
+
+        if output.status.success() {
+            Ok(std::str::from_utf8(&output.stdout)?.trim().to_string())
+        } else {
+            let message = String::from_utf8_lossy(&output.stderr);
+
+            Err(format!(
+                "Error launching command {:?}, error = '{}'",
+                self.command_for_stake_snapshot(stake_pool_id),
+                message
+            )
+            .into())
+        }
+    }
+
     async fn launch_epoch(&self) -> Result<String, Box<dyn Error + Sync + Send>> {
         let output = self.command_for_epoch().output().await?;
 
@@ -127,6 +166,45 @@ impl CardanoCliChainObserver {
     fn parse_string<'a>(&'a self, string: &'a str) -> IResult<&str, f64> {
         nom::number::complete::double(string)
     }
+
+    async fn get_current_stake_value(
+        &self,
+        stake_pool_id: &str,
+    ) -> Result<u64, ChainObserverError> {
+        let stake_pool_snapshot_output = self
+            .cli_runner
+            .launch_stake_snapshot(stake_pool_id)
+            .await
+            .map_err(ChainObserverError::General)?;
+        let stake_pool_snapshot: Value = serde_json::from_str(&stake_pool_snapshot_output)
+            .map_err(|e| {
+                ChainObserverError::InvalidContent(
+                    format!(
+                        "Error: {:?}, output was = '{}'",
+                        e, stake_pool_snapshot_output
+                    )
+                    .into(),
+                )
+            })?;
+        if let Value::Number(stake_pool_stake) = &stake_pool_snapshot["poolStakeMark"] {
+            return stake_pool_stake.as_u64().ok_or_else(|| {
+                ChainObserverError::InvalidContent(
+                    format!(
+                        "Error: could not parse stake pool value as u64 {:?}",
+                        stake_pool_stake
+                    )
+                    .into(),
+                )
+            });
+        }
+        Err(ChainObserverError::InvalidContent(
+            format!(
+                "Error: could not parse stake pool snapshot {:?}",
+                stake_pool_snapshot
+            )
+            .into(),
+        ))
+    }
 }
 
 #[async_trait]
@@ -149,6 +227,7 @@ impl ChainObserver for CardanoCliChainObserver {
             Ok(None)
         }
     }
+
     async fn get_current_stake_distribution(
         &self,
     ) -> Result<Option<StakeDistribution>, ChainObserverError> {
@@ -166,11 +245,17 @@ impl ChainObserver for CardanoCliChainObserver {
                 continue;
             }
 
-            if let Ok((_, f)) = self.parse_string(words[1]) {
-                let stake: u64 = (f * 1_000_000_000.0).round() as u64;
+            let stake_pool_id = words[0];
+            let stake_fraction = words[1];
+
+            if let Ok((_, _f)) = self.parse_string(stake_fraction) {
+                // This block is a fix:
+                // the stake retrieved was computed on the current epoch, when we need a value computed on the previous epoch
+                // in 'let stake: u64 = (f * 1_000_000_000.0).round() as u64;'
+                let stake: u64 = self.get_current_stake_value(stake_pool_id).await?;
 
                 if stake > 0 {
-                    let _ = stake_distribution.insert(words[0].to_string(), stake);
+                    let _ = stake_distribution.insert(stake_pool_id.to_string(), stake);
                 }
             } else {
                 return Err(ChainObserverError::InvalidContent(
@@ -208,6 +293,36 @@ pool1qz2vzszautc2c8mljnqre2857dpmheq7kgt6vav0s38tvvhxm6w   1.051e-6
             Ok(output.to_string())
         }
 
+        async fn launch_stake_snapshot(
+            &self,
+            stake_pool_id: &str,
+        ) -> Result<String, Box<dyn Error + Sync + Send>> {
+            let mut output = r#"
+{
+    "poolStakeGo": 1000000,
+    "poolStakeSet": 2000000,
+    "poolStakeMark": 3000000,
+    "activeStakeGo": 5000000,
+    "activeStakeSet": 7000000,
+    "activeStakeMark": 9000000
+}
+"#;
+            if stake_pool_id == "pool1qpqvz90w7qsex2al2ejjej0rfgrwsguch307w8fraw7a7adf6g8" {
+                output = r#"
+                {
+                    "poolStakeGo": 0,
+                    "poolStakeSet": 0,
+                    "poolStakeMark": 0,
+                    "activeStakeGo": 5000000,
+                    "activeStakeSet": 7000000,
+                    "activeStakeMark": 9000000
+                }
+                "#
+            }
+
+            Ok(output.to_string())
+        }
+
         async fn launch_epoch(&self) -> Result<String, Box<dyn Error + Sync + Send>> {
             let output = r#"
 {
@@ -273,6 +388,22 @@ pool1qz2vzszautc2c8mljnqre2857dpmheq7kgt6vav0s38tvvhxm6w   1.051e-6
         );
     }
 
+    #[tokio::test]
+    async fn test_get_current_stake_value() {
+        let observer = CardanoCliChainObserver::new(Box::new(TestCliRunner {}));
+        let stake = observer
+            .get_current_stake_value("pool1qqyjr9pcrv97gwrueunug829fs5znw6p2wxft3fvqkgu5f4qlrg")
+            .await
+            .expect("get current stake value should not fail");
+        assert_eq!(3_000_000, stake);
+
+        let stake = observer
+            .get_current_stake_value("pool1qpqvz90w7qsex2al2ejjej0rfgrwsguch307w8fraw7a7adf6g8")
+            .await
+            .expect("get current stake value should not fail");
+        assert_eq!(0, stake);
+    }
+
     #[tokio::test]
     async fn test_get_current_stake_distribution() {
         let observer = CardanoCliChainObserver::new(Box::new(TestCliRunner {}));
@@ -281,16 +412,15 @@ pool1qz2vzszautc2c8mljnqre2857dpmheq7kgt6vav0s38tvvhxm6w   1.051e-6
             .await
             .unwrap()
             .unwrap();
-
         assert_eq!(7, results.len());
         assert_eq!(
-            2_493_000,
+            3_000_000,
             *results
                 .get("pool1qqyjr9pcrv97gwrueunug829fs5znw6p2wxft3fvqkgu5f4qlrg")
                 .unwrap()
         );
         assert_eq!(
-            1_051,
+            3_000_000,
             *results
                 .get("pool1qz2vzszautc2c8mljnqre2857dpmheq7kgt6vav0s38tvvhxm6w")
                 .unwrap()


Test patch:
diff --git a/mithril-test-lab/mithril-devnet/devnet-mkfiles.sh b/mithril-test-lab/mithril-devnet/devnet-mkfiles.sh
index a1a615c41..2f7ec8d6d 100755
--- a/mithril-test-lab/mithril-devnet/devnet-mkfiles.sh
+++ b/mithril-test-lab/mithril-devnet/devnet-mkfiles.sh
@@ -588,10 +588,26 @@ do
         echo ">>>> Activated!"
         POOL_IDX=1
         ./pools.sh | while read POOL_ID ; do
-            echo ">>>> Found PoolId: \$POOL_ID"
+            echo ">>>> Detected PoolId: \$POOL_ID"
             echo PARTY_ID=\${POOL_ID} > node-pool\${POOL_IDX}/pool.env
             POOL_IDX=\$(( \$POOL_IDX + 1))
         done
+        ./pools.sh | while read POOL_ID ; do
+            echo ">>>> Retrieve stakes PoolId: \$POOL_ID"
+            while true
+            do
+                POOL_STAKE_PREVIOUS_EPOCH=\$(CARDANO_NODE_SOCKET_PATH=node-pool${N}/ipc/node.sock ./cardano-cli query stake-snapshot \\
+                    --stake-pool-id \$POOL_ID \\
+                    --testnet-magic ${NETWORK_MAGIC} | jq .poolStakeMark)
+                if [ "\$POOL_STAKE_PREVIOUS_EPOCH" != "0" ] ; then
+                    break
+                else
+                    echo ">>>> Not retrievable yet"
+                    sleep 2
+                fi
+            done
+            echo ">>>> Stakes retrieved PoolId: \$POOL_ID / \$POOL_STAKE_PREVIOUS_EPOCH"
+        done
         break
     else
         echo ">>>> Not activated yet"
diff --git a/mithril-test-lab/mithril-end-to-end/src/end_to_end_spec.rs b/mithril-test-lab/mithril-end-to-end/src/end_to_end_spec.rs
index 5f5a884d6..0194786bb 100644
--- a/mithril-test-lab/mithril-end-to-end/src/end_to_end_spec.rs
+++ b/mithril-test-lab/mithril-end-to-end/src/end_to_end_spec.rs
@@ -4,7 +4,7 @@ use mithril_common::chain_observer::{CardanoCliChainObserver, ChainObserver};
 use mithril_common::digesters::ImmutableFile;
 use mithril_common::entities::{Certificate, Epoch, EpochSettings, Snapshot};
 use reqwest::StatusCode;
-use slog_scope::info;
+use slog_scope::{info, warn};
 use std::error::Error;
 use std::path::Path;
 use std::sync::Arc;
@@ -21,14 +21,16 @@ impl Spec {
 
     pub async fn run(&mut self) -> Result<(), Box<dyn Error>> {
         let aggregator_endpoint = self.infrastructure.aggregator().endpoint();
-
         wait_for_enough_immutable(self.infrastructure.aggregator().db_directory()).await?;
-        // Epoch 3 since:
-        // * Epoch(0) won't produce anything since the Cardano network is still booting up.
-        // * It needs signers from the previous epoch.
-        // * Epoch(2) is still too early for some environements (ie: CI) to have the aggregator
-        // waiting for signatures long enough for signers to register.
-        let mut target_epoch = Epoch(3);
+        let start_epoch = self
+            .infrastructure
+            .chain_observer()
+            .get_current_epoch()
+            .await?
+            .unwrap_or_default();
+
+        // Wait 1 epoch after start epoch for the aggregator to be able to bootstrap a genesis certificate
+        let mut target_epoch = start_epoch + 1;
         wait_for_target_epoch(
             self.infrastructure.chain_observer(),
             target_epoch,
@@ -123,10 +125,13 @@ async fn wait_for_epoch_settings(aggregator_endpoint: &str) -> Result<EpochSetti
                     info!("Aggregator ready"; "epoch_settings"  => #?epoch_settings);
                     Ok(Some(epoch_settings))
                 }
-                s if s.is_server_error() => Err(format!(
-                    "Server error while waiting for the Aggregator, http code: {}",
-                    s
-                )),
+                s if s.is_server_error() => {
+                    warn!(
+                        "Server error while waiting for the Aggregator, http code: {}",
+                        s
+                    );
+                    Ok(None)
+                }
                 _ => Ok(None),
             },
             Err(_) => Ok(None),