Skip to content

Let's spell stripped correctly #747

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

Merged
merged 2 commits into from
Feb 1, 2023

Conversation

cclauss
Copy link
Contributor

@cclauss cclauss commented Jan 31, 2023

Summary of Changes

Fixes # (if applicable - add the number of issue this PR addresses)

Test Data or Screenshots

By submitting this pull request, you are confirming the following:
  • I have reviewed the Contribution Guidelines.
  • I have performed a self-review of my own code.
  • I have updated my repository to match the develop branch.
  • I have included test data or screenshots that prove my fix is effective or that my feature works.
  • I have checked that all tests work and swiftlint is not throwing any errors/warnings.

@cclauss
Copy link
Contributor Author

cclauss commented Jan 31, 2023

Should this be stripped or striped or some other word?

@JeneaVranceanu
Copy link
Collaborator

I'd actually name it publicKey as well.
Probably, it was intended to be named stripped as in an uncompressed form we remove the first byte of 65 bytes as it's a static prefix 0x04.

According to https://www.oreilly.com/library/view/mastering-ethereum/9781491971932/ch04.html (search for Table 4-1. Serialized EC public key prefixes) Ethereum only uses uncompressed public keys of 65 bytes in length where the first byte is always 04 and the compressed public keys have either 02 or 03 prefix. I'd suggest modifying the whole function to the following:

/// Convert a public key to the corresponding ``EthereumAddress``. Accepts public keys in compressed (33 bytes), uncompressed (65 bytes)
/// or uncompressed without prefix (64 bytes) format.
///
/// - Parameter publicKey: compressed 33, non-compressed (65 bytes) or non-compressed without prefix (64 bytes)
/// - Returns: 20 bytes of address data.
static func publicToAddressData(_ publicKey: Data) -> Data? {
    var publicKey = publicKey
    if publicKey.count == 33 {
        guard (publicKey[0] == 2 || publicKey[0] == 3),
              let decompressedKey = SECP256K1.combineSerializedPublicKeys(keys: [publicKey], outputCompressed: false) else {
            return nil
        }
        publicKey = decompressedKey
    }
    if publicKey.count == 65 {
        guard publicKey[0] == 4 else {
            return nil
        }
        publicKey = publicKey[1...64]
    } else if publicKey.count != 64 {
        return nil
    }
    let sha3 = publicKey.sha3(.keccak256)
    let addressData = sha3[12...31]
    return addressData
}

@JeneaVranceanu
Copy link
Collaborator

@cclauss I've updated your PR. Will push test cases in a separate one.

@JeneaVranceanu
Copy link
Collaborator

Test cases added in #751

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants