Skip to content

Initial commit for reworked select k indices function. #2733

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.

Sign up for GitHub

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

Jump to bottom
Draft
wants to merge 3 commits into
base: main
Choose a base branch
from
Draft

Initial commit for reworked select k indices function. #2733

wants to merge 3 commits into from
+589 −6

Conversation

@damrobi
Copy link
Collaborator

@damrobi damrobi commented Oct 10, 2025

Content

This PR includes a new function to perform the selection of k unique indices for the aggregated signature of mithril-stm. The goal of the new function is to improve readability and performances (memory and time) if possible.

Pre-submit checklist

  • Branch
    • Tests are provided (if possible)
    • Crates versions are updated (if relevant)
    • CHANGELOG file is updated (if relevant)
    • Commit sequence broadly makes sense
    • Key commits have useful messages
  • PR
    • All check jobs of the CI have succeeded
    • 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)
    • Add ADR blog post or Dev ADR entry (if relevant)
    • No new TODOs introduced

Comments

Issue(s)

Relates to #YYY or Closes #YYY

@github-actions
Copy link

github-actions bot commented Oct 10, 2025 •
edited
Loading

Test Results

    3 files   -     1    141 suites   - 23   48m 3s ⏱️ + 24m 31s
2 209 tests +   14  2 209 ✅ +   14  0 💤 ±0  0 ❌ ±0 
5 707 runs   - 1 053  5 707 ✅  - 1 053  0 💤 ±0  0 ❌ ±0 

Results for commit e3c2b19. ± Comparison against base commit 9f90c66.

This pull request removes 98 and adds 112 tests. Note that renamed tests count towards both. 
mithril-aggregator ‑ database::query::signed_entity::get_signed_entity::tests::cardano_stake_distribution_by_epoch_returns_records_filtered_by_epoch
mithril-aggregator ‑ database::query::signed_entity::get_signed_entity::tests::cardano_stake_distribution_by_epoch_returns_records_returns_only_cardano_stake_distribution_records
mithril-aggregator ‑ database::query::signed_entity::get_signed_entity::tests::test_get_signed_entity_records
mithril-aggregator ‑ http_server::routes::signer_routes::tests::registered_signers_registration_epoch::error_if_given_epoch_is_not_a_number_nor_latest
mithril-aggregator ‑ http_server::routes::signer_routes::tests::registered_signers_registration_epoch::use_epoch_service_current_epoch_if_latest
mithril-aggregator ‑ http_server::routes::signer_routes::tests::registered_signers_registration_epoch::use_given_epoch_if_valid_number
mithril-aggregator ‑ services::aggregator_client::tests::remote_certificate_retriever::test_get_latest_certificate_details
mithril-aggregator ‑ services::aggregator_client::tests::remote_certificate_retriever::test_get_latest_genesis_certificate
mithril-aggregator ‑ services::aggregator_client::tests::test_4xx_errors_are_handled_as_remote_server_logical
mithril-aggregator ‑ services::aggregator_client::tests::test_5xx_errors_are_handled_as_remote_server_technical
…

mithril-aggregator ‑ database::query::signed_entity::get_signed_entity::tests::by_signed_entity_and_epoch_returns_records_filtered_by_discriminant
mithril-aggregator ‑ database::query::signed_entity::get_signed_entity::tests::by_signed_entity_and_epoch_returns_records_filtered_by_epoch
mithril-aggregator ‑ database::query::signed_entity::get_signed_entity::tests::test_get_all_records
mithril-aggregator ‑ database::query::signed_entity::get_signed_entity::tests::test_get_record_by_id
mithril-aggregator ‑ database::query::signed_entity::get_signed_entity::tests::test_get_record_by_signed_entity_type
mithril-aggregator ‑ database::repository::signed_entity_store::tests::get_last_signed_entities_by_type_and_epoch_when_nothing_found
mithril-aggregator ‑ database::repository::signed_entity_store::tests::get_last_signed_entities_by_type_and_epoch_when_signed_entity_found_for_epoch
mithril-aggregator ‑ http_server::parameters::tests::dont_overflow_if_epoch_minus_offset_is_negative
mithril-aggregator ‑ http_server::parameters::tests::error_if_given_epoch_is_not_a_number_nor_latest_nor_latest_minus_a_number
mithril-aggregator ‑ http_server::parameters::tests::test_expanded_epoch_apply_additional_offset
…

