Tests/mpc dkg #99

mohammadalfaiyazbitgo · 2025-07-30T18:39:19Z

No description provided.

pranavjain97 · 2025-08-05T23:26:29Z

src/api/enclaved/mpcFinalize.ts

+  debugLogger('MPC Finalize request received');
+  debugLogger('Request inputs:', {
+    source: req.decoded.source,
+    coin: req.decoded.coin,
+    encryptedData: req.decoded.encryptedData,
+    encryptedDataKey: req.decoded.encryptedDataKey,
+    bitgoKeyChain: req.decoded.bitgoKeyChain,
+    counterPartyGpgPub: req.decoded.counterPartyGpgPub,
+    counterPartyKeyShare: req.decoded.counterPartyKeyShare,
+    bitgoKeyChainKeyShares: req.decoded.bitgoKeyChain.keyShares.map((keyShare: any) =>
+      JSON.stringify(keyShare),
+    ),
+  });
+


pranavjain97 · 2025-08-05T23:27:17Z

src/api/enclaved/mpcFinalize.ts

+    coin: coinInstance,
+    verifySignatures: [
+      source === 'user' ? sourceGpgPub : counterPartyGpgPub,
+      source === 'user' ? counterPartyGpgPub : sourceGpgPub,


did u mean source === 'backup here?

no,

async verifyWalletSignatures( userGpgPub: string, backupGpgPub: string, bitgoKeychain: Keychain, decryptedShare: string, verifierIndex: 1 | 2, useHardcodedBitGoKeys?: { env: EnvironmentName; pubKeyType: 'nitro' | 'onprem'; } ): Promise<void> {

if the source is 'user' then we have to provide the source gpg key for userGpgKey, and vice versa.

pranavjain97 · 2025-08-05T23:27:33Z

src/api/enclaved/mpcFinalize.ts

+      sourceIndex,
+      {
+        env: req.bitgo.env,
+        pubKeyType: 'onprem',


what's this for?

we were fetching constants from bitgo in the sdk method for this check, i added some changes to the SDK to use the hard coded ones if these options were given. BitGo/BitGoJS@520d70d

pranavjain97 · 2025-08-05T23:28:14Z

src/api/enclaved/mpcInitialize.ts

+  debugLogger('MPC Initialize request received');
+  debugLogger('Request inputs:', { source: req.decoded.source });


is this helpful? also, we use logger.debug for all the logs

pranavjain97 · 2025-08-05T23:29:32Z

src/api/enclaved/utils.ts

+): Promise<EDDSA.KeyCombine> {
+  const eddsa = await bitgoSdk.Eddsa.initialize();
+  return eddsa.keyCombine(...params);
+}


why do we need a helper just for this? It might be clearer to do this at the caller

this was because getting a mock to work on this method was next to impossible so i removed it out and mocked it.

Copilot

Pull Request Overview

This PR implements MPC (Multi-Party Computation) Distributed Key Generation (DKG) functionality with comprehensive test coverage. The changes include adding error stack traces to response handling, refactoring MPC finalize operations to use utility functions, and introducing extensive test suites for both MPC initialization and finalization workflows.

Enhances error debugging by including stack traces in error responses
Refactors MPC finalize operations to use centralized utility functions for better maintainability
Adds comprehensive test coverage for MPC initialization and finalization workflows
Updates package dependencies to latest beta versions

Reviewed Changes

Copilot reviewed 10 out of 11 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
src/shared/responseHandler.ts	Adds stack trace information to error responses for better debugging
src/api/advancedWalletManager/utils.ts	Introduces utility functions for EDDSA operations and wallet signature verification
src/api/advancedWalletManager/mpcInitialize.ts	Improves error handling by using specific BadRequestError
src/api/advancedWalletManager/mpcFinalize.ts	Refactors to use utility functions and improves parameter structure
src/advancedWalletManager/routers/advancedWalletManagerApiSpec.ts	Removes redundant error handling now managed by responseHandler
src/tests/api/master/recoveryWalletMpcV2.test.ts	Updates chain ID in test expectations
src/tests/api/advancedWalletManager/mpcInitialize.test.ts	Adds comprehensive test suite for MPC initialization
src/tests/api/advancedWalletManager/mpcFinalize.test.ts	Adds comprehensive test suite for MPC finalization
package.json	Updates BitGo SDK dependencies to latest beta versions
masterBitgoExpress.json	Updates API schema to include stack trace in error responses

Copilot · 2025-08-06T17:12:46Z

src/api/advancedWalletManager/mpcFinalize.ts

+    const combinedKey = await eddsaKeyCombine([
+      sourcePrivateShare,
+      [bitgoToSourceYShare, counterPartyToSourceYShare],
    ]);


The parameters passed to eddsaKeyCombine don't match the function signature. The function expects Parameters<Eddsa['keyCombine']> but is receiving an array with sourcePrivateShare and an array of yShares. This should be sourcePrivateShare as first parameter and the array of yShares as second parameter, not wrapped in an outer array.

Suggested change

const combinedKey = await eddsaKeyCombine([

sourcePrivateShare,

[bitgoToSourceYShare, counterPartyToSourceYShare],

]);

const combinedKey = await eddsaKeyCombine(

sourcePrivateShare,

[bitgoToSourceYShare, counterPartyToSourceYShare],

);

pranavjain97

lgtm; just fix CI and double check the copilot comment

- added tests for mpc finalize - added tests for mpc initalize Ticket: WP-5338

mohammadalfaiyazbitgo force-pushed the tests/mpc-dkg branch from b9545d0 to aa18717 Compare July 30, 2025 18:39

mohammadalfaiyazbitgo requested review from Copilot and pranavjain97 and removed request for pranavjain97 July 30, 2025 18:39

This comment was marked as outdated.

Sign in to view

mohammadalfaiyazbitgo force-pushed the tests/mpc-dkg branch 2 times, most recently from 4999da5 to 5b3e4d5 Compare August 1, 2025 14:13

pranavjain97 requested changes Aug 5, 2025

View reviewed changes

mohammadalfaiyazbitgo force-pushed the tests/mpc-dkg branch from 5b3e4d5 to 4ecfe36 Compare August 6, 2025 16:57

mohammadalfaiyazbitgo requested a review from pranavjain97 August 6, 2025 16:58

mohammadalfaiyazbitgo force-pushed the tests/mpc-dkg branch 2 times, most recently from e2502dd to 5f2b647 Compare August 6, 2025 17:11

mohammadalfaiyazbitgo requested a review from Copilot August 6, 2025 17:12

Copilot AI reviewed Aug 6, 2025

View reviewed changes

mohammadalfaiyazbitgo force-pushed the tests/mpc-dkg branch 3 times, most recently from 086ee67 to 24bb820 Compare August 6, 2025 17:21

pranavjain97 previously approved these changes Aug 6, 2025

View reviewed changes

mohammadalfaiyazbitgo force-pushed the tests/mpc-dkg branch from 24bb820 to 5519d8c Compare August 6, 2025 17:31

fix(awm): force signature verification with hardocded bitgo keys

bd3baa1

- added tests for mpc finalize - added tests for mpc initalize Ticket: WP-5338

mohammadalfaiyazbitgo dismissed pranavjain97’s stale review via bd3baa1 August 6, 2025 17:31

mohammadalfaiyazbitgo force-pushed the tests/mpc-dkg branch from 5519d8c to bd3baa1 Compare August 6, 2025 17:31

pranavjain97 approved these changes Aug 6, 2025

View reviewed changes

pranavjain97 merged commit 36ac2de into master Aug 6, 2025
4 checks passed

pranavjain97 deleted the tests/mpc-dkg branch August 6, 2025 17:37

		debugLogger('MPC Initialize request received');
		debugLogger('Request inputs:', { source: req.decoded.source });

Tests/mpc dkg #99

Tests/mpc dkg #99

Uh oh!

Conversation

mohammadalfaiyazbitgo commented Jul 30, 2025

Uh oh!

This comment was marked as outdated.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Copilot AI Aug 6, 2025

Choose a reason for hiding this comment

Uh oh!

pranavjain97 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants