Skip to content
This repository has been archived by the owner on Oct 19, 2024. It is now read-only.

Add a simple constructor for Wallet #1530

Merged
merged 2 commits into from
Jul 30, 2022
Merged

Conversation

moh-eulith
Copy link
Contributor

@moh-eulith moh-eulith commented Jul 28, 2022

Motivation

I implemented Signer, but realized that I had to copy a large amount of code (Wallet and more) just to get a working implementation.

Solution

A plain constructor for Wallet allows other implementation of Signer to use this code.

PR Checklist

  • Added Tests: correctness is by inspection of the one liner
  • Added Documentation: yes
  • Updated the changelog: I don't understand the changelog. It's from a long time ago?

@gakonst
Copy link
Owner

gakonst commented Jul 30, 2022

Can you give an example of when you had to implement your own DigestSigner? This doesn't make full sense to me on why it'd be needed

@moh-eulith
Copy link
Contributor Author

There are devices (and custodians and what not) out there than can do ECDSA signatures: given a hashed message, spit out a 64 byte signature from a secretly held key.

Of course having a raw signature is far from being able to talk to the Eth chain. Wallet and associated code do all that heavy lifting.

Honestly, the question seems a bit odd given that the code is already generified over the digest signer, for the very practical reason that there are 2 different implementations even in the library itself (private key and yubi key).

@gakonst
Copy link
Owner

gakonst commented Jul 30, 2022

Hmm I was thinking that you could do this by implementing a Signer and then the SignerMiddleware does all the lifting, but I can see the need to expose a constructor anyway

ethers-signers/src/wallet/mod.rs Outdated Show resolved Hide resolved
@gakonst gakonst merged commit 7306a3c into gakonst:master Jul 30, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants