yenom / BitcoinKit

Bitcoin protocol toolkit for Swift
MIT License
841 stars 261 forks source link

Mnemonic: Exposing word lists and adding checksum validation. #227

Closed Sajjon closed 4 years ago

Sajjon commented 4 years ago

Description of the Change

Major cleanup of Mnemonic, especially generation of it.

I've exposed the Mnemonic word lists so that wallets can present them. Imagine a Restore Account with Mneemonic-flow inside a wallet, where the user needs to manually input the mnemonic, if we know the language, we can now present suggestions based on input and the word list. i.e. with user types "ca" we can display all words starting with "cat" (["cat", "catalog", "catch", "category", "cattle"]) to the user.

In accordance with BIP39 guidelines, I've added support for mnemonic checksum:

Although using a mnemonic not generated by the algorithm described in "Generating the mnemonic" section is possible, this is not advised and software must compute a checksum for the mnemonic sentence using a wordlist and issue a warning if it is invalid.

Checksum validation is by default turned on, so this initializer:

// "try" needed since we are passing a throwing closure (default argument), performing validation
try Mnemonic.seed(mnemonic: words)

is performing checksum validation, and throws an error if the mnemonic is not checksummed. I made this initializer rethrowing, which is neat because then we can get a non-throwing version of the same init by using this

// no "try" needed, since non-throwing closure passed, thus initializer is not (re-)throwing.
Mnemonic.seed(mnemonic: words) { _ in }

The checksum validation implementation is inspired by this python code, which I refer to in the function.

Alternate Designs

We could altogether remove the validation closure for the function Mnemonic.seed....

Benefits

We can now present the user with the warning about non checksummed mnemonics! Which the BIP39 suggests.

Possible Drawbacks

In order to make the code for the checksum validation as clean as possible I introduced two types, UInt11 and BitArray, resulting in more code, but shorter and more clear implementation of the validation method.

Applicable Issues

Mainly good for wallets, using mnemonic. Validation might also be good for (backend)services.

Sajjon commented 4 years ago

@usatie let me know if you want me to change file headers (probably right?)

usatie commented 4 years ago

@Sajjon Thanks for your PR! Yes, I want you to change the file header, and also please pass all the test both on apple platforms and Linux.

CI should be green 👍 https://travis-ci.org/yenom/BitcoinKit/jobs/586104674

usatie commented 4 years ago

I recommend you to re-create your branch from the current yenom/master branch. And then, add your files and changes.

In other words, I prefer rebase master over merge master.

Commit history with a lot of merge commits and unrelated commits like 9331179 and 83e9ab0 is not recommended.

Sajjon commented 4 years ago

@usatie Fixed!

However CI is failed, that is because Travis CI-pipeline is using Xcode 10.3, it needs to be using XCode 11 (GM seed 2 is available).

usatie commented 4 years ago

Thank you for your change!

Oh, you want to use Self, right? Actually, I ran into the same error just a minute ago in my branch and fixed :D

I think forcing all users to Xcode11 is too early, can you fix the part to pass the test with Xcode10.3?

usatie commented 4 years ago

You can debug the test on Linux by swift test command :D

Sajjon commented 4 years ago

@usatie So! I think I've fixed everything, also, thanks to the two new types UInt11 and BitArray, I've managed to do a major cleanup of Mnemonic Generation, from this (IMO) not-so-readable code:

Earlier

static func generate(entropy: Data, language: Language = .english) -> [String] {
    let list = wordList(for: language)
    var bin = String(entropy.flatMap { ("00000000" + String($0, radix: 2)).suffix(8) })

    let hash = Crypto.sha256(entropy)
    let bits = entropy.count * 8
    let cs = bits / 32

    let hashbits = String(hash.flatMap { ("00000000" + String($0, radix: 2)).suffix(8) })
    let checksum = String(hashbits.prefix(cs))
    bin += checksum

    var mnemonic = [String]()
    for i in 0..<(bin.count / 11) {
        let wi = Int(bin[bin.index(bin.startIndex, offsetBy: i * 11)..<bin.index(bin.startIndex, offsetBy: (i + 1) * 11)], radix: 2)!
        mnemonic.append(String(list[wi]))
    }
    return mnemonic
}

Now

static func generate(entropy: Data, language: Language = .english) throws -> [String] {

    guard let strength = Mnemonic.Strength(byteCount: entropy.count) else {
        throw Error.unsupportedByteCountOfEntropy(got: entropy.count)
    }

    let words = wordList(for: language)
    let hash = Crypto.sha256(entropy)

    let checkSumBits = BitArray(data: hash).prefix(strength.checksumLengthInBits)

    let bits = BitArray(data: entropy) + checkSumBits
    let wordIndices = bits.splitIntoChunks(ofSize: Mnemonic.WordList.sizeLog2)
        .map { UInt11(bitArray: $0)! }
        .map { $0.asInt }

    let mnemonic = wordIndices.map { words[$0] }

    try validateChecksumOf(mnemonic: mnemonic, language: language)
    return mnemonic
}

Which gives a much more clear intent of what is going on, and also avoids this long and actually completely unreadable oneliner: let wi = Int(bin[bin.index(bin.startIndex, offsetBy: i * 11)..<bin.index(bin.startIndex, offsetBy: (i + 1) * 11)], radix: 2)!

Apart from that we also perform checksum assertion and length of entropy assertion, with clear errors thrown.

Feedback is most welcome!

Sajjon commented 4 years ago

@usatie also added IDETemplateMacros.plist, to enforce same file header for all collaborators.

Sajjon commented 4 years ago

Finally found all compilation errors caused by Swift 5.1 (Xcode 11) syntax. So now Travis passes all tests :D