Closed ypopovych closed 5 years ago
Merging #77 into master will increase coverage by
0.2%
. The diff coverage is68.55%
.
@@ Coverage Diff @@
## master #77 +/- ##
=========================================
+ Coverage 75.14% 75.35% +0.2%
=========================================
Files 64 64
Lines 4152 4187 +35
=========================================
+ Hits 3120 3155 +35
Misses 1032 1032
Impacted Files | Coverage Δ | |
---|---|---|
Web3/Classes/Keychain/Secp256k1+CTXCreator.swift | 68.42% <ø> (ø) |
|
Web3/Classes/PromiseKit/Promisable.swift | 100% <ø> (ø) |
:arrow_up: |
...s/Core/Json/EthereumTransactionReceiptObject.swift | 0% <0%> (ø) |
:arrow_up: |
...3/Classes/ContractABI/Contract/SolidityEvent.swift | 64.44% <0%> (ø) |
:arrow_up: |
Web3/Classes/ContractABI/ABI/Eth+ABI.swift | 0% <0%> (ø) |
:arrow_up: |
Web3/Classes/Core/Json/EthereumLogObject.swift | 0% <0%> (ø) |
:arrow_up: |
Web3/Classes/Core/Toolbox/BytesConvertible.swift | 0% <0%> (ø) |
:arrow_up: |
...Classes/Core/Toolbox/String+BytesConvertible.swift | 78.57% <0%> (ø) |
:arrow_up: |
Web3/Classes/ContractABI/Contract/ERC165.swift | 0% <0%> (ø) |
:arrow_up: |
.../Classes/Core/Json/EthereumTransactionObject.swift | 50% <0%> (-22.23%) |
:arrow_down: |
... and 35 more |
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 d1cbe0a...08a132f. Read the comment docs.
all tests passed
@Ybrin @pixelmatrix guys, can we start the process of merging? I will need the updated library in my own libraries, and I don't want to fork it. It's better to update yours.
The Swift 5 stuff looks good to me. My only suggestion there is that Hashable is able to be computed for many custom structs now, so many of the custom hash(into:) methods may be unnecessary.
Personally I'd prefer this PR only introduces a single change (Swift 5 OR Keychain changes), but this is much cleaner than the previous one. I'm not sure how @Ybrin will feel about the Keychain stuff and splitting it into more submodules, but this seems fine to me.
About the logic behind Keychain extraction.
We have our own keychain implementation (with Rust based HD wallet). And in current realization, Web3 adds a dependency to secp256k1.swift
, which we will never use. If we will split it, it can be used in both ways.
@Ybrin @pixelmatrix any news?
@ypopovych
Updated PR with compatibility with Swift 5 only.
Also moved
EthereumPrivateKey
andEthereumPublicKey
to own package. For someone who wants to use the library with own signature/key management parts. Enabled by default for CocoaPods.