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.

Sign up for GitHub

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

Jump to bottom

feat(sdk-coin-icp): added address creation and validation logic #5390

Merged
merged 1 commit into from
Jan 29, 2025
Merged

feat(sdk-coin-icp): added address creation and validation logic #5390

merged 1 commit into from
Jan 29, 2025
+234 −11

Conversation

mohd-kashif
Copy link
Contributor

TICKET: WIN-4247

@mohd-kashif mohd-kashif self-assigned this Jan 20, 2025
Base automatically changed from WIN-4245 to master January 20, 2025 09:10
@mohd-kashif mohd-kashif marked this pull request as ready for review January 20, 2025 09:13
@mohd-kashif mohd-kashif requested review from a team as code owners January 20, 2025 09:13
OttoAllmendinger
OttoAllmendinger previously requested changes Jan 20, 2025
View reviewed changes
modules/sdk-coin-icp/src/icp.ts Outdated Show resolved Hide resolved
@mohd-kashif mohd-kashif force-pushed the WIN-4247 branch 2 times, most recently from 38ee515 to 1b0e289 Compare January 20, 2025 10:13
@mohd-kashif mohd-kashif force-pushed the WIN-4247 branch 2 times, most recently from 7d382a0 to d947a3c Compare January 21, 2025 03:43
zahin-mohammad
zahin-mohammad previously approved these changes Jan 23, 2025
View reviewed changes
Copy link
Contributor

@zahin-mohammad zahin-mohammad left a comment

Choose a reason for hiding this comment

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

lgtm, just confused why we have the ordering changes

modules/bitgo/tsconfig.json Outdated Show resolved Hide resolved
tsconfig.packages.json Outdated Show resolved Hide resolved
alebusse
alebusse requested changes Jan 23, 2025
View reviewed changes
Copy link
Contributor

@alebusse alebusse left a comment

Choose a reason for hiding this comment

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

rebase please

@mohd-kashif mohd-kashif marked this pull request as draft January 24, 2025 11:57
@mohd-kashif mohd-kashif force-pushed the WIN-4247 branch 3 times, most recently from 678f480 to cd2de0d Compare January 24, 2025 15:54
@mohd-kashif mohd-kashif marked this pull request as ready for review January 24, 2025 16:10
zahin-mohammad
zahin-mohammad previously approved these changes Jan 24, 2025
View reviewed changes
alebusse
alebusse reviewed Jan 27, 2025
View reviewed changes
Copy link
Contributor

@alebusse alebusse left a comment •
edited
Loading

Choose a reason for hiding this comment

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

why the curve is eddsa in the statics?

BitGoJS/modules/statics/src/coins.ts

Lines 1578 to 1588 in 8ee3d50

account(
'35254b6a-5370-4e22-844b-be504b510103',
'icp',
'Internet Computer',
Networks.main.icp,
8,
UnderlyingAsset.ICP,
BaseUnit.ICP,
ICP_FEATURES,
KeyCurve.Ed25519
),

@mohd-kashif mohd-kashif force-pushed the WIN-4247 branch 2 times, most recently from 5677af9 to d51839c Compare January 28, 2025 09:47
@mohd-kashif
Copy link
Contributor Author

why the curve is eddsa in the statics?

BitGoJS/modules/statics/src/coins.ts

Lines 1578 to 1588 in 8ee3d50

account(
'35254b6a-5370-4e22-844b-be504b510103',
'icp',
'Internet Computer',
Networks.main.icp,
8,
UnderlyingAsset.ICP,
BaseUnit.ICP,
ICP_FEATURES,
KeyCurve.Ed25519
),

changed to Secp256k1

@OttoAllmendinger OttoAllmendinger removed their request for review January 28, 2025 17:01
@mohd-kashif mohd-kashif force-pushed the WIN-4247 branch 2 times, most recently from 0ab434a to 2c77d35 Compare January 28, 2025 19:02
alebusse
alebusse previously approved these changes Jan 28, 2025
View reviewed changes
@mohd-kashif mohd-kashif requested a review from alebusse January 29, 2025 16:26
@mohd-kashif mohd-kashif merged commit b480997 into master Jan 29, 2025
8 checks passed
@mohd-kashif mohd-kashif deleted the WIN-4247 branch January 29, 2025 19:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@alebusse alebusse alebusse approved these changes

@margueriteblair margueriteblair margueriteblair approved these changes

@kaustubhbitgo kaustubhbitgo Awaiting requested review from kaustubhbitgo kaustubhbitgo is a code owner automatically assigned from BitGo/wallet-platform

@DinshawKothari DinshawKothari Awaiting requested review from DinshawKothari

@adarsh312 adarsh312 Awaiting requested review from adarsh312

@saravanan7mani saravanan7mani Awaiting requested review from saravanan7mani

@sachushaji sachushaji Awaiting requested review from sachushaji

@OttoAllmendinger OttoAllmendinger Awaiting requested review from OttoAllmendinger

@zahin-mohammad zahin-mohammad Awaiting requested review from zahin-mohammad

Assignees

@mohd-kashif mohd-kashif

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
7 participants
@mohd-kashif @OttoAllmendinger @sachushaji @saravanan7mani @zahin-mohammad @margueriteblair @alebusse