Skip to content

Include an implementation of Goldilocks field and tests for FPMul#76

Closed
hdvanegasm wants to merge 47 commits intodevfrom
small-field
Closed

Include an implementation of Goldilocks field and tests for FPMul#76
hdvanegasm wants to merge 47 commits intodevfrom
small-field

Conversation

@hdvanegasm
Copy link
Contributor

In this PR, I added:

  • Implementation for Goldilocks field.
  • Tests with Goldilocks field for random bit generation.
  • Unit tests for fixed-point multiplication.

cadaniluk and others added 30 commits December 29, 2025 16:48
Limits on the number of times a subprotocol can be executed
as well as on other protocol-specific resources such as storage are set.

Related to that, session IDs are checked to minimize the target surface for
malicious messages.
This means
	- protocol-specific checks such as sub ID == 0
	- check that instance ID matches the one of the current instance

To test session IDs, a preprocessing test with a lot of shares and triples
has been added.
The network test only worked in release mode, probably because
functions were blocking in threads (such as accept/recv) and these
threads were cancelled, leading to cancelling while locks were still
held. This is fixed by simply accepting/receiving once instead of
infinitely.

Additionally, some memory leaks are removed and appropriate freeing
functions are added.
- implemented Pedersen commitments with basic tests.
- Avss and Random share generation with avss
- Rand() function added
Add automated CI pipeline with:
- Build job (release mode)
- Test job (all integration tests)
- Lint job (rustfmt + clippy)

Triggers on push to main/dev/feature branches and PRs.

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Replace incorrect action name 'dtolnay/rust-action@stable' with the correct
'dtolnay/rust-toolchain@stable' at lines 21, 45, and 68. This fixes the CI
failure on PR #69.

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Applied cargo fmt and cargo clippy fixes across the codebase to ensure
clean CI builds. Updated CI workflow to use targeted test command that
runs only the stoffelmpc-mpc library tests.

Changes:
- CI: Updated test command from `cargo test` to `cargo test -p stoffelmpc-mpc --lib`
- Formatting: Applied cargo fmt to 33 files across mpc/ and network/ crates
- Linting: Fixed clippy warnings including unused imports, style issues
- Added crate-level lint allows in mpc/src/lib.rs for acceptable style patterns

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Ensures consistent Rust version between local development and CI to prevent
environment-specific lint failures. Fixes unnecessary_unwrap clippy warning
in bad_fake_network.rs by using if-let pattern.

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Use iterator pattern instead of index-based loop for cleaner code.

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Implement missing `rand` trait method in HoneyBadgerMPCNode
- Update SubProtocolCounters to use Mutex<Option<u8>> type
- Add async .await to get_next() counter calls
- Fix unused imports in pedersen.rs test module with #[cfg(test)]
- Add LimitError variant to RandBitError enum
- Update get_or_create_storage to return Result for session limits
- Fix Box::from_raw to use drop() for proper memory freeing
- Prefix unused sessionid variable with underscore
- Fix test to properly check session storage limit

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Replace fixed 500ms sleep with a polling loop that waits for specific
conditions to be met. This fixes intermittent CI failures caused by
the sleep not being long enough on slower machines.

The polling loop checks every 50ms with a 10 second timeout, ensuring
the test is both fast on fast machines and reliable on slow ones.

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
…s-ci

ci: Add GitHub Actions CI workflow (STO-386)
- Added a new mpc protocol for ADKG

- Added triple generation, removed unnecessary traits, redefined feldman shares for avss mpc

- Added multiplication for avss, tests,removed unused imports
- Added session ID trait, protocol specific message wrappers and corressponding changes
* Fixed-point and integer operations
* division by constants,
* preprocessing-storage traits
* testing
# Conflicts:
#	Cargo.toml
#	mpc/Cargo.toml
#	mpc/src/common/mod.rs
#	mpc/src/ffi/c_bindings/honey_badger_mpc_client/mod.rs
#	mpc/src/honeybadger/fpdiv/fpdiv_const.rs
#	mpc/src/honeybadger/fpmul/fpmul.rs
#	mpc/src/honeybadger/mod.rs
#	mpc/src/honeybadger/preprocessing.rs
#	mpc/tests/node_test.rs
This commit combines three PRs previously reviewed separately:
  1. avoid premature clearing of messages
  2. some testing improvements
  3. some input improvements

In more detail:

1.
- See STO-177.

2.
- better Debug impl for SessionId
- avoid premature dropping of receivers
	- the same problem as STO-38.
- avoid premature dropping of receiver
	- See STO-176.
- make program exit on panic in any thread
- add preprocessing_e2e test with BadFakeNetwork

3.
- allow InputServer::init to be called before InputServer::input_handler
- remove storage for random shares
- only send masked inputs when reconstruction of random values has succeeded
  for the first time to avoid sending them multiple times
- do not store more messages than 2t+1 to prevent a malicious node
  from spoofing sender IDs to act as if more nodes were sending random shares
- add InputServer::wait_for_all_inputs, which either waits a given duration
  and then fails, or returns all inputs if they have arrived in this time;
  the previous solution used less encapsulation and always waited a fixed
  amount of time with no possibility to continue earlier
- use one lock per InputClient instead of multiple
- add some documentation
hdvanegasm and others added 7 commits February 16, 2026 16:02
# Conflicts:
#	mpc/src/honeybadger/batch_recon/batch_recon.rs
#	mpc/src/honeybadger/fpmul/mod.rs
#	mpc/src/honeybadger/fpmul/rand_bit.rs
#	mpc/src/honeybadger/mod.rs
#	mpc/src/honeybadger/mul/multiplication.rs
During the addition of these tests, I found an error in truncation and fixed-point multiplication. Run the tests for more insights.
@hdvanegasm hdvanegasm requested review from GarryFCR and removed request for GarryFCR February 17, 2026 02:27
@hdvanegasm
Copy link
Contributor Author

There are some fixes still pending. Those fixes are related to the use of AvssSessionId inside HoneyBadgerMPCNode. This PR is still work in progress.

The fix is not completed yet. I think that the routing for AVSS protocols is not yet ready.
The fix is not completed yet. I think that the routing for AVSS protocols is not yet ready.
The fix is not completed yet. I think that the routing for AVSS protocols is not yet ready.
@hdvanegasm
Copy link
Contributor Author

All errors are now solved and the PR is ready to be reviewed.

SendError,
}

impl<F, G> SecretKey<F, Shamirshare<F>, G> for FeldmanShamirShare<F, G>
Copy link
Collaborator

Choose a reason for hiding this comment

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

this has been deleted, its not required

@@ -1,8 +1,10 @@
use crate::avss_mpc::AvssSessionId;
Copy link
Collaborator

Choose a reason for hiding this comment

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

dont use this session id type here, keep it generic. The reason this exists in common is so that other mpc protocols can use it and not only avss_mpc.

) -> Result<Vec<Self::Sint>, Self::Error>;
}

pub trait SecretKey<F, S, G>
Copy link
Collaborator

Choose a reason for hiding this comment

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

The following is not needed anymore

pub output_channel: Sender<SessionId>,
}

#[derive(Debug)]
Copy link
Collaborator

Choose a reason for hiding this comment

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

we wont need the store or state here, check out the channel-fixes branch its quite simplified.

pub share_r_2: Option<Vec<GF256>>,
pub share_b_2: Vec<GF256>, //PrandBitD output
pub share_b_p: Vec<RobustShare<G>>, //PrandBitD/PrandBitL output
pub protocol_state: ProtocolState,
Copy link
Collaborator

Choose a reason for hiding this comment

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

this is also added in channel fixes

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is about the ProtocolState.

@hdvanegasm
Copy link
Contributor Author

I will close this pull request as I think it is better to branch from dev and do the changes manually, and then merge back into dev. This will save me from resolving conflicts about other things that has nothing to do with my contribution with potential critical mistakes.

@hdvanegasm hdvanegasm closed this Mar 9, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants