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

Extend the case-sensitive checksum to ICAPs #70

Closed
simenfd opened this issue Feb 28, 2016 · 6 comments
Closed

Extend the case-sensitive checksum to ICAPs #70

simenfd opened this issue Feb 28, 2016 · 6 comments
Labels

Comments

@simenfd
Copy link

simenfd commented Feb 28, 2016

Abstract

Extend the Yet another cool checksum address encoding #55 to ICAP addresses

Rationale

The ICAP format has both a Direct and Basic format, which only differs by one character in length; this is risky, given the weak checksum of only 6.6 bits (two digits: 00-99). There are real risks that someone might add a character by mistake and accidentally create a valid but unintended ICAP. Similar issue was raised here

IBANs are by specifications case insensitive, making it backwards compatible to alter the case for the purpose of checksums.

Implementation

Applying the Yet another checksum to the ICAP Direct and Basic. The case-change should only be applied to the "address" section of the ICAP, not the 4 first characters. In effect, the final ICAP will look like this:

Basic (34 chars):
XE7338O073kYGtWWzN0f2Wz0R8Px5ZPpZs

Direct (35 chars):
XE73038O073kYGtWwZN0f2Wz0R8pX5ZpPzS

Note: The change in case above is only for illustration, I haven't really calculated the real checksum. That should be trivial if people find this EIP useful.

The problems that this page raises, that an user enters one extra character without changing the checksum, is in this proposal almost reduced to zero.

The number of checksum bits increases from 6.5 bits to 6.5 + 30(26/36) ~ 28 bit => 1/268 million*

@simenfd simenfd changed the title Extend the case-sensitive-checksum to ICAPs Extend the case-sensitive checksum to ICAPs Feb 28, 2016
@axic
Copy link
Member

axic commented Feb 28, 2016

Some sources state that IBANs are case-insensitive (and many opensource tools implement them accordingly), yet this registry on SWIFT ( https://www.swift.com/sites/default/files/resources/swift_standards_ibanregistry.pdf ) suggest it might not be?

This should be answered before considering a case-sensitive scheme.

@simenfd
Copy link
Author

simenfd commented Feb 29, 2016

There are some noteworthy points here:http://www.boards.ie/vbulletin/showthread.php?t=2057209257

But the ISO specification (ISO 13616-1:2007) says:
In addition, given the possibility that domestic accounts may use lower case alpha characters, ISO 13616 remains unchanged on this point, i.e. lower case characters continue to be allowed, although the check digit algorithm will continue to be case independent.

Still, given what is stated in that forum, it seems like many interfaces will convert lowercase to uppercase in the banking system. One could make the case-validator ignore IBANs with all upper case characters to prevent problems with legacy bankingsoftware converting it. Though, I don't think this will be a real problem.

@simenfd
Copy link
Author

simenfd commented Mar 11, 2016

// Changes the case of a valid ICAP address to a checksumed one
func ICAPAddExtendedChecksum(s string) string {
    lowerICAP := strings.ToLower(s)
    upperICAP := strings.ToUpper(s)
    addressHash := Sha3([]byte(lowerICAP)) // Normalize to lower prior to hashing
    hexHash := hex.EncodeToString(addressHash)
    var checksumICAP string = upperICAP[:4] // XExx (where xx is checksum) is untouched
    for i := 4; i < len(lowerICAP); i++ {   // Start at the 4th character
        l, _ := strconv.ParseInt(string(hexHash[i]), 16, 16)
        if l > 7 {
            checksumICAP += string(upperICAP[i])
        } else {
            checksumICAP += string(lowerICAP[i])
        }
    }
    return checksumICAP
}

func ICAPValidateExtended(s string) error {
    // Validate the ordinary checksum
    _, err := ICAPToAddress(strings.ToUpper(s))
    if err != nil {
        return err
    }
    // Validate casing
    correctCasing := ICAPAddExtendedChecksum(s) // Recalculate
    for i := 0; i < len(s); i++ {
        if correctCasing[i] != s[i] {
            return errors.New("Invalid ICAP casing")
        }
    }
    return nil
}

Some test results:

    icap_test.go:71: Original: XE499OG1EH8ZZI0KXC6N83EKGT1BM97P2O7 ->
    icap_test.go:73: Checksum: XE499Og1eh8zzI0KxC6n83EkGT1bm97p2o7

    icap_test.go:71: Original: XE1222Q908LN1QBBU6XUQSO1OHWJIOS46OO ->
    icap_test.go:73: Checksum: XE1222Q908lN1qbbU6XUqSO1oHwJIOS46oo

    icap_test.go:71: Original: XE7338O073KYGTWWZN0F2WZ0R8PX5ZPPZS ->
    icap_test.go:73: Checksum: XE7338o073kYgTWwzN0F2wZ0r8Px5ZppZS

@wanderer wanderer added the ERC label Apr 14, 2016
@gavofyork
Copy link

makes a lot of sense. shouldn't be too hard to put into web3.js...

@github-actions
Copy link

There has been no activity on this issue for two months. It will be closed in a week if no further activity occurs. If you would like to move this EIP forward, please respond to any outstanding feedback or add a comment indicating that you have addressed all required feedback and are ready for a review.

@github-actions github-actions bot added the stale label Jan 17, 2022
@github-actions
Copy link

This issue was closed due to inactivity. If you are still pursuing it, feel free to reopen it and respond to any feedback or request a review in a comment.

bumblefudge added a commit to bumblefudge/EIPs that referenced this issue Feb 16, 2024
Chia Blockchain CAIP-2 specification
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

6 participants