feat(mbe): api to sign and send a txrequest #63
Merged
Conversation
1bf290e to
505c288
Compare
505c288 to
3c11b17
Compare
Contributor
Author
|
@claude review the PR |
Contributor
Author
|
@claude review this pr |
There was a problem hiding this comment.
Pull Request Overview
Adds a new endpoint to sign and send MPC transaction requests and integrates it into the API spec, router, handler, tests, and OpenAPI JSON.
- Introduces
handleSignAndSendTxRequesthandler and wires it intoMasterApiSpecand router - Defines request/response types (
SignMpcRequest/SignMpcResponse) and adds tests for ECDSA and EdDSA flows - Updates
masterBitgoExpress.jsonto include the new/txrequest/{txRequestId}/signAndSendpath
Reviewed Changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| src/api/master/routers/masterApiSpec.ts | Added new route spec and registration for signAndSend |
| src/api/master/handlers/handleSignAndSendTxRequest.ts | Implemented MPC sign-and-send handler logic |
| src/tests/api/master/signAndSendTxRequest.test.ts | Added tests covering successful and pending flows |
| masterBitgoExpress.json | Updated OpenAPI spec with new path (and removed passphrase parameter) |
Comments suppressed due to low confidence (5)
masterBitgoExpress.json:258
- The
walletPassphraseproperty was removed from the consolidate request schema here but likely unintentionally. Reintroduce or confirm removal if intended.
},
src/api/master/routers/masterApiSpec.ts:228
- Using
t.anyfor the 200 response is too permissive. Define a specific codec that matches the expected response properties (e.g.,txid,tx, optionalpendingApproval, etc.).
200: t.any,
masterBitgoExpress.json:634
- The OpenAPI spec for the signAndSend endpoint has an empty schema for the 200 response—please specify the returned object structure to aid consumers.
"schema": {}
src/api/master/routers/masterApiSpec.ts:222
- [nitpick]
SignMpcRequestis generic; consider renaming toSignAndSendTxRequestBodyfor clarity and consistency with the endpoint.
const SignMpcRequest = {
src/tests/api/master/signAndSendTxRequest.test.ts:22
- [nitpick] Tests cover successful and pending cases but lack negative scenarios (e.g., wallet not found, wrong wallet type, or mismatched
commonKeychain). Consider adding those to improve coverage.
describe('POST /api/:coin/wallet/:walletId/txrequest/:txRequestId/signAndSend', () => {
3c11b17 to
9fe64d7
Compare
mohammadalfaiyazbitgo
approved these changes
Jul 11, 2025
| } | ||
|
|
||
| if (wallet.type() !== 'cold' || wallet.subType() !== 'onPrem') { | ||
| throw new Error('Wallet is not an on-prem wallet'); |
Contributor
There was a problem hiding this comment.
nit: Advanced Wallet
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.