♻️ This comment has been updated with latest results.

@damrobi damrobi temporarily deployed to testing-preview October 10, 2025 10:10 — with GitHub Actions Inactive
github-advanced-security[bot]
github-advanced-security bot found potential problems Nov 5, 2025
View reviewed changes
mithril-stm/src/aggregate_signature/basic_verifier.rs
use std::collections::{BTreeMap, HashMap, HashSet};
use std::time::Instant;

use rug::integer::IsPrime;

Check warning

Code scanning / clippy

unused import: rug::integer::IsPrime Warning

unused import: rug::integer::IsPrime
mithril-stm/src/aggregate_signature/basic_verifier.rs
let mut removal_idx_by_vk: HashMap<&SingleSignatureWithRegisteredParty, Vec<Index>> =
HashMap::new();

let mut basic_verif_time = 0;

Check warning

Code scanning / clippy

variable basic_verif_time is assigned to, but never used Warning

variable basic_verif_time is assigned to, but never used
mithril-stm/src/aggregate_signature/basic_verifier.rs
HashMap::new();

let mut basic_verif_time = 0;
let now = Instant::now();

Check warning

Code scanning / clippy

unused variable: now Warning

unused variable: now
mithril-stm/src/aggregate_signature/basic_verifier.rs
let mut dedup_sigs: HashSet<SingleSignatureWithRegisteredParty> = HashSet::new();
let mut count: u64 = 0;

let now = Instant::now();

Check warning

Code scanning / clippy

unused variable: now Warning

unused variable: now
mithril-stm/src/aggregate_signature/basic_verifier.rs
msg: &[u8],
sigs: &[SingleSignatureWithRegisteredParty],
) -> Result<Vec<SingleSignatureWithRegisteredParty>, AggregationError> {
let now = Instant::now();

Check warning

Code scanning / clippy

unused variable: now Warning

unused variable: now
mithril-stm/src/aggregate_signature/basic_verifier.rs
let (idx_by_mtidx, btm) = Self::get_k_indices(total_stake, params, msg, sigs);
// println!("First function took: {:?}ms.", now.elapsed().as_millis());

let now = Instant::now();

Check warning

Code scanning / clippy

unused variable: now Warning

unused variable: now
mithril-stm/src/aggregate_signature/basic_verifier.rs
msg: &[u8],
sigs: &[SingleSignatureWithRegisteredParty],
) -> Result<Vec<SingleSignatureWithRegisteredParty>, AggregationError> {
let now = Instant::now();

Check warning

Code scanning / clippy

unused variable: now Warning

unused variable: now
mithril-stm/src/aggregate_signature/basic_verifier.rs
let btm = Self::get_k_indices_opti(total_stake, params, msg, sigs);
// println!("First function OPTI? took: {:?}ms.\n", now.elapsed().as_millis());

let now = Instant::now();

Check warning

Code scanning / clippy

unused variable: now Warning

unused variable: now
mithril-stm/src/aggregate_signature/basic_verifier.rs
sig_by_mt_index: BTreeMap<Index, &SingleSignatureWithRegisteredParty>,
) -> Result<Vec<SingleSignatureWithRegisteredParty>, AggregationError> {
let mut valid_idx_for_mt_idx: HashMap<u64, Vec<u64>> = HashMap::new();
let mut valid_idx_for_sig: HashMap<&SingleSignatureWithRegisteredParty, Vec<u64>> = HashMap::new();

Check warning

Code scanning / clippy

variable does not need to be mutable Warning

variable does not need to be mutable
mithril-stm/src/aggregate_signature/basic_verifier.rs
sig_by_mt_index: BTreeMap<Index, &SingleSignatureWithRegisteredParty>,
) -> Result<Vec<SingleSignatureWithRegisteredParty>, AggregationError> {
let mut valid_idx_for_mt_idx: HashMap<u64, Vec<u64>> = HashMap::new();
let mut valid_idx_for_sig: HashMap<&SingleSignatureWithRegisteredParty, Vec<u64>> = HashMap::new();

Check warning

Code scanning / clippy

unused variable: valid_idx_for_sig Warning

unused variable: valid_idx_for_sig
github-advanced-security[bot]
github-advanced-security bot found potential problems Nov 5, 2025
View reviewed changes
Copy link

@github-advanced-security github-advanced-security bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

clippy found more than 20 potential problems in the proposed changes. Check the Files changed tab for more details.

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

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@damrobi