debug sig verification failure

aptos-move/aptos-vm/src/keyless_validation.rs

@@ -142,23 +142,29 @@ pub(crate) fn validate_authenticators(
 
     for (pk, sig) in authenticators {
         let jwk = get_jwk_for_authenticator(&patched_jwks, pk, sig)?;
+        println!("1");
 
         match &sig.sig {
             ZkpOrOpenIdSig::Groth16Zkp(proof) => match jwk {
                 JWK::RSA(rsa_jwk) => {
+                    println!("2");
                     if proof.exp_horizon_secs > config.max_exp_horizon_secs {
+                        println!("{} > {}", proof.exp_horizon_secs, config.max_exp_horizon_secs);
                         return Err(invalid_signature!("The expiration horizon is too long"));
                     }
 
+                    println!("3");
                     // If an `aud` override was set for account recovery purposes, check that it is
                     // in the allow-list on-chain.
                     if proof.override_aud_val.is_some() {
                         config.is_allowed_override_aud(proof.override_aud_val.as_ref().unwrap())?;
                     }
 
+                    println!("4");
                     let public_inputs_hash = get_public_inputs_hash(sig, pk, &rsa_jwk, config)
                         .map_err(|_| invalid_signature!("Could not compute public inputs hash"))?;
 
+                    println!("5");
                     // The training wheels signature is only checked if a training wheels PK is set on chain
                     if training_wheels_pk.is_some() {
                         proof
@@ -167,13 +173,15 @@ pub(crate) fn validate_authenticators(
                                 &public_inputs_hash,
                             )
                             .map_err(|_| {
+                                println!("bad tw_sig");
                                 invalid_signature!("Could not verify training wheels signature")
                             })?;
                     }
 
+                    println!("6");
                     proof
                         .verify_proof(public_inputs_hash, pvk)
-                        .map_err(|_| invalid_signature!("Proof verification failed"))?;
+                        .map_err(|_| { println!("bad proof"); invalid_signature!("Proof verification failed") })?;
                 },
                 JWK::Unsupported(_) => return Err(invalid_signature!("JWK is not supported")),
             },
@@ -208,5 +216,7 @@ pub(crate) fn validate_authenticators(
             },
         }
     }
+
+    println!("great success!");
     Ok(())
 }

aptos-move/e2e-move-tests/Cargo.toml

@@ -28,7 +28,6 @@ aptos-package-builder = { workspace = true }
 aptos-types = { workspace = true }
 aptos-vm = { workspace = true, features = ["testing"] }
 aptos-vm-genesis = { workspace = true }
-ark-ff = { workspace = true }
 bcs = { workspace = true }
 claims = { workspace = true }
 hex = { workspace = true }

aptos-move/e2e-move-tests/src/harness.rs

@@ -655,6 +655,7 @@ impl MoveHarness {
     }
 
     /// Reads the resource data `T`.
