Wp 4622/api ts mbe #18

mohammadalfaiyazbitgo · 2025-06-09T02:11:14Z

No description provided.

Copilot

Pull Request Overview

This PR refactors the BitGo Express integration by updating type definitions and introducing new API routers and response handlers.

Updated request types to use the new BitGo type and added an isBitGoRequest type guard.
Introduced new routers for health check, enclaved express, and API spec endpoints using the @api-ts libraries.
Adjusted wallet generation to integrate with the updated BitGo type and its associated casting.

Reviewed Changes

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

Show a summary per file

File	Description
src/types/request.ts	Replaced BitGoBase with BitGo and added a type guard for client request verification.
src/shared/responseHandler.ts	Added a unified response handler wrapper for service functions.
src/routes/master.ts	Removed legacy health check and BitGo setup middleware; integrated new router-based endpoints.
src/masterBitgoExpress/routers/*	Added new API routers for health check, enclaved express, and API spec endpoints integration.
src/masterBitgoExpress/generateWallet.ts	Updated the wallet generation code to use an explicit cast to BitGoBase—reflecting a change in type usage.
package.json	Added new dependencies for the @api-ts libraries and updated version references.

Comments suppressed due to low confidence (1)

src/masterBitgoExpress/generateWallet.ts:121

The use of double casting ('bitgo as unknown as BitGoBase') indicates a potential type mismatch between the BitGo type in requests and the BitGoBase expected by the Wallet constructor. Consider updating the types or refactoring the Wallet constructor to accept the BitGo type directly to maintain type safety.

wallet: new Wallet(bitgo as unknown as BitGoBase, baseCoin, newWallet),

pranavjain97

You're converting routes from master and calling them "enclave" spec

pranavjain97 · 2025-06-09T19:41:08Z

src/masterBitgoExpress/generateWallet.ts


  const result: WalletWithKeychains = {
-    wallet: new Wallet(bitgo, baseCoin, newWallet),
+    wallet: new Wallet(bitgo as unknown as BitGoBase, baseCoin, newWallet),


Why is this needed? Shoudn't GenerateWalletOptions have bitgo as BitGoBase?

pranavjain97 · 2025-06-09T19:47:16Z

src/routes/master.ts

-    next();
-  };
-}
+import { createEnclavedApiRouter } from '../masterBitgoExpress/routers/enclavedApiSpec';


This should be called createMasterApiRouter

pranavjain97 · 2025-06-09T19:48:03Z

src/masterBitgoExpress/routers/enclavedExpressHealth.ts

+});
+
+// Create router with handlers
+export function createEnclavedExpressRouter(


This is master express API router

pranavjain97 · 2025-06-09T19:48:33Z

src/masterBitgoExpress/routers/enclavedApiSpec.ts

+});
+
+// Create router with handlers
+export function createEnclavedApiRouter(


Suggested change

export function createEnclavedApiRouter(

export function createMasterApiRouter(

pranavjain97 · 2025-06-09T19:48:49Z

src/masterBitgoExpress/routers/enclavedApiSpec.ts

+};
+
+// API Specification
+export const EnclavedApiSpec = apiSpec({


Suggested change

export const EnclavedApiSpec = apiSpec({

export const MasterApiSpec = apiSpec({

pranavjain97 · 2025-06-09T19:49:10Z

src/masterBitgoExpress/routers/enclavedApiSpec.ts

@@ -0,0 +1,111 @@
+import * as t from 'io-ts';


All this is masterApiSpec

pranavjain97

Also, why do we need a new routers folder. Can we use the existing routes folder and add them to the master file

mohammadalfaiyazbitgo · 2025-06-09T20:01:18Z

Also, why do we need a new routers folder. Can we use the existing routes folder and add them to the master file

file becomes too large, around or just over 500 lines as is and will become even more larger once we add in the tss routes + recovery. Would prefer if we could keep things encapsulated

mohammadalfaiyazbitgo · 2025-06-09T20:02:13Z

You're converting routes from master and calling them "enclave" spec

Yeah originally had named it such so it would be referencing the api on enclaved that master is calling but in hindsight that is kinda confusing.

Ticket: WP-4622

pranavjain97 · 2025-06-09T20:21:08Z

src/masterBitgoExpress/routers/enclavedExpressHealth.ts

+};
+
+// API Specification
+export const EnclavedExpressApiSpec = apiSpec({


Suggested change

export const EnclavedExpressApiSpec = apiSpec({

export const MasterExpressApiSpec = apiSpec({

mohammadalfaiyazbitgo requested review from Copilot and pranavjain97 June 9, 2025 02:11

This comment was marked as outdated.

Sign in to view

mohammadalfaiyazbitgo requested a review from Copilot June 9, 2025 02:50

This comment was marked as outdated.

Sign in to view

mohammadalfaiyazbitgo force-pushed the WP-4622/api-ts-mbe branch 2 times, most recently from 8a58773 to 1797b29 Compare June 9, 2025 18:58

mohammadalfaiyazbitgo marked this pull request as ready for review June 9, 2025 18:58

mohammadalfaiyazbitgo requested a review from Copilot June 9, 2025 18:58

Copilot AI reviewed Jun 9, 2025

View reviewed changes

mohammadalfaiyazbitgo enabled auto-merge June 9, 2025 19:49

pranavjain97 requested changes Jun 9, 2025

View reviewed changes

pranavjain97 reviewed Jun 9, 2025

View reviewed changes

mohammadalfaiyazbitgo force-pushed the WP-4622/api-ts-mbe branch from 1797b29 to 51a181e Compare June 9, 2025 19:58

feat(mbe): move routes to api-ts

f98962b

Ticket: WP-4622

mohammadalfaiyazbitgo force-pushed the WP-4622/api-ts-mbe branch from 51a181e to f98962b Compare June 9, 2025 20:03

mohammadalfaiyazbitgo requested a review from pranavjain97 June 9, 2025 20:03

pranavjain97 approved these changes Jun 9, 2025

View reviewed changes

mohammadalfaiyazbitgo merged commit 6de52b8 into master Jun 9, 2025
3 checks passed

mohammadalfaiyazbitgo deleted the WP-4622/api-ts-mbe branch June 9, 2025 20:22

	export function createEnclavedApiRouter(
	export function createMasterApiRouter(

	export const EnclavedApiSpec = apiSpec({
	export const MasterApiSpec = apiSpec({

	export const EnclavedExpressApiSpec = apiSpec({
	export const MasterExpressApiSpec = apiSpec({

Wp 4622/api ts mbe #18

Wp 4622/api ts mbe #18

Uh oh!

Conversation

mohammadalfaiyazbitgo commented Jun 9, 2025

Uh oh!

This comment was marked as outdated.

Uh oh!

This comment was marked as outdated.

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

pranavjain97 left a comment

Choose a reason for hiding this comment

Uh oh!

pranavjain97 Jun 9, 2025

Choose a reason for hiding this comment

Uh oh!

pranavjain97 Jun 9, 2025

Choose a reason for hiding this comment

Uh oh!

pranavjain97 Jun 9, 2025

Choose a reason for hiding this comment

Uh oh!

pranavjain97 Jun 9, 2025

Choose a reason for hiding this comment

Uh oh!

pranavjain97 Jun 9, 2025

Choose a reason for hiding this comment

Uh oh!

pranavjain97 Jun 9, 2025

Choose a reason for hiding this comment

Uh oh!

pranavjain97 left a comment

Choose a reason for hiding this comment

Uh oh!

mohammadalfaiyazbitgo commented Jun 9, 2025

Uh oh!

mohammadalfaiyazbitgo commented Jun 9, 2025

Uh oh!

pranavjain97 Jun 9, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants