Add 20-byte account id to subxt_core (#1638)

* Add accountId20 impl to subxt_core

closes #1576

Cargo.lock

@@ -4995,6 +4995,7 @@ dependencies = [
  "hashbrown 0.14.5",
  "hex",
  "impl-serde",
+ "keccak-hash",
  "parity-scale-codec",
  "primitive-types",
  "scale-bits",

codegen/src/lib.rs

@@ -358,7 +358,7 @@ fn default_derives(crate_path: &syn::Path) -> DerivesRegistry {
 fn default_substitutes(crate_path: &syn::Path) -> TypeSubstitutes {
     let mut type_substitutes = TypeSubstitutes::new();
 
-    let defaults: [(syn::Path, syn::Path); 12] = [
+    let defaults: [(syn::Path, syn::Path); 13] = [
         (
             parse_quote!(bitvec::order::Lsb0),
             parse_quote!(#crate_path::utils::bits::Lsb0),
@@ -371,6 +371,10 @@ fn default_substitutes(crate_path: &syn::Path) -> TypeSubstitutes {
             parse_quote!(sp_core::crypto::AccountId32),
             parse_quote!(#crate_path::utils::AccountId32),
         ),
+        (
+            parse_quote!(fp_account::AccountId20),
+            parse_quote!(#crate_path::utils::AccountId20),
+        ),
         (
             parse_quote!(sp_runtime::multiaddress::MultiAddress),
             parse_quote!(#crate_path::utils::MultiAddress),

core/Cargo.toml

@@ -64,6 +64,9 @@ sp-core = { workspace = true, optional = true }
 sp-runtime = { workspace = true, optional = true }
 tracing = { workspace = true, default-features = false }
 
+# AccountId20
+keccak-hash = { workspace = true}
+
 [dev-dependencies]
 assert_matches = { workspace = true }
 bitvec = { workspace = true }

core/src/utils/account_id20.rs

@@ -0,0 +1,163 @@
+// Copyright 2019-2024 Parity Technologies (UK) Ltd.
+// This file is dual-licensed as Apache-2.0 or GPL-3.0.
+// see LICENSE for license details.
+
+//! `AccountId20` is a repressentation of Ethereum address derived from hashing the public key.
+
+use core::fmt::Display;
+
+use alloc::format;
+use alloc::string::String;
+use codec::{Decode, Encode};
+use keccak_hash::keccak;
+use serde::{Deserialize, Serialize};
+
+#[derive(
+    Copy,
+    Clone,
+    Eq,
+    PartialEq,
+    Ord,
+    PartialOrd,
+    Encode,
+    Decode,
+    Debug,
+    scale_encode::EncodeAsType,
+    scale_decode::DecodeAsType,
+    scale_info::TypeInfo,
+)]
+/// Ethereum-compatible `AccountId`.
+pub struct AccountId20(pub [u8; 20]);
+
+impl AsRef<[u8]> for AccountId20 {
+    fn as_ref(&self) -> &[u8] {
+        &self.0[..]
+    }
+}
+
+impl AsRef<[u8; 20]> for AccountId20 {
+    fn as_ref(&self) -> &[u8; 20] {
+        &self.0
+    }
+}
+
+impl From<[u8; 20]> for AccountId20 {
+    fn from(x: [u8; 20]) -> Self {
+        AccountId20(x)
+    }
+}
+
+impl AccountId20 {
+    /// Convert to a public key hash
+    pub fn checksum(&self) -> String {
+        let hex_address = hex::encode(self.0);
+        let hash = keccak(hex_address.as_bytes());
+
+        let mut checksum_address = String::with_capacity(42);
+        checksum_address.push_str("0x");
+
+        for (i, ch) in hex_address.chars().enumerate() {
+            // Get the corresponding nibble from the hash
+            let nibble = hash[i / 2] >> (if i % 2 == 0 { 4 } else { 0 }) & 0xf;
+
+            if nibble >= 8 {
+                checksum_address.push(ch.to_ascii_uppercase());
+            } else {
+                checksum_address.push(ch);
+            }
+        }
+
+        checksum_address
+    }
+}
+
+/// An error obtained from trying to interpret a hex encoded string into an AccountId20
+#[derive(Clone, Copy, Eq, PartialEq, Debug)]
+#[allow(missing_docs)]
+pub enum FromChecksumError {
+    BadLength,
+    InvalidChecksum,
+    InvalidPrefix,
+}
+
+impl Display for FromChecksumError {
+    fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result {
+        match self {
+            FromChecksumError::BadLength => write!(f, "Length is bad"),
+            FromChecksumError::InvalidChecksum => write!(f, "Invalid checksum"),
+            FromChecksumError::InvalidPrefix => write!(f, "Invalid checksum prefix byte."),
+        }
+    }
+}
+
+#[cfg(feature = "std")]
+impl std::error::Error for FromChecksumError {}
+
+impl Serialize for AccountId20 {
+    fn serialize<S>(&self, serializer: S) -> Result<S::Ok, S::Error>
+    where
+        S: serde::Serializer,
+    {
+        serializer.serialize_str(&self.checksum())
+    }
+}
+
+impl<'de> Deserialize<'de> for AccountId20 {
+    fn deserialize<D>(deserializer: D) -> Result<Self, D::Error>
+    where
+        D: serde::Deserializer<'de>,
+    {
+        String::deserialize(deserializer)?
+            .parse::<AccountId20>()
+            .map_err(|e| serde::de::Error::custom(format!("{e:?}")))
+    }
+}
+
+impl core::fmt::Display for AccountId20 {
+    fn fmt(&self, f: &mut core::fmt::Formatter) -> core::fmt::Result {
+        write!(f, "{}", self.checksum())
+    }
+}
+
+impl core::str::FromStr for AccountId20 {
+    type Err = FromChecksumError;
+    fn from_str(s: &str) -> Result<Self, Self::Err> {
+        if s.len() != 42 {
+            return Err(FromChecksumError::BadLength);
+        }
+        if !s.starts_with("0x") {
+            return Err(FromChecksumError::InvalidPrefix);
+        }
+        hex::decode(&s.as_bytes()[2..])
+            .map_err(|_| FromChecksumError::InvalidChecksum)?
+            .try_into()
+            .map(AccountId20)
+            .map_err(|_| FromChecksumError::BadLength)
+    }
+}
+
+#[cfg(test)]
+mod test {
+    use super::*;
+
+    #[test]
+    fn deserialisation() {
+        let key_hashes = vec![
+            "0xf24FF3a9CF04c71Dbc94D0b566f7A27B94566cac",
+            "0x3Cd0A705a2DC65e5b1E1205896BaA2be8A07c6e0",
+            "0x798d4Ba9baf0064Ec19eB4F0a1a45785ae9D6DFc",
+            "0x773539d4Ac0e786233D90A233654ccEE26a613D9",
+            "0xFf64d3F6efE2317EE2807d223a0Bdc4c0c49dfDB",
+            "0xC0F0f4ab324C46e55D02D0033343B4Be8A55532d",
+        ];
+
+        for key_hash in key_hashes {
+            let parsed: AccountId20 = key_hash.parse().expect("Failed to parse");
+
+            let encoded = parsed.checksum();
+
+            // `encoded` should be equal to the initial key_hash
+            assert_eq!(encoded, key_hash);
+        }
+    }
+}

core/src/utils/mod.rs

@@ -5,6 +5,7 @@
 //! Miscellaneous utility helpers.
 
 mod account_id;
+mod account_id20;
 pub mod bits;
 mod era;
 mod multi_address;
@@ -21,6 +22,7 @@ use codec::{Compact, Decode, Encode};
 use derive_where::derive_where;
 
 pub use account_id::AccountId32;
+pub use account_id20::AccountId20;
 pub use era::Era;
 pub use multi_address::MultiAddress;
 pub use multi_signature::MultiSignature;

signer/Cargo.toml

@@ -49,7 +49,9 @@ secrecy = { workspace = true }
 regex = { workspace = true, features = ["unicode"] }
 hex = { workspace = true, features = ["alloc"] }
 cfg-if = { workspace = true }
-codec = { package = "parity-scale-codec", workspace = true, features = ["derive"] }
+codec = { package = "parity-scale-codec", workspace = true, features = [
+    "derive",
+] }
 sp-crypto-hashing = { workspace = true }
 pbkdf2 = { workspace = true }
 sha2 = { workspace = true }
@@ -58,7 +60,10 @@ zeroize = { workspace = true }
 bip39 = { workspace = true }
 bip32 = { workspace = true, features = ["alloc", "secp256k1"], optional = true }
 schnorrkel = { workspace = true, optional = true }
-secp256k1 = { workspace = true, optional = true, features = ["alloc", "recovery"] }
+secp256k1 = { workspace = true, optional = true, features = [
+    "alloc",
+    "recovery",
+] }
 keccak-hash = { workspace = true, optional = true }
 
 # We only pull this in to enable the JS flag for schnorrkel to use.

signer/src/eth.rs

@@ -6,7 +6,6 @@
 
 use crate::ecdsa;
 use alloc::format;
-use alloc::string::String;
 use core::fmt::{Display, Formatter};
 use core::str::FromStr;
 use keccak_hash::keccak;
@@ -17,6 +16,15 @@ const SECRET_KEY_LENGTH: usize = 32;
 /// Bytes representing a private key.
 pub type SecretKeyBytes = [u8; SECRET_KEY_LENGTH];
 
+/// The public key for an [`Keypair`] key pair. This is the uncompressed variant of [`ecdsa::PublicKey`].
+pub struct PublicKey(pub [u8; 65]);
+
+impl AsRef<[u8]> for PublicKey {
+    fn as_ref(&self) -> &[u8] {
+        &self.0
+    }
+}
+
 /// An ethereum keypair implementation.
 #[derive(Debug, Clone, PartialEq, Eq)]
 pub struct Keypair(ecdsa::Keypair);
@@ -89,18 +97,10 @@ impl Keypair {
             .map_err(|_| Error::InvalidSeed)
     }
 
-    /// Obtain the [`ecdsa::PublicKey`] of this keypair.
-    pub fn public_key(&self) -> ecdsa::PublicKey {
-        self.0.public_key()
-    }
-
-    /// Obtains the public address of the account by taking the last 20 bytes
-    /// of the Keccak-256 hash of the public key.
-    pub fn account_id(&self) -> AccountId20 {
+    /// Obtain the [`eth::PublicKey`] of this keypair.
+    pub fn public_key(&self) -> PublicKey {
         let uncompressed = self.0 .0.public_key().serialize_uncompressed();
-        let hash = keccak(&uncompressed[1..]).0;
-        let hash20 = hash[12..].try_into().expect("should be 20 bytes");
-        AccountId20(hash20)
+        PublicKey(uncompressed)
     }
 
     /// Signs an arbitrary message payload.
@@ -113,7 +113,6 @@ impl Keypair {
         Signature(self.0.sign_prehashed(message_hash).0)
     }
 }
-
 /// A derivation path. This can be parsed from a valid derivation path string like
 /// `"m/44'/60'/0'/0/0"`, or we can construct one using the helpers [`DerivationPath::empty()`]
 /// and [`DerivationPath::eth()`].
@@ -168,45 +167,6 @@ impl AsRef<[u8; 65]> for Signature {
     }
 }
 
-/// A 20-byte cryptographic identifier.
-#[derive(Debug, Copy, Clone, PartialEq, Eq, codec::Encode)]
-pub struct AccountId20(pub [u8; 20]);
-
-impl AccountId20 {
-    fn checksum(&self) -> String {
-        let hex_address = hex::encode(self.0);
-        let hash = keccak(hex_address.as_bytes());
-
-        let mut checksum_address = String::with_capacity(42);
-        checksum_address.push_str("0x");
-
-        for (i, ch) in hex_address.chars().enumerate() {
-            // Get the corresponding nibble from the hash
-            let nibble = hash[i / 2] >> (if i % 2 == 0 { 4 } else { 0 }) & 0xf;
-
-            if nibble >= 8 {
-                checksum_address.push(ch.to_ascii_uppercase());
-            } else {
-                checksum_address.push(ch);
-            }
-        }
-
-        checksum_address
-    }
-}
-
-impl AsRef<[u8]> for AccountId20 {
-    fn as_ref(&self) -> &[u8] {
-        &self.0
-    }
-}
-
-impl Display for AccountId20 {
-    fn fmt(&self, f: &mut Formatter<'_>) -> core::fmt::Result {
-        write!(f, "{}", self.checksum())
-    }
-}
-
 /// Verify that some signature for a message was created by the owner of the [`PublicKey`].
 ///
 /// ```rust
@@ -219,12 +179,20 @@ impl Display for AccountId20 {
 /// let public_key = keypair.public_key();
 /// assert!(eth::verify(&signature, message, &public_key));
 /// ```
-pub fn verify<M: AsRef<[u8]>>(sig: &Signature, message: M, pubkey: &ecdsa::PublicKey) -> bool {
+pub fn verify<M: AsRef<[u8]>>(sig: &Signature, message: M, pubkey: &PublicKey) -> bool {
     let message_hash = keccak(message.as_ref());
     let wrapped =
         Message::from_digest_slice(message_hash.as_bytes()).expect("Message is 32 bytes; qed");
+    let Ok(signature) = secp256k1::ecdsa::Signature::from_compact(&sig.as_ref()[..64]) else {
+        return false;
+    };
+    let Ok(pk) = secp256k1::PublicKey::from_slice(&pubkey.0) else {
+        return false;
+    };
 
-    ecdsa::internal::verify(&sig.0, &wrapped, pubkey)
+    secp256k1::Secp256k1::verification_only()
+        .verify_ecdsa(&wrapped, &signature, &pk)
+        .is_ok()
 }
 
 /// An error handed back if creating a keypair fails.
@@ -290,36 +258,68 @@ pub mod dev {
 
 #[cfg(feature = "subxt")]
 mod subxt_compat {
+    use super::*;
     use subxt_core::config::Config;
     use subxt_core::tx::signer::Signer as SignerT;
-
-    use super::*;
+    use subxt_core::utils::AccountId20;
+    use subxt_core::utils::MultiAddress;
 
     impl<T: Config> SignerT<T> for Keypair
     where
-        T::AccountId: From<AccountId20>,
-        T::Address: From<AccountId20>,
+        T::AccountId: From<PublicKey>,
+        T::Address: From<PublicKey>,
         T::Signature: From<Signature>,
     {
         fn account_id(&self) -> T::AccountId {
-            self.account_id().into()
+            self.public_key().into()
         }
 
         fn address(&self) -> T::Address {
-            self.account_id().into()
+            self.public_key().into()
         }
 
         fn sign(&self, signer_payload: &[u8]) -> T::Signature {
             self.sign(signer_payload).into()
         }
     }
+
+    impl PublicKey {
+        /// Obtains the public address of the account by taking the last 20 bytes
+        /// of the Keccak-256 hash of the public key.
+        pub fn to_account_id(&self) -> AccountId20 {
+            let hash = keccak(&self.0[1..]).0;
+            let hash20 = hash[12..].try_into().expect("should be 20 bytes");
+            AccountId20(hash20)
+        }
+        /// A shortcut to obtain a [`MultiAddress`] from a [`PublicKey`].
+        /// We often want this type, and using this method avoids any
+        /// ambiguous type resolution issues.
+        pub fn to_address<T>(self) -> MultiAddress<AccountId20, T> {
+            MultiAddress::Address20(self.to_account_id().0)
+        }
+    }
+
+    impl From<PublicKey> for AccountId20 {
+        fn from(value: PublicKey) -> Self {
+            value.to_account_id()
+        }
+    }
+
+    impl<T> From<PublicKey> for MultiAddress<AccountId20, T> {
+        fn from(value: PublicKey) -> Self {
+            let address: AccountId20 = value.into();
+            MultiAddress::Address20(address.0)
+        }
+    }
 }
 
 #[cfg(test)]
+#[cfg(feature = "subxt")]
 mod test {
     use bip39::Mnemonic;
     use proptest::prelude::*;
     use secp256k1::Secp256k1;
+    use subxt_core::utils::AccountId20;
 
     use subxt_core::{config::*, tx::signer::Signer as SignerT, utils::H256};
 
@@ -392,7 +392,7 @@ mod test {
         fn check_subxt_signer_implementation_matches(keypair in keypair(), msg in ".*") {
             let msg_as_bytes = msg.as_bytes();
 
-            assert_eq!(SubxtSigner::account_id(&keypair), keypair.account_id());
+            assert_eq!(SubxtSigner::account_id(&keypair), keypair.public_key().to_account_id());
             assert_eq!(SubxtSigner::sign(&keypair, msg_as_bytes), keypair.sign(msg_as_bytes));
         }
 
@@ -405,8 +405,9 @@ mod test {
                 let hash20 = hash[12..].try_into().expect("should be 20 bytes");
                 AccountId20(hash20)
             };
-
-            assert_eq!(keypair.account_id(), account_id);
+            let account_id_derived_from_pk: AccountId20 = keypair.public_key().to_account_id();
+            assert_eq!(account_id_derived_from_pk, account_id);
+            assert_eq!(keypair.public_key().to_account_id(), account_id);
 
         }
 
@@ -465,7 +466,7 @@ mod test {
         ];
 
         for (case_idx, (keypair, exp_account_id, exp_priv_key)) in cases.into_iter().enumerate() {
-            let act_account_id = keypair.account_id().to_string();
+            let act_account_id = keypair.public_key().to_account_id().checksum();
             let act_priv_key = format!("0x{}", &keypair.0 .0.display_secret());
 
             assert_eq!(
@@ -610,7 +611,7 @@ mod test {
         fn test_account_derivation_1() {
             let kp = Keypair::from_secret_key(KEY_1).expect("valid keypair");
             assert_eq!(
-                kp.account_id().to_string(),
+                kp.public_key().to_account_id().checksum(),
                 "0x976f8456E4e2034179B284A23C0e0c8f6d3da50c"
             );
         }
@@ -619,7 +620,7 @@ mod test {
         fn test_account_derivation_2() {
             let kp = Keypair::from_secret_key(KEY_2).expect("valid keypair");
             assert_eq!(
-                kp.account_id().to_string(),
+                kp.public_key().to_account_id().checksum(),
                 "0x420e9F260B40aF7E49440ceAd3069f8e82A5230f"
             );
         }
@@ -628,7 +629,7 @@ mod test {
         fn test_account_derivation_3() {
             let kp = Keypair::from_secret_key(KEY_3).expect("valid keypair");
             assert_eq!(
-                kp.account_id().to_string(),
+                kp.public_key().to_account_id().checksum(),
                 "0x9cce34F7aB185c7ABA1b7C8140d620B4BDA941d6"
             );
         }

subxt/examples/tx_basic_frontier.rs

@@ -6,7 +6,8 @@
 #![allow(missing_docs)]
 
 use subxt::OnlineClient;
-use subxt_signer::eth::{dev, AccountId20, Signature};
+use subxt_core::utils::AccountId20;
+use subxt_signer::eth::{dev, Signature};
 
 #[subxt::subxt(runtime_metadata_path = "../artifacts/frontier_metadata_small.scale")]
 mod eth_runtime {}
@@ -25,28 +26,20 @@ impl subxt::Config for EthRuntimeConfig {
     type AssetId = u32;
 }
 
-// This helper makes it easy to use our `AccountId20`'s with generated
-// code that expects a generated `eth_runtime::runtime_types::fp_account:AccountId20` type.
-impl From<AccountId20> for eth_runtime::runtime_types::fp_account::AccountId20 {
-    fn from(a: AccountId20) -> Self {
-        eth_runtime::runtime_types::fp_account::AccountId20(a.0)
-    }
-}
-
 #[tokio::main]
 async fn main() -> Result<(), Box<dyn std::error::Error>> {
     let api = OnlineClient::<EthRuntimeConfig>::from_insecure_url("ws://127.0.0.1:9944").await?;
 
     let alith = dev::alith();
     let baltathar = dev::baltathar();
-    let dest = baltathar.account_id();
+    let dest = baltathar.public_key().to_account_id();
 
     println!("baltathar pub:  {}", hex::encode(baltathar.public_key().0));
     println!("baltathar addr: {}", hex::encode(dest));
 
     let balance_transfer_tx = eth_runtime::tx()
         .balances()
-        .transfer_allow_death(dest.into(), 10_001);
+        .transfer_allow_death(dest, 10_001);
 
     let events = api
         .tx()