Skip to content

Conversation

pranavjain97
Copy link
Contributor

@pranavjain97 pranavjain97 commented Oct 21, 2025

Ticket: WP-6378

@pranavjain97 pranavjain97 changed the title feat: add MPC EdDSA support for isWalletAddress feat: implement isWalletAddress for mpc eddsa coins Oct 21, 2025
@pranavjain97 pranavjain97 force-pushed the WP-6378-isWalletAddress-coins branch from 49214eb to d971ec0 Compare October 21, 2025 20:32
@pranavjain97 pranavjain97 force-pushed the WP-6378-isWalletAddress-coins branch from d971ec0 to e9982fe Compare October 21, 2025 21:20
@pranavjain97 pranavjain97 marked this pull request as ready for review October 22, 2025 14:40
@pranavjain97 pranavjain97 requested review from a team as code owners October 22, 2025 14:40
dpkjnr
dpkjnr previously approved these changes Oct 22, 2025
Copy link
Contributor

@mtexeira-simtlix mtexeira-simtlix left a comment

Choose a reason for hiding this comment

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

A couple of nits/suggestions and comments, thanks for the changes!

zahin-mohammad
zahin-mohammad previously approved these changes Oct 22, 2025
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 a few nits

throw new InvalidAddressError(`invalid address: ${address}`);
}

if (!keychains || keychains.length === 0) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
if (!keychains || keychains.length === 0) {
if (!keychains?.length) {

Comment on lines 160 to 162
keychains: {
commonKeychain: string;
}[];
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you define a type for this instead of in-lining it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

nvm; replaced with Keychains

* @throws {InvalidAddressError} if the address is invalid
* @throws {Error} if required parameters are missing or invalid
*/
export async function verifyEddsaTssWalletAddress(
Copy link
Contributor

Choose a reason for hiding this comment

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

We should have coin agnostic tests for this.

@pranavjain97 pranavjain97 dismissed stale reviews from zahin-mohammad and dpkjnr via 9513595 October 22, 2025 22:04
Copy link
Contributor Author

@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.

addressed comments

@pranavjain97 pranavjain97 force-pushed the WP-6378-isWalletAddress-coins branch from 9513595 to a64baf3 Compare October 22, 2025 22:36
@pranavjain97 pranavjain97 marked this pull request as ready for review October 23, 2025 03:53
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.

4 participants