- 
                Notifications
    
You must be signed in to change notification settings  - Fork 300
 
feat: added isWalletAddress implementation for canton #7410
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
base: master
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
          
 yes makes sense, I'll change the implementation  | 
    
3da889c    to
    2268dba      
    Compare
  
    There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
a better way to test this is to create two wallets with different key triples, create two addresses, and assert that isWalletAddress is true for the correct wallet and false for the other wallet
| const commonKeychain = extractCommonKeychain(keychains); | ||
| const MPC = await EDDSAMethods.getInitializedMpcInstance(); | ||
| const derivationPath = 'm/0'; | ||
| const derivedPublicKey = MPC.deriveUnhardened(commonKeychain, derivationPath).slice(0, 64); | ||
| const publicKeyBase64 = Buffer.from(derivedPublicKey, 'hex').toString('base64'); | ||
| const rootAddressFingerprint = utils.getAddressFromPublicKey(publicKeyBase64); | ||
| const rootAddress = `${rootAddressFingerprint.slice(0, 5)}::${rootAddressFingerprint}`; | ||
| return addressPart === rootAddress; | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please re-use the generic methods used for other ED coins.
  async isWalletAddress(params: TssVerifyAddressOptions): Promise<boolean> {
    return verifyEddsaTssWalletAddress(
      params,
      (address) => this.isValidAddress(address),
      (publicKey) => this.getAddressFromPublicKey(publicKey)
    );
  }
you just need to change the implementation of the two call back functions.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey @zahin-mohammad , canton is a memo based coin so I can't directly use this function. I was thinking of adding,
export async function verifyEddsaMemoBasedWalletAddress(
params: TssVerifyAddressOptions,
isValidAddress: (address: string) => boolean,
getAddressFromPublicKey: (publicKey: string) => string
): Promise {
const { keychains, address, index } = params;
const [addressPart, memoId] = address.split('?memoId=');
if (memoId && memoId !== index) {
throw new InvalidAddressError(invalid memoId: ${memoId});
}
if (!isValidAddress(addressPart)) {
throw new InvalidAddressError(invalid address: ${addressPart});
}
const commonKeychain = extractCommonKeychain(keychains);
const MPC = await EDDSAMethods.getInitializedMpcInstance();
const derivationPath = 'm/0';
const derivedPublicKey = MPC.deriveUnhardened(commonKeychain, derivationPath).slice(0, 64);
const expectedAddress = getAddressFromPublicKey(derivedPublicKey);
return addressPart === expectedAddress;
}
This method and use it when sdk-core is published. I'll create a ticket in backlog for this refactor, what do you think?
Ticket: COIN-6323