keyring: remove hardcoded default passphrase on NewMnemonic #8662
Conversation
fedekunze
requested review from
aaronc,
alessio and
alexanderbez
as code owners
fedekunze
added
T: API Breaking
crypto/keyring/keyring.go
Outdated
| @@ -498,12 +501,16 @@ func (ks keystore) NewMnemonic(uid string, language Language, hdPath string, alg | |||
| return nil, "", err | |||
| } | |||
|
|
|||
| info, err := ks.NewAccount(uid, mnemonic, DefaultBIP39Passphrase, hdPath, algo) | |||
| if passphrase == "default" { | |||
There was a problem hiding this comment.
Mmmm. Why don't we rely on the zero value ""?
There was a problem hiding this comment.
in case the default passphrase changes
There was a problem hiding this comment.
so I think we'll need to modify the command line interfaces as well, correct?
…s-sdk into fedekunze/8635-key-mnemonic
|
Test cases need update |
|
i would also suggest possibly removing the |
Why don't you use the bip39 library instead if that's your use case? |
I think the same argument can be made about why |
…s-sdk into fedekunze/8635-key-mnemonic
…s-sdk into fedekunze/8635-key-mnemonic
fedekunze
added
the
A:automerge
Codecov Report
@@ Coverage Diff @@
## master #8662 +/- ##
==========================================
- Coverage 61.47% 61.46% -0.02%
==========================================
Files 659 659
Lines 37916 37921 +5
==========================================
- Hits 23308 23307 -1
- Misses 12168 12171 +3
- Partials 2440 2443 +3
|
closes #8635