Update Mnemonic creation to support arrays #687
Conversation
add array support and some code cleanup
cleanup and add support for array phrase
pharms-eth
changed the title
| } | ||
| private struct HDversion{ | ||
| public static var privatePrefix: Data? = Data.fromHex("0x0488ADE4") | ||
| public static var publicPrefix: Data? = Data.fromHex("0x0488B21E") |
There was a problem hiding this comment.
Any specific reason type was changed to Data??
Also please apply formating to the code.
| let hmacKey = "Bitcoin seed".data(using: .ascii)! | ||
| let hmac: Authenticator = HMAC(key: hmacKey.bytes, variant: HMAC.Variant.sha2(.sha512)) | ||
|
|
||
| guard let hmacKey = "Bitcoin seed".data(using: .ascii) else {return nil} |
There was a problem hiding this comment.
Please, add spaces near curly braces.
We are slowly but steadily updating such pieces of code.
| let hmac: Authenticator = HMAC(key: hmacKey.bytes, variant: HMAC.Variant.sha2(.sha512)) | ||
|
|
||
| guard let hmacKey = "Bitcoin seed".data(using: .ascii) else {return nil} | ||
| let hmac:Authenticator = HMAC(key: hmacKey.bytes, variant: HMAC.Variant.sha2(.sha512)) |
There was a problem hiding this comment.
Space is missing before type declaration: hmac:Authenticator
| get { | ||
| return privateKey != nil | ||
| } | ||
| } |
There was a problem hiding this comment.
I'd leave that in place only because it was public.
The fewer breaking changes we have the better.
Add a // TODO: remove with v4 to it.
| @@ -98,6 +97,18 @@ public class BIP32Keystore: AbstractKeystore { | |||
| try self.init(seed: seed, password: password, prefixPath: prefixPath, aesMode: aesMode) | |||
| } | |||
|
|
|||
| //TODO: Unit Test | |||
| //TODO: merge and cleanup with above code | |||
| public convenience init?(mnemonicsPhrase: [String], password: String, mnemonicsPassword: String = "", language: BIP39Language = .english, prefixPath: String = HDNode.defaultPathMetamaskPrefix, aesMode: String = "aes-128-cbc") throws { | |||
There was a problem hiding this comment.
Suggestion: add newline characters after each comma so that it's easier to read.
|
|
||
| public func deriveWithoutPrivateKey(index: UInt32, hardened: Bool = false) -> HDNode? { | ||
| var entropy: [UInt8] // derive public key when is itself public key | ||
| if index >= (UInt32(1) << 31) || hardened { |
There was a problem hiding this comment.
The (UInt32(1) << 31) is used 7 times across this file.
Let's move it out as a constant. Not a huge gain but at least we can assign a name to it like maxIterationIndex which is good.
| /** | ||
| Initializes a new mnemonics set with the provided bitsOfEntropy. | ||
| **/ |
There was a problem hiding this comment.
Something to remove.
| **/ | ||
| /// Initializes a new mnemonics set with the provided bitsOfEntropy. | ||
| /// - Parameters: | ||
| /// - bitsOfEntropy: 128 - 12 words, 192 - 18 words , 256 - 24 words in output. |
There was a problem hiding this comment.
18 words , 256 - redundant space.
| .compactMap { binary in | ||
| Int(binary, radix: 2) | ||
| } | ||
| .map { index in | ||
| language.words[index] | ||
| } |
There was a problem hiding this comment.
Formatting is required.
Please make sure all files you worked with are formatted.
| let status = SecRandomCopyBytes(kSecRandomDefault, entropy_bytes.count, &entropy_bytes) | ||
|
|
||
| if status != errSecSuccess { // Always test the status. | ||
|
|
There was a problem hiding this comment.
At least return nil is missing.
I'd also add NSLog with some Error: .... reason here (based on status) ....
| static func randomBytes(length: Int) -> Data? { | ||
| let entropy_bit_size = length//128 | ||
| //# valid_entropy_bit_sizes = [128, 160, 192, 224, 256], count: [12, 15, 18, 21, 24] | ||
| var entropy_bytes = [UInt8](repeating: 0, count: entropy_bit_size)// / 8) |
There was a problem hiding this comment.
Some more code to remove // / 8)
| } | ||
| return nil | ||
|
|
||
| let source1 = MTLCreateSystemDefaultDevice()?.makeBuffer(length: length)?.hash.description.data(using: .utf8) |
There was a problem hiding this comment.
I'm not sure but maybe you can answer: will it be compatible with all platforms we support?
Except for Linux of course.
I suppose it should be but can say for sure.
|
@pharms-eth Any updates? |
sorry just got back, will update this week |
|
@pharms-eth Any plans to update this PR? Let me know if you are not and/or will not be able to update it anytime soon. |
|
@pharms-eth I'm closing this PR as the same was implemented in #792 |
provide support for the Mnemonic string to be an array of the words