Conversation
Add a new setup module providing a framework for managing multi-party protocol setup steps. Includes SetupStep trait for step lifecycle management, StepPhase enum for state tracking, ExchangeConfig for exchange configuration, and template implementations for keys, nonces, and signatures steps
Add a new setup orchestration system with SetupEngine that provides a state machine-based approach to managing multi-party protocol setup steps. This enables cleaner separation of concerns and allows protocols to opt-in gradually to the new system.
This is the next generation Program implementation that: - Uses SetupEngine for orchestrating setup steps - Delegates aggregation responsibility to protocols - Provides cleaner separation of concerns - Allows protocols to opt-in gradually via UsesSetupSteps trait
…-leaders -> leader -> all non-leaders)
… leader broadcast
…ndler Remove the UsesSetupSteps trait and integrate setup steps functionality directly into the ProtocolHandler trait. This simplifies the architecture by eliminating an extra trait layer
- Removed use_broadcasting from ExchangeConfig struct and Default impl - Updated exchange_config() in KeysStep, NoncesStep, and SignaturesStep
Rename the collaboration protocol to aggregated key protocol to better reflect its purpose of generating aggregated MuSig2 keys.
Rename the template_steps directory to steps for better clarity and consistency.
Add default implementations for optional protocol methods in the ProtocolHandler trait to reduce boilerplate code in simple protocols like AggregatedKeyProtocol.
Replace manual impl Default implementations with #[derive(Default)] for KeysStep, NoncesStep, and SignaturesStep. These are unit structs where the manual implementation was redundant, as it only called new().
…on.rs structure - Remove unused UTXO funding section that was not being used in the test - Remove unused wallet funding initialization - Replace manual bitcoin/bitcoind setup with prepare_bitcoin() helper - Add bitcoind cleanup at test end (matching integration.rs pattern) - Clean up unused imports (Bitcoind, BitcoinClient, Wallet, etc.) - Remove unused MIN_TX_FEE constant
…_steps() - Replace Option<Vec<Box<dyn SetupStep>>> with Option<Vec<SetupStepName>> in ProtocolHandler::setup_steps() - Add SetupStepName enum (Keys, Nonces, Signatures) for type-safe step names - Implement factory function create_setup_step() to create steps from enum - Update SetupEngine::new() to accept Vec<SetupStepName> instead of Vec<Box<dyn SetupStep>> - Remove trait object allocations at protocol definition time - Update all protocol implementations (AggregatedKeyProtocol, default implementation) - Update tests to use enum variants instead of string literals
Move all setup-related logic from ProgramV2 to SetupEngine to improve separation of concerns. ProgramV2 should only care about whether setup is complete or not, not the internal details of setup steps.
…control - Remove duplicate save() call in BitVMX after receive_setup_data() to prevent state overwrites that caused tick() to re-enter build() and send duplicate messages. ProgramV2 now handles saving internally. - Write state to legacy key in ProgramV2::save() so is_active_program() can correctly identify Ready programs and skip them in process_programs(). - Add send_setup_completed() trait method to ProtocolHandler to allow protocols to suppress SetupCompleted messages. AggregatedKeyProtocol returns false to maintain backward compatibility with SetupKey callers that only expect AggregatedPubkey responses. - Clean up aggregated_key test by removing unused helper struct and setup completion assertions that are no longer relevant.
- Remove special-case handling for single-participant protocols in ProgramV2::setup() This allows SetupEngine to handle all cases uniformly through the normal setup flow - Add single-participant handling in AggregatedKeyProtocol::build_protocol() When only one participant exists, use their own key directly instead of attempting MuSig2 aggregation (which requires at least 2 participants) - Add test_aggregated_key_single_participant() test Verifies that single-participant protocols work correctly through SetupEngine without any special-case bypass logic
- Remove deprecated `collaborate` module and all collaboration processing/storage paths - Read aggregated pubkey/keypair derivation from globals (`final_aggregated_key`) - Update aggregated-key single participant test to use `SetupKey` and assert globals match API response
- Remove confirmation_threshold parameter from SubscribeToTransaction and SubscribeToRskPegin API messages - Simplify tick() return type from Result<bool> to Result<()> - Remove unused GracefulShutdown trait implementation - Update imports: use PubKeyHash from bitvmx_broker instead of bitvmx_operator_comms - Add None parameters to TypesToMonitor calls for consistency - Improve test infrastructure: - Add existence checks before removing directories - Add Docker container cleanup logic to prevent conflicts - Update shutdown policy tests to match new tick() signature - Code formatting and cleanup (remove unused imports, improve formatting)
Resolved conflict in tests/common/mod.rs by combining imports: - Added Path from std::path (needed for check_bitvmx_cpu_built) - Kept Command from std::process (needed for Docker cleanup) - Added warn to tracing imports (used in helper functions)
99aa681 to
1e27abc
Compare
jonasmartin
left a comment
There was a problem hiding this comment.
comments on aggregated_key
| let aggregated_key = if aggregated_pub_keys.len() == 1 { | ||
| tracing::info!("AggregatedKeyProtocol: Single participant, using own key directly"); | ||
| *my_key | ||
| } else { |
There was a problem hiding this comment.
this case is not valid. just resturn error if there is not enough participants
There was a problem hiding this comment.
Fixed: Removed the invalid single-participant case and added validation to require at least 2 participants. The protocol now returns an error if there are not enough participants for MuSig2 aggregation.
src/program/setup/exchange_config.rs
Outdated
There was a problem hiding this comment.
Not sure if exchange_config is needed, but if it's, remove the custom implementations from keys_steps, nonces_step and signatures_step so the base impl in the steupStep trait is used
There was a problem hiding this comment.
At some point on this step something similar to this needs to be done:
https://github.com/FairgateLabs/rust-bitvmx-client/blob/main/src/program/program.rs#L520-L525
Replace manual match-based trait delegation with enum_dispatch macro, consistent with the existing ProtocolHandler/ProtocolType pattern.
src/bitvmx.rs
Outdated
| participants_keys: Option<Vec<PublicKey>>, | ||
| _participants_keys: Option<Vec<PublicKey>>, |
There was a problem hiding this comment.
There was a usecase used by union where the public keys where provided externally instead of generated, we need to support that case
…ust-bitvmx-client into feature/protocol-workflow
Protocol Workflow Feature
Overview
This PR introduces a major refactoring of the protocol setup and execution workflow in BitVMX, introducing a new
SetupEngineorchestration system andProgramV2implementation that provides better separation of concerns, improved testability, and more robust protocol initialization.Key Features
1. SetupEngine - Protocol Setup Orchestration
Introduces a new
SetupEnginethat orchestrates multi-step protocol setup processes:ProtocolHandler::setup_steps()2. ProgramV2 - Next Generation Program Implementation
New
ProgramV2implementation that:SetupEnginefor orchestrating setup stepsKey differences from Program (Legacy):
prepare_aggregated_keys()- protocols handle their own aggregationSetupEngineto orchestrate setup steps3. AggregatedKeyProtocol
Replaces the old
Collaborationflow with a newAggregatedKeyProtocol:globalsunderfinal_aggregated_keyBreaking Changes
Removed Features
Collaborationclass and related methods have been removedprocess_collaboration_message()process_collaboration()get_collaboration()save_collaboration()mark_collaboration_as_complete()src/collaborate.rs: File removedMigration Path
Programimplementation continue to worksetup_v2()API to leverage ProgramV2AggregatedKeyProtocolinstead ofCollaborationTechnical Details
SetupEngine Architecture
ProgramV2 State Machine
Leader Broadcast Flow
Testing
New Tests
test_aggregated_key: Multi-participant aggregated key generation testtest_aggregated_key_single_participant: Single-participant scenario testTest Improvements
Files Changed
New Files
src/program/setup/setup_engine.rs- SetupEngine implementationsrc/program/setup/setup_step.rs- Setup step definitionssrc/program/program_v2.rs- ProgramV2 implementationsrc/program/protocols/aggregated_key.rs- AggregatedKeyProtocoltests/aggregated_key.rs- Aggregated key testsModified Files
src/bitvmx.rs- Updated to support ProgramV2, removed Collaboration flowsrc/types.rs- Updated message types, added ProgramVersion enumsrc/api.rs- Added setup_v2() methodsrc/client.rs- Updated API calls to match new signaturessrc/main.rs- Updated tick() return type handlingtests/common/mod.rs- Improved Docker container cleanupRemoved Files
src/collaborate.rs- Replaced by AggregatedKeyProtocolPerformance Considerations
Backward Compatibility
Programimplementation remains availablesetup_v2()APIFuture Work
Checklist
Related Issues
This PR addresses the need for: