Fix pallet custom signatures issues

AstarNetwork · Sep 15, 2021 · 9a3c89d · 9a3c89d
1 parent 4791291
commit 9a3c89d
Show file tree

Hide file tree

Showing 5 changed files with 37 additions and 30 deletions.
diff --git a/Cargo.lock b/Cargo.lock
diff --git a/frame/custom-signatures/Cargo.toml b/frame/custom-signatures/Cargo.toml
@@ -1,6 +1,6 @@
 [package]
 name = "pallet-custom-signatures"
-version = "4.0.0"
+version = "4.1.0"
 authors = ["Stake Technologies <devops@stake.co.jp>"]
 edition = "2018"
 license = "Apache-2.0"
@@ -20,7 +20,7 @@ frame-system = { git = "https://github.com/paritytech/substrate", branch = "polk
 
 [dev-dependencies]
 pallet-balances = { git = "https://github.com/paritytech/substrate", branch = "polkadot-v0.9.9" }
-libsecp256k1 = "0.3.2"
+libsecp256k1 = "0.6.0"
 hex-literal = "0.2.1"
 sp-keyring = { git = "https://github.com/paritytech/substrate", branch = "polkadot-v0.9.9" }
 sp-core = { git = "https://github.com/paritytech/substrate", branch = "polkadot-v0.9.9" }

diff --git a/frame/custom-signatures/src/ethereum.rs b/frame/custom-signatures/src/ethereum.rs
@@ -38,16 +38,12 @@ impl sp_std::convert::TryFrom<Vec<u8>> for EthereumSignature {
 }
 
 /// Constructs the message that Ethereum RPC's `personal_sign` and `eth_sign` would sign.
+///
+/// Note: sign message hash to escape of message length estimation.
 pub fn signable_message(what: &[u8]) -> Vec<u8> {
-    let mut l = what.len();
-    let mut rev = Vec::new();
-    while l > 0 {
-        rev.push(b'0' + (l % 10) as u8);
-        l /= 10;
-    }
-    let mut v = b"\x19Ethereum Signed Message:\n".to_vec();
-    v.extend(rev.into_iter().rev());
-    v.extend_from_slice(what);
+    let hash = keccak_256(what);
+    let mut v = b"\x19Ethereum Signed Message:\n32".to_vec();
+    v.extend_from_slice(&hash[..]);
     v
 }
 
@@ -61,7 +57,7 @@ impl Verify for EthereumSignature {
         mut msg: L,
         account: &<Self::Signer as IdentifyAccount>::AccountId,
     ) -> bool {
-        let msg = keccak_256(&signable_message(&msg.get()));
+        let msg = keccak_256(&signable_message(msg.get()));
         match secp256k1_ecdsa_recover_compressed(&self.0, &msg).ok() {
             Some(public) => {
                 let signer = Self::Signer::from(ecdsa::Public::from_raw(public));
@@ -79,9 +75,9 @@ fn verify_should_works() {
 
     let msg = "test eth signed message";
     let pair = ecdsa::Pair::from_seed(&hex![
-        "7e9c7ad85df5cdc88659f53e06fb2eb9bab3ebc59083a3190eaf2c730332529c"
+        "ac0974bec39a17e36ba4a6b4d238ff944bacb478cbed5efcae784d7bf4f2ff80"
     ]);
     let account = <MultiSignature as Verify>::Signer::from(pair.public()).into_account();
-    let signature = EthereumSignature(hex!["dd0992d40e5cdf99db76bed162808508ac65acd7ae2fdc8573594f03ed9c939773e813181788fc02c3c68f3fdc592759b35f6354484343e18cb5317d34dab6c61b"]);
+    let signature = EthereumSignature(hex!["f5d5cc953828e3fb0d81f3176d88fa5c73d3ad3dc4bc7a8061b03a6db2cd73337778df75a1443e8c642f6ceae0db39b90c321ac270ad7836695cae76f703f3031c"]);
     assert_eq!(signature.verify(msg.as_ref(), &account), true);
 }
diff --git a/frame/custom-signatures/src/lib.rs b/frame/custom-signatures/src/lib.rs
@@ -110,7 +110,6 @@ pub mod pallet {
                 nonce == frame_system::Pallet::<T>::account_nonce(signer.clone()),
                 Error::<T>::BadNonce,
             );
-            frame_system::Pallet::<T>::inc_account_nonce(signer.clone());
 
             let signature = <T as Config>::Signature::try_from(signature)
                 .map_err(|_| Error::<T>::DecodeFailure)?;
@@ -121,6 +120,9 @@ pub mod pallet {
                 Error::<T>::InvalidSignature
             );
 
+            // Increment account nonce
+            frame_system::Pallet::<T>::inc_account_nonce(signer.clone());
+
             // Processing fee
             let tx_fee = T::Currency::withdraw(
                 &signer,
@@ -148,7 +150,7 @@ pub mod pallet {
             signature: &T::Signature,
             nonce: &T::Index,
         ) -> bool {
-            let payload = (T::CallMagicNumber::get(), nonce.clone(), call.clone());
+            let payload = (T::CallMagicNumber::get(), *nonce, call.clone());
             signature.verify(&payload.encode()[..], signer)
         }
     }

diff --git a/frame/custom-signatures/src/tests.rs b/frame/custom-signatures/src/tests.rs
@@ -116,19 +116,21 @@ fn new_test_ext() -> sp_io::TestExternalities {
 /// Simple `eth_sign` implementation, should be equal to exported by RPC
 fn eth_sign(seed: &[u8; 32], data: &[u8]) -> Vec<u8> {
     let call_msg = ethereum::signable_message(data);
-    let ecdsa_msg = secp256k1::Message::parse(&keccak_256(&call_msg));
-    let secret = secp256k1::SecretKey::parse(&seed).expect("valid seed");
-    let mut ecdsa: ecdsa::Signature = secp256k1::sign(&ecdsa_msg, &secret).into();
+    let ecdsa_msg = libsecp256k1::Message::parse(&keccak_256(&call_msg));
+    let secret = libsecp256k1::SecretKey::parse(&seed).expect("valid seed");
+    let (signature, recovery_id) = libsecp256k1::sign(&ecdsa_msg, &secret);
+    let mut out = Vec::new();
+    out.extend_from_slice(&signature.serialize()[..]);
     // Fix recovery ID: Ethereum uses 27/28 notation
-    ecdsa.as_mut()[64] += 27;
-    Vec::from(ecdsa.as_ref() as &[u8])
+    out.push(recovery_id.serialize() + 27);
+    out
 }
 
 #[test]
 fn eth_sign_works() {
-    let seed = hex!["7e9c7ad85df5cdc88659f53e06fb2eb9bab3ebc59083a3190eaf2c730332529c"];
-    let text = b"Hello Plasm";
-    let signature = hex!["79eec99d7f5b321c1b75d2fc044b555f9afdbc4f9b43a011085f575b216f85c452a04373d487671852dca4be4fe5fd90836560afe709d1dab45ab18bc936c2111c"];
+    let seed = hex!["ac0974bec39a17e36ba4a6b4d238ff944bacb478cbed5efcae784d7bf4f2ff80"];
+    let text = b"Hello Astar";
+    let signature = hex!["0cc6d5de6db06727fe43a260e7c9a417be3daab9b0e4e65e276f543e5c2f3de67e9e26d903d5301181e13033f61692db2dca67c1f8992b62476eaf8cb3a597101c"];
     assert_eq!(eth_sign(&seed, &text[..]), signature);
 }
 
@@ -199,18 +201,17 @@ fn balance_transfer() {
     })
 }
 
-/* TODO: enable it when UI fixtures will be ready
 #[test]
 fn call_fixtures() {
     use sp_core::crypto::Ss58Codec;
 
-    let seed = hex!["7e9c7ad85df5cdc88659f53e06fb2eb9bab3ebc59083a3190eaf2c730332529c"];
+    let seed = hex!["ac0974bec39a17e36ba4a6b4d238ff944bacb478cbed5efcae784d7bf4f2ff80"];
     let pair = ecdsa::Pair::from_seed(&seed);
     assert_eq!(
         MultiSigner::from(pair.public())
             .into_account()
             .to_ss58check(),
-        "5Geeci7qCoYHyg9z2AwfpiT4CDryvxYyD7SAUdfNBz9CyDSb",
+        "5EGynCAEvv8NLeHx8vDMvb8hTcEcMYUMWCDQEEncNEfNWB2W",
     );
 
     let dest =
@@ -221,8 +222,14 @@ fn call_fixtures() {
         hex!["0000c4305fb88b6ccb43d6552dc11d18e7b0ee3185247adcc6e885eb284adf6c563da10f"],
     );
 
-    let signature = hex!["96cd8087ef720b0ec10d96996a8bbb45005ba3320d1dde38450a56f77dfd149720cc2e6dcc8f09963aad4cdf5ec15e103ce56d0f4c7a753840217ef1787467a01c"];
     let payload = (0xff50u16, 0u32, call.clone());
+    assert_eq!(
+        payload.encode(),
+        hex![
+            "50ff000000000000c4305fb88b6ccb43d6552dc11d18e7b0ee3185247adcc6e885eb284adf6c563da10f"
+        ],
+    );
+
+    let signature = hex!["6ecb474240df46ee5cde8f51cf5ccf4c75d15ac3c1772aea6c8189604263c98b16350883438c4eaa447ebcb6889d516f70351fd704bb3521072cd2fccc7c99dc1c"];
     assert_eq!(eth_sign(&seed, payload.encode().as_ref()), signature)
 }
-*/