Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(EIP712): parsing of TypedData payload; encoding + hashing; #833

Merged
Show file tree
Hide file tree
Changes from 16 commits
Commits
Show all changes
24 commits
Select commit Hold shift + click to select a range
354589f
feat(EIP712): parsing of TypedData payload; encoding + hashing;
JeneaVranceanu Oct 18, 2023
0342f91
featEIP712): parser test
JeneaVranceanu Oct 18, 2023
a9b23fb
chore(EIP712): renamed test file
JeneaVranceanu Oct 18, 2023
ce459b6
feat(EIP712): type encoding, type hashing, circular dependency checks…
JeneaVranceanu Oct 18, 2023
89595ad
chore(EIP712): added link to the tests + implementation source
JeneaVranceanu Oct 18, 2023
27d5b61
feat(EIP712): impl of encodeData, structHash and signHash functions f…
JeneaVranceanu Oct 18, 2023
07965a1
fix: typo
JeneaVranceanu Oct 18, 2023
5493dd0
chore(EIP712): new test + moved EIP712 reused data into one file;
JeneaVranceanu Oct 18, 2023
4669bfe
fix(EIP712): fixed values in tests since personal-message-hashing is …
JeneaVranceanu Oct 19, 2023
b8e55b2
chore(EIP712): example for EIP712Parser;
JeneaVranceanu Oct 19, 2023
c01f6a1
fix: ParsingError and AbstractKeystoreError are now extensions of Loc…
JeneaVranceanu Oct 31, 2023
41828fb
feat: support for eth_signTypedDataV4 payload parsing
JeneaVranceanu Nov 3, 2023
c6da90f
chore: make parsing errors more granular
JeneaVranceanu Nov 3, 2023
e40c3f8
fix: order custom subtypes (no primitives!) in alphabetical order whe…
JeneaVranceanu Nov 3, 2023
4990084
chore: added new test for Invalid Order Signature error
JeneaVranceanu Nov 3, 2023
d69d594
fix: updated test case to match the expected result
JeneaVranceanu Nov 3, 2023
40a9a3d
chore: updated error message for entropyOf
JeneaVranceanu Nov 3, 2023
cfd8ee9
chore: updated error messages for EIP712Parser
JeneaVranceanu Nov 3, 2023
0b0b3fa
chore: error message fix
JeneaVranceanu Nov 3, 2023
68d8457
chore: updated error message
JeneaVranceanu Nov 4, 2023
111d33e
chore: docs update
JeneaVranceanu Nov 4, 2023
e77af0a
fix: EIP712 - change type to AbstractKeystore from BIP32Keystore
JeneaVranceanu Nov 7, 2023
bd1544d
Merge branch 'feat/eip712-dynamic-parsing' of github.com:JeneaVrancea…
JeneaVranceanu Nov 7, 2023
6cc06db
fix: parsing and encoding of "bytes"
JeneaVranceanu Nov 20, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 28 additions & 3 deletions Sources/Web3Core/EthereumABI/ABIParsing.swift
Original file line number Diff line number Diff line change
Expand Up @@ -7,16 +7,41 @@ import Foundation

extension ABI {

public enum ParsingError: Swift.Error {
public enum ParsingError: LocalizedError {
case invalidJsonFile
case elementTypeInvalid
case elementTypeInvalid(_ desc: String? = nil)
case elementNameInvalid
case functionInputInvalid
case functionOutputInvalid
case eventInputInvalid
case parameterTypeInvalid
case parameterTypeNotFound
case abiInvalid
Comment on lines 11 to 19
Copy link
Collaborator Author

@JeneaVranceanu JeneaVranceanu Nov 3, 2023

Choose a reason for hiding this comment

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

I've decided not to add (_ desc: String? = nil) to all error types as it would trigger more changes across the library.
It's better to do that updated in a separate PR.


public var errorDescription: String? {
var errorMessage: [String?]
switch self {
case .invalidJsonFile:
errorMessage = ["invalidJsonFile"]
case .elementTypeInvalid(let desc):
errorMessage = ["elementTypeInvalid", desc]
Copy link
Collaborator

Choose a reason for hiding this comment

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

I wonder if this line is a cause that we're messing here with an array of Strings rather a plain String?

case .elementNameInvalid:
errorMessage = ["elementNameInvalid"]
case .functionInputInvalid:
errorMessage = ["functionInputInvalid"]
case .functionOutputInvalid:
errorMessage = ["functionOutputInvalid"]
case .eventInputInvalid:
errorMessage = ["eventInputInvalid"]
case .parameterTypeInvalid:
errorMessage = ["parameterTypeInvalid"]
case .parameterTypeNotFound:
errorMessage = ["parameterTypeNotFound"]
case .abiInvalid:
errorMessage = ["abiInvalid"]
}
return errorMessage.compactMap { $0 }.joined(separator: " ")
}
}

enum TypeParsingExpressions {
Expand All @@ -39,7 +64,7 @@ extension ABI.Record {
public func parse() throws -> ABI.Element {
let typeString = self.type ?? "function"
guard let type = ABI.ElementType(rawValue: typeString) else {
throw ABI.ParsingError.elementTypeInvalid
throw ABI.ParsingError.elementTypeInvalid("Invalid ABI type \(typeString).")
}
return try parseToElement(from: self, type: type)
}
Expand Down
2 changes: 1 addition & 1 deletion Sources/Web3Core/EthereumABI/ABITypeParser.swift
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ public struct ABITypeParser {

public static func parseTypeString(_ string: String) throws -> ABI.Element.ParameterType {
let (type, tail) = recursiveParseType(string)
guard let t = type, tail == nil else {throw ABI.ParsingError.elementTypeInvalid}
guard let t = type, tail == nil else { throw ABI.ParsingError.elementTypeInvalid("Invalid ABI type \(string).") }
return t
}

Expand Down
31 changes: 25 additions & 6 deletions Sources/Web3Core/KeystoreManager/AbstractKeystore.swift
Original file line number Diff line number Diff line change
Expand Up @@ -11,11 +11,30 @@ public protocol AbstractKeystore {
func UNSAFE_getPrivateKeyData(password: String, account: EthereumAddress) throws -> Data
}

public enum AbstractKeystoreError: Error {
case noEntropyError
case keyDerivationError
case aesError
case invalidAccountError
public enum AbstractKeystoreError: LocalizedError {
case noEntropyError(_ additionalDescription: String? = nil)
case keyDerivationError(_ additionalDescription: String? = nil)
case aesError(_ additionalDescription: String? = nil)
case invalidAccountError(_ additionalDescription: String? = nil)
case invalidPasswordError
case encryptionError(String)
case encryptionError(_ additionalDescription: String? = nil)

public var errorDescription: String? {
var errorMessage: [String?]
switch self {
case .noEntropyError(let additionalDescription):
errorMessage = ["Entropy error (e.g. failed to generate a random array of bytes).", additionalDescription]
Copy link
Collaborator

Choose a reason for hiding this comment

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

I wonder if this line is a cause that we're messing here with an array of Strings rather a plain String? Here it looks quite more necessary than in the previous alike code snippet, yet I wonder if it's a best way in terms of code intention clarity to implement such behaviour?

case .keyDerivationError(let additionalDescription):
errorMessage = ["Key derivation error.", additionalDescription]
case .aesError(let additionalDescription):
errorMessage = ["AES error.", additionalDescription]
case .invalidAccountError(let additionalDescription):
errorMessage = ["Invalid account error.", additionalDescription]
case .invalidPasswordError:
errorMessage = ["Invalid password error."]
case .encryptionError(let additionalDescription):
errorMessage = ["Encryption error.", additionalDescription]
}
return errorMessage.compactMap { $0 }.joined(separator: " ")
}
}
115 changes: 57 additions & 58 deletions Sources/Web3Core/KeystoreManager/BIP32Keystore.swift

Large diffs are not rendered by default.

8 changes: 5 additions & 3 deletions Sources/Web3Core/KeystoreManager/BIP39.swift
Original file line number Diff line number Diff line change
Expand Up @@ -95,11 +95,13 @@ public class BIP39 {
}

private static func entropyOf(size: Int) throws -> Data {
let isCorrectSize = size >= 128 && size <= 256 && size.isMultiple(of: 32)
let randomBytesCount = size / 8
guard
size >= 128 && size <= 256 && size.isMultiple(of: 32),
let entropy = Data.randomBytes(length: size/8)
isCorrectSize,
Copy link
Collaborator

Choose a reason for hiding this comment

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

Although it got better, it still doesn't fit with our convention of guard else formatting.

let entropy = Data.randomBytes(length: randomBytesCount)
else {
throw AbstractKeystoreError.noEntropyError
throw AbstractKeystoreError.noEntropyError("BIP39. \(!isCorrectSize ? "Requested entropy of wrong bits size \(size)." : "Failed to generated \(randomBytesCount) of random bytes.")")
}
return entropy
}
Expand Down
62 changes: 27 additions & 35 deletions Sources/Web3Core/KeystoreManager/EthereumKeystoreV3.swift
Original file line number Diff line number Diff line change
Expand Up @@ -23,13 +23,13 @@ public class EthereumKeystoreV3: AbstractKeystore {
}

public func UNSAFE_getPrivateKeyData(password: String, account: EthereumAddress) throws -> Data {
if self.addresses?.count == 1 && account == self.addresses?.last {
guard let privateKey = try? self.getKeyData(password) else {
if account == addresses?.last {
guard let privateKey = try? getKeyData(password) else {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Maybe we can make this bit a bit more straightforward? Like changing the if let above to an another guard statement.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Upd: oh, I see that the execution flow continues after that line:32 throw statement. Maybe we should add at least TODO: refactor me at the very beginning of the whole file, as I believe this function should be split in to, but not this time?

throw AbstractKeystoreError.invalidPasswordError
}
return privateKey
}
throw AbstractKeystoreError.invalidAccountError
throw AbstractKeystoreError.invalidAccountError("EthereumKeystoreV3. Cannot get private key: keystore doesn't contain information about given address \(account.address).")
}

// Class
Expand Down Expand Up @@ -77,7 +77,7 @@ public class EthereumKeystoreV3: AbstractKeystore {
defer {
Data.zero(&newPrivateKey)
}
try encryptDataToStorage(password, keyData: newPrivateKey, aesMode: aesMode)
try encryptDataToStorage(password, privateKey: newPrivateKey, aesMode: aesMode)
}

public init?(privateKey: Data, password: String, aesMode: String = "aes-128-cbc") throws {
Expand All @@ -87,68 +87,60 @@ public class EthereumKeystoreV3: AbstractKeystore {
guard SECP256K1.verifyPrivateKey(privateKey: privateKey) else {
return nil
}
try encryptDataToStorage(password, keyData: privateKey, aesMode: aesMode)
try encryptDataToStorage(password, privateKey: privateKey, aesMode: aesMode)
}

fileprivate func encryptDataToStorage(_ password: String, keyData: Data?, dkLen: Int = 32, N: Int = 4096, R: Int = 6, P: Int = 1, aesMode: String = "aes-128-cbc") throws {
if keyData == nil {
throw AbstractKeystoreError.encryptionError("Encryption without key data")
fileprivate func encryptDataToStorage(_ password: String, privateKey: Data, dkLen: Int = 32, N: Int = 4096, R: Int = 6, P: Int = 1, aesMode: String = "aes-128-cbc") throws {
if privateKey.count != 32 {
throw AbstractKeystoreError.encryptionError("EthereumKeystoreV3. Attempted encryption with private key of length != 32. Given private key length is \(privateKey.count).")
}
let saltLen = 32
guard let saltData = Data.randomBytes(length: saltLen) else {
throw AbstractKeystoreError.noEntropyError
throw AbstractKeystoreError.noEntropyError("EthereumKeystoreV3. Failed to generate random bytes: `Data.randomBytes(length: \(saltLen))`.")
}
guard let derivedKey = scrypt(password: password, salt: saltData, length: dkLen, N: N, R: R, P: P) else {
throw AbstractKeystoreError.keyDerivationError
throw AbstractKeystoreError.keyDerivationError("EthereumKeystoreV3. Scrypt function failed.")
}
let last16bytes = Data(derivedKey[(derivedKey.count - 16)...(derivedKey.count - 1)])
let encryptionKey = Data(derivedKey[0...15])
guard let IV = Data.randomBytes(length: 16) else {
throw AbstractKeystoreError.noEntropyError
throw AbstractKeystoreError.noEntropyError("EthereumKeystoreV3. Failed to generate random bytes: `Data.randomBytes(length: 16)`.")
}
var aesCipher: AES?
switch aesMode {
var aesCipher: AES
switch aesMode.lowercased() {
case "aes-128-cbc":
aesCipher = try? AES(key: encryptionKey.bytes, blockMode: CBC(iv: IV.bytes), padding: .noPadding)
aesCipher = try AES(key: encryptionKey.bytes, blockMode: CBC(iv: IV.bytes), padding: .noPadding)
case "aes-128-ctr":
aesCipher = try? AES(key: encryptionKey.bytes, blockMode: CTR(iv: IV.bytes), padding: .noPadding)
aesCipher = try AES(key: encryptionKey.bytes, blockMode: CTR(iv: IV.bytes), padding: .noPadding)
default:
aesCipher = nil
throw AbstractKeystoreError.aesError("EthereumKeystoreV3. AES error: given AES mode can be one of 'aes-128-cbc' or 'aes-128-ctr'. Instead '\(aesMode)' was given.")
}
if aesCipher == nil {
throw AbstractKeystoreError.aesError
}
guard let encryptedKey = try aesCipher?.encrypt(keyData!.bytes) else {
throw AbstractKeystoreError.aesError
}
let encryptedKeyData = Data(encryptedKey)
var dataForMAC = Data()
dataForMAC.append(last16bytes)
dataForMAC.append(encryptedKeyData)

let encryptedKeyData = Data(try aesCipher.encrypt(privateKey.bytes))
let dataForMAC = last16bytes + encryptedKeyData
let mac = dataForMAC.sha3(.keccak256)
let kdfparams = KdfParamsV3(salt: saltData.toHexString(), dklen: dkLen, n: N, p: P, r: R, c: nil, prf: nil)
let cipherparams = CipherParamsV3(iv: IV.toHexString())
let crypto = CryptoParamsV3(ciphertext: encryptedKeyData.toHexString(), cipher: aesMode, cipherparams: cipherparams, kdf: "scrypt", kdfparams: kdfparams, mac: mac.toHexString(), version: nil)
guard let pubKey = Utilities.privateToPublic(keyData!) else {
throw AbstractKeystoreError.keyDerivationError
guard let publicKey = Utilities.privateToPublic(privateKey) else {
throw AbstractKeystoreError.keyDerivationError("EthereumKeystoreV3. Failed to derive public key from given private key. `Utilities.privateToPublic(privateKey)` returned `nil`.")
}
guard let addr = Utilities.publicToAddress(pubKey) else {
throw AbstractKeystoreError.keyDerivationError
guard let addr = Utilities.publicToAddress(publicKey) else {
throw AbstractKeystoreError.keyDerivationError("EthereumKeystoreV3. Failed to derive address from derived public key. `Utilities.publicToAddress(publicKey)` returned `nil`.")
}
self.address = addr
let keystoreparams = KeystoreParamsV3(address: addr.address.lowercased(), crypto: crypto, id: UUID().uuidString.lowercased(), version: 3)
self.keystoreParams = keystoreparams
}

public func regenerate(oldPassword: String, newPassword: String, dkLen: Int = 32, N: Int = 4096, R: Int = 6, P: Int = 1) throws {
var keyData = try self.getKeyData(oldPassword)
if keyData == nil {
throw AbstractKeystoreError.encryptionError("Failed to decrypt a keystore")
guard var privateKey = try getKeyData(oldPassword) else {
throw AbstractKeystoreError.encryptionError("EthereumKeystoreV3. Failed to decrypt a keystore")
}
defer {
Data.zero(&keyData!)
Data.zero(&privateKey)
}
try self.encryptDataToStorage(newPassword, keyData: keyData!, aesMode: self.keystoreParams!.crypto.cipher)
try self.encryptDataToStorage(newPassword, privateKey: privateKey, aesMode: self.keystoreParams!.crypto.cipher)
}

fileprivate func getKeyData(_ password: String) throws -> Data? {
Expand Down
2 changes: 1 addition & 1 deletion Sources/Web3Core/KeystoreManager/KeystoreManager.swift
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ public class KeystoreManager: AbstractKeystore {

public func UNSAFE_getPrivateKeyData(password: String, account: EthereumAddress) throws -> Data {
guard let keystore = walletForAddress(account) else {
throw AbstractKeystoreError.invalidAccountError
throw AbstractKeystoreError.invalidAccountError("KeystoreManager: no keystore/wallet found for given address. Address `\(account.address)`.")
}
return try keystore.UNSAFE_getPrivateKeyData(password: password, account: account)
}
Expand Down
2 changes: 1 addition & 1 deletion Sources/Web3Core/Transaction/CodableTransaction.swift
Original file line number Diff line number Diff line change
Expand Up @@ -156,7 +156,7 @@ public struct CodableTransaction {
let result = self.attemptSignature(privateKey: privateKey, useExtraEntropy: useExtraEntropy)
if result { return }
}
throw AbstractKeystoreError.invalidAccountError
throw AbstractKeystoreError.invalidAccountError("Failed to sign transaction with given private key.")
}

// actual signing algorithm implementation
Expand Down
2 changes: 1 addition & 1 deletion Sources/Web3Core/Utility/String+Extension.swift
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,7 @@ extension String {
let to16 = utf16.index(utf16.startIndex, offsetBy: nsRange.location + nsRange.length, limitedBy: utf16.endIndex),
let from = from16.samePosition(in: self),
let to = to16.samePosition(in: self)
else { return nil }
else { return nil }
return from ..< to
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,12 @@ public class EIP712 {
public typealias Bytes = Data
}

// FIXME: this type is wrong - The minimum number of optional fields is 5, and those are
// string name the user readable name of signing domain, i.e. the name of the DApp or the protocol.
// string version the current major version of the signing domain. Signatures from different versions are not compatible.
// uint256 chainId the EIP-155 chain id. The user-agent should refuse signing if it does not match the currently active chain.
// address verifyingContract the address of the contract that will verify the signature. The user-agent may do contract specific phishing prevention.
// bytes32 salt an disambiguating salt for the protocol. This can be used as a domain separator of last resort.
public struct EIP712Domain: EIP712Hashable {
Comment on lines +19 to 25
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This is something to deal with later.

public let chainId: EIP712.UInt256?
public let verifyingContract: EIP712.Address
Expand Down Expand Up @@ -54,6 +60,8 @@ public extension EIP712Hashable {
result = ABIEncoder.encodeSingleType(type: .uint(bits: 256), value: field)!
case is EIP712.Address:
result = ABIEncoder.encodeSingleType(type: .address, value: field)!
case let boolean as Bool:
result = ABIEncoder.encodeSingleType(type: .uint(bits: 8), value: boolean ? 1 : 0)!
case let hashable as EIP712Hashable:
result = try hashable.hash()
default:
Expand All @@ -64,16 +72,19 @@ public extension EIP712Hashable {
preconditionFailure("Not solidity type")
}
}
guard result.count == 32 else { preconditionFailure("ABI encode error") }
guard result.count % 32 == 0 else { preconditionFailure("ABI encode error") }
parameters.append(result)
}
return Data(parameters.flatMap { $0.bytes }).sha3(.keccak256)
}
}

public func eip712encode(domainSeparator: EIP712Hashable, message: EIP712Hashable) throws -> Data {
let data = try Data([UInt8(0x19), UInt8(0x01)]) + domainSeparator.hash() + message.hash()
return data.sha3(.keccak256)
public func eip712hash(domainSeparator: EIP712Hashable, message: EIP712Hashable) throws -> Data {
try eip712hash(domainSeparatorHash: domainSeparator.hash(), messageHash: message.hash())
}

public func eip712hash(domainSeparatorHash: Data, messageHash: Data) -> Data {
(Data([UInt8(0x19), UInt8(0x01)]) + domainSeparatorHash + messageHash).sha3(.keccak256)
}

// MARK: - Additional private and public extensions with support members
Expand Down
Loading