Skip to content
This repository has been archived by the owner on Nov 15, 2023. It is now read-only.

Using array-bytes for Array/Bytes/Hex Operation #12111

Closed
AurevoirXavier opened this issue Aug 25, 2022 · 10 comments · Fixed by #12190
Closed

Using array-bytes for Array/Bytes/Hex Operation #12111

AurevoirXavier opened this issue Aug 25, 2022 · 10 comments · Fixed by #12190

Comments

@AurevoirXavier
Copy link
Contributor

AurevoirXavier commented Aug 25, 2022

Hi! I'm the author of array-bytes.

This crate is completely optimized for Substrate development.

It's pretty useful and easy to use. And it's completely no-std.

Did you consider replacing hex crate, HexDisplay struct, and any other array/bytes/hex operation with this crate?

@bkchr
Copy link
Member

bkchr commented Aug 25, 2022

This crate is completely optimized for Substrate development.

Can you elaborate?

@AurevoirXavier
Copy link
Contributor Author

AurevoirXavier commented Aug 25, 2022

Can you elaborate?

Sorry, let me elaborate on this.

There is no performance optimization. But this crate makes the developer's life much easier (IMO).

It is completely no-std. So, you don't need to write something like array-bytes/std in the toml file.

It provides a lot of handy functions.

For example, the sp_core::H256 has implemented the From<[u8; 32]>. With this crate, you could write something like:

// better error handling
let hash = array_bytes::hex_into::<H256, _>("0x840cff528f0a427cf1405cc474c76b369890fc01951c73c71ae1e2ad1e8baa7d").expect("valid hex; qed");
// also provides an infallible method
let hash = array_bytes::hex_into_unchecked::<H256, _>("0x840cff528f0a427cf1405cc474c76b369890fc01951c73c71ae1e2ad1e8baa7d");

// For `AccountId32`
let account = array_bytes::hex_into::<AccountId32, _>("0x840cff528f0a427cf1405cc474c76b369890fc01951c73c71ae1e2ad1e8baa7d").expect("valid hex; qed");
let account = array_bytes::slice_into::<AccountId32, _>(&[100, 118, 109, 58, 0, 0, 0, 0, 0, 0, 0, 213, 244, 148, 7, 4, 235, 76, 229, 228, 181, 24, 119, 212, 155, 88, 163, 233, 53, 49, 182, 96]).expect("valid data; qed");

And it supports a dynamic hex2bytes method. Not like hex crate, it requires a &'static str at compile time.

And in the runtime development, we work with the rust array/slice/vec every day. It provides some functions to transform the dynamic type into a fixed length array, example.

Furthermore, if you are working with offchain-worker or genesis data. You might need to interact with the JSON object. It provides some useful deserializing functions. For example:

"header": {
    "block_number": "0x123"
}

#[derive(Debug, PartialEq, Deserialize)]
struct Header {
	#[serde(deserialize_with = "array_bytes::de_hex2num")]
	block_number: u32,
}

So, I hope Substrate could use this crate. And let more Substrate developers know/enjoy such convenient functions/tools.

@bkchr
Copy link
Member

bkchr commented Aug 25, 2022

I'm fine with switching to this crate.

@gilescope
Copy link
Contributor

https://github.com/hack-ink/array-bytes/blob/38fb70e60306f2d6096bc2453fbcec150c59237c/src/lib.rs#L243 - Shouldn't these unchecked functions be marked as unsafe functions? I should not be able to call them from safe code because they are not safe.

@AurevoirXavier
Copy link
Contributor Author

hack-ink/array-bytes@38fb70e/src/lib.rs#L243 - Shouldn't these unchecked functions be marked as unsafe functions? I should not be able to call them from safe code because they are not safe.

In https://paritytech.github.io/substrate/master/sc_service/index.html?search=unchecked, there are also a lot of unchecked functions.

IMHO, unchecked is a marker, you should know what you are doing.
For me, I only use unchecked functions in tests or some panic-ble parts.
Just like you can write .unwrap() in runtime code but you do not.

But any discussion is welcome.

@bkchr
Copy link
Member

bkchr commented Sep 7, 2022

This function is not really unchecked as @gilescope is saying. You are using there transmute without checking the input. You should use str::from_utf8.

@AurevoirXavier
Copy link
Contributor Author

This function is not really unchecked as @gilescope is saying. You are using there transmute without checking the input. You should use str::from_utf8.

For this context (hex conversion), str::from_utf8 do a lot of unnecessary checks. I'll consider marking this function as unsafe.

@AurevoirXavier
Copy link
Contributor Author

AurevoirXavier commented Sep 7, 2022

@shawntabrizi
Copy link
Member

Would make sense to me to have SR Labs or someone else audit the crate before we switch to it?

@AurevoirXavier
Copy link
Contributor Author

Would make sense to me to have SR Labs or someone else audit the crate before we switch to it?

I'm looking forward to this.

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

Successfully merging a pull request may close this issue.

4 participants