Skip to content

feat(mbe,ebe)!: migrate to beta packages and deprecate bitgo package#81

Merged
mohammadalfaiyazbitgo merged 2 commits intomasterfrom
feature-migrate-bitgo-beta
Jul 23, 2025
Merged

feat(mbe,ebe)!: migrate to beta packages and deprecate bitgo package#81
mohammadalfaiyazbitgo merged 2 commits intomasterfrom
feature-migrate-bitgo-beta

Conversation

@mohammadalfaiyazbitgo
Copy link
Copy Markdown
Contributor

@mohammadalfaiyazbitgo mohammadalfaiyazbitgo commented Jul 22, 2025

Breaking Change: Update all packages to bitgo-beta and deprecate usage of bitgo package

Ticket: WP-5336

This comment was marked as outdated.

Sign in to view

@mohammadalfaiyazbitgo mohammadalfaiyazbitgo force-pushed the feature-migrate-bitgo-beta branch 2 times, most recently from 04fd623 to 9f3fd27 Compare July 22, 2025 20:46
Copilot AI reviewed Jul 22, 2025
View reviewed changes
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 migrates from the deprecated bitgo package to the new @bitgo-beta packages and introduces a comprehensive coin factory for dynamic coin loading. The changes provide better modularity and prepare for future package deprecation.

  • Updates all package imports from @bitgo/* to @bitgo-beta/* equivalents
  • Introduces a new coinFactory to handle dynamic coin registration and loading
  • Replaces direct bitgo.coin() calls with coinFactory.getCoin() throughout the codebase

Reviewed Changes

Copilot reviewed 50 out of 51 changed files in this pull request and generated 1 comment.

File Description
src/shared/coinFactory.ts New factory for dynamic coin loading with comprehensive coin family support
src/types/request.ts Updates BitGo type from BitGo to BitGoAPI
src/shared/transactionUtils.ts Package migration and bug fix for MPCUnsignedTx validation
package.json Major dependency updates to use @bitgo-beta packages and version management
Comments suppressed due to low confidence (2)

package.json:2

  • [nitpick] The package name still uses @bitgo/ namespace while all dependencies have been migrated to @bitgo-beta/. Consider updating the package name to maintain consistency.
  "name": "@bitgo/enclaved-bitgo-express",

src/tests/routes.test.ts:36

  • The test expects the package name to be @bitgo-beta/enclaved-bitgo-express but the actual package.json still shows @bitgo/enclaved-bitgo-express. This inconsistency will cause the test to fail.
      response.body.should.have.property('name', '@bitgo-beta/enclaved-bitgo-express');

Comment thread src/shared/transactionUtils.ts

export function isMPCUnsignedTx(tx: any): tx is MPCUnsignedTx {
return 'unsignedTx' in tx && isMPCTx(tx);
return 'unsignedTx' in tx && isMPCTx(tx.unsignedTx);
Copy link

Copilot AI Jul 22, 2025

Choose a reason for hiding this comment

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

The logic change from isMPCTx(tx) to isMPCTx(tx.unsignedTx) appears to be a bug fix. The function should validate the unsignedTx property of the transaction object, not the entire transaction object itself.

Suggested change
return 'unsignedTx' in tx && isMPCTx(tx.unsignedTx);
return tx && 'unsignedTx' in tx && isMPCTx(tx.unsignedTx);

Copilot uses AI. Check for mistakes.
@mohammadalfaiyazbitgo mohammadalfaiyazbitgo force-pushed the feature-migrate-bitgo-beta branch from 9f3fd27 to f4e31c0 Compare July 22, 2025 20:52
pranavjain97
pranavjain97 previously approved these changes Jul 22, 2025
View reviewed changes
Copy link
Copy Markdown
Contributor

@pranavjain97 pranavjain97 left a comment

Choose a reason for hiding this comment

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

Nice LFG 🚀

Comment thread src/shared/transactionUtils.ts

export function isMPCUnsignedTx(tx: any): tx is MPCUnsignedTx {
return 'unsignedTx' in tx && isMPCTx(tx);
return 'unsignedTx' in tx && isMPCTx(tx.unsignedTx);
Copy link
Copy Markdown
Contributor

@pranavjain97 pranavjain97 Jul 22, 2025

Choose a reason for hiding this comment

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

was this intended?

pranavjain97
pranavjain97 reviewed Jul 22, 2025
View reviewed changes
Copy link
Copy Markdown
Contributor

@pranavjain97 pranavjain97 left a comment

Choose a reason for hiding this comment

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

How come beta has BitGoAPI but stable uses BitGoBase?

pranavjain97
pranavjain97 reviewed Jul 22, 2025
View reviewed changes
Copy link
Copy Markdown
Contributor

@pranavjain97 pranavjain97 left a comment

Choose a reason for hiding this comment

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

I'd also re-test some of the happy paths E2E to ensure nothing has broken

Comment thread src/api/enclaved/handlers/recoveryMultisigTransaction.ts Outdated
import { AbstractUtxoCoin } from '@bitgo/abstract-utxo';
import { SignFinalOptions } from '@bitgo-beta/abstract-eth';
import { AbstractUtxoCoin } from '@bitgo-beta/abstract-utxo';
import { HalfSignedUtxoTransaction, MethodNotImplementedError, TransactionRecipient } from 'bitgo';
Copy link
Copy Markdown
Contributor

@pranavjain97 pranavjain97 Jul 22, 2025

Choose a reason for hiding this comment

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

^

@mohammadalfaiyazbitgo mohammadalfaiyazbitgo force-pushed the feature-migrate-bitgo-beta branch 2 times, most recently from 055f4d0 to 8be5a10 Compare July 23, 2025 00:27
@mohammadalfaiyazbitgo
Copy link
Copy Markdown
Contributor Author

mohammadalfaiyazbitgo commented Jul 23, 2025

I'd also re-test some of the happy paths E2E to ensure nothing has broken

Yeah i did an ecdsa recovery & wallet generation. I'll do a multisig create and withdraw tomorrow.

@mohammadalfaiyazbitgo
Copy link
Copy Markdown
Contributor Author

mohammadalfaiyazbitgo commented Jul 23, 2025

was this intended?

Yeah the object being passed to isMPCTx needs to be the unsignedTx field.

@mohammadalfaiyazbitgo mohammadalfaiyazbitgo force-pushed the feature-migrate-bitgo-beta branch 2 times, most recently from 991a23f to dee4382 Compare July 23, 2025 01:01
@mohammadalfaiyazbitgo
Copy link
Copy Markdown
Contributor Author

mohammadalfaiyazbitgo commented Jul 23, 2025

@pranavjain97 tested a multisig wallet create & withdrawal, and an ecdsa create and withdrawal.

@mohammadalfaiyazbitgo mohammadalfaiyazbitgo force-pushed the feature-migrate-bitgo-beta branch from dee4382 to 22ef7b5 Compare July 23, 2025 15:18
@mohammadalfaiyazbitgo
feat(mbe, ebe): migrate to beta packages and deprecate bitgo package
8429316 
BREAKING CHANGE: Migrating packages to beta package
TICKET: WP-5336
@mohammadalfaiyazbitgo
feat(mbe, ebe): remove bitgo package and bump express
16ed80e 
BREAKING CHANGE: Removed bitgo package
Ticket: WP-5336
@mohammadalfaiyazbitgo mohammadalfaiyazbitgo force-pushed the feature-migrate-bitgo-beta branch from 22ef7b5 to 16ed80e Compare July 23, 2025 15:21
@mohammadalfaiyazbitgo mohammadalfaiyazbitgo merged commit 525671d into master Jul 23, 2025
4 checks passed
@mohammadalfaiyazbitgo mohammadalfaiyazbitgo deleted the feature-migrate-bitgo-beta branch July 23, 2025 17:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

Copilot code review Copilot Copilot left review comments

@pranavjain97 pranavjain97 pranavjain97 approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@mohammadalfaiyazbitgo @pranavjain97