Skip to content

feat(mbe): use custom signing fns for mpcv2 signing/sendmany#61

Merged
pranavjain97 merged 10 commits intomasterfrom
WP-5232-mpcv2-signing-custom-fns
Jul 10, 2025
Merged

feat(mbe): use custom signing fns for mpcv2 signing/sendmany#61
pranavjain97 merged 10 commits intomasterfrom
WP-5232-mpcv2-signing-custom-fns

Conversation

@pranavjain97
Copy link
Copy Markdown
Contributor

@pranavjain97 pranavjain97 commented Jul 8, 2025

Ticket: WP-5232

@pranavjain97 pranavjain97 force-pushed the WP-5232-mpcv2-signing-custom-fns branch 5 times, most recently from d7a913a to 4ab0f79 Compare July 8, 2025 21:33
@pranavjain97 pranavjain97 requested a review from Copilot July 8, 2025 21:33

This comment was marked as outdated.

@pranavjain97 pranavjain97 force-pushed the WP-5232-mpcv2-signing-custom-fns branch 3 times, most recently from fd13360 to cf25a92 Compare July 8, 2025 22:55
@pranavjain97 pranavjain97 force-pushed the WP-5232-mpcv2-signing-custom-fns branch from cf25a92 to f4288f9 Compare July 9, 2025 20:42
@pranavjain97 pranavjain97 changed the title feat(mbe): use custom signing fns for mpcv2 signing feat(mbe): use custom signing fns for mpcv2 signing/sendmany Jul 9, 2025
Comment thread src/api/master/clients/enclavedExpressClient.ts
Comment on lines +62 to +65
} else {
// For non-ECDSA algorithms, return the original parameters
return req.decoded as SendManyOptions;
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The function name createEcdsaMPCv2SendParams doesn't match the comment. makes it seem the function is specifc for mpcv2 but you handle other wallet types as well.

Copy link
Copy Markdown
Contributor Author

@pranavjain97 pranavjain97 Jul 9, 2025

Choose a reason for hiding this comment

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

not sure what you mean; it only returns custom fns for mpcv2 + the given send params

Comment on lines +103 to +107
// if (params.commonKeychain && signingKeychain.commonKeychain !== params.commonKeychain) {
// throw new Error(
// `Common keychain provided does not match the keychain on wallet for ${params.source}`,
// );
// }
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

remove comments.

Comment on lines +112 to +115
if (wallet.multisigType() === 'tss') {
if (signingKeychain.source === 'backup') {
throw new Error('Backup MPC signing not supported for sendMany');
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

it's not supported for utxos either from what I recall.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

sure - but not part of mpc branch

Copy link
Copy Markdown
Contributor

@mohammadalfaiyazbitgo mohammadalfaiyazbitgo left a comment

Choose a reason for hiding this comment

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

Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR replaces legacy MPC signing flows with custom ECDSA MPCv2 signing functions, renames GPG key parameters for consistency, and consolidates EDDSA and ECDSA handlers to work directly on TxRequest objects.

  • Rename bitgoGpgPubKey to bitgoPublicGpgKey in API spec and handlers.
  • Introduce createEcdsaMPCv2CustomSigners and wire it into sendMany and transaction endpoints.
  • Refactor EDDSA and ECDSA MPCv2 signing code to accept full TxRequest objects and remove outdated methods.

Reviewed Changes

Copilot reviewed 12 out of 12 changed files in this pull request and generated no comments.

Show a summary per file
File Description
src/enclavedBitgoExpress/routers/enclavedApiSpec.ts Rename GPG key field and restrict shareType to known literals
src/api/master/handlers/transactionRequests.ts Add unified signAndSendTxRequests for EDDSA and ECDSA MPC flows
src/api/master/handlers/handleSendMany.ts Implement ECDSA MPCv2 custom signers and remove old code paths
src/api/master/handlers/generateWallet.ts Update ECDSA key generation import to new ecdsaMPCv2 module
src/api/master/handlers/eddsa.ts Refactor EDDSA signing to accept TxRequest and rename key param
src/api/master/handlers/ecdsaMPCv2.ts Create custom ECDSA MPCv2 signer generators and refactor sender
src/api/master/clients/enclavedExpressClient.ts Update MPCv2 methods to include source/pub and rename param
src/api/enclaved/handlers/signMpcTransaction.ts Update unified handler to use renamed fields and typed shareType
src/tests/api/master/sendMany.test.ts Adjust tests for new sendMany flow and rename parameter stubs
src/tests/api/master/eddsa.test.ts Skip EDDSA tests until new custom flow is in place
src/tests/api/master/ecdsa.test.ts Switch to new signAndSendEcdsaMPCv2FromTxRequest entry point
src/tests/api/enclaved/signMpcTransaction.test.ts Update tests for renamed GPG key field and share-type handling
Comments suppressed due to low confidence (3)

src/api/master/handlers/handleSendMany.ts:8

  • Imported PrebuildTransactionResult but the code uses PrebuildTransactionOptions. Replace or add the correct import for PrebuildTransactionOptions from @bitgo/sdk-core to ensure the type is available.
  PrebuildTransactionResult,

src/api/master/handlers/handleSendMany.ts:5

  • [nitpick] The KeyIndices import is not used in this file and can be removed to improve readability and avoid dead code.
  KeyIndices,

src/api/master/handlers/ecdsaMPCv2.ts:7

  • [nitpick] The SupplementGenerateWalletOptions import is not referenced elsewhere in this module and should be removed to reduce unused dependencies.
  SupplementGenerateWalletOptions,

@pranavjain97 pranavjain97 merged commit 15c27bd into master Jul 10, 2025
3 checks passed
@pranavjain97 pranavjain97 deleted the WP-5232-mpcv2-signing-custom-fns branch July 10, 2025 16:23
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.

3 participants