Conversation
- Introduced comprehensive integration tests for ADKG (Asynchronous Distributed Key Generation), including unit tests for secret key objects, simulated network setups, built-in validations, and end-to-end workflows across multiple parties. - Refactored `connect_as_server` and `connect_as_client` methods in networking APIs to simplify argument handling. - Added `adkg_engine` module to the networking layer for managing ADKG-related operations.
…open_share_in_exp` functionality - Modularize HoneyBadger-specific logic using `#[cfg(feature = "honeybadger")]` for enhanced feature control. - Introduce `open_share_in_exp` implementation for Lagrange-based sharing in the exponents. - Update random share handling with synchronized preprocessing pools. - Add `--mpc-backend` CLI flag and backend validation to support multiple MPC backends.
…imitives - Implemented `AdkgQuicServer` with QUIC networking, ECDH key exchange, AVSS message routing, and configuration support. - Introduced `MpcBackendKind` enum for runtime selection of MPC backends with feature-gated options for HoneyBadger and ADKG. - Added tests for DKG primitives `random_share` and `open_share_in_exp` to validate distributed key generation and public key reconstruction.
|
This repository is currently set to inactive in Gecko Security. To enable automated security scanning for pull requests, please activate it in your Gecko Security dashboard. |
…DKG curve configuration, and improve share handling - Introduced generic session ID types in ADKG and HoneyBadger engines for better flexibility and encapsulation. - Enhanced ADKG with support for multiple elliptic curve configurations (BLS12-381, BN254) using a new `AdkgCurveConfig` enum. - Replaced fixed session and share types with generic constraints to support a wider range of key generation protocols. - Improved message handling by modifying receive loops to include sender IDs and extending testing for ADKG operations over different curve configurations. - Refactored serialization and deserialization routines for RobustShare and secret keys for better portability and compatibility.
|
This repository is currently set to inactive in Gecko Security. To enable automated security scanning for pull requests, please activate it in your Gecko Security dashboard. |
…ctionality - Renamed `sender_id` to `local_party_id` for clarity. - Removed unused `assign_sender_ids` and `is_fully_connected` methods. - Simplified Cargo dependency for `stoffelnet` by removing the specific commit reference.
|
This repository is currently set to inactive in Gecko Security. To enable automated security scanning for pull requests, please activate it in your Gecko Security dashboard. |
crates/stoffel-vm/src/net/backend.rs
Outdated
| } | ||
| } | ||
|
|
||
| /// Whether this backend supports distributed key generation. |
There was a problem hiding this comment.
It would be more accurate to say whether this backend supports/is safe for elliptic curves
crates/stoffel-vm/src/net/backend.rs
Outdated
| #[cfg(feature = "honeybadger")] | ||
| MpcBackendKind::HoneyBadger => true, | ||
| #[cfg(feature = "adkg")] | ||
| MpcBackendKind::Adkg => false, |
There was a problem hiding this comment.
We need client inputs for this too! It's just that the client and the server are the same
| @@ -0,0 +1,183 @@ | |||
| //! MPC backend selection. | |||
There was a problem hiding this comment.
Nitpick about the naming. As mentioned in mpc-protocols, what was built for the implementation of ADKG is generalizable and not just for ADKG. So, we should map the naming accordingly.
| /// Using `AdkgMpcEngine<F, G>` directly with an untested pair may produce | ||
| /// incorrect results. Always prefer constructing engines via `AdkgCurveConfig`. | ||
| #[derive(Debug, Clone, Copy, PartialEq, Eq)] | ||
| pub enum AdkgCurveConfig { |
There was a problem hiding this comment.
Gotta add Curve25519 and Ristretto
| /// | ||
| /// Generic over field `F` and curve group `G` where `G: CurveGroup<ScalarField = F>`. | ||
| #[derive(Clone, Debug)] | ||
| pub struct AdkgSecretKey<F: FftField, G: CurveGroup<ScalarField = F>> { |
There was a problem hiding this comment.
We don't need this here because it will be done in the language.
| /// | ||
| /// This party initiates the AVSS protocol with a randomly generated secret. | ||
| /// All parties will receive Feldman-verifiable shares. | ||
| pub async fn generate_key(&self) -> Result<AdkgSecretKey<F, G>, String> { |
There was a problem hiding this comment.
This is essentially rand() that we've been talking about
| } | ||
| } | ||
|
|
||
| /// Get a secret key by session ID |
There was a problem hiding this comment.
Hmm. It makes sense to want to retrieve generated random material that have been previously stored. Not sure if this should be retrieved via session ID though as they are not meant to be long lived.
There was a problem hiding this comment.
Potential solution is to simply call a method to retrieve a saved random share. Since this is stored in a key-value store, the key will need to be the same across all nodes but the value would be different for all nodes.
| } | ||
|
|
||
| /// Encode a RobustShare to bytes using CanonicalSerialize | ||
| fn encode_robustshare(share: &RobustShare<F>) -> Result<Vec<u8>, String> { |
There was a problem hiding this comment.
Don't need robust share here because HBMPC already has it. Also, the point of this specific engine is mainly because we care about doing elliptic curve operations which we can't easily do with robust shares.
| (ShareType::SecretInt { .. }, Value::I64(v)) => { | ||
| let secret = F::from(*v as u64); | ||
| let mut rng = ark_std::test_rng(); | ||
| let shares = RobustShare::compute_shares(secret, self.n, self.t, None, &mut rng) |
There was a problem hiding this comment.
So this would be AVSS share (redundant for the sake of clarity) instead of robust share.
| _left: &[u8], | ||
| _right: &[u8], | ||
| ) -> Result<Vec<u8>, String> { | ||
| Err("AVSS protocol does not support secure multiplication (requires Beaver triples)".into()) |
There was a problem hiding this comment.
is this not implemented in mpc-protocols?
…e consensus/client operation handling - Introduced `supports_*` methods and sub-trait accessors (`as_client_ops`, `as_consensus`) for modular trait handling in MPC engines. - Replaced engine-specific downcasting with generic sub-trait dispatch for consensus and client operations. - Added feature-independent registration for RBC and ABA builtins. - Implemented `FromStr` for `MpcBackendKind` and introduced cancellation handling in ADKG server. - Enhanced ADKG server with a shutdown mechanism using `CancellationToken`. - Simplified error messages and updated documentation for client input hydration and protocol-specific support checks. - Updated Cargo dependencies to include `tokio-util`.
…tions - Replaced references to ADKG with AVSS across codebase, including backend selection and integration tests. - Updated builtins and test modules to align with AVSS protocols and elliptic curve capabilities. - Improved error handling for unsupported curve configurations and added backward-compatible aliases for ADKG identifiers. - Refactored object creation and verification methods for AVSS shares, ensuring proper key name handling and commitment consistency. - Adjusted VM programs and end-to-end tests to demonstrate AVSS functionality.
- Replaced MPC engine dependency with direct secure RNG-based implementations. - Simplified `Mpc.rand` to generate 32 bytes via `rand::thread_rng`. - Updated `Mpc.rand_int` to accept bit lengths (8, 16, 32, 64) and return corresponding unsigned integers. - Removed legacy secret-sharing logic and cleaned up error handling for unsupported configurations.
- Replaced `rand::thread_rng` with `rand::rng` for improved consistency. - Updated random generation methods to use `random()` instead of `gen()`.
| } else if let Some(_rest) = arg.strip_prefix("--stun-servers") { | ||
| } else if let Some(_rest) = arg.strip_prefix("--servers") { | ||
| } else if let Some(_rest) = arg.strip_prefix("--mpc-backend") { | ||
| } else if let Some(_rest) = arg.strip_prefix("--adkg-curve") { |
There was a problem hiding this comment.
This isn't just specific to AVSS. You'll need to set the field/group for HBMPC too
| /// authentication of the sender's identity. Falls back to `usize::MAX` if | ||
| /// deserialization fails (the subsequent `process` call will also fail). | ||
| #[cfg(feature = "honeybadger")] | ||
| fn extract_sender_id(data: &[u8]) -> usize { |
There was a problem hiding this comment.
Isn't this also the case for AVSS?
Is there not a nicer way of doing this across both?
There was a problem hiding this comment.
Since stoffelnet uses ALPN now, is this still necessary?
| @@ -83,6 +125,8 @@ async fn main() { | |||
| } else if let Some(_rest) = arg.strip_prefix("--expected-clients") { | |||
There was a problem hiding this comment.
Is this still necessary with ALPN?
| /// | ||
| /// Generic over `(F, G)` where `F` is the scalar field and `G` is the curve group. | ||
| /// Use `Bls12381AdkgServer` or `Bn254AdkgServer` type aliases. | ||
| pub struct AdkgQuicServer<F, G> |
There was a problem hiding this comment.
Why isn't this server structure more or less the same as the HBMPC one? Also, what's the rational here for ECDH?
…proved curve handling - Renamed `STOFFEL_EXPECTED_CLIENTS` to `STOFFEL_EXPECTED_CLIENT_COUNT` across the configuration. - Removed deprecated `STOFFEL_CLIENT_ID` and added validation in the entrypoint script. - Refactored `HoneyBadgerMpcEngine` to support extensible field and curve configurations with type parameters. - Enhanced client share handling with serialization and deserialization for compatibility. - Updated receive loops to separate server-to-server and client-to-server connections. - Improved AVSS ECDH key handling and extended curve support for BN254, Ed25519, and Curve25519 in MPC engine logic.
… field configuration - Introduced `MpcCurveConfig` enum with parsing, validation, and backend compatibility checks for supported curves (BLS12-381, BN254, Curve25519, Ed25519). - Added corresponding `MpcFieldKind` enum for scalar field identification. - Implemented `SupportedMpcField` trait to associate scalar fields with their curve configurations. - Included tests for curve name parsing, validation, and backend support.
- Updated feature flags in integration test module definitions: `honeybadger` -> `hb_itest`.
| use std::str::FromStr; | ||
|
|
||
| use ark_bls12_381::Fr; | ||
| #[cfg(feature = "honeybadger")] |
There was a problem hiding this comment.
Why is this conditional? AVSS needs it too
| use ark_bls12_381::Fr; | ||
| #[cfg(feature = "honeybadger")] | ||
| use ark_ec::{CurveGroup, PrimeGroup}; | ||
| #[cfg(feature = "honeybadger")] |
| use std::fs::File; | ||
| use std::sync::Arc; | ||
| use std::time::Duration; | ||
| #[cfg(feature = "honeybadger")] |
| use std::time::Duration; | ||
| #[cfg(feature = "honeybadger")] | ||
| use std::collections::HashSet; | ||
| #[cfg(feature = "honeybadger")] |
| use serde::{Deserialize, Serialize}; | ||
| use stoffel_vm::core_vm::VirtualMachine; | ||
| use stoffel_vm::net::{MpcBackendKind, MpcCurveConfig}; | ||
| #[cfg(feature = "honeybadger")] |
There was a problem hiding this comment.
This can stay because it's directly related to the HBMPC engine
| use stoffel_vm::net::{honeybadger_node_opts, spawn_receive_loops}; | ||
| use stoffel_vm::runtime_hooks::{HookContext, HookEvent}; | ||
| use stoffel_vm_types::compiled_binary::CompiledBinary; | ||
| #[cfg(feature = "honeybadger")] |
| use stoffelnet::transports::quic::{NetworkManager, QuicNetworkConfig, QuicNetworkManager}; | ||
| use stoffelnet::transports::quic::{NetworkManager, QuicNetworkManager}; | ||
| use tokio::sync::mpsc; | ||
| #[cfg(feature = "adkg")] |
There was a problem hiding this comment.
We gotta rename this. We'll do it once the most important stuff is fixed first.
- Introduced comprehensive integration tests for AVSS distributed key generation and public key extraction. - Implemented full flow covering node setup, ECDH key exchange, AVSS share generation, and commitment validation. - Verified consistency of Feldman commitments and VM-based public key extraction across parties. - Added support for multiple independent DKG sessions and public key validation.
| } | ||
| } | ||
|
|
||
| #[cfg(feature = "honeybadger")] |
There was a problem hiding this comment.
From line 65 to 228, it's not clear why these are only for honeybadger. The functionality looks relevant to AVSS too
| } | ||
|
|
||
| // Client mode: connect to MPC servers and provide inputs | ||
| // Client mode: connect to MPC servers and provide inputs (HoneyBadger only) |
There was a problem hiding this comment.
Hmm, this is not unique to HBMPC. I guess the confusing part is that our target use case for AVSS is such that the client and MPC node are one instead of separate. But that doesn't discount use cases where they can be separate.
| // Client mode: connect to MPC servers and provide inputs | ||
| // Client mode: connect to MPC servers and provide inputs (HoneyBadger only) | ||
| #[cfg(feature = "honeybadger")] | ||
| if as_client { |
There was a problem hiding this comment.
This block is thicc. Is there anyway to refactor it?
| ); | ||
|
|
||
| // Macro to avoid duplicating ADKG setup for each curve | ||
| macro_rules! setup_adkg { |
There was a problem hiding this comment.
Why not apply this to HBMPC too?
| #[cfg(feature = "honeybadger")] | ||
| use stoffelmpc_mpc::honeybadger::{HoneyBadgerMPCClient, HoneyBadgerMPCNode}; | ||
| #[cfg(feature = "honeybadger")] | ||
| use stoffelnet::network_utils::ClientId; |
There was a problem hiding this comment.
Why does this have the honeybadger conditional attribute?
- Replaced "adkg" with "avss" across features, modules, and backend selection. - Introduced stricter guards for invalid FFI calls, oversized buffers, and unreasonable argument counts. - Refactored AVSS async operations with unified `block_on_avss_new` helper. - Improved AVSS QUIC server key handling with duplicate detection and validation. - Added tests for AVSS public key exchange and validation logic.
- Introduced `init_crypto_provider` and `setup_test_tracing` utilities for test environment initialization. - Added comprehensive error handling in FFI functions for invalid inputs and type conversions. - Refactored buffer management in FFI functions to use shared `write_ffi_result_bytes` and `free_ffi_result_bytes` helpers. - Restructured AVSS engine creation with modular curve initialization and robust failure logging. - Included new tests for invalid curve configurations and null/empty public key maps.
…on test for negative fixed-point handling - Updated `field_from_i64` and `field_to_value` implementations to use `curve::field_from_i64` and `curve::field_to_i64` helpers. - Added a regression test to verify negative fixed-point value round-trip through AVSS encode/share/reconstruct/decode pipeline.
…kg" to "avss" across modules - Introduced `UNKNOWN_SENDER_ID` in `open_registry` to improve readability and maintainability for unknown sender handling. - Replaced all occurrences of `usize::MAX` with the new constant in message validation and connection handling logic. - Renamed "adkg" to "avss" consistently across codebase, including FFI modules, integration tests, and backend configuration. - Enhanced error messages and logging for improved debug clarity, ensuring validation proper handling of unknown and mismatched sender IDs.
…nd tests - Introduced optional `auth_token` field in discovery messages to enable authenticated registration. - Added environment variable `STOFFEL_AUTH_TOKEN` for configuring shared secrets in bootnode and parties. - Implemented validation for `auth_token` in both party and session registration workflows. - Updated entrypoint script, documentation, and Docker Compose to enforce `STOFFEL_AUTH_TOKEN` for non-client roles. - Added integration tests to verify authentication prevents party impersonation and session injection attacks. - Updated error handling and logging for invalid or missing authentication tokens.
No description provided.