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

CoreVerifier setup does not compute total_stake correctly #1306

Closed
2 tasks
jpraynaud opened this issue Oct 19, 2023 · 0 comments · Fixed by #1308
Closed
2 tasks

CoreVerifier setup does not compute total_stake correctly #1306

jpraynaud opened this issue Oct 19, 2023 · 0 comments · Fixed by #1308
Assignees
Labels
bug ⚠️ Something isn't working cryptography 🔐 Cryptography related

Comments

@jpraynaud
Copy link
Member

Why

The Coreverifer of the mithril-stm does not correctly compute the total_stake during the setup: this can lead to division by zero when the num-integer-backend is used.

What

The total_stake variable is not mutable and is always 0 when returned by the setup function:

pub fn setup(public_signers: &[(VerificationKey, Stake)]) -> Self {
    let total_stake: Stake = 0;
    let mut unique_parties = HashSet::new();
    for signer in public_signers.iter() {
        let (_, overflow) = total_stake.overflowing_add(signer.1);
        if overflow {
            panic!("Total stake overflow");
        }
        unique_parties.insert(MTLeaf(signer.0, signer.1));
    }

    let mut eligible_parties: Vec<_> = unique_parties.into_iter().collect();
    eligible_parties.sort_unstable();
    CoreVerifier {
        eligible_parties,
        total_stake,
    }
}

The rug backend does not detect a division by zero, probably because it is converted to a float before dividing.

How

  • Fix the computation of the total_stake when creating the CoreVerifier
  • Add a test that verifies the computation of total_stake
@jpraynaud jpraynaud added bug ⚠️ Something isn't working cryptography 🔐 Cryptography related labels Oct 19, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug ⚠️ Something isn't working cryptography 🔐 Cryptography related
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants