Skip to content

Conversation

@lokesh-bitgo
Copy link
Contributor

Ticket: WP-5427

@lokesh-bitgo lokesh-bitgo marked this pull request as ready for review November 3, 2025 06:32
@lokesh-bitgo lokesh-bitgo requested a review from a team as a code owner November 3, 2025 06:32
Copy link
Contributor

@rahulhegde-bitgo rahulhegde-bitgo left a comment

Choose a reason for hiding this comment

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

LGTM

danielzhao122
danielzhao122 previously approved these changes Nov 3, 2025
@lokesh-bitgo lokesh-bitgo force-pushed the WP-5427-express-migrate-api-v-2-coin-wallet-id-sendmany-to-typed-routes branch from 4f0014c to 7abc5ce Compare November 4, 2025 11:11
* @param req
*/
async function handleV2SendMany(req: express.Request) {
async function handleV2SendMany(req: ExpressApiRouteRequest<'express.v2.wallet.sendmany', 'post'>) {
Copy link
Contributor

Choose a reason for hiding this comment

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

While this works, we lose the type saftey of api-ts when using things like createTSSSendParams and createSendParams. Can we have a follow up to fix these?

As long as we have those two utilities, the codec and the actual request body we are acting on won't be aligned.

* This endpoint supports the full set of parameters available in the BitGo SDK
* for building, signing, and sending transactions to multiple recipients.
*/
export const SendManyRequestBody = {
Copy link
Contributor

Choose a reason for hiding this comment

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

How can everything be optional?

Copy link
Contributor

Choose a reason for hiding this comment

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

These codecs will eventually generate our api-docs, so it will be an issue if we advertise params as required, then change to optional, then back to required.

Comment on lines +392 to +393
/** Transaction request object */
txRequest: t.unknown,
Copy link
Contributor

Choose a reason for hiding this comment

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

This will look like a breaking change in our docs.

Please cross-reference https://developers.bitgo.com/reference/expresswalletsendmany to ensure we aren't introducing breaking changes, and if we are, we should only be introducing them to match what the code is currently doing.

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.

5 participants