Fixed Aptos verifyMessage to use SDK's own Ed25519 verification#79
Fixed Aptos verifyMessage to use SDK's own Ed25519 verification#79SergeyG-Solicy wants to merge 1 commit intomainfrom
Conversation
Review Summary by QodoUse Aptos SDK's native Ed25519 verification instead of node-forge
WalkthroughsDescription• Replaced node-forge dependency with Aptos SDK's native Ed25519 verification • Simplified verifyMessage implementation using SDK's built-in cryptographic methods • Removed manual signature and public key validation logic • Eliminated external dependency on node-forge library Diagramflowchart LR
A["verifyMessage method"] --> B["Create Ed25519PublicKey from string"]
A --> C["Create Ed25519Signature from bytes"]
B --> D["Call SDK's verifySignature method"]
C --> D
D --> E["Return boolean result"]
F["Remove node-forge dependency"] -.-> E
File Changes1. src/multichain/core/aptos.ts
|
Code Review by Qodo
1. Public key not validated
|
WalkthroughRefactors Ed25519 signature verification in the Aptos module by replacing manual node-forge-based verification with SDK-provided Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/multichain/core/aptos.ts`:
- Around line 434-439: The verification uses a raw JS string while signMessage
signs new TextEncoder().encode(message), causing a mismatch for hex-looking
strings; update the call to pubKey.verifySignature in Aptos-related code to pass
the same UTF-8 bytes (e.g., new TextEncoder().encode(message)) instead of the
raw string so Ed25519PublicKey.verifySignature and the earlier signMessage use
identical byte input.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 73e72635-527a-4e5a-aed8-876d9e1ddea1
📒 Files selected for processing (1)
src/multichain/core/aptos.ts
| const pubKey = new Ed25519PublicKey(publicKey) | ||
| const sig = new Ed25519Signature(hexToUint8Array(signature)) | ||
|
|
||
| // Validate input sizes | ||
| if (signatureBytes.length !== 64) { | ||
| throw new Error("Invalid signature length. Ed25519 signatures must be 64 bytes.") | ||
| } | ||
| if (publicKeyBytes.length !== 32) { | ||
| throw new Error("Invalid public key length. Ed25519 public keys must be 32 bytes.") | ||
| } | ||
|
|
||
| // Use node-forge for Ed25519 signature verification | ||
| const isValid = forge.pki.ed25519.verify({ | ||
| message: message, | ||
| encoding: "utf8", | ||
| signature: signatureBytes, | ||
| publicKey: publicKeyBytes | ||
| return pubKey.verifySignature({ | ||
| message, | ||
| signature: sig, |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
For the @aptos-labs/ts-sdkversion used in this repository, how does Ed25519PublicKey.verifySignature interpretmessage when it is passed a JavaScript string? Does it verify UTF-8 bytes, or does it expect hex/HexInput semantics?
💡 Result:
In @aptos-labs/ts-sdk (e.g., v1.16.0), Ed25519PublicKey.verifySignature({ message }) treats a JavaScript string as follows:
- If the string is a valid hex string per the SDK’s
Hex.isValidrules (optional0xprefix, non-empty, even-length, only hex chars), it is interpreted as hex bytes (i.e., HexInput semantics), not UTF-8. [1][2] - Otherwise, it converts the string to UTF-8 bytes via
Buffer.from(message, "utf8")and verifies those bytes. [2][1]
So: it’s “hex-if-it-looks-like-hex, otherwise UTF-8.” [2][1]
Sources: [1] https://app.unpkg.com/%40aptos-labs/ts-sdk%401.16.0/files/dist/esm/chunk-CX4BEIQT.mjs, [2] https://app.unpkg.com/%40aptos-labs/ts-sdk%401.16.0/files/dist/esm/chunk-F7EMGK4M.mjs, [3] https://app.unpkg.com/%40aptos-labs/ts-sdk%401.16.0/files/dist/esm/chunk-BCUSI3N6.mjs
Verify the same UTF-8 bytes that signMessage signs.
Line 412 signs new TextEncoder().encode(message), but Line 438 passes the raw JS string into verifySignature. The @aptos-labs/ts-sdk applies a heuristic when interpreting string input: if it looks like valid hex, it treats it as hex bytes; otherwise, it converts to UTF-8.
For the test case "Hello Aptos!", this works because it is not valid hex, so the SDK falls back to UTF-8. However, any message that happens to be a valid hex string (e.g., "aabbcc") will be signed as UTF-8 bytes but verified as hex bytes, causing verification to fail.
Encode the message before verification to make both paths use identical bytes and avoid this fragility.
Proposed fix
async verifyMessage(
message: string,
signature: string,
publicKey: string
): Promise<boolean> {
try {
+ const messageBytes = new TextEncoder().encode(message)
const pubKey = new Ed25519PublicKey(publicKey)
const sig = new Ed25519Signature(hexToUint8Array(signature))
return pubKey.verifySignature({
- message,
+ message: messageBytes,
signature: sig,
})
} catch (error) {
console.error("Failed to verify message:", error)
return false🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/multichain/core/aptos.ts` around lines 434 - 439, The verification uses a
raw JS string while signMessage signs new TextEncoder().encode(message), causing
a mismatch for hex-looking strings; update the call to pubKey.verifySignature in
Aptos-related code to pass the same UTF-8 bytes (e.g., new
TextEncoder().encode(message)) instead of the raw string so
Ed25519PublicKey.verifySignature and the earlier signMessage use identical byte
input.
1 participant
Summary by CodeRabbit