Skip to content

feat(sdk-coin-cosmos): add cosmos shared coin functionalities #6522

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

Closed
wants to merge 0 commits into from

Conversation

baltiyal
Copy link
Contributor

TICKET: COIN-4881

@baltiyal baltiyal force-pushed the COIN-4881-shared-cosmos-functionality branch 3 times, most recently from c2d458f to 534fc57 Compare July 24, 2025 14:52
@MohammedRyaan786 MohammedRyaan786 requested a review from Copilot July 24, 2025 16:12
Copy link

@Copilot 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 adds shared coin functionalities for Cosmos-based cryptocurrencies by implementing a unified SDK that can handle multiple Cosmos chains through configuration rather than requiring individual coin modules.

  • Implements a shared Cosmos coin class that uses statics configuration instead of individual modules
  • Adds core utilities for address validation, transaction building, and key pair management
  • Provides a registration system to automatically register all Cosmos coins with the shared implementation

Reviewed Changes

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

Show a summary per file
File Description
modules/sdk-coin-cosmos/src/register.ts Adds registration function to automatically register Cosmos coins using shared implementation
modules/sdk-coin-cosmos/src/lib/utils.ts Implements Cosmos utilities for address validation and amount validation using network configuration
modules/sdk-coin-cosmos/src/lib/transactionBuilderFactory.ts Creates transaction builders for various Cosmos transaction types
modules/sdk-coin-cosmos/src/lib/keyPair.ts Implements key pair management and address derivation for Cosmos networks
modules/sdk-coin-cosmos/src/lib/index.ts Exports library components for external use
modules/sdk-coin-cosmos/src/index.ts Updates main module exports to include new shared functionality
modules/sdk-coin-cosmos/src/cosmosSharedCoin.ts Enhances shared coin implementation with network configuration and concrete method implementations
modules/sdk-coin-cosmos/package.json Adds required dependencies for Cosmos functionality
modules/bitgo/test/browser/browser.spec.ts Updates browser test to include CosmosSharedCoin in exclusions
Comments suppressed due to low confidence (1)

modules/sdk-coin-cosmos/src/cosmosSharedCoin.ts:87

  • [nitpick] The assertion error message could be more helpful by suggesting what action to take. Consider: env config is missing for ${family} in ${env}. Please ensure the environment configuration includes the nodeUrl for this coin family.
    assert(cosmosConfig?.[family]?.nodeUrl, `env config is missing for ${family} in ${env}`);

throw new InvalidTransactionError('Invalid transaction');
}
} catch (e) {
throw new InvalidTransactionError('Invalid transaction: ' + e.message);
Copy link
Preview

Copilot AI Jul 24, 2025

Choose a reason for hiding this comment

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

Error message concatenation can result in unclear messages if e.message is undefined. Consider using template literals or ensuring e.message exists: throw new InvalidTransactionError('Invalid transaction: ' + (e.message || 'Unknown error'));

Suggested change
throw new InvalidTransactionError('Invalid transaction: ' + e.message);
throw new InvalidTransactionError(`Invalid transaction: ${e.message || 'Unknown error'}`);

Copilot uses AI. Check for mistakes.

yarn.lock Outdated
@@ -6801,14 +6801,6 @@ ansi-styles@^6.0.0, ansi-styles@^6.1.0:
resolved "https://registry.npmjs.org/ansi-styles/-/ansi-styles-6.2.1.tgz#0e62320cf99c21afff3b3012192546aacbfb05c5"
Copy link
Contributor

Choose a reason for hiding this comment

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

how come so many deletions? is it normal?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

tbh, not sure.
Cleaned the present yarn-lock and made it again using yarn install and got similar or same result

@baltiyal baltiyal marked this pull request as ready for review July 25, 2025 11:01
@baltiyal baltiyal requested review from a team as code owners July 25, 2025 11:01
@baltiyal baltiyal force-pushed the COIN-4881-shared-cosmos-functionality branch from 534fc57 to e72c320 Compare July 25, 2025 12:18
@baltiyal baltiyal closed this Jul 25, 2025
@baltiyal baltiyal force-pushed the COIN-4881-shared-cosmos-functionality branch from e72c320 to d779dff Compare July 25, 2025 12:19
@baltiyal
Copy link
Contributor Author

This branch was closed by mistake.
Please review the updated PR: #6548
@MohammedRyaan786 has addressed your comments in the new branch.
@bhavidhingra — the yarn.lock file is not included in the new branch as it was conflicting with master. So, rebased, and it was not needed after that.

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.

3 participants