Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

BIP38 Support #2908

Open
nformant1 opened this issue Apr 5, 2022 · 18 comments
Open

BIP38 Support #2908

nformant1 opened this issue Apr 5, 2022 · 18 comments

Comments

@nformant1
Copy link
Contributor

nformant1 commented Apr 5, 2022

Hi there,

please add a BIP38 support for dogecoin core.

The core wallet is the reference implementation of all functions and paper wallets are still a thing in the Dogecoin community so I'd like to see this feature in the core wallet.

BIP38 specs: https://github.com/bitcoin/bips/blob/master/bip-0038.mediawiki

Feature Request

Description
The Qt wallet should be able to create BIP38 encrypted paper wallets and the import of an encrypted private key should be supported.

Solution
The Qt wallet could have a check box to encrypt the key and prompt for a password (and a verify-input to check the entered password).

Known dependencies:

Qt
Menue: File / Print paper wallets / new prompt to add password or ask for encryption
"Print Your Paper Wallets" window needs that info too after it offers to print new wallets

Console
importprivkey should have an optional argument or detect automatically that it is an encrypted private key
dumpprivkey could could have an optional argument to encrypt the private key

Cheers / thx!

@patricklodder patricklodder added this to To do in Operation Such Frensly via automation Apr 6, 2022
@patricklodder
Copy link
Member

Thanks! Overall, I think this is a good enhancement. I've added this to Operation Such Frensly because the final target is a release-independent UI improvement.

Although we'd normally say "RPC first", I'm not sure about the proposed console (=rpc) targets because sending custom encryption keys over network / typing it on the console means the secret has cleartext exposure and could potentially be logged. A counter-argument could be that lock/unlock wallet does this too. I would like to hear some opinions from others about this aspect of the requested change: does it add significant risk?

My second question is: is AES256/ECB a good enough to secure key data?. Arguably for a cleartext that consists of random bytes (from the paper wallet generator), it's probably okay. But for BIP32-derived keys (from dumpprivkey) I'm not sure.

Third: Please note that BIP38 is still in draft. We should do some additional in-depth review of the BIP itself because of the lack of upstream reviews / implementations.

Once we've identified targets, we can break down the individual changes into smaller packages and manage them from this issue.

@gewure
Copy link

gewure commented Jun 12, 2022

I'd like to look further into this proposed feature

@patricklodder from my feel it does not add extra exposure, as lock/unlock requires secrets too as you remarked. i see no real other way of doing this?

@patricklodder
Copy link
Member

That's awesome. I'd recommend splitting it up in 3 parts:

  1. Generic BIP38 implementation per the proposed conventions and tests
  2. Either add a new cli tool or implement an RPC endpoint that encodes and decodes keys using BIP38 (I think the former is more secure than the latter)
  3. Add it to existing functionality, in whatever order we prefer
    1. RPC endpoints
    2. Importprivkey UI
    3. Paper wallet generator (may be the easiest one to start with)

@envisiondata
Copy link

I would like to work on this.

@patricklodder
Copy link
Member

I would like to work on this.

Awesome! If we can help you in any way just let us know ❤️

@dogecoin dogecoin deleted a comment from SpringDogeEast Jul 26, 2022
@alamshafil
Copy link
Contributor

I was taking a look at this and tried to make a extremely basic implementation of BIP 38. So far I only coded no compression, and no EC multiply generation. However, I am stuck on scrypt part when trying to use these parameters: n=16384, r=8, p=8, length=64. I have looked at other scrypt implementations to have a idea on how the parameters are used but I keep ending up with a SEGFAULT. I might be able to figure if given time but any help would be helpful.

Since BIP 38 is still draft, here are the comments about it:
https://github.com/bitcoin/bips/wiki/Comments:BIP-0038

@gewure
Copy link

gewure commented Aug 3, 2022

I would like to work on this.
Perfect.

@patricklodder can you make issues for the single tasks, splitted up as you proposed?

Also is are you guys in some channel where we could discuss things?
@alamshafil @envisiondata

greetings

@envisiondata
Copy link

@gewure I do not have a specific channel to communicate on. Let me know which you prefer and I'll sign up for it.

@alamshafil
Copy link
Contributor

I'll join to since I have some progress. We can use Discord or Matrix using Element. I am open to any platform of choice.

@gewure
Copy link

gewure commented Aug 3, 2022

I'll join to since I have some progress. We can use Discord or Matrix using Element. I am open to any platform of choice.

Ok, ill join your link, sir :]

@envisiondata
Copy link

envisiondata commented Aug 3, 2022 via email

@alamshafil
Copy link
Contributor

Discord: Furious#0695

@patricklodder
Copy link
Member

Hey guys, do you have a repo to collaborate / do work refinement on? I talked to @rllola (slowly because our schedules didn't align at all) and was playing with the idea that maybe we can host a for-purpose repo at @ShibeTechnology, to stage work and collab?

lmk if that's needed. If you already have a staging repo, please lmk and I'll help with the refinement part there, as asked above.

@alamshafil
Copy link
Contributor

I personally have been working on a local branch for testing. Much progress hasn't been made since the my last post. The code right now is a mess because I hoped to get something basic working then refine it. Much discussion hasn't been made outside of GitHub.

I feel like having a dedicated repo will make collaborative work easier.

@patricklodder
Copy link
Member

I feel like having a dedicated repo will make collaborative work easier.

@gewure @envisiondata @nformant1 - could you please share your thoughts? If you agree with Shafil, I'll help setting this up.

@envisiondata
Copy link

@patricklodder I'm on a side project taking up my weekends and have not done anything on this.

@patricklodder
Copy link
Member

@envisiondata that's ok, evolution not revolution. If there is a staging area you can simply contribute there ❤️

@gewure
Copy link

gewure commented Oct 21, 2022

Hey there, lately been a little busy - but am now again looking into this. I shortly discussed the implementation with @alamshafil and it looks good what he done as far as i can say - i try to be on the discord frequently.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

No branches or pull requests

6 participants
@patricklodder @envisiondata @gewure @alamshafil @nformant1 and others