Skip to content

feat: v4 signature#2280

Merged
zhouxl merged 2 commits intodevfrom
310/972-fix-v4-signature
Jan 16, 2026
Merged

feat: v4 signature#2280
zhouxl merged 2 commits intodevfrom
310/972-fix-v4-signature

Conversation

@zhouxl
Copy link
Copy Markdown
Collaborator

@zhouxl zhouxl commented Jan 16, 2026

Related Issue

Summary of Changes

Need Regression Testing

  • Yes
  • No

Risk Assessment

  • Low
  • Medium
  • High

Additional Notes

Screenshots (if applicable)

@github-actions
Copy link
Copy Markdown

github-actions bot commented Jan 16, 2026

PR Summary

Updated the EVM signature generation process to use a V4 signature approach. The changes modify how the EVM signature is created by using Keccak256 hashing on the JWT data and signing with the Ethereum key directly, rather than using the previous ethSign method. Added error handling with HUD error displays and a new signFailed error case.

Changes

File Summary
FRW/Foundation/Bridge/RNBridgeError.swift Added a new signFailed case to the RNBridgeError enum to handle signature failure scenarios.
FRW/Foundation/Bridge/TurboModule+Onboard.swift Refactored EVM signature generation to use Keccak256 hashing on JWT data and sign with the Ethereum key directly via getKeyForCoin. Added HUD error displays for invalidMnemonic, invalidSignData, and signFailed errors. The evmSignature is now formatted with hex prefix instead of using hexString.

autogenerated by presubmit.ai

@github-actions
Copy link
Copy Markdown

github-actions bot commented Jan 16, 2026

Dependency Review

✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.

Scanned Files

None

Copy link
Copy Markdown

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

LGTM!

Review Summary

Commits Considered (1)
Files Processed (2)
  • FRW/Foundation/Bridge/RNBridgeError.swift (1 hunk)
  • FRW/Foundation/Bridge/TurboModule+Onboard.swift (1 hunk)
Actionable Comments (0)
Skipped Comments (2)
  • FRW/Foundation/Bridge/TurboModule+Onboard.swift [170-170]

    possible bug: "Potential inconsistent hex formatting between signatures."

  • FRW/Foundation/Bridge/RNBridgeError.swift [20-20]

    maintainability: "New error case is defined but not used in the PR."

Copy link
Copy Markdown

@github-actions github-actions bot 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 needs attention.

Review Summary

Commits Considered (1)
  • bcdce96: feat: update sign for eoa
Files Processed (1)
  • FRW/Foundation/Bridge/TurboModule+Onboard.swift (1 hunk)
Actionable Comments (1)
  • FRW/Foundation/Bridge/TurboModule+Onboard.swift [172-172]

    possible issue: "Potential signature format issue for Ethereum V4 signatures."

Skipped Comments (1)
  • FRW/Foundation/Bridge/TurboModule+Onboard.swift [166-166]

    enhancement: "Missing HUD error display for ethAddress failure."

HUD.error(WalletError.invalidSignData)
throw WalletError.invalidSignData
}
let digest = Hash.keccak256(data: jwtData)
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

The code signs the raw Keccak256 hash of the JWT data directly. For Ethereum V4 (EIP-712) typed data signatures, the standard requires a specific prefix (\x19Ethereum Signed Message:\n + message length) to be prepended before hashing, or proper EIP-712 domain separator handling. Signing raw hashes without proper prefixing can lead to signature replay attacks or incompatibility with standard Ethereum signature verification.

@zhouxl zhouxl marked this pull request as ready for review January 16, 2026 05:49
@zhouxl zhouxl requested a review from a team as a code owner January 16, 2026 05:49
@zhouxl zhouxl merged commit 689c5f2 into dev Jan 16, 2026
4 checks passed
@zhouxl zhouxl deleted the 310/972-fix-v4-signature branch January 16, 2026 05:50
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.

2 participants