Skip to content

Conversation

@bobo-k2
Copy link
Contributor

@bobo-k2 bobo-k2 commented May 23, 2025

This PR enables a dApp registration with EVM account. Until now the registration was possible with SS58 account only.

@bobo-k2 bobo-k2 requested review from Copilot and gtg7784 May 23, 2025 11:22
Copy link

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 pull request adds support for dApp registration using an EVM account by introducing new signature verification logic and address conversion.

  • Introduces EVM signature verification with ethers.verifyMessage
  • Converts EVM addresses to SS58 format when constructing the verification payload
  • Updates the developer check to use the converted SS58 address

): Promise<boolean> {
const api = this._apiFactory.getApiInstance(network);
const isEvmSigner = isEthereumAddress(senderAddress);
const ss58SenderAddress = isEvmSigner ? evmToAddress(senderAddress, ASTAR_SS58_FORMAT) : senderAddress;
Copy link

Copilot AI May 23, 2025

Choose a reason for hiding this comment

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

[nitpick] Consider adding an inline comment explaining why the sender address is converted to SS58 format for EVM signers and how this conversion aligns with the expected payload format.

Copilot uses AI. Check for mistakes.
Comment on lines +75 to +76
const signerAddress = ethers.verifyMessage(messageToVerify, signature);
isValidSignature = signerAddress.toLowerCase() === senderAddress.toLowerCase();
Copy link

Copilot AI May 23, 2025

Choose a reason for hiding this comment

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

Consider wrapping ethers.verifyMessage in a try-catch block to gracefully handle potential exceptions during signature verification for EVM signers.

Suggested change
const signerAddress = ethers.verifyMessage(messageToVerify, signature);
isValidSignature = signerAddress.toLowerCase() === senderAddress.toLowerCase();
try {
const signerAddress = ethers.verifyMessage(messageToVerify, signature);
isValidSignature = signerAddress.toLowerCase() === senderAddress.toLowerCase();
} catch (error) {
console.error('Error verifying EVM signature:', error);
isValidSignature = false;
}

Copilot uses AI. Check for mistakes.
@github-actions
Copy link

Visit the preview URL for this PR (updated for commit 1b4c096):

https://astar-token-api--pr171-feat-register-dapp-h-xya6fcmg.web.app

(expires Fri, 30 May 2025 11:23:14 GMT)

🔥 via Firebase Hosting GitHub Action 🌎

Sign: f99fa4f4f8f3bb450e6e842f7e1c7783d6d896a3

@tksarah
Copy link

tksarah commented May 26, 2025

This is an intriguing improvement.
Is this, in other words, one of the improvements aimed at making it easier for the Soneium project to be listed on dApp Staking in the future?
Thank you

@bobo-k2 bobo-k2 merged commit 2774456 into prod May 26, 2025
8 checks passed
@bobo-k2 bobo-k2 deleted the feat/register-dapp-h160 branch May 26, 2025 08:59
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