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

uni code bloat #458

Open
matklad opened this issue Jul 5, 2021 · 1 comment
Open

uni code bloat #458

matklad opened this issue Jul 5, 2021 · 1 comment

Comments

@matklad
Copy link
Contributor

matklad commented Jul 5, 2021

We have this snippet of code in the sdk right now:

match value.to_lowercase().as_str() {
"ed25519" => Ok(CurveType::ED25519),
"secp256k1" => Ok(CurveType::SECP256K1),
_ => Err(ParsePublicKeyError { kind: ParsePublicKeyErrorKind::UnknownCurve }),
}

It has two problems. The minor one is that to_lowercase allocates a string, while we can do this comparison without allocations.

The bigger problem is that .to_lowercase() does unicode-aware case conversion. Such conversion requires a sizable unicode table:

https://github.com/rust-lang/rust/blob/5249414809d40fe22eca0c36105a2f71b9006e04/library/core/src/unicode/unicode_data.rs#L576-L1386

This table is a part of libcore, and gets linked with every contract. It's not possible to opt-out of unicode support (which is sort-of a design bug for core), but it can be eliminated by the linker if it isn't actually used. Naturally, .to_lowercase() uses the table and prevents the linker from eliminating it.

We should do the following:

@austinabell
Copy link
Contributor

This should be removed, but not going to close the issue yet because all code paths of the SDK haven't been tested that this isn't included

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: NEW❗
Development

No branches or pull requests

2 participants