Skip to content

Add message signing to manager#753

Merged
ScreamingHawk merged 16 commits into
masterfrom
messages
May 19, 2025
Merged

Add message signing to manager#753
ScreamingHawk merged 16 commits into
masterfrom
messages

Conversation

@ScreamingHawk

@ScreamingHawk ScreamingHawk commented May 15, 2025

Copy link
Copy Markdown
Contributor
  • Adds ERC-191 and ERC-712 message signing to core/wallet and wdk/manager
  • Adds arbitrum sepolia to networks for demo dapp
  • Updates wallet contract addresses to contracts with ERC-1271 fixes
  • Uses 6492 for signature validation
  • Corrects 6492 signature encoding for chained signatures
  • Updates existing tests to use anvil

@ScreamingHawk ScreamingHawk requested review from a team as code owners May 15, 2025 00:02
@Agusx1211 Agusx1211 requested a review from Copilot May 16, 2025 09:41

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull Request Overview

This PR introduces message signing functionality and integrates support for a new Arbitrum Sepolia network in the demo dapp. The changes include new types and endpoints for message signing, removal of the no‐longer-needed EnvelopeStatus type, and updates to both the core wallet and database modules to support the new feature.

Reviewed Changes

Copilot reviewed 10 out of 10 changed files in this pull request and generated no comments.

Show a summary per file
File Description
packages/wallet/wdk/src/sequence/types/transaction-request.ts Removed the EnvelopeStatus type no longer required by the updated message signing feature
packages/wallet/wdk/src/sequence/types/signature-request.ts Added a new SignMessage action in the action-to-payload mapping and Actions object
packages/wallet/wdk/src/sequence/types/message-request.ts Introduced new types for handling message signing requests and responses
packages/wallet/wdk/src/sequence/types/index.ts Updated exports to include the new message types and re-arranged type exports accordingly
packages/wallet/wdk/src/sequence/messages.ts Added a new module to handle the complete lifecycle of message signing requests
packages/wallet/wdk/src/sequence/manager.ts Integrated new message signing endpoints and updated module initialization
packages/wallet/wdk/src/dbs/messages.ts Created a new database module to store message signing data
packages/wallet/wdk/src/dbs/index.ts Exported the new messages DB module
packages/wallet/primitives/src/network.ts Added Arbitrum Sepolia network configuration to the supported networks
packages/wallet/core/src/wallet.ts Enhanced the Wallet class with methods to prepare and build message signatures
Comments suppressed due to low confidence (1)

packages/wallet/wdk/src/sequence/types/transaction-request.ts:18

  • Removal of the EnvelopeStatus type could impact other parts of the system that depend on it; please double-check that all affected modules have been updated to use the new message status types.
export type EnvelopeStatus = 'requested' | 'defined' | 'formed' | 'relayed'

Comment thread packages/wallet/wdk/src/sequence/manager.ts Outdated
@ScreamingHawk ScreamingHawk marked this pull request as draft May 18, 2025 19:28
@ScreamingHawk

Copy link
Copy Markdown
Contributor Author

Switched this PR to draft as signing flow is incorrect, signatures are not validating.

@ScreamingHawk ScreamingHawk marked this pull request as ready for review May 19, 2025 02:24
@ScreamingHawk

Copy link
Copy Markdown
Contributor Author

Ready now @Agusx1211

@Agusx1211 Agusx1211 requested a review from Copilot May 19, 2025 18:46

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull Request Overview

Adds end-to-end message signing support (ERC-191 & ERC-712) to the Wallet & Manager, integrates off-chain ERC-6492 validation, expands network configs, and bumps default contract addresses.

  • Introduce Messages module in WDK for request/complete flows and persistence
  • Extend Manager API and DB schema to handle message signing alongside transactions
  • Update primitives to wrap chained signatures with ERC-6492 and validate them off-chain

Reviewed Changes

Copilot reviewed 21 out of 21 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
packages/wallet/wdk/src/sequence/messages.ts Introduces Messages service with request, complete, listeners
packages/wallet/wdk/src/sequence/manager.ts Extends Manager with message signing API & database hooks
packages/wallet/wdk/src/dbs/messages.ts Adds Messages IndexedDB store
packages/wallet/wdk/src/sequence/types/message-request.ts Defines MessageRequest, MessageRequested, MessageSigned
packages/wallet/wdk/src/sequence/types/signature-request.ts Adds SignMessage action in signature mapping
packages/wallet/wdk/src/sequence/types/index.ts Re-exports new message types and reorders transaction exports
packages/wallet/wdk/test/wdk.test.ts Removes placeholder test (coverage needed)
packages/wallet/primitives/src/signature.ts Fixes ERC-6492 suffix wrapping in encodeSignature
packages/wallet/primitives/src/erc-6492.ts Implements off-chain ERC-6492 isValid helper
packages/wallet/primitives/src/network.ts Adds Arbitrum Sepolia to network list
packages/wallet/primitives/src/constants.ts Updates default contract addresses to ERC-1271-patched versions
Comments suppressed due to low confidence (3)

packages/wallet/wdk/test/wdk.test.ts:1

  • The placeholder WDK test file was removed without adding any new tests for the message signing flows; consider adding unit/integration tests for Messages.request, Messages.complete, and listener methods.
import { describe, it, expect } from 'vitest'

packages/wallet/wdk/src/sequence/types/transaction-request.ts:18

  • Removed EnvelopeStatus is a breaking change to the public types; ensure consumers are updated or provide a deprecation path if external code relied on this type.
export type EnvelopeStatus = 'requested' | 'defined' | 'formed' | 'relayed'

packages/wallet/wdk/src/sequence/manager.ts:500

  • [nitpick] Method name completedMessageSignature is inconsistent with requestMessageSignature and deleteMessageRequest; consider renaming to completeMessageSignature for verb consistency.
public async completedMessageSignature(messageOrSignatureId: string) {

Comment thread packages/wallet/wdk/src/sequence/manager.ts
Comment thread packages/wallet/wdk/src/dbs/messages.ts
Comment thread packages/wallet/primitives/src/erc-6492.ts Outdated
ScreamingHawk and others added 3 commits May 20, 2025 07:03
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
@ScreamingHawk ScreamingHawk merged commit 1c00450 into master May 19, 2025
3 checks passed
@ScreamingHawk ScreamingHawk deleted the messages branch May 19, 2025 20:18
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.

3 participants