Open
Conversation
Consolidated Tests Results 2026-02-25 - 17:24:58Test ResultsDetails
test-reporter: Run #591
🎉 All tests passed!TestsView All Tests
🍂 No flaky tests in this run. Github Test Reporter by CTRF 💚 🔄 This comment has been updated |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Most of the diff is mechanical import rewiring. Focus review on the new crates and the open questions below.
What this does
Extracts the pure-algebra layer out of
threshold-fheinto its own crate –threshold-algebra, abbreviated asalgebra– along with three smaller utility crates.The goal is to make
threshold-fhethinner and thus allow for more compilation parallelism. Smaller code units provides a clear(-er) dependency boundary (no IO, no async, no tokio). This is the first step of several.New crates
threshold-algebracore/threshold-algebra/base_ring,bivariate,galois_fields,galois_rings,poly,error_correction,syndrome,structure_traits— plusrole,sharingand thePRSSConversionstrait (see open questions)threshold-hashingcore/threshold-hashing/hash_element,unsafe_hash_list,DomainSep,DIGEST_BYTESetc. — extracted fromthreshold-fheerror-utilscore/error-utils/anyhow_error_and_log/anyhow_error_and_warn_loghelpers (lived inthreshold-fhe's internalerrormodule)thread-handlescore/thread-handles/threshold-fheand some cleanup&testsOther notable changes
gen→ggenrename inchoreography::grpcandnetworking— avoids clashing with the Rustgenreserved keyword. While we are still on the 2021 edition, tools likerust-analyzerflagsgenas a syntax error. Suggestions for a better rename welcome!Roleis no longer re-exported fromexecution— consumers now import fromalgebra::roledirectly.sharingis no longer re-exported fromexecution— consumers import fromalgebra::sharing.632a6e7f) after seeing the compiler spend a lot of time and RAM generating full debug info. The downside is that inspecting lvars during debugging is not possible with the partial debug info; backtraces are however intact.Open questions for reviewers
Should
Role(andRoleTrait,RoleKind,TwoSetsRole,DualRole) live in its own crate?I moved it from
threshold-fheintothreshold-algebra, but several types inexecution::runtime(sessions, party) still depend on it heavily. Some role-related pieces are still left inexecution, which is a bit of a code smell. A standalonethreshold-rolecrate might make the dependency graph cleaner. Thoughts?Same question for
sharing::{shamir, share}.ShamirSharings,Share,InputOp,RevealOpare now inthreshold-algebra, but they're really protocol-level types used by execution. Should they stay in algebra or be extracted to a mini-crate? Currently there is code left inexecution/sharing, which is not great.PRSSConversionstrait is now defined inthreshold-algebra/src/lib.rs.It exists so that galois ring types can implement it, but it's a protocol thing more than an algebra concern. Where should it live? It's re-exported in
execution::small_execution::prf. This is a tricky one, ideas welcome.Outstanding TODOs
threshold-algebra/Cargo.toml:14lazy_staticdep (kms-internal#2907)threshold-algebra/Cargo.toml:30non-wasmmight be unnecessary.threshold-algebra/src/role.rs:107derive_morebefore — still needed?threshold-algebra/src/structure_traits.rs:140,159embed_role_to_exceptional_sequenceandDerivetrait could take 1-based indexes instead ofRole— simplify? Can we get rid ofRoleand the other types fromalgebra?threshold-algebra/src/sharing/shamir.rs:629threshold-fhe.threshold-algebra/src/lib.rs:12-16threshold-hashing/src/lib.rs:7DomainSep/DSEP_LEN/DIGEST_BYTESconstants — own "types and consts" crate?