Skip to content

Conversation

@yash-atreya
Copy link
Contributor

@yash-atreya yash-atreya commented May 27, 2025

Motivation

Currently, the EthereumWallet encapsulates a dyn TxSigner which only gives access to transaction signing capabilities and NOT message / hash signing e.g signing eip712 typed data.

Solution

  • FullSigner gives access to both tx sign and message signing.
  • Wrap FullSigner in dedicated type ArcFullSigner to disambiguate function calls on the signer that are equivalent but from different traits - e.g the .address(..) exists on both TxSigner and Signer requiring fully qualified syntax at callsite by the user, using ArcFullSigner type removes the need for that.
  • Use the ArcFullSigner in EthereumWallet
  • Add sign_hash_with method

PR Checklist

  • Added Tests
  • Added Documentation
  • Breaking changes

@yash-atreya yash-atreya changed the title feat(network): use FullSigner in EthereumWallet to sign hashes feat(network): use FullSigner in EthereumWallet to sign typed data May 27, 2025
@yash-atreya yash-atreya self-assigned this May 27, 2025
@yash-atreya yash-atreya moved this to In Progress in Alloy May 27, 2025
@yash-atreya yash-atreya marked this pull request as ready for review June 3, 2025 13:53
@yash-atreya yash-atreya moved this from In Progress to Ready for Review in Alloy Jun 3, 2025
@yash-atreya yash-atreya changed the title feat(network): use FullSigner in EthereumWallet to sign typed data feat(network): use FullSigner in EthereumWallet to sign data Jun 5, 2025
Copy link
Member

@mattsse mattsse left a comment

Choose a reason for hiding this comment

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

I think this makes sense,

some nits, can we also send a patch pr to foundry to make sure this doesn't break anything major?

@github-project-automation github-project-automation bot moved this from Ready for Review to In Progress in Alloy Jun 5, 2025
@yash-atreya yash-atreya enabled auto-merge (squash) June 24, 2025 13:24
@yash-atreya yash-atreya moved this from In Progress to Ready for Review in Alloy Jun 24, 2025
Copy link
Member

@mattsse mattsse left a comment

Choose a reason for hiding this comment

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

marking this a blocked for now because breaking, but def something we want to do

@yash-atreya yash-atreya merged commit e1fb9d9 into main Jul 2, 2025
27 checks passed
@yash-atreya yash-atreya deleted the yash/arc-full-signer branch July 2, 2025 13:01
@github-project-automation github-project-automation bot moved this from Ready for Review to Done in Alloy Jul 2, 2025
@grandizzy grandizzy moved this from Done to Completed in Alloy Jul 8, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Completed

Development

Successfully merging this pull request may close these issues.

3 participants