+    /// WARNING: Does not work with resource groups (because set_resource does not work?).
     pub fn read_resource<T: DeserializeOwned>(
         &self,
         addr: &AccountAddress,
@@ -714,6 +715,7 @@ impl MoveHarness {
     }
 
     /// Write the resource data `T`.
+    /// WARNING: Does not work with resource groups.
     pub fn set_resource<T: Serialize>(
         &mut self,
         addr: AccountAddress,

aptos-move/e2e-move-tests/src/tests/keyless_feature_gating.rs

@@ -1,35 +1,36 @@
 // Copyright © Aptos Foundation
 
-use crate::MoveHarness;
+use crate::{assert_success, build_package, tests::common, MoveHarness};
 use aptos_cached_packages::aptos_stdlib;
 use aptos_crypto::SigningKey;
 use aptos_language_e2e_tests::account::{Account, TransactionBuilder};
 use aptos_types::{
-    keyless::{test_utils::get_sample_groth16_sig_and_pk, KeylessPublicKey, KeylessSignature},
+    keyless::{
+        test_utils::{
+            get_sample_esk, get_sample_groth16_sig_and_pk, get_sample_iss, get_sample_jwk,
+        },
+        Configuration, KeylessPublicKey, KeylessSignature, ZkpOrOpenIdSig,
+    },
     on_chain_config::FeatureFlag,
     transaction::{
-        authenticator::{AnyPublicKey, AuthenticationKey},
-        SignedTransaction, TransactionStatus,
+        authenticator::{AnyPublicKey, AuthenticationKey, EphemeralSignature},
+        Script, SignedTransaction, TransactionStatus,
     },
 };
-use aptos_types::keyless::test_utils::{get_sample_esk, get_sample_jwk};
-use aptos_types::keyless::{Configuration, get_public_inputs_hash, Groth16ZkpAndStatement, ZkpOrOpenIdSig};
-use aptos_types::transaction::authenticator::EphemeralSignature;
 use move_core_types::{
-    account_address::AccountAddress, vm_status::StatusCode::FEATURE_UNDER_GATING,
+    account_address::AccountAddress,
+    transaction_argument::TransactionArgument,
+    vm_status::StatusCode::{FEATURE_UNDER_GATING, INVALID_SIGNATURE},
 };
-use ark_ff::{BigInteger, PrimeField};
-
-// TODO(keyless): Initialize keyless_account.move
 
 #[test]
-fn test_keyless_feature_gating() {
+fn test_keyless_feature_is_disabled() {
     let mut h = MoveHarness::new_with_features(vec![], vec![FeatureFlag::KEYLESS_ACCOUNTS]);
 
     let (sig, pk) = get_sample_groth16_sig_and_pk();
     let bob = h.new_account_at(AccountAddress::from_hex_literal("0xb0b").unwrap());
 
-    let transaction = get_keyless_signed_txn(&mut h, sig, pk, bob);
+    let transaction = get_keyless_groth16_txn(&mut h, sig, pk, bob);
 
     let output = h.run_raw(transaction);
     match output.status() {
@@ -42,8 +43,31 @@ fn test_keyless_feature_gating() {
     }
 }
 
+#[test]
+fn test_keyless_feature_is_enabled() {
+    let mut h = MoveHarness::new_with_features(vec![FeatureFlag::KEYLESS_ACCOUNTS], vec![]);
+
+    let (sig, pk) = get_sample_groth16_sig_and_pk();
+    let bob = h.new_account_at(AccountAddress::from_hex_literal("0xb0b").unwrap());
+
+    // initialize JWK
+    run_setup_script(&mut h);
+
+    let transaction = get_keyless_groth16_txn(&mut h, sig, pk, bob);
+
+    let output = h.run_raw(transaction);
+    match output.status() {
+        TransactionStatus::Discard(status) => {
+            assert_eq!(*status, INVALID_SIGNATURE)
+        },
+        _ => {
+            panic!("Expected to get INVALID_SIGNATURE DiscardedVMStatus")
+        },
+    }
+}
+
 /// Creates and funds a new account at `pk` and sends coins to `recipient`.
-fn get_keyless_signed_txn(
+fn get_keyless_groth16_txn(
     h: &mut MoveHarness,
     mut sig: KeylessSignature,
     pk: KeylessPublicKey,
@@ -64,23 +88,6 @@ fn get_keyless_signed_txn(
 
     let esk = get_sample_esk();
     sig.ephemeral_signature = EphemeralSignature::ed25519(esk.sign(&raw_txn).unwrap());
-    let jwk = get_sample_jwk();
-    // TODO: might need new_for_devnet() here; forget how the MoveHarness is initialized
-    let config = Configuration::new_for_testing();
-
-    let public_inputs_hash: Option<[u8; 32]> = if let ZkpOrOpenIdSig::Groth16Zkp(_) = &sig.sig {
-        // This will only calculate the hash if it's needed, avoiding unnecessary computation.
-        Some(
-            get_public_inputs_hash(&sig, &pk, &jwk, &config)
-                .unwrap()
-                .into_bigint()
-                .to_bytes_le()
-                .try_into()
-                .expect("expected 32-byte public inputs hash"),
-        )
-    } else {
-        None
-    };
 
     // Compute the training wheels signature if not present
     match &mut sig.sig {
@@ -94,3 +101,41 @@ fn get_keyless_signed_txn(
     let transaction = SignedTransaction::new_keyless(raw_txn, pk, sig);
     transaction
 }
+
+fn run_setup_script(h: &mut MoveHarness) {
+    let core_resources = h.new_account_at(AccountAddress::from_hex_literal("0xA550C18").unwrap());
+
+    let package = build_package(
+        common::test_dir_path("keyless_setup.data/pack"),
+        aptos_framework::BuildOptions::default(),
+    )
+    .expect("building package must succeed");
+
+    let txn = h.create_publish_built_package(&core_resources, &package, |_| {});
+    assert_success!(h.run(txn));
+
+    let script = package.extract_script_code()[0].clone();
+
+    let iss = get_sample_iss();
+    let jwk = get_sample_jwk();
+    let config = Configuration::new_for_testing();
+
+    let txn = TransactionBuilder::new(core_resources.clone())
+        .script(Script::new(script, vec![], vec![
+            TransactionArgument::U8Vector(iss.into_bytes()),
+            TransactionArgument::U8Vector(jwk.kid.into_bytes()),
+            TransactionArgument::U8Vector(jwk.alg.into_bytes()),
+            TransactionArgument::U8Vector(jwk.e.into_bytes()),
+            TransactionArgument::U8Vector(jwk.n.into_bytes()),
+            TransactionArgument::U64(config.max_exp_horizon_secs)
+        ]))
+        .sequence_number(h.sequence_number(&core_resources.address()))
+        .max_gas_amount(1_000_000)
+        .gas_unit_price(1)
+        .sign();
+
+    // NOTE: We cannot write the Configuration and Groth16Verification key via MoveHarness::set_resource
+    // because it does not (yet) work with resource groups.
+
+    assert_success!(h.run(txn));
+}

aptos-move/e2e-move-tests/src/tests/keyless_setup.data/pack/Move.toml

@@ -0,0 +1,6 @@
+[package]
+name = 'InsertJwk'
+version = "0.0.0"
+
+[dependencies]
+AptosFramework = { local = "../../../../../framework/aptos-framework" }

aptos-move/e2e-move-tests/src/tests/keyless_setup.data/pack/sources/main.move

@@ -0,0 +1,24 @@
+script {
+    use aptos_framework::jwks;
+    use aptos_framework::aptos_governance;
+    use aptos_framework::keyless_account;
+    use std::string::utf8;
+
+    fun main(core_resources: &signer, iss: vector<u8>, kid: vector<u8>, alg: vector<u8>, e: vector<u8>, n: vector<u8>, max_exp_horizon_secs: u64) {
+        let fx = aptos_governance::get_signer_testnet_only(core_resources, @aptos_framework);
+        let jwk = jwks::new_rsa_jwk(
+            utf8(kid),
+            utf8(alg),
+            utf8(e),
+            utf8(n)
+        );
+
+        let patches = vector[
+            jwks::new_patch_remove_all(),
+            jwks::new_patch_upsert_jwk(iss, jwk),
+        ];
+        jwks::set_patches(&fx, patches);
+
+        keyless_account::update_max_exp_horizon(&fx, max_exp_horizon_secs);
+    }
+}

aptos-move/e2e-move-tests/src/tests/mod.rs

@@ -20,6 +20,7 @@ mod generate_upgrade_script;
 mod governance_updates;
 mod infinite_loop;
 mod init_module;
+mod keyless_feature_gating;
 mod lazy_natives;
 mod max_loop_depth;
 mod memory_quota;
@@ -49,4 +50,3 @@ mod type_too_large;
 mod vector_numeric_address;
 mod vm;
 mod vote;
-mod keyless_feature_gating;

types/src/keyless/configuration.rs

@@ -17,7 +17,7 @@ use move_core_types::{
 use serde::{Deserialize, Serialize};
 
 /// Reflection of aptos_framework::keyless_account::Configuration
-#[derive(Serialize, Deserialize, Debug)]
+#[derive(Serialize, Deserialize, Eq, PartialEq, Debug)]
 pub struct Configuration {
     pub override_aud_vals: Vec<String>,
     pub max_signatures_per_txn: u16,

types/src/keyless/groth16_vk.rs

@@ -17,7 +17,7 @@ use serde::{Deserialize, Serialize};
 use std::fmt::{Display, Formatter};
 
 /// Reflection of aptos_framework::keyless_account::Groth16VerificationKey
-#[derive(Serialize, Deserialize, Debug)]
+#[derive(Serialize, Deserialize, Eq, PartialEq, Debug)]
 pub struct Groth16VerificationKey {
     pub alpha_g1: Vec<u8>,
     pub beta_g2: Vec<u8>,