Skip to content
This repository has been archived by the owner on May 28, 2019. It is now read-only.

Try to fix PublicKey.compressed, add more tests #57

Merged
merged 3 commits into from
Sep 13, 2018

Conversation

hewigovens
Copy link
Contributor

check y is even or odd

@codecov-io
Copy link

codecov-io commented Sep 12, 2018

Codecov Report

Merging #57 into master will increase coverage by 0.06%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #57      +/-   ##
==========================================
+ Coverage    81.2%   81.27%   +0.06%     
==========================================
  Files          74       74              
  Lines        2943     2953      +10     
==========================================
+ Hits         2390     2400      +10     
  Misses        553      553
Impacted Files Coverage Δ
Tests/PurposeTests.swift 100% <ø> (ø) ⬆️
Tests/VeChain/VechainTransactionTests.swift 100% <100%> (ø) ⬆️
Tests/HDWalletTests.swift 100% <100%> (ø) ⬆️
Tests/Bitcoin/BitcoinAddressTests.swift 100% <100%> (ø) ⬆️
Sources/PublicKey.swift 76% <100%> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update fb4db48...7091445. Read the comment docs.

XCTAssertEqual(uncompressed2.compressed.legacyBitcoinAddress(prefix: 0).description, "1J7mdg5rbQyUHENYdx39WVWK7fsLpEoXZy")

let uncompressed3 = PublicKey(data: Data(hexString: "042de45bea3dada528eee8a1e04142d3e04fad66119d971b6019b0e3c02266b79142158aa83469db1332a880a2d5f8ce0b3bba542b3e32df0740ccbfb01c275e42")!)!
XCTAssertTrue(!uncompressed3.isCompressed)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you use XCTAssertFalse instead

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

@@ -38,7 +38,7 @@ public struct PublicKey: Hashable, CustomStringConvertible {
if isCompressed {
return self
}
let prefix: UInt8 = 0x02 | (data[1] & 0x01)
let prefix: UInt8 = 0x02 | (data[64] & 0x01)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

from:
https://bitcoin.stackexchange.com/questions/76181/bitcoin-public-keys-from-uncompressed-to-compressed-version-with-code-sample/76183#76183
Compression is very simply, if the Y value of the pubkey is odd, place an 0x03 byte. If it is even, place a 0x02 byte

And we been using 0x02 and 0x01, can you explain this part?

Copy link
Contributor Author

@hewigovens hewigovens Sep 12, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. & | are bitwise operators, here is for fast computing
  2. data[64] & 0x01 tests if y is even or odd (you convert it to binary and do bitwise AND)
  3. 0x02 | does bitwise OR 0 / 1 (y) and results 0x02 | 0x03.

@@ -33,7 +33,8 @@ class HDWalletTests: XCTestCase {
func testDeriveBitcoin() {
let wallet = HDWallet(mnemonic: words, passphrase: passphrase)
let key = wallet.getKey(at: Bitcoin().derivationPath(at: 0))
let publicKey = key.publicKey(compressed: true)
let publicKey = key.publicKey().compressed
XCTAssertEqual(publicKey.description, key.publicKey(compressed: true).description)

XCTAssertEqual("026fc80db3a34e7c6bfc6d7d9a53aeba9e706b309c9cf7ee96fa9c36ff7fd92a20", publicKey.description)
XCTAssertEqual("3FJjnZNXC6FWQ2UJAaKL3Vme2EJavfgnXe", publicKey.bitcoinAddress(prefix: Bitcoin().payToScriptHashAddressPrefix).description)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

publicKey.bitcoinAddress(prefix: Bitcoin().payToScriptHashAddressPrefix)
=>
key.publicKey().bitcoinAddress(prefix: Bitcoin().payToScriptHashAddressPrefix)

Can we do it this way? Our goal to make sure it will be using compressed version without specifying it previously

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it would break others if publicKey() is compressed by default, e.g ethereum.

@hewigovens hewigovens merged commit f9cdd2d into master Sep 13, 2018
@vikmeup vikmeup deleted the pubkey-compressed branch September 13, 2018 06:49
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.

None yet

4 participants