From bf059bad17bd90fe7ae195847ee8648bf92f1f31 Mon Sep 17 00:00:00 2001 From: Alin Tomescu Date: Wed, 28 Feb 2024 19:58:23 -0800 Subject: [PATCH 1/4] clearer feature gating for keyless TXNs --- aptos-move/aptos-vm/src/aptos_vm.rs | 15 ++++++++++++++- aptos-move/aptos-vm/src/keyless_validation.rs | 15 +++++---------- 2 files changed, 19 insertions(+), 11 deletions(-) diff --git a/aptos-move/aptos-vm/src/aptos_vm.rs b/aptos-move/aptos-vm/src/aptos_vm.rs index 33335f5202852..4a91693d4ae80 100644 --- a/aptos-move/aptos-vm/src/aptos_vm.rs +++ b/aptos-move/aptos-vm/src/aptos_vm.rs @@ -1320,7 +1320,20 @@ impl AptosVM { let authenticators = aptos_types::keyless::get_authenticators(transaction) .map_err(|_| VMStatus::error(StatusCode::INVALID_SIGNATURE, None))?; - keyless_validation::validate_authenticators(&authenticators, self.features(), resolver)?; + + // If there are keyless TXN authenticators, validate them all. + if !authenticators.is_empty() { + // Feature-gating keyless TXNs: if they are *not* enabled, return `FEATURE_UNDER_GATING`, + // which will discard the TXN from being put on-chain. + if !self.features().is_keyless_enabled() { + return Err(VMStatus::error(StatusCode::FEATURE_UNDER_GATING, None)); + } + keyless_validation::validate_authenticators( + &authenticators, + self.features(), + resolver, + )?; + } // The prologue MUST be run AFTER any validation. Otherwise you may run prologue and hit // SEQUENCE_NUMBER_TOO_NEW if there is more than one transaction from the same sender and diff --git a/aptos-move/aptos-vm/src/keyless_validation.rs b/aptos-move/aptos-vm/src/keyless_validation.rs index ad82b287b0ed1..7f5e410590b94 100644 --- a/aptos-move/aptos-vm/src/keyless_validation.rs +++ b/aptos-move/aptos-vm/src/keyless_validation.rs @@ -103,22 +103,17 @@ pub(crate) fn validate_authenticators( features: &Features, resolver: &impl AptosMoveResolver, ) -> Result<(), VMStatus> { - // Feature gating for (_, sig) in authenticators { - if !features.is_keyless_enabled() && matches!(sig.sig, ZkpOrOpenIdSig::Groth16Zkp { .. }) { - return Err(VMStatus::error(StatusCode::FEATURE_UNDER_GATING, None)); - } - if (!features.is_keyless_enabled() || !features.is_keyless_zkless_enabled()) - && matches!(sig.sig, ZkpOrOpenIdSig::OpenIdSig { .. }) + // Feature-gating for keyless-but-zkless TXNs: If keyless TXNs *are* enabled, and (1) this + // is a ZKless transaction but (2) ZKless TXNs are not yet enabled, discard the TXN from + // being put on-chain. + if matches!(sig.sig, ZkpOrOpenIdSig::OpenIdSig { .. }) + && !features.is_keyless_zkless_enabled() { return Err(VMStatus::error(StatusCode::FEATURE_UNDER_GATING, None)); } } - if authenticators.is_empty() { - return Ok(()); - } - let config = &get_configs_onchain(resolver)?; if authenticators.len() > config.max_signatures_per_txn as usize { return Err(invalid_signature!("Too many keyless authenticators")); From a3361e67e253d0e8e5b382e9d73fae1421ad1943 Mon Sep 17 00:00:00 2001 From: Alin Tomescu Date: Thu, 29 Feb 2024 11:53:12 -0800 Subject: [PATCH 2/4] added e2e test for feature gating --- Cargo.lock | 1 + aptos-move/e2e-move-tests/Cargo.toml | 1 + .../src/tests/keyless_feature_gating.rs | 96 +++++++++++++++++++ aptos-move/e2e-move-tests/src/tests/mod.rs | 1 + aptos-move/e2e-tests/src/account.rs | 13 +++ 5 files changed, 112 insertions(+) create mode 100644 aptos-move/e2e-move-tests/src/tests/keyless_feature_gating.rs diff --git a/Cargo.lock b/Cargo.lock index 569641a30dfd1..42e283d806a10 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -7302,6 +7302,7 @@ dependencies = [ "aptos-vm", "aptos-vm-genesis", "aptos-vm-types", + "ark-ff", "bcs 0.1.4", "claims", "hex", diff --git a/aptos-move/e2e-move-tests/Cargo.toml b/aptos-move/e2e-move-tests/Cargo.toml index a5ad28e4e45b5..e06c06cdf9751 100644 --- a/aptos-move/e2e-move-tests/Cargo.toml +++ b/aptos-move/e2e-move-tests/Cargo.toml @@ -28,6 +28,7 @@ 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 } diff --git a/aptos-move/e2e-move-tests/src/tests/keyless_feature_gating.rs b/aptos-move/e2e-move-tests/src/tests/keyless_feature_gating.rs new file mode 100644 index 0000000000000..63e5def01ccb7 --- /dev/null +++ b/aptos-move/e2e-move-tests/src/tests/keyless_feature_gating.rs @@ -0,0 +1,96 @@ +// Copyright © Aptos Foundation + +use crate::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}, + on_chain_config::FeatureFlag, + transaction::{ + authenticator::{AnyPublicKey, AuthenticationKey}, + 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, +}; +use ark_ff::{BigInteger, PrimeField}; + +// TODO(keyless): Initialize keyless_account.move + +#[test] +fn test_keyless_feature_gating() { + 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 output = h.run_raw(transaction); + match output.status() { + TransactionStatus::Discard(status) => { + assert_eq!(*status, FEATURE_UNDER_GATING) + }, + _ => { + panic!("Expected to get FEATURE_UNDER_GATING DiscardedVMStatus") + }, + } +} + +/// Creates and funds a new account at `pk` and sends coins to `recipient`. +fn get_keyless_signed_txn( + h: &mut MoveHarness, + mut sig: KeylessSignature, + pk: KeylessPublicKey, + recipient: Account, +) -> SignedTransaction { + let apk = AnyPublicKey::keyless(pk.clone()); + let addr = AuthenticationKey::any_key(apk.clone()).account_address(); + let account = h.store_and_fund_account(&Account::new_from_addr(addr), 100000000, 0); + + let payload = aptos_stdlib::aptos_coin_transfer(*recipient.address(), 1); + let raw_txn = TransactionBuilder::new(account.clone()) + .payload(payload) + .sequence_number(h.sequence_number(account.address())) + .max_gas_amount(1_000_000) + .gas_unit_price(1) + .sign() // dummy signature, which we discard below + .into_raw_transaction(); + + 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 { + ZkpOrOpenIdSig::Groth16Zkp(proof) => { + // Training wheels should be disabled. + proof.training_wheels_signature = None + }, + ZkpOrOpenIdSig::OpenIdSig(_) => {}, + } + + let transaction = SignedTransaction::new_keyless(raw_txn, pk, sig); + transaction +} diff --git a/aptos-move/e2e-move-tests/src/tests/mod.rs b/aptos-move/e2e-move-tests/src/tests/mod.rs index a8f9e1283792e..9fb01dfe381c9 100644 --- a/aptos-move/e2e-move-tests/src/tests/mod.rs +++ b/aptos-move/e2e-move-tests/src/tests/mod.rs @@ -49,3 +49,4 @@ mod type_too_large; mod vector_numeric_address; mod vm; mod vote; +mod keyless_feature_gating; diff --git a/aptos-move/e2e-tests/src/account.rs b/aptos-move/e2e-tests/src/account.rs index 306b6800e3f1f..3dd3b9db8b8cc 100644 --- a/aptos-move/e2e-tests/src/account.rs +++ b/aptos-move/e2e-tests/src/account.rs @@ -30,6 +30,8 @@ pub const DEFAULT_EXPIRATION_TIME: u64 = 4_000_000; /// /// Tests will typically create a set of `Account` instances to run transactions on. This type /// encodes the logic to operate on and verify operations on any Aptos account. +/// +/// TODO: This is pleistocene-age code must be brought up to speed, since our accounts are not just Ed25519-based. #[derive(Debug, Clone, Eq, PartialEq)] pub struct Account { addr: AccountAddress, @@ -58,6 +60,17 @@ impl Account { Self::with_keypair(privkey, pubkey) } + /// Creates an account with a specific address + /// TODO: Currently stores a dummy SK/PK pair. + pub fn new_from_addr(addr: AccountAddress) -> Self { + let (privkey, pubkey) = KeyGen::from_os_rng().generate_ed25519_keypair(); + Self { + addr, + privkey, + pubkey, + } + } + /// Creates a new account with the given keypair. /// /// Like with [`Account::new`], the account returned by this constructor is a purely logical From 5201ae4b2d67b4d34b84ef2efa318795e89fb4cc Mon Sep 17 00:00:00 2001 From: Alin Tomescu Date: Thu, 29 Feb 2024 15:08:52 -0800 Subject: [PATCH 3/4] added e2e test when feature is enabled --- Cargo.lock | 1 - aptos-move/aptos-vm/src/keyless_validation.rs | 1 + aptos-move/e2e-move-tests/Cargo.toml | 1 - .../proptest-regressions/tests/aggregator.txt | 8 ++ aptos-move/e2e-move-tests/src/harness.rs | 2 + .../src/tests/keyless_feature_gating.rs | 122 +++++++++++----- .../tests/keyless_setup.data/pack/Move.toml | 6 + .../keyless_setup.data/pack/sources/main.move | 24 ++++ aptos-move/e2e-move-tests/src/tests/mod.rs | 2 +- .../src/tests/offer_rotation_capability.rs | 2 +- .../src/tests/offer_signer_capability.rs | 2 +- .../src/tests/rotate_auth_key.rs | 25 ++-- aptos-move/e2e-tests/src/account.rs | 131 ++++++++++-------- .../e2e-testsuite/src/tests/verify_txn.rs | 20 +-- .../move/aptosvm_publish_and_run.rs | 2 +- types/src/keyless/configuration.rs | 2 +- types/src/keyless/groth16_vk.rs | 2 +- 17 files changed, 224 insertions(+), 129 deletions(-) create mode 100644 aptos-move/e2e-move-tests/proptest-regressions/tests/aggregator.txt create mode 100644 aptos-move/e2e-move-tests/src/tests/keyless_setup.data/pack/Move.toml create mode 100644 aptos-move/e2e-move-tests/src/tests/keyless_setup.data/pack/sources/main.move diff --git a/Cargo.lock b/Cargo.lock index 42e283d806a10..569641a30dfd1 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -7302,7 +7302,6 @@ dependencies = [ "aptos-vm", "aptos-vm-genesis", "aptos-vm-types", - "ark-ff", "bcs 0.1.4", "claims", "hex", diff --git a/aptos-move/aptos-vm/src/keyless_validation.rs b/aptos-move/aptos-vm/src/keyless_validation.rs index 7f5e410590b94..8432e106223a2 100644 --- a/aptos-move/aptos-vm/src/keyless_validation.rs +++ b/aptos-move/aptos-vm/src/keyless_validation.rs @@ -208,5 +208,6 @@ pub(crate) fn validate_authenticators( }, } } + Ok(()) } diff --git a/aptos-move/e2e-move-tests/Cargo.toml b/aptos-move/e2e-move-tests/Cargo.toml index e06c06cdf9751..a5ad28e4e45b5 100644 --- a/aptos-move/e2e-move-tests/Cargo.toml +++ b/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 } diff --git a/aptos-move/e2e-move-tests/proptest-regressions/tests/aggregator.txt b/aptos-move/e2e-move-tests/proptest-regressions/tests/aggregator.txt new file mode 100644 index 0000000000000..7c8d718698c51 --- /dev/null +++ b/aptos-move/e2e-move-tests/proptest-regressions/tests/aggregator.txt @@ -0,0 +1,8 @@ +# Seeds for failure cases proptest has generated in the past. It is +# automatically read and these particular cases re-run before any +# novel cases are generated. +# +# It is recommended to check this file in to source control so that +# everyone who runs the test benefits from these saved cases. +cc 0d06e418a86d5a07e5ac65d22e86d78a819989e1e0815db4750efcc12dc9d905 # shrinks to block_split = Whole +cc c5ad38cf3be8dea3f90d1504b070a030228a03829c63964d2855e8946d17ffe6 # shrinks to block_split = Whole diff --git a/aptos-move/e2e-move-tests/src/harness.rs b/aptos-move/e2e-move-tests/src/harness.rs index 4c4353ce9fa9d..7b20a153c4863 100644 --- a/aptos-move/e2e-move-tests/src/harness.rs +++ b/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( &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( &mut self, addr: AccountAddress, diff --git a/aptos-move/e2e-move-tests/src/tests/keyless_feature_gating.rs b/aptos-move/e2e-move-tests/src/tests/keyless_feature_gating.rs index 63e5def01ccb7..749a0610df8e5 100644 --- a/aptos-move/e2e-move-tests/src/tests/keyless_feature_gating.rs +++ b/aptos-move/e2e-move-tests/src/tests/keyless_feature_gating.rs @@ -1,35 +1,35 @@ // 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_crypto::{hash::CryptoHash, SigningKey}; +use aptos_language_e2e_tests::account::{Account, AccountPublicKey, 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, Transaction, 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, }; -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 +42,24 @@ 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); + assert_success!(output.status().clone()); +} + /// 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, @@ -51,36 +67,28 @@ fn get_keyless_signed_txn( ) -> SignedTransaction { let apk = AnyPublicKey::keyless(pk.clone()); let addr = AuthenticationKey::any_key(apk.clone()).account_address(); - let account = h.store_and_fund_account(&Account::new_from_addr(addr), 100000000, 0); + let account = h.store_and_fund_account( + &Account::new_from_addr(addr, AccountPublicKey::Keyless(pk.clone())), + 100000000, + 0, + ); + + println!("Actual address: {}", addr.to_hex()); + println!("Account address: {}", account.address().to_hex()); let payload = aptos_stdlib::aptos_coin_transfer(*recipient.address(), 1); + //println!("Payload: {:?}", payload); let raw_txn = TransactionBuilder::new(account.clone()) .payload(payload) .sequence_number(h.sequence_number(account.address())) .max_gas_amount(1_000_000) .gas_unit_price(1) - .sign() // dummy signature, which we discard below - .into_raw_transaction(); + .raw(); + + println!("RawTxn sender: {:?}", raw_txn.sender()); 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 { @@ -92,5 +100,47 @@ fn get_keyless_signed_txn( } let transaction = SignedTransaction::new_keyless(raw_txn, pk, sig); + println!( + "Submitted TXN hash: {}", + Transaction::UserTransaction(transaction.clone()).hash() + ); 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)); +} diff --git a/aptos-move/e2e-move-tests/src/tests/keyless_setup.data/pack/Move.toml b/aptos-move/e2e-move-tests/src/tests/keyless_setup.data/pack/Move.toml new file mode 100644 index 0000000000000..9c710048b40d9 --- /dev/null +++ b/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" } diff --git a/aptos-move/e2e-move-tests/src/tests/keyless_setup.data/pack/sources/main.move b/aptos-move/e2e-move-tests/src/tests/keyless_setup.data/pack/sources/main.move new file mode 100644 index 0000000000000..2f7a0631421a7 --- /dev/null +++ b/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, kid: vector, alg: vector, e: vector, n: vector, 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); + } +} diff --git a/aptos-move/e2e-move-tests/src/tests/mod.rs b/aptos-move/e2e-move-tests/src/tests/mod.rs index 9fb01dfe381c9..f3cb033dd467b 100644 --- a/aptos-move/e2e-move-tests/src/tests/mod.rs +++ b/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; diff --git a/aptos-move/e2e-move-tests/src/tests/offer_rotation_capability.rs b/aptos-move/e2e-move-tests/src/tests/offer_rotation_capability.rs index 0f319e70af454..6406384979fe2 100644 --- a/aptos-move/e2e-move-tests/src/tests/offer_rotation_capability.rs +++ b/aptos-move/e2e-move-tests/src/tests/offer_rotation_capability.rs @@ -59,7 +59,7 @@ pub fn offer_rotation_capability_v2( aptos_stdlib::account_offer_rotation_capability( rotation_proof_signed.to_bytes().to_vec(), 0, - offerer_account.pubkey.to_bytes().to_vec(), + offerer_account.pubkey.to_bytes(), *delegate_account.address(), ) )); diff --git a/aptos-move/e2e-move-tests/src/tests/offer_signer_capability.rs b/aptos-move/e2e-move-tests/src/tests/offer_signer_capability.rs index b5c244cf79413..3286afcb164f1 100644 --- a/aptos-move/e2e-move-tests/src/tests/offer_signer_capability.rs +++ b/aptos-move/e2e-move-tests/src/tests/offer_signer_capability.rs @@ -49,7 +49,7 @@ fn offer_signer_capability_v2() { aptos_stdlib::account_offer_signer_capability( signature.to_bytes().to_vec(), 0, - account_alice.pubkey.to_bytes().to_vec(), + account_alice.pubkey.to_bytes(), *account_bob.address(), ) )); diff --git a/aptos-move/e2e-move-tests/src/tests/rotate_auth_key.rs b/aptos-move/e2e-move-tests/src/tests/rotate_auth_key.rs index b97fe37a71cab..9559ab0a89b84 100644 --- a/aptos-move/e2e-move-tests/src/tests/rotate_auth_key.rs +++ b/aptos-move/e2e-move-tests/src/tests/rotate_auth_key.rs @@ -36,7 +36,7 @@ fn rotate_auth_key_ed25519_to_ed25519() { *account1.address(), 0, account2.privkey.clone(), - account2.pubkey.clone(), + account2.pubkey.to_bytes(), ); // verify that we can still get to account1's originating address @@ -60,7 +60,7 @@ fn rotate_auth_key_ed25519_to_multi_ed25519() { *account1.address(), 0, private_key, - public_key, + public_key.to_bytes(), ); // verify that we can still get to account1's originating address @@ -82,10 +82,10 @@ fn rotate_auth_key_twice() { *account1.address(), 0, account2.privkey.clone(), - account2.pubkey.clone(), + account2.pubkey.to_bytes(), ); // rotate account1's keypair to account2 - account1.rotate_key(account2.privkey, account2.pubkey); + account1.rotate_key(account2.privkey, account2.pubkey.as_ed25519().unwrap()); // verify that we can still get to account1's originating address verify_originating_address(&mut harness, account1.auth_key(), *account1.address(), 1); @@ -98,9 +98,9 @@ fn rotate_auth_key_twice() { *account1.address(), 1, account3.privkey.clone(), - account3.pubkey.clone(), + account3.pubkey.to_bytes(), ); - account1.rotate_key(account3.privkey, account3.pubkey); + account1.rotate_key(account3.privkey, account3.pubkey.as_ed25519().unwrap()); verify_originating_address(&mut harness, account1.auth_key(), *account1.address(), 2); } @@ -174,10 +174,7 @@ fn run_rotate_auth_key_with_rotation_capability( ) } -pub fn assert_successful_key_rotation_transaction< - S: SigningKey + ValidCryptoMaterial, - V: ValidCryptoMaterial, ->( +pub fn assert_successful_key_rotation_transaction( from_scheme: u8, to_scheme: u8, harness: &mut MoveHarness, @@ -185,7 +182,7 @@ pub fn assert_successful_key_rotation_transaction< originator: AccountAddress, sequence_number: u64, new_private_key: S, - new_public_key: V, + new_public_key_bytes: Vec, ) { // Construct a proof challenge struct that proves that // the user intends to rotate their auth key. @@ -196,7 +193,7 @@ pub fn assert_successful_key_rotation_transaction< sequence_number, originator, current_auth_key: AccountAddress::from_bytes(current_account.auth_key()).unwrap(), - new_public_key: new_public_key.to_bytes().to_vec(), + new_public_key: new_public_key_bytes.clone(), }; let rotation_msg = bcs::to_bytes(&rotation_proof).unwrap(); @@ -211,9 +208,9 @@ pub fn assert_successful_key_rotation_transaction< ¤t_account, aptos_stdlib::account_rotate_authentication_key( from_scheme, - current_account.pubkey.to_bytes().to_vec(), + current_account.pubkey.to_bytes(), to_scheme, - new_public_key.to_bytes().to_vec(), + new_public_key_bytes, signature_by_curr_privkey.to_bytes().to_vec(), signature_by_new_privkey.to_bytes().to_vec(), ) diff --git a/aptos-move/e2e-tests/src/account.rs b/aptos-move/e2e-tests/src/account.rs index 3dd3b9db8b8cc..dd605f125e810 100644 --- a/aptos-move/e2e-tests/src/account.rs +++ b/aptos-move/e2e-tests/src/account.rs @@ -13,10 +13,11 @@ use aptos_types::{ account_config::{self, AccountResource, CoinStoreResource}, chain_id::ChainId, event::{EventHandle, EventKey}, + keyless::KeylessPublicKey, state_store::state_key::StateKey, transaction::{ - authenticator::AuthenticationKey, EntryFunction, RawTransaction, Script, SignedTransaction, - TransactionPayload, + authenticator::{AnyPublicKey, AuthenticationKey}, + EntryFunction, RawTransaction, Script, SignedTransaction, TransactionPayload, }, write_set::{WriteOp, WriteSet, WriteSetMut}, }; @@ -26,6 +27,28 @@ use move_core_types::move_resource::MoveStructType; // TTL is 86400s. Initial time was set to 0. pub const DEFAULT_EXPIRATION_TIME: u64 = 4_000_000; +#[derive(Debug, Clone, Eq, PartialEq)] +pub enum AccountPublicKey { + Ed25519(Ed25519PublicKey), + Keyless(KeylessPublicKey), +} + +impl AccountPublicKey { + pub fn to_bytes(&self) -> Vec { + match self { + AccountPublicKey::Ed25519(pk) => pk.to_bytes().to_vec(), + AccountPublicKey::Keyless(pk) => pk.to_bytes(), + } + } + + pub fn as_ed25519(&self) -> Option { + match self { + AccountPublicKey::Ed25519(pk) => Some(pk.clone()), + AccountPublicKey::Keyless(_) => None, + } + } +} + /// Details about a Aptos account. /// /// Tests will typically create a set of `Account` instances to run transactions on. This type @@ -36,9 +59,10 @@ pub const DEFAULT_EXPIRATION_TIME: u64 = 4_000_000; pub struct Account { addr: AccountAddress, /// The current private key for this account. + /// TODO: When `pubkey` is of type `AccountPublicKey::Keyless`, this will be undefined. pub privkey: Ed25519PrivateKey, /// The current public key for this account. - pub pubkey: Ed25519PublicKey, + pub pubkey: AccountPublicKey, } impl Account { @@ -62,8 +86,8 @@ impl Account { /// Creates an account with a specific address /// TODO: Currently stores a dummy SK/PK pair. - pub fn new_from_addr(addr: AccountAddress) -> Self { - let (privkey, pubkey) = KeyGen::from_os_rng().generate_ed25519_keypair(); + pub fn new_from_addr(addr: AccountAddress, pubkey: AccountPublicKey) -> Self { + let (privkey, _) = KeyGen::from_os_rng().generate_ed25519_keypair(); Self { addr, privkey, @@ -80,7 +104,7 @@ impl Account { Account { addr, privkey, - pubkey, + pubkey: AccountPublicKey::Ed25519(pubkey), } } @@ -96,7 +120,7 @@ impl Account { Account { addr, privkey, - pubkey, + pubkey: AccountPublicKey::Ed25519(pubkey), } } @@ -107,7 +131,7 @@ impl Account { pub fn new_genesis_account(address: AccountAddress) -> Self { Account { addr: address, - pubkey: GENESIS_KEYPAIR.1.clone(), + pubkey: AccountPublicKey::Ed25519(GENESIS_KEYPAIR.1.clone()), privkey: GENESIS_KEYPAIR.0.clone(), } } @@ -147,14 +171,20 @@ impl Account { /// Changes the keys for this account to the provided ones. pub fn rotate_key(&mut self, privkey: Ed25519PrivateKey, pubkey: Ed25519PublicKey) { self.privkey = privkey; - self.pubkey = pubkey; + self.pubkey = AccountPublicKey::Ed25519(pubkey); } /// Computes the authentication key for this account, as stored on the chain. /// /// This is the same as the account's address if the keys have never been rotated. pub fn auth_key(&self) -> Vec { - AuthenticationKey::ed25519(&self.pubkey).to_vec() + match &self.pubkey { + AccountPublicKey::Ed25519(pk) => AuthenticationKey::ed25519(pk), + AccountPublicKey::Keyless(pk) => { + AuthenticationKey::any_key(AnyPublicKey::keyless(pk.clone())) + }, + } + .to_vec() } pub fn transaction(&self) -> TransactionBuilder { @@ -245,31 +275,26 @@ impl TransactionBuilder { self } - pub fn raw(self) -> RawTransaction { + pub fn raw(&self) -> RawTransaction { RawTransaction::new( *self.sender.address(), self.sequence_number.expect("sequence number not set"), - self.program.expect("transaction payload not set"), + self.program.clone().expect("transaction payload not set"), self.max_gas_amount.unwrap_or(gas_costs::TXN_RESERVED), self.gas_unit_price.unwrap_or(0), self.ttl.unwrap_or(DEFAULT_EXPIRATION_TIME), - ChainId::test(), + self.chain_id.unwrap_or_else(ChainId::test), //ChainId::test(), ) } pub fn sign(self) -> SignedTransaction { - RawTransaction::new( - *self.sender.address(), - self.sequence_number.expect("sequence number not set"), - self.program.expect("transaction payload not set"), - self.max_gas_amount.unwrap_or(gas_costs::TXN_RESERVED), - self.gas_unit_price.unwrap_or(0), - self.ttl.unwrap_or(DEFAULT_EXPIRATION_TIME), - self.chain_id.unwrap_or_else(ChainId::test), - ) - .sign(&self.sender.privkey, self.sender.pubkey) - .unwrap() - .into_inner() + self.raw() + .sign( + &self.sender.privkey, + self.sender.pubkey.as_ed25519().unwrap(), + ) + .unwrap() + .into_inner() } pub fn sign_multi_agent(self) -> SignedTransaction { @@ -283,22 +308,14 @@ impl TransactionBuilder { .iter() .map(|signer| &signer.privkey) .collect(); - RawTransaction::new( - *self.sender.address(), - self.sequence_number.expect("sequence number not set"), - self.program.expect("transaction payload not set"), - self.max_gas_amount.unwrap_or(gas_costs::TXN_RESERVED), - self.gas_unit_price.unwrap_or(0), - self.ttl.unwrap_or(DEFAULT_EXPIRATION_TIME), - ChainId::test(), - ) - .sign_multi_agent( - &self.sender.privkey, - secondary_signer_addresses, - secondary_private_keys, - ) - .unwrap() - .into_inner() + self.raw() + .sign_multi_agent( + &self.sender.privkey, + secondary_signer_addresses, + secondary_private_keys, + ) + .unwrap() + .into_inner() } pub fn sign_fee_payer(self) -> SignedTransaction { @@ -312,25 +329,17 @@ impl TransactionBuilder { .iter() .map(|signer| &signer.privkey) .collect(); - let fee_payer = self.fee_payer.unwrap(); - RawTransaction::new( - *self.sender.address(), - self.sequence_number.expect("sequence number not set"), - self.program.expect("transaction payload not set"), - self.max_gas_amount.unwrap_or(gas_costs::TXN_RESERVED), - self.gas_unit_price.unwrap_or(0), - self.ttl.unwrap_or(DEFAULT_EXPIRATION_TIME), - ChainId::test(), - ) - .sign_fee_payer( - &self.sender.privkey, - secondary_signer_addresses, - secondary_private_keys, - *fee_payer.address(), - &fee_payer.privkey, - ) - .unwrap() - .into_inner() + let fee_payer = self.fee_payer.clone().unwrap(); + self.raw() + .sign_fee_payer( + &self.sender.privkey, + secondary_signer_addresses, + secondary_private_keys, + *fee_payer.address(), + &fee_payer.privkey, + ) + .unwrap() + .into_inner() } } @@ -461,7 +470,7 @@ impl AccountData { pub fn to_bytes(&self) -> Vec { let account = AccountResource::new( self.sequence_number, - AuthenticationKey::ed25519(&self.account.pubkey).to_vec(), + self.account.auth_key(), self.coin_register_events.clone(), self.key_rotation_events.clone(), ); diff --git a/aptos-move/e2e-testsuite/src/tests/verify_txn.rs b/aptos-move/e2e-testsuite/src/tests/verify_txn.rs index 2ca7a3dc76788..737e811c3d256 100644 --- a/aptos-move/e2e-testsuite/src/tests/verify_txn.rs +++ b/aptos-move/e2e-testsuite/src/tests/verify_txn.rs @@ -40,7 +40,7 @@ fn verify_signature() { *sender.address(), 0, &private_key, - sender.account().pubkey.clone(), + sender.account().pubkey.as_ed25519().unwrap(), program, ); @@ -70,9 +70,9 @@ fn verify_multi_agent_invalid_sender_signature() { vec![*secondary_signer.address()], 10, &private_key, - sender.account().pubkey.clone(), + sender.account().pubkey.as_ed25519().unwrap(), vec![&secondary_signer.account().privkey], - vec![secondary_signer.account().pubkey.clone()], + vec![secondary_signer.account().pubkey.as_ed25519().unwrap()], None, ); assert_prologue_parity!( @@ -100,9 +100,9 @@ fn verify_multi_agent_invalid_secondary_signature() { vec![*secondary_signer.address()], 10, &sender.account().privkey, - sender.account().pubkey.clone(), + sender.account().pubkey.as_ed25519().unwrap(), vec![&private_key], - vec![secondary_signer.account().pubkey.clone()], + vec![secondary_signer.account().pubkey.as_ed25519().unwrap()], None, ); assert_prologue_parity!( @@ -134,16 +134,16 @@ fn verify_multi_agent_duplicate_secondary_signer() { ], 10, &sender.account().privkey, - sender.account().pubkey.clone(), + sender.account().pubkey.as_ed25519().unwrap(), vec![ &secondary_signer.account().privkey, &third_signer.account().privkey, &secondary_signer.account().privkey, ], vec![ - secondary_signer.account().pubkey.clone(), - third_signer.account().pubkey.clone(), - secondary_signer.account().pubkey.clone(), + secondary_signer.account().pubkey.as_ed25519().unwrap(), + third_signer.account().pubkey.as_ed25519().unwrap(), + secondary_signer.account().pubkey.as_ed25519().unwrap(), ], None, ); @@ -212,7 +212,7 @@ fn verify_simple_payment() { .max_gas_amount(100_000) .gas_unit_price(1) .raw() - .sign(&sender.account().privkey, sender.account().pubkey.clone()) + .sign(&sender.account().privkey, sender.account().pubkey.as_ed25519().unwrap()) .unwrap() .into_inner(); diff --git a/testsuite/fuzzer/fuzz/fuzz_targets/move/aptosvm_publish_and_run.rs b/testsuite/fuzzer/fuzz/fuzz_targets/move/aptosvm_publish_and_run.rs index 0b52df4d2db67..b1445d8f1e94d 100644 --- a/testsuite/fuzzer/fuzz/fuzz_targets/move/aptosvm_publish_and_run.rs +++ b/testsuite/fuzzer/fuzz/fuzz_targets/move/aptosvm_publish_and_run.rs @@ -350,7 +350,7 @@ fn run_case(mut input: RunnableState) -> Result<(), Corpus> { let raw_tx = tx.raw(); let tx = match input.tx_auth_type { Authenticator::Ed25519 { sender: _ } => raw_tx - .sign(&sender_acc.privkey, sender_acc.pubkey) + .sign(&sender_acc.privkey, sender_acc.pubkey.as_ed25519().unwrap()) .map_err(|_| Corpus::Keep)? .into_inner(), Authenticator::MultiAgent { diff --git a/types/src/keyless/configuration.rs b/types/src/keyless/configuration.rs index 265258466f154..5076667a0241c 100644 --- a/types/src/keyless/configuration.rs +++ b/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, pub max_signatures_per_txn: u16, diff --git a/types/src/keyless/groth16_vk.rs b/types/src/keyless/groth16_vk.rs index 2260d0d0390f6..b1c1bf6a08fa1 100644 --- a/types/src/keyless/groth16_vk.rs +++ b/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, pub beta_g2: Vec, From 9deead1776093f06dcf1357678ea1794e9aa1fb8 Mon Sep 17 00:00:00 2001 From: Alin Tomescu Date: Fri, 1 Mar 2024 11:34:52 -0800 Subject: [PATCH 4/4] added e2e test for zkless path --- .../src/tests/keyless_feature_gating.rs | 54 ++++++++++++++++--- .../e2e-testsuite/src/tests/verify_txn.rs | 5 +- 2 files changed, 52 insertions(+), 7 deletions(-) diff --git a/aptos-move/e2e-move-tests/src/tests/keyless_feature_gating.rs b/aptos-move/e2e-move-tests/src/tests/keyless_feature_gating.rs index 749a0610df8e5..0aea703c64ede 100644 --- a/aptos-move/e2e-move-tests/src/tests/keyless_feature_gating.rs +++ b/aptos-move/e2e-move-tests/src/tests/keyless_feature_gating.rs @@ -8,6 +8,7 @@ use aptos_types::{ keyless::{ test_utils::{ get_sample_esk, get_sample_groth16_sig_and_pk, get_sample_iss, get_sample_jwk, + get_sample_openid_sig_and_pk, }, Configuration, KeylessPublicKey, KeylessSignature, ZkpOrOpenIdSig, }, @@ -23,13 +24,13 @@ use move_core_types::{ }; #[test] -fn test_keyless_feature_is_disabled() { +fn test_keyless_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_groth16_txn(&mut h, sig, pk, bob); + let transaction = get_keyless_txn(&mut h, sig, pk, bob); let output = h.run_raw(transaction); match output.status() { @@ -43,7 +44,7 @@ fn test_keyless_feature_is_disabled() { } #[test] -fn test_keyless_feature_is_enabled() { +fn test_keyless_enabled() { let mut h = MoveHarness::new_with_features(vec![FeatureFlag::KEYLESS_ACCOUNTS], vec![]); let (sig, pk) = get_sample_groth16_sig_and_pk(); @@ -52,14 +53,55 @@ fn test_keyless_feature_is_enabled() { // initialize JWK run_setup_script(&mut h); - let transaction = get_keyless_groth16_txn(&mut h, sig, pk, bob); + let transaction = get_keyless_txn(&mut h, sig, pk, bob); + + let output = h.run_raw(transaction); + assert_success!(output.status().clone()); +} + +#[test] +fn test_keyless_enabled_but_zkless_disabled() { + let mut h = MoveHarness::new_with_features(vec![FeatureFlag::KEYLESS_ACCOUNTS], vec![ + FeatureFlag::KEYLESS_BUT_ZKLESS_ACCOUNTS, + ]); + + let (sig, pk) = get_sample_openid_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_txn(&mut h, sig, pk, bob); + + let output = h.run_raw(transaction); + match output.status() { + TransactionStatus::Discard(status) => { + assert_eq!(*status, FEATURE_UNDER_GATING) + }, + _ => { + panic!("Expected to get FEATURE_UNDER_GATING DiscardedVMStatus") + }, + } +} + +#[test] +fn test_keyless_enabled_but_zkless_enabled() { + let mut h = MoveHarness::new_with_features(vec![FeatureFlag::KEYLESS_ACCOUNTS], vec![]); + + let (sig, pk) = get_sample_openid_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_txn(&mut h, sig, pk, bob); let output = h.run_raw(transaction); assert_success!(output.status().clone()); } /// Creates and funds a new account at `pk` and sends coins to `recipient`. -fn get_keyless_groth16_txn( +fn get_keyless_txn( h: &mut MoveHarness, mut sig: KeylessSignature, pk: KeylessPublicKey, @@ -134,7 +176,7 @@ fn run_setup_script(h: &mut MoveHarness) { TransactionArgument::U8Vector(jwk.n.into_bytes()), TransactionArgument::U64(config.max_exp_horizon_secs), ])) - .sequence_number(h.sequence_number(&core_resources.address())) + .sequence_number(h.sequence_number(core_resources.address())) .max_gas_amount(1_000_000) .gas_unit_price(1) .sign(); diff --git a/aptos-move/e2e-testsuite/src/tests/verify_txn.rs b/aptos-move/e2e-testsuite/src/tests/verify_txn.rs index 737e811c3d256..9a7b2f54183a1 100644 --- a/aptos-move/e2e-testsuite/src/tests/verify_txn.rs +++ b/aptos-move/e2e-testsuite/src/tests/verify_txn.rs @@ -212,7 +212,10 @@ fn verify_simple_payment() { .max_gas_amount(100_000) .gas_unit_price(1) .raw() - .sign(&sender.account().privkey, sender.account().pubkey.as_ed25519().unwrap()) + .sign( + &sender.account().privkey, + sender.account().pubkey.as_ed25519().unwrap(), + ) .unwrap() .into_inner();