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.

Sign up for GitHub

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

Jump to bottom

chore: support Display/FromStr for AccountId20 in no_std #1426

Merged
merged 2 commits into from
May 15, 2024
Merged

chore: support Display/FromStr for AccountId20 in no_std #1426

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
eb358a8
chore: support `Display`/`FromStr` for `AccountId20` in no_std
yjhmelody May 13, 2024
db49ed9
arrange code and impl `Hash` for `AccountId20`
yjhmelody May 13, 2024
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
22 changes: 13 additions & 9 deletions primitives/account/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,12 @@
// limitations under the License.

#![cfg_attr(not(feature = "std"), no_std)]
#![warn(unused_crate_dependencies)]

extern crate alloc;

use alloc::string::{String, ToString};
use core::fmt;

use scale_codec::{Decode, Encode, MaxEncodedLen};
use scale_info::TypeInfo;
Expand All @@ -28,14 +34,13 @@ use sp_runtime_interface::pass_by::PassByInner;
/// A fully Ethereum-compatible `AccountId`.
/// Conforms to H160 address and ECDSA key standards.
/// Alternative to H256->H160 mapping.
#[derive(Copy, Clone, Eq, PartialEq, Ord, PartialOrd, Default)]
#[derive(Copy, Clone, Eq, PartialEq, Ord, PartialOrd, Default, Hash)]
#[derive(Encode, Decode, MaxEncodedLen, TypeInfo)]
pub struct AccountId20(pub [u8; 20]);

#[cfg(feature = "serde")]
impl_serde::impl_fixed_hash_serde!(AccountId20, 20);

#[cfg(feature = "std")]
Copy link

Choose a reason for hiding this comment

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

This is not a correct change. H160::from_str implementation is only available in primitive-types (that sp-core re-exports) if fixed-hash/rustc-hex feature is enabled, which is in turn enabled by primitive-types/rustc-hex, which sp-core only enables when std feature is used.

As the result this can (and does) fail for downstream users unless feature unification accidentally enables fixed-hash/rustc-hex. This should either be reverted or sp-core needs to enable primitive-types/rustc-hex unconditionally.

impl core::str::FromStr for AccountId20 {
type Err = &'static str;

Expand All @@ -46,9 +51,8 @@ impl core::str::FromStr for AccountId20 {
}
}

#[cfg(feature = "std")]
impl core::fmt::Display for AccountId20 {
fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result {
impl fmt::Display for AccountId20 {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
let address = hex::encode(self.0).trim_start_matches("0x").to_lowercase();
let address_hash = hex::encode(keccak_256(address.as_bytes()));

Expand All @@ -73,8 +77,8 @@ impl core::fmt::Display for AccountId20 {
}
}

impl core::fmt::Debug for AccountId20 {
fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result {
impl fmt::Debug for AccountId20 {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
write!(f, "{:?}", H160(self.0))
}
}
Expand Down Expand Up @@ -231,8 +235,8 @@ impl sp_runtime::traits::IdentifyAccount for EthereumSigner {
}
}

impl core::fmt::Display for EthereumSigner {
fn fmt(&self, fmt: &mut core::fmt::Formatter) -> core::fmt::Result {
impl fmt::Display for EthereumSigner {
fn fmt(&self, fmt: &mut fmt::Formatter) -> fmt::Result {
write!(fmt, "{:?}", H160::from(self.0))
}
}
Expand Down