wasm secp256k1 sig verification

.changelog/unreleased/bug-fixes/1599-wasm-secp256k1-sig.md

@@ -0,0 +1,2 @@
+- Fix signature verification with secp256k1 in WASM VPs.
+  ([\#1599](https://github.com/anoma/namada/pull/1599))

apps/src/lib/config/genesis.rs

@@ -995,9 +995,14 @@ pub fn genesis(num_validators: u64) -> Genesis {
         public_key: None,
         storage: HashMap::default(),
     };
-    let implicit_accounts = vec![ImplicitAccount {
-        public_key: wallet::defaults::daewon_keypair().ref_to(),
-    }];
+    let implicit_accounts = vec![
+        ImplicitAccount {
+            public_key: wallet::defaults::daewon_keypair().ref_to(),
+        },
+        ImplicitAccount {
+            public_key: wallet::defaults::ester_keypair().ref_to(),
+        },
+    ];
     let default_user_tokens = token::Amount::whole(1_000_000);
     let default_key_tokens = token::Amount::whole(1_000);
     let mut balances: HashMap<Address, token::Amount> = HashMap::from_iter([

apps/src/lib/wallet/defaults.rs

@@ -3,8 +3,9 @@
 #[cfg(any(test, feature = "dev"))]
 pub use dev::{
     addresses, albert_address, albert_keypair, bertha_address, bertha_keypair,
-    christel_address, christel_keypair, daewon_address, daewon_keypair, keys,
-    validator_address, validator_keypair, validator_keys,
+    christel_address, christel_keypair, daewon_address, daewon_keypair,
+    ester_address, ester_keypair, keys, validator_address, validator_keypair,
+    validator_keys,
 };
 use namada::ledger::wallet::alias::Alias;
 use namada::ledger::{eth_bridge, governance, pos};
@@ -107,6 +108,7 @@ mod dev {
             ("bertha".into(), bertha_keypair()),
             ("christel".into(), christel_keypair()),
             ("daewon".into(), daewon_keypair()),
+            ("ester".into(), ester_keypair()),
             ("validator".into(), validator_keypair()),
         ]
     }
@@ -137,6 +139,7 @@ mod dev {
             ("bertha".into(), bertha_address()),
             ("christel".into(), christel_address()),
             ("daewon".into(), daewon_address()),
+            ("ester".into(), ester_address()),
         ];
         let token_addresses = tokens()
             .into_iter()
@@ -166,6 +169,11 @@ mod dev {
         (&daewon_keypair().ref_to()).into()
     }
 
+    /// An implicit user address for testing & development
+    pub fn ester_address() -> Address {
+        (&ester_keypair().ref_to()).into()
+    }
+
     /// An established validator address for testing & development
     pub fn validator_address() -> Address {
         Address::decode("atest1v4ehgw36ggcnsdee8qerswph8y6ry3p5xgunvve3xaqngd3kxc6nqwz9gseyydzzg5unys3ht2n48q").expect("The token address decoding shouldn't fail")
@@ -219,6 +227,18 @@ mod dev {
         ed_sk.try_to_sk().unwrap()
     }
 
+    pub fn ester_keypair() -> common::SecretKey {
+        // generated from
+        // [`namada::types::key::secp256k1::gen_keypair`]
+        let bytes = [
+            54, 144, 147, 226, 3, 93, 132, 247, 42, 126, 90, 23, 200, 155, 122,
+            147, 139, 93, 8, 204, 135, 178, 40, 152, 5, 227, 175, 204, 102,
+            239, 154, 66,
+        ];
+        let sk = secp256k1::SecretKey::try_from_slice(&bytes).unwrap();
+        sk.try_to_sk().unwrap()
+    }
+
     pub fn validator_keypair() -> common::SecretKey {
         // generated from
         // [`namada::types::key::ed25519::gen_keypair`]

core/Cargo.toml

@@ -25,9 +25,9 @@ ferveo-tpke = [
 wasm-runtime = [
   "rayon",
 ]
-# secp256k1 key signing and verification, disabled in WASM build by default as 
-# it bloats the build a lot
-secp256k1-sign-verify = [
+# secp256k1 key signing, disabled in WASM build by default as it bloats the 
+# build a lot
+secp256k1-sign = [
   "libsecp256k1/hmac",
 ]
 

core/src/types/key/secp256k1.rs

@@ -477,14 +477,14 @@ impl super::SigScheme for SigScheme {
 
     /// Sign the data with a key
     fn sign(keypair: &SecretKey, data: impl AsRef<[u8]>) -> Self::Signature {
-        #[cfg(not(any(test, feature = "secp256k1-sign-verify")))]
+        #[cfg(not(any(test, feature = "secp256k1-sign")))]
         {
             // to avoid `unused-variables` warn
             let _ = (keypair, data);
-            panic!("\"secp256k1-sign-verify\" feature must be enabled");
+            panic!("\"secp256k1-sign\" feature must be enabled");
         }
 
-        #[cfg(any(test, feature = "secp256k1-sign-verify"))]
+        #[cfg(any(test, feature = "secp256k1-sign"))]
         {
             use sha2::{Digest, Sha256};
             let hash = Sha256::digest(data.as_ref());
@@ -500,31 +500,21 @@ impl super::SigScheme for SigScheme {
         data: &T,
         sig: &Self::Signature,
     ) -> Result<(), VerifySigError> {
-        #[cfg(not(any(test, feature = "secp256k1-sign-verify")))]
-        {
-            // to avoid `unused-variables` warn
-            let _ = (pk, data, sig);
-            panic!("\"secp256k1-sign-verify\" feature must be enabled");
-        }
-
-        #[cfg(any(test, feature = "secp256k1-sign-verify"))]
-        {
-            use sha2::{Digest, Sha256};
-            let bytes = &data
-                .try_to_vec()
-                .map_err(VerifySigError::DataEncodingError)?[..];
-            let hash = Sha256::digest(bytes);
-            let message = &libsecp256k1::Message::parse_slice(hash.as_ref())
-                .expect("Error parsing given data");
-            let is_valid = libsecp256k1::verify(message, &sig.0, &pk.0);
-            if is_valid {
-                Ok(())
-            } else {
-                Err(VerifySigError::SigVerifyError(format!(
-                    "Error verifying secp256k1 signature: {}",
-                    libsecp256k1::Error::InvalidSignature
-                )))
-            }
+        use sha2::{Digest, Sha256};
+        let bytes = &data
+            .try_to_vec()
+            .map_err(VerifySigError::DataEncodingError)?[..];
+        let hash = Sha256::digest(bytes);
+        let message = &libsecp256k1::Message::parse_slice(hash.as_ref())
+            .expect("Error parsing given data");
+        let is_valid = libsecp256k1::verify(message, &sig.0, &pk.0);
+        if is_valid {
+            Ok(())
+        } else {
+            Err(VerifySigError::SigVerifyError(format!(
+                "Error verifying secp256k1 signature: {}",
+                libsecp256k1::Error::InvalidSignature
+            )))
         }
     }
 
@@ -533,28 +523,18 @@ impl super::SigScheme for SigScheme {
         data: &[u8],
         sig: &Self::Signature,
     ) -> Result<(), VerifySigError> {
-        #[cfg(not(any(test, feature = "secp256k1-sign-verify")))]
-        {
-            // to avoid `unused-variables` warn
-            let _ = (pk, data, sig);
-            panic!("\"secp256k1-sign-verify\" feature must be enabled");
-        }
-
-        #[cfg(any(test, feature = "secp256k1-sign-verify"))]
-        {
-            use sha2::{Digest, Sha256};
-            let hash = Sha256::digest(data);
-            let message = &libsecp256k1::Message::parse_slice(hash.as_ref())
-                .expect("Error parsing raw data");
-            let is_valid = libsecp256k1::verify(message, &sig.0, &pk.0);
-            if is_valid {
-                Ok(())
-            } else {
-                Err(VerifySigError::SigVerifyError(format!(
-                    "Error verifying secp256k1 signature: {}",
-                    libsecp256k1::Error::InvalidSignature
-                )))
-            }
+        use sha2::{Digest, Sha256};
+        let hash = Sha256::digest(data);
+        let message = &libsecp256k1::Message::parse_slice(hash.as_ref())
+            .expect("Error parsing raw data");
+        let is_valid = libsecp256k1::verify(message, &sig.0, &pk.0);
+        if is_valid {
+            Ok(())
+        } else {
+            Err(VerifySigError::SigVerifyError(format!(
+                "Error verifying secp256k1 signature: {}",
+                libsecp256k1::Error::InvalidSignature
+            )))
         }
     }
 }

genesis/e2e-tests-single-node.toml

@@ -36,6 +36,7 @@ Bertha = 1000000
 Christel = 1000000
 "Christel.public_key" = 100
 Daewon = 1000000
+Ester = 1000000
 faucet = 9223372036
 "faucet.public_key" = 100
 "validator-0.public_key" = 100
@@ -48,6 +49,7 @@ Albert = 1000000
 Bertha = 1000000
 Christel = 1000000
 Daewon = 1000000
+Ester = 1000000
 faucet = 9223372036854
 
 [token.ETH]
@@ -58,6 +60,7 @@ Albert = 1000000
 Bertha = 1000000
 Christel = 1000000
 Daewon = 1000000
+Ester = 1000000
 faucet = 9223372036854
 
 [token.DOT]
@@ -68,6 +71,7 @@ Albert = 1000000
 Bertha = 1000000
 Christel = 1000000
 Daewon = 1000000
+Ester = 1000000
 faucet = 9223372036854
 
 [token.Schnitzel]
@@ -78,6 +82,7 @@ Albert = 1000000
 Bertha = 1000000
 Christel = 1000000
 Daewon = 1000000
+Ester = 1000000
 faucet = 9223372036854
 
 [token.Apfel]
@@ -88,6 +93,7 @@ Albert = 1000000
 Bertha = 1000000
 Christel = 1000000
 Daewon = 1000000
+Ester = 1000000
 faucet = 9223372036854
 
 [token.Kartoffel]
@@ -99,6 +105,7 @@ Albert = 1000000
 Bertha = 1000000
 Christel = 1000000
 Daewon = 1000000
+Ester = 1000000
 faucet = 9223372036854
 
 # Some established accounts present at genesis.
@@ -120,6 +127,8 @@ vp = "vp_masp"
 
 [implicit.Daewon]
 
+[implicit.Ester]
+
 # Wasm VP definitions
 
 # Implicit VP

shared/Cargo.toml

@@ -85,7 +85,7 @@ namada-sdk = [
 multicore = ["masp_proofs/multicore"]
 
 [dependencies]
-namada_core = {path = "../core", default-features = false, features = ["secp256k1-sign-verify"]}
+namada_core = {path = "../core", default-features = false, features = ["secp256k1-sign"]}
 namada_proof_of_stake = {path = "../proof_of_stake", default-features = false}
 async-std.workspace = true
 async-trait = {version = "0.1.51", optional = true}
@@ -134,7 +134,7 @@ rand_core = {version = "0.6", default-features = false, optional = true}
 zeroize.workspace = true
 
 [dev-dependencies]
-namada_core = {path = "../core", default-features = false, features = ["secp256k1-sign-verify", "testing", "ibc-mocks"]}
+namada_core = {path = "../core", default-features = false, features = ["secp256k1-sign", "testing", "ibc-mocks"]}
 namada_test_utils = {path = "../test_utils"}
 assert_matches.workspace = true
 async-trait.workspace = true

tests/src/e2e/ledger_tests.rs

@@ -405,7 +405,7 @@ fn ledger_txs_and_queries() -> Result<()> {
             "--node",
             &validator_one_rpc,
         ],
-        // Submit a token transfer tx (from an implicit account)
+        // Submit a token transfer tx (from an ed25519 implicit account)
         vec![
             "transfer",
             "--source",
@@ -425,6 +425,26 @@ fn ledger_txs_and_queries() -> Result<()> {
             "--node",
             &validator_one_rpc,
         ],
+        // Submit a token transfer tx (from a secp256k1 implicit account)
+        vec![
+            "transfer",
+            "--source",
+            ESTER,
+            "--target",
+            ALBERT,
+            "--token",
+            NAM,
+            "--amount",
+            "10.1",
+            "--gas-amount",
+            "0",
+            "--gas-limit",
+            "0",
+            "--gas-token",
+            NAM,
+            "--node",
+            &validator_one_rpc,
+        ],
         // 3. Submit a transaction to update an account's validity
         // predicate
         vec![

tests/src/e2e/setup.rs

@@ -795,6 +795,7 @@ pub mod constants {
     pub const CHRISTEL: &str = "Christel";
     pub const CHRISTEL_KEY: &str = "Christel-key";
     pub const DAEWON: &str = "Daewon";
+    pub const ESTER: &str = "Ester";
     pub const MATCHMAKER_KEY: &str = "matchmaker-key";
     pub const MASP: &str = "atest1v4ehgw36xaryysfsx5unvve4g5my2vjz89p52sjxxgenzd348yuyyv3hg3pnjs35g5unvde4ca36y5";
 

wasm/checksums.json

@@ -1,21 +1,21 @@
 {
-    "tx_bond.wasm": "tx_bond.9b8df8657be80f070485e15199dcbf6b920c5b8929c95846788a7277322ac0b8.wasm",
-    "tx_change_validator_commission.wasm": "tx_change_validator_commission.cab8564fb682e6acf8e2d9140e9b8dac2872b6525ccece45e24b274070819833.wasm",
-    "tx_ibc.wasm": "tx_ibc.1f9962f14fc04e44e2459795216579b985c66b547f40df6e1d0d6117a04ce058.wasm",
-    "tx_init_account.wasm": "tx_init_account.a5082699b37e9704302af3ff5e6f67d5f02ee97d4369c98309d6e616bd57b779.wasm",
-    "tx_init_proposal.wasm": "tx_init_proposal.b997fb0a7a5369437cb5a37b74684ddc4eff5c451566f3d0538a717964481302.wasm",
-    "tx_init_validator.wasm": "tx_init_validator.c6470ec72d09a0841f818e356081e9231613b9375a677ad34431f31246ec725f.wasm",
-    "tx_reveal_pk.wasm": "tx_reveal_pk.ffdc2241c8fac28e52d33961b4c6813dfec1ae9ffafdd0b7d8f307c3093dfeb8.wasm",
-    "tx_transfer.wasm": "tx_transfer.b9356405dd27fe0405f32e0bc86d3aee3d228fbcbe7108f9b71128d52fbd67ea.wasm",
-    "tx_unbond.wasm": "tx_unbond.45aee4615d36c6553e613dfdca756ae2856993b24c8f46a2fe274db2ae1da07f.wasm",
-    "tx_unjail_validator.wasm": "tx_unjail_validator.df38aab0c05fe42b8036dc72c9b9fa4563116f62030b66f5080918c2994f723a.wasm",
-    "tx_update_vp.wasm": "tx_update_vp.54981ff2f04596ddcc69894f75f51c0ecd3b000c74d35ec91f8899a1dc2dbb72.wasm",
-    "tx_vote_proposal.wasm": "tx_vote_proposal.ab17eedfa0ee7db45300732d1d31cb4db81322a69824f936a07739f2c43115e2.wasm",
-    "tx_withdraw.wasm": "tx_withdraw.47bb8bef9719b4367d6dfbcf64e07af9bb97df752b249afd67b248dbff65c152.wasm",
-    "vp_implicit.wasm": "vp_implicit.70e268b7367c9d1b1c6adc4ed216989eedee257cbdf4e8dc931a4430126f3ef3.wasm",
-    "vp_masp.wasm": "vp_masp.f80362039d989503d4d2eccf070a8cb3feec775c804567b78b0619cf173f1b5f.wasm",
-    "vp_testnet_faucet.wasm": "vp_testnet_faucet.45f611a0bb2b437246c612710c88e1cf79fc2e8520078e410bd08b139d62514f.wasm",
-    "vp_token.wasm": "vp_token.d9f64fd1d645ee50200bad803fe288a43123eda601894f69eead6dfa4703c915.wasm",
-    "vp_user.wasm": "vp_user.8f293808d127ffbb14619ff28775a902d87d672bfd6fb49a59b55acfc43beaa8.wasm",
-    "vp_validator.wasm": "vp_validator.348d5b9a088781d0dc4c02e3541add718f2fa2fd6e221b47c8130cd50ec68637.wasm"
+    "tx_bond.wasm": "tx_bond.7348cad11839290ba23b6137a71e6f78565ae8bf0e792969abc0aa2b573a7fe4.wasm",
+    "tx_change_validator_commission.wasm": "tx_change_validator_commission.79c474a15c9d318194fe217581f651d8f9e06eb404b77ed31c9be00f96925e30.wasm",
+    "tx_ibc.wasm": "tx_ibc.70fe0637cc3d872208be7d879fc961c1b2f8d235384680e7ba6c6d554ca94b01.wasm",
+    "tx_init_account.wasm": "tx_init_account.204960496ddc6bc6eb55b6bd5678f23a1769f125b048e16156a5002037ef3fe5.wasm",
+    "tx_init_proposal.wasm": "tx_init_proposal.e8825e8df6f64ff869920a5de4789729efbcb751d0df9e9e4771b22f2c06b6c9.wasm",
+    "tx_init_validator.wasm": "tx_init_validator.49adb7bff5a6ecbf8ca1f0f522063a5d5825435a342771812d752be25ba8dad4.wasm",
+    "tx_reveal_pk.wasm": "tx_reveal_pk.6c5446b11c8286cb6bf2b8d57f8be3c9b1bf40805f0f58b178e44b5fd9501dc5.wasm",
+    "tx_transfer.wasm": "tx_transfer.13a89b38706652ebad3502eba461608b0180a400ad595ff89199c51b8d02e5af.wasm",
+    "tx_unbond.wasm": "tx_unbond.a4a0ee502605ff1141fca1bd592809bff467c75485ce8e0fd52ab9b8f6aabbfe.wasm",
+    "tx_unjail_validator.wasm": "tx_unjail_validator.75fcf5d64d2126ac509816fb460920683d95f77c948612e42eff3a1b8d846ab3.wasm",
+    "tx_update_vp.wasm": "tx_update_vp.e2d26d05785334a4dfe3b9e604478aefadd66bd2b0bafdfa4e188e08af4f56ce.wasm",
+    "tx_vote_proposal.wasm": "tx_vote_proposal.91c5686ac57610e1387603d61da653073e1575e5806b9f9f105eb118e695066c.wasm",
+    "tx_withdraw.wasm": "tx_withdraw.eca51a0ac20d45c775adc3975ffdd8f8e74fb9c54e93f95a8561439e69c899ff.wasm",
+    "vp_implicit.wasm": "vp_implicit.5e6d47fd63dd9df730ddfdbfd8f0dbc500c48aa46c95f3afbe82b1b5b7d75fa1.wasm",
+    "vp_masp.wasm": "vp_masp.e2f00d5c896f0011abb6becd744066dc6fab5536d136e8a4b53b66b329403ee5.wasm",
+    "vp_testnet_faucet.wasm": "vp_testnet_faucet.7167d806af804f7c309ce29d4486e9f83cbcd4a141328fa19dbc28ed2bee206b.wasm",
+    "vp_token.wasm": "vp_token.0edf53e3d92466e886514ce2ed381adfeca0fe9a426e6c3c00b41c1a9e327088.wasm",
+    "vp_user.wasm": "vp_user.598d409c316b3e7fa787d56f14954de1235ad2053eebbb5c872a029c8e510b9f.wasm",
+    "vp_validator.wasm": "vp_validator.b7546d3992c4c305d38ca7123f996c69ab1d677af224bbfafac0179e7b83829e.wasm"
 }