Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor(sdk-coin-atom): use abstract-cosmos #4141

Merged
merged 1 commit into from
Dec 26, 2023

Conversation

hitansh-bitgo
Copy link
Contributor

Ticket: WIN-1192

@hitansh-bitgo hitansh-bitgo force-pushed the WIN-1192-atom-refactor branch 2 times, most recently from dccf889 to f83c550 Compare December 10, 2023 10:28
@hitansh-bitgo hitansh-bitgo marked this pull request as ready for review December 11, 2023 04:35
@hitansh-bitgo hitansh-bitgo requested review from a team as code owners December 11, 2023 04:35
@hitansh-bitgo hitansh-bitgo force-pushed the WIN-1192-atom-refactor branch 2 times, most recently from c8eab8f to 259b04b Compare December 11, 2023 16:21
export { KeyPair } from './keyPair';
export { Transaction } from './transaction';
Copy link
Contributor

Choose a reason for hiding this comment

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

Removal of all these exports completely from here is a breaking change since /src/index.ts exposes these to end users.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added some of the exports back from abstract-cosmos & added a breaking change footer in the commit message for the ones still being removed completely

export const validDenoms = ['natom', 'uatom', 'matom', 'atom'];
export const accountAddressRegex = /^(cosmos)1(['qpzry9x8gf2tvdw0s3jn54khce6mua7l']{38})$/;
export const validatorAddressRegex = /^(cosmosvaloper)1(['qpzry9x8gf2tvdw0s3jn54khce6mua7l']{38})$/;
export const contractAddressRegex = /^(cosmos)1(['qpzry9x8gf2tvdw0s3jn54khce6mua7l']+)$/;
Copy link
Contributor

Choose a reason for hiding this comment

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

why removing contractAddressRegex ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Contract support was only added in abstract-cosmos & other chains but wasn't added for Atom it seems, so this was unused. I've updated the PR with the contract support changes needed for atom as well.

@@ -1438,7 +1438,7 @@ export enum KeyCurve {
* This enum contains the base units for the coins that BitGo supports
*/
export enum BaseUnit {
ATOM = 'uATOM',
ATOM = 'uatom',
Copy link
Contributor

Choose a reason for hiding this comment

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

Will this break anything in WP or we use lowercase there ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We were using lowercase everywhere except here

Ticket: WIN-1192

BREAKING CHANGE: restructured exports
- Removed constants overlapping with @bitgo/abstract-cosmos
- Removed common Interfaces exported by @bitgo/abstract-cosmos

Code dependent on the previously exported Interfaces/Constants needs to
be updated to import these from @bitgo/abstract-cosmos instead
@hitansh-bitgo hitansh-bitgo merged commit 2c3a9ca into master Dec 26, 2023
10 checks passed
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