Skip to content

Commit

Permalink
Addressed review comments
Browse files Browse the repository at this point in the history
  • Loading branch information
Aregnaz Harutyunyan committed Dec 12, 2024
1 parent 48593ee commit e1c46bf
Show file tree
Hide file tree
Showing 22 changed files with 2,440 additions and 2,513 deletions.
19 changes: 5 additions & 14 deletions api/src/transactions.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,6 @@ use aptos_api_types::{
MAX_RECURSIVE_TYPES_ALLOWED, U64,
};
use aptos_crypto::{hash::CryptoHash, signing_message};
use aptos_types::transaction::automation::AutomationTransactionPayload;
use aptos_types::{
account_address::AccountAddress,
mempool_status::MempoolStatusCode,
Expand Down Expand Up @@ -1033,19 +1032,11 @@ impl TransactionsApi {
})?;
// Verify the signed transaction
match signed_transaction.payload() {
TransactionPayload::Automation(auto_payload) => match auto_payload {
AutomationTransactionPayload::EntryFunction(e) => {
TransactionsApi::validate_entry_function_payload_format(
ledger_info,
e,
)?;
},
AutomationTransactionPayload::EntryFunctionArguments(args) => {
TransactionsApi::validate_entry_function_payload_format(
ledger_info,
args.inner_payload(),
)?;
},
TransactionPayload::Automation(params) => {
TransactionsApi::validate_entry_function_payload_format(
ledger_info,
params.automated_function(),
)?;
},
TransactionPayload::EntryFunction(entry_function) => {
TransactionsApi::validate_entry_function_payload_format(
Expand Down
58 changes: 26 additions & 32 deletions api/types/src/convert.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,10 @@

use crate::{
transaction::{
AutomationTransactionPayload as ApiAutomationTransactionPayload, BlockEpilogueTransaction, DecodedTableData, DeleteModule, DeleteResource, DeleteTableItem,
DeletedTableData, MultisigPayload, MultisigTransactionPayload, StateCheckpointTransaction,
UserTransactionRequestInner, WriteModule, WriteResource, WriteTableItem,
AutomationRegistrationParams, BlockEpilogueTransaction, DecodedTableData, DeleteModule,
DeleteResource, DeleteTableItem, DeletedTableData, MultisigPayload,
MultisigTransactionPayload, StateCheckpointTransaction, UserTransactionRequestInner,
WriteModule, WriteResource, WriteTableItem,
},
view::{ViewFunction, ViewRequest},
Address, Bytecode, DirectWriteSet, EntryFunctionId, EntryFunctionPayload, Event,
Expand All @@ -22,9 +23,7 @@ use aptos_crypto::{hash::CryptoHash, HashValue};
use aptos_logger::{sample, sample::SampleRate};
use aptos_resource_viewer::AptosValueAnnotator;
use aptos_storage_interface::DbReader;
use aptos_types::transaction::automation::{
AutomationTransactionArguments, AutomationTransactionPayload,
};
use aptos_types::transaction::automation::RegistrationParams;
use aptos_types::{
access_path::{AccessPath, Path},
chain_id::ChainId,
Expand Down Expand Up @@ -281,8 +280,8 @@ impl<'a, S: StateView> MoveConverter<'a, S> {
use aptos_types::transaction::TransactionPayload::*;
let ret = match payload {
Script(s) => TransactionPayload::ScriptPayload(s.try_into()?),
Automation(AutomationTransactionPayload::EntryFunction(fun)) | EntryFunction(fun) => {
let entry_function_payload = self.try_to_entry_function_payload(fun)?;
EntryFunction(fun) => {
let entry_function_payload = self.try_into_entry_function_payload(fun)?;
TransactionPayload::EntryFunctionPayload(entry_function_payload)
},
Multisig(multisig) => {
Expand All @@ -292,7 +291,7 @@ impl<'a, S: StateView> MoveConverter<'a, S> {
entry_function,
) => {
let entry_function_payload =
self.try_to_entry_function_payload(entry_function)?;
self.try_into_entry_function_payload(entry_function)?;
Some(MultisigTransactionPayload::EntryFunctionPayload(
entry_function_payload,
))
Expand All @@ -309,15 +308,11 @@ impl<'a, S: StateView> MoveConverter<'a, S> {

// Deprecated.
ModuleBundle(_) => bail!("Module bundle payload has been removed"),
Automation(AutomationTransactionPayload::EntryFunctionArguments(args)) => {
let (
inner_payload,
max_gas_amount,
gas_price_cap,
expiration_timestamp_secs,
) = args.into_inner();
let auto_payload = ApiAutomationTransactionPayload {
inner_payload: self.try_to_entry_function_payload(inner_payload)?,
Automation(params) => {
let (inner_payload, max_gas_amount, gas_price_cap, expiration_timestamp_secs) =
params.into_inner();
let auto_payload = AutomationRegistrationParams {
automated_function: self.try_into_entry_function_payload(inner_payload)?,
expiration_timestamp_secs,
max_gas_amount,
gas_price_cap,
Expand Down Expand Up @@ -629,7 +624,7 @@ impl<'a, S: StateView> MoveConverter<'a, S> {

let ret = match payload {
TransactionPayload::EntryFunctionPayload(entry_func_payload) => {
let entry_function = self.try_to_aptos_core_entry_function(entry_func_payload)?;
let entry_function = self.try_into_supra_core_entry_function(entry_func_payload)?;
Target::EntryFunction(entry_function)
},
TransactionPayload::ScriptPayload(script) => {
Expand Down Expand Up @@ -662,7 +657,7 @@ impl<'a, S: StateView> MoveConverter<'a, S> {
match payload {
MultisigTransactionPayload::EntryFunctionPayload(entry_function) => {
let entry_function =
self.try_to_aptos_core_entry_function(entry_function)?;
self.try_into_supra_core_entry_function(entry_function)?;
Some(
aptos_types::transaction::MultisigTransactionPayload::EntryFunction(
entry_function,
Expand All @@ -684,20 +679,19 @@ impl<'a, S: StateView> MoveConverter<'a, S> {
bail!("Module bundle payload has been removed")
},
TransactionPayload::AutomationPayload(payload) => {
let ApiAutomationTransactionPayload {
inner_payload,
let AutomationRegistrationParams {
automated_function,
expiration_timestamp_secs,
max_gas_amount,
gas_price_cap,
} = payload;
let aptos_inner_payload = self.try_to_aptos_core_entry_function(inner_payload)?;
Target::Automation(AutomationTransactionPayload::EntryFunctionArguments(
AutomationTransactionArguments::new(
aptos_inner_payload,
expiration_timestamp_secs,
max_gas_amount,
gas_price_cap,
),
let core_automated_function =
self.try_into_supra_core_entry_function(automated_function)?;
Target::Automation(RegistrationParams::new(
core_automated_function,
expiration_timestamp_secs,
max_gas_amount,
gas_price_cap,
))
},
};
Expand Down Expand Up @@ -1008,7 +1002,7 @@ impl<'a, S: StateView> MoveConverter<'a, S> {
Ok(id.to_string())
}

fn try_to_entry_function_payload(&self, fun: EntryFunction) -> Result<EntryFunctionPayload> {
fn try_into_entry_function_payload(&self, fun: EntryFunction) -> Result<EntryFunctionPayload> {
let (module, function, ty_args, args) = fun.into_inner();
let func_args = self
.inner
Expand All @@ -1035,7 +1029,7 @@ impl<'a, S: StateView> MoveConverter<'a, S> {
})
}

fn try_to_aptos_core_entry_function(
fn try_into_supra_core_entry_function(
&self,
entry_function: EntryFunctionPayload,
) -> Result<EntryFunction> {
Expand Down
12 changes: 6 additions & 6 deletions api/types/src/transaction.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -946,7 +946,7 @@ pub enum TransactionPayload {
ModuleBundlePayload(DeprecatedModuleBundlePayload),

MultisigPayload(MultisigPayload),
AutomationPayload(AutomationTransactionPayload),
AutomationPayload(AutomationRegistrationParams),
}

impl VerifyInput for TransactionPayload {
Expand Down Expand Up @@ -1064,17 +1064,17 @@ impl VerifyInput for MultisigPayload {
}

#[derive(Clone, Debug, PartialEq, Eq, Serialize, Deserialize, Object)]
pub struct AutomationTransactionPayload {
pub inner_payload: EntryFunctionPayload,
pub struct AutomationRegistrationParams {
pub automated_function: EntryFunctionPayload,
pub expiration_timestamp_secs: u64,
pub max_gas_amount: u64,
pub gas_price_cap: u64,
}

impl VerifyInput for AutomationTransactionPayload {
impl VerifyInput for AutomationRegistrationParams {
fn verify(&self) -> anyhow::Result<()> {
self.inner_payload.function.verify()?;
for type_arg in self.inner_payload.type_arguments.iter() {
self.automated_function.function.verify()?;
for type_arg in self.automated_function.type_arguments.iter() {
type_arg.verify(0)?;
}
Ok(())
Expand Down
137 changes: 23 additions & 114 deletions aptos-move/aptos-vm/src/aptos_vm.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ use aptos_logger::{enabled, prelude::*, Level};
use aptos_metrics_core::TimerHelper;
#[cfg(any(test, feature = "testing"))]
use aptos_types::state_store::StateViewId;
use aptos_types::transaction::automation::AutomationTransactionPayload;
use aptos_types::transaction::automation::RegistrationParams;
use aptos_types::{
account_config::{self, new_block_event_key, AccountResource},
block_executor::{
Expand Down Expand Up @@ -887,7 +887,7 @@ impl AptosVM {
gas_meter: &mut impl AptosGasMeter,
traversal_context: &mut TraversalContext<'a>,
txn_data: &TransactionMetadata,
payload: &'a AutomationTransactionPayload,
registration_params: &'a RegistrationParams,
log_context: &AdapterLogSchema,
new_published_modules_loaded: &mut bool,
change_set_configs: &ChangeSetConfigs,
Expand All @@ -899,63 +899,32 @@ impl AptosVM {
message: None,
})
});
if !payload.is_valid() {
return Err(VMStatus::error(
StatusCode::INVALID_AUTOMATION_PAYLOAD,
Some(AutomationTransactionPayload::entry_function_reference()),
));
}

gas_meter.charge_intrinsic_gas_for_transaction(txn_data.transaction_size())?;
if txn_data.is_keyless() {
gas_meter.charge_keyless()?;
}

match payload {
AutomationTransactionPayload::EntryFunction(entry_fn) => {
session.execute(|session| {
self.validate_automation_txn_inner_payload(
session,
gas_meter,
txn_data.senders(),
entry_fn,
)
})?;
session.execute(|session| {
self.validate_and_execute_entry_function(
resolver,
session,
gas_meter,
traversal_context,
txn_data.senders(),
entry_fn,
txn_data,
)
})?
},
AutomationTransactionPayload::EntryFunctionArguments(args) => {
session.execute(|session| {
self.validate_automation_txn_inner_payload_v2(
session,
gas_meter,
txn_data.senders(),
args.inner_payload(),
)
})?;
let entry_function = EntryFunction::from(args.clone());
session.execute(|session| {
self.validate_and_execute_entry_function(
resolver,
session,
gas_meter,
traversal_context,
txn_data.senders(),
&entry_function,
txn_data,
)
})?
},
};
session.execute(|session| {
self.validate_automated_function(
session,
gas_meter,
txn_data.senders(),
registration_params.automated_function(),
)
})?;
let register_entry_function = EntryFunction::from(registration_params.clone());
session.execute(|session| {
self.validate_and_execute_entry_function(
resolver,
session,
gas_meter,
traversal_context,
txn_data.senders(),
&register_entry_function,
txn_data,
)
})?;

session.execute(|session| {
self.resolve_pending_code_publish(
Expand Down Expand Up @@ -2559,68 +2528,8 @@ impl AptosVM {
})
}

/// Validates inner payload/entry function of automation transaction to be valid.
/// It is expected that the first actual argument of the automation transaction payload is
/// the bcs serialized payload/entry function representing automation task.
fn validate_automation_txn_inner_payload(
&self,
session: &mut SessionExt,
gas_meter: &mut impl AptosGasMeter,
senders: Vec<AccountAddress>,
automation_entry_fn: &EntryFunction,
) -> Result<(), VMStatus> {
if automation_entry_fn.args().is_empty() {
return Err(VMStatus::error(
StatusCode::INVALID_AUTOMATION_PAYLOAD_ARGUMENTS,
Some("Automation transaction payload arguments are empty".to_string()),
));
}
// first we should deserialize actual parameter of Vec<u8> which represents bytes of the EntryFunction.
let param_bytes = &automation_entry_fn.args()[0];
let inner_entry_function = bcs::from_bytes::<Vec<u8>>(param_bytes)
.and_then(|payload_bytes| bcs::from_bytes::<EntryFunction>(&payload_bytes))
.map_err(|e| {
VMStatus::error(
StatusCode::FAILED_TO_DESERIALIZE_ARGUMENT,
Some(format!(
"Automation transaction payload first argument is \
expected to be BCS bytes of entry-function {}",
e
)),
)
})?;

let function = session.load_function(
inner_entry_function.module(),
inner_entry_function.function(),
inner_entry_function.ty_args(),
)?;
let struct_constructors_enabled =
self.features().is_enabled(FeatureFlag::STRUCT_CONSTRUCTORS);
let (_, _, _, actual_args) = inner_entry_function.into_inner();
// By constructing args we are making sure that function execution in scope of automated
// task will not fail due to invalid arguments passed
verifier::transaction_arg_validation::validate_combine_signer_and_txn_args(
session,
gas_meter,
senders,
actual_args,
&function,
struct_constructors_enabled,
)
.map_err(|e| {
VMStatus::error(
StatusCode::INVALID_AUTOMATION_INNER_PAYLOAD,
Some(format!(
"Automation transaction inner payload validation failed. Details: {e:?}",
)),
)
})
.map(drop)
}

/// Checks inner payload/entry function of automation transaction to be valid.
fn validate_automation_txn_inner_payload_v2(
fn validate_automated_function(
&self,
session: &mut SessionExt,
gas_meter: &mut impl AptosGasMeter,
Expand Down
2 changes: 1 addition & 1 deletion aptos-move/aptos-vm/src/keyless_validation.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -296,4 +296,4 @@ pub(crate) fn validate_authenticators(
}

Ok(())
}
}
Loading

0 comments on commit e1c46bf

Please sign in to comment.