Skip to content

Commit

Permalink
Updated automation::registry to be non-entry and made direct call fro…
Browse files Browse the repository at this point in the history
…m native layer
  • Loading branch information
Aregnaz Harutyunyan committed Dec 12, 2024
1 parent b590ac2 commit 01c361f
Show file tree
Hide file tree
Showing 12 changed files with 60 additions and 235 deletions.
2 changes: 1 addition & 1 deletion aptos-move/aptos-debugger/src/aptos_debugger.rs
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,7 @@ impl AptosDebugger {
gas_meter,
auto_payload.module_id().clone(),
auto_payload.function().to_owned(),
auto_payload.ty_args().to_vec(),
auto_payload.ty_args(),
),
TransactionPayload::EntryFunction(entry_func) => GasProfiler::new_function(
gas_meter,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -281,7 +281,6 @@ crate::gas_schedule::macros::define_gas_parameters!(
[transaction_context_gas_unit_price_base: InternalGas, {RELEASE_V1_12.. => "transaction_context.gas_unit_price.base"}, 735],
[transaction_context_chain_id_base: InternalGas, {RELEASE_V1_12.. => "transaction_context.chain_id.base"}, 735],
[transaction_context_entry_function_payload_base: InternalGas, {RELEASE_V1_12.. => "transaction_context.entry_function_payload.base"}, 735],
[transaction_context_automation_registration_base: InternalGas, {RELEASE_V1_12.. => "transaction_context.automation_registration.base"}, 735],
[transaction_context_entry_function_payload_per_byte_in_str: InternalGasPerByte, {RELEASE_V1_12.. => "transaction_context.entry_function_payload.per_abstract_memory_unit"}, 18],
[transaction_context_multisig_payload_base: InternalGas, {RELEASE_V1_12.. => "transaction_context.multisig_payload.base"}, 735],
[transaction_context_multisig_payload_per_byte_in_str: InternalGasPerByte, {RELEASE_V1_12.. => "transaction_context.multisig_payload.per_abstract_memory_unit"}, 18],
Expand Down
44 changes: 38 additions & 6 deletions aptos-move/aptos-vm/src/aptos_vm.rs
Original file line number Diff line number Diff line change
Expand Up @@ -798,6 +798,8 @@ impl AptosVM {
Ok(())
}



fn execute_script_or_entry_function<'a, 'r, 'l>(
&'l self,
resolver: &'r impl AptosMoveResolver,
Expand Down Expand Up @@ -913,16 +915,14 @@ impl AptosVM {
registration_params.automated_function(),
)
})?;
let register_entry_function = EntryFunction::from(registration_params.clone());
// let register_entry_function = EntryFunction::from(registration_params.clone());
session.execute(|session| {
self.validate_and_execute_entry_function(
resolver,
self.execute_automation_registration(
session,
gas_meter,
traversal_context,
txn_data.senders(),
&register_entry_function,
txn_data,
txn_data.sender(),
registration_params
)
})?;

Expand Down Expand Up @@ -953,6 +953,38 @@ impl AptosVM {
)
}

fn execute_automation_registration(
&self,
session: &mut SessionExt,
gas_meter: &mut impl AptosGasMeter,
traversal_context: &mut TraversalContext,
sender: AccountAddress,
registration_params: &RegistrationParams,
) -> Result<(), VMStatus> {
// Note: Feature gating is needed here because the traversal of the dependencies could
// result in shallow-loading of the modules and therefore subtle changes in
// the error semantics.
if self.gas_feature_version >= 15 {
let module_id = traversal_context
.referenced_module_ids
.alloc(registration_params.module_id().clone());
session.check_dependencies_and_charge_gas(
gas_meter,
traversal_context,
[(module_id.address(), module_id.name())],
)?;
}
let args = registration_params.serialized_args_with_sender(sender);

session.execute_function_bypass_visibility(registration_params.module_id(),
registration_params.function(),
registration_params.ty_args(),
args,
gas_meter,
traversal_context)?;
Ok(())
}

fn charge_change_set(
&self,
change_set: &mut VMChangeSet,
Expand Down
2 changes: 1 addition & 1 deletion aptos-move/e2e-tests/src/executor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -689,7 +689,7 @@ impl FakeExecutor {
gas_meter,
auto_payload.module_id().clone(),
auto_payload.function().to_owned(),
auto_payload.ty_args().to_vec(),
auto_payload.ty_args(),
),
TransactionPayload::EntryFunction(entry_func) => GasProfiler::new_function(
gas_meter,
Expand Down
35 changes: 0 additions & 35 deletions aptos-move/e2e-testsuite/src/tests/automation_registration.rs
Original file line number Diff line number Diff line change
Expand Up @@ -133,41 +133,6 @@ fn check_successful_registration() {
assert_eq!(next_task_id, 2);
}

#[test]
fn check_registration_from_non_automation_context() {
let mut test_context = AutomationRegistrationTestContext::new();
// Prepare inner-entry-function to be automated.
let dest_account = test_context.new_account_data(0, 0);
let inner_entry_function =
aptos_framework_sdk_builder::supra_coin_mint(dest_account.address().clone(), 100)
.into_entry_function();
let inner_entry_function_bytes =
bcs::to_bytes(&inner_entry_function).expect("Can't serialize entry function");

let entry_with_register = aptos_framework_sdk_builder::automation_registry_register(
inner_entry_function_bytes,
3600,
100,
100,
);
let user_txn_with_register_entry = test_context
.txn_sender
.account()
.transaction()
.payload(entry_with_register)
.sequence_number(0)
.sign();

let output = test_context.execute_transaction(user_txn_with_register_entry);
match output.status() {
TransactionStatus::Keep(ExecutionStatus::MoveAbort { code, .. }) => {
//ENOT_AUTOMATION_TXN_CONTEXT
assert_eq!(*code, 7);
},
_ => panic!("Unexpected transaction status: {output:?}"),
}
}

#[test]
fn check_invalid_automation_txn() {
let mut test_context = AutomationRegistrationTestContext::new();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -147,14 +147,6 @@ pub enum EntryFunctionCall {
cap_update_table: Vec<u8>,
},

/// Registers a new automation task entry.
AutomationRegistryRegister {
payload_tx: Vec<u8>,
expiry_time: u64,
max_gas_amount: u64,
gas_price_cap: u64,
},

/// Update Automation gas limit
AutomationRegistryUpdateAutomationGasLimit {
automation_gas_limit: u64,
Expand Down Expand Up @@ -1199,14 +1191,6 @@ impl EntryFunctionCall {
new_public_key_bytes,
cap_update_table,
),
AutomationRegistryRegister {
payload_tx,
expiry_time,
max_gas_amount,
gas_price_cap,
} => {
automation_registry_register(payload_tx, expiry_time, max_gas_amount, gas_price_cap)
},
AutomationRegistryUpdateAutomationGasLimit {
automation_gas_limit,
} => automation_registry_update_automation_gas_limit(automation_gas_limit),
Expand Down Expand Up @@ -2149,32 +2133,6 @@ pub fn account_rotate_authentication_key_with_rotation_capability(
))
}

/// Registers a new automation task entry.
pub fn automation_registry_register(
payload_tx: Vec<u8>,
expiry_time: u64,
max_gas_amount: u64,
gas_price_cap: u64,
) -> TransactionPayload {
TransactionPayload::EntryFunction(EntryFunction::new(
ModuleId::new(
AccountAddress::new([
0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
0, 0, 0, 1,
]),
ident_str!("automation_registry").to_owned(),
),
ident_str!("register").to_owned(),
vec![],
vec![
bcs::to_bytes(&payload_tx).unwrap(),
bcs::to_bytes(&expiry_time).unwrap(),
bcs::to_bytes(&max_gas_amount).unwrap(),
bcs::to_bytes(&gas_price_cap).unwrap(),
],
))
}

/// Update Automation gas limit
pub fn automation_registry_update_automation_gas_limit(
automation_gas_limit: u64,
Expand Down Expand Up @@ -5371,19 +5329,6 @@ mod decoder {
}
}

pub fn automation_registry_register(payload: &TransactionPayload) -> Option<EntryFunctionCall> {
if let TransactionPayload::EntryFunction(script) = payload {
Some(EntryFunctionCall::AutomationRegistryRegister {
payload_tx: bcs::from_bytes(script.args().get(0)?).ok()?,
expiry_time: bcs::from_bytes(script.args().get(1)?).ok()?,
max_gas_amount: bcs::from_bytes(script.args().get(2)?).ok()?,
gas_price_cap: bcs::from_bytes(script.args().get(3)?).ok()?,
})
} else {
None
}
}

pub fn automation_registry_update_automation_gas_limit(
payload: &TransactionPayload,
) -> Option<EntryFunctionCall> {
Expand Down Expand Up @@ -7265,10 +7210,6 @@ static SCRIPT_FUNCTION_DECODER_MAP: once_cell::sync::Lazy<EntryFunctionDecoderMa
"account_rotate_authentication_key_with_rotation_capability".to_string(),
Box::new(decoder::account_rotate_authentication_key_with_rotation_capability),
);
map.insert(
"automation_registry_register".to_string(),
Box::new(decoder::automation_registry_register),
);
map.insert(
"automation_registry_update_automation_gas_limit".to_string(),
Box::new(decoder::automation_registry_update_automation_gas_limit),
Expand Down
24 changes: 0 additions & 24 deletions aptos-move/framework/src/natives/transaction_context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -394,26 +394,6 @@ fn native_multisig_payload_internal(
}
}

/// Checks whether user-transaction context is from on the automation registration transaction,
/// i.e. original transaction has Automation registration payload.
fn native_is_automation_registration_internal(
context: &mut SafeNativeContext,
mut _ty_args: Vec<Type>,
_args: VecDeque<Value>,
) -> SafeNativeResult<SmallVec<[Value; 1]>> {
context.charge(TRANSACTION_CONTEXT_AUTOMATION_REGISTRATION_BASE)?;

let user_transaction_context_opt = get_user_transaction_context_opt_from_context(context);

if let Some(transaction_context) = user_transaction_context_opt {
Ok(smallvec![Value::bool(transaction_context.is_automation_registration())])
} else {
Err(SafeNativeError::Abort {
abort_code: error::invalid_state(abort_codes::ETRANSACTION_CONTEXT_NOT_AVAILABLE),
})
}
}

fn get_user_transaction_context_opt_from_context<'a>(
context: &'a SafeNativeContext,
) -> &'a Option<UserTransactionContext> {
Expand Down Expand Up @@ -451,10 +431,6 @@ pub fn make_all(
"multisig_payload_internal",
native_multisig_payload_internal,
),
(
"is_automation_registration_internal",
native_is_automation_registration_internal,
),
("txn_app_hash_internal", native_txn_app_hash_internal),
];

Expand Down
15 changes: 2 additions & 13 deletions aptos-move/framework/supra-framework/doc/automation_registry.md
Original file line number Diff line number Diff line change
Expand Up @@ -261,16 +261,6 @@ Invalid gas price: it cannot be zero



<a id="0x1_automation_registry_ENOT_AUTOMATION_TXN_CONTEXT"></a>

Entry function is called not from automation registration transaction context.


<pre><code><b>const</b> <a href="automation_registry.md#0x1_automation_registry_ENOT_AUTOMATION_TXN_CONTEXT">ENOT_AUTOMATION_TXN_CONTEXT</a>: u64 = 7;
</code></pre>



<a id="0x1_automation_registry_EREGITRY_NOT_FOUND"></a>

Registry Id not found
Expand Down Expand Up @@ -498,7 +488,7 @@ Calculate and collect registry charge from user
Registers a new automation task entry.


<pre><code><b>public</b> entry <b>fun</b> <a href="automation_registry.md#0x1_automation_registry_register">register</a>(owner: &<a href="../../aptos-stdlib/../move-stdlib/doc/signer.md#0x1_signer">signer</a>, payload_tx: <a href="../../aptos-stdlib/../move-stdlib/doc/vector.md#0x1_vector">vector</a>&lt;u8&gt;, expiry_time: u64, max_gas_amount: u64, gas_price_cap: u64)
<pre><code><b>public</b> <b>fun</b> <a href="automation_registry.md#0x1_automation_registry_register">register</a>(owner: &<a href="../../aptos-stdlib/../move-stdlib/doc/signer.md#0x1_signer">signer</a>, payload_tx: <a href="../../aptos-stdlib/../move-stdlib/doc/vector.md#0x1_vector">vector</a>&lt;u8&gt;, expiry_time: u64, max_gas_amount: u64, gas_price_cap: u64)
</code></pre>


Expand All @@ -507,14 +497,13 @@ Registers a new automation task entry.
<summary>Implementation</summary>


<pre><code><b>public</b> entry <b>fun</b> <a href="automation_registry.md#0x1_automation_registry_register">register</a>(
<pre><code><b>public</b> <b>fun</b> <a href="automation_registry.md#0x1_automation_registry_register">register</a>(
owner: &<a href="../../aptos-stdlib/../move-stdlib/doc/signer.md#0x1_signer">signer</a>,
payload_tx: <a href="../../aptos-stdlib/../move-stdlib/doc/vector.md#0x1_vector">vector</a>&lt;u8&gt;,
expiry_time: u64,
max_gas_amount: u64,
gas_price_cap: u64
) <b>acquires</b> <a href="automation_registry.md#0x1_automation_registry_AutomationRegistry">AutomationRegistry</a> {
<b>assert</b>!(<a href="transaction_context.md#0x1_transaction_context_is_automation_registration">transaction_context::is_automation_registration</a>(), <a href="automation_registry.md#0x1_automation_registry_ENOT_AUTOMATION_TXN_CONTEXT">ENOT_AUTOMATION_TXN_CONTEXT</a>);
<b>let</b> registry_data = <b>borrow_global_mut</b>&lt;<a href="automation_registry.md#0x1_automation_registry_AutomationRegistry">AutomationRegistry</a>&gt;(@supra_framework);

// todo : well formedness check of payload_tx
Expand Down
51 changes: 0 additions & 51 deletions aptos-move/framework/supra-framework/doc/transaction_context.md
Original file line number Diff line number Diff line change
Expand Up @@ -30,8 +30,6 @@
- [Function `chain_id_internal`](#0x1_transaction_context_chain_id_internal)
- [Function `entry_function_payload`](#0x1_transaction_context_entry_function_payload)
- [Function `entry_function_payload_internal`](#0x1_transaction_context_entry_function_payload_internal)
- [Function `is_automation_registration`](#0x1_transaction_context_is_automation_registration)
- [Function `is_automation_registration_internal`](#0x1_transaction_context_is_automation_registration_internal)
- [Function `txn_app_hash`](#0x1_transaction_context_txn_app_hash)
- [Function `txn_app_hash_internal`](#0x1_transaction_context_txn_app_hash_internal)
- [Function `account_address`](#0x1_transaction_context_account_address)
Expand Down Expand Up @@ -736,55 +734,6 @@ This function aborts if called outside of the transaction prologue, execution, o



</details>

<a id="0x1_transaction_context_is_automation_registration"></a>

## Function `is_automation_registration`

Returns true if current user transaction has automation registration payload type. Otherwise, return <code><b>false</b></code>.
This function aborts if called outside of the transaction prologue, execution, or epilogue phases.


<pre><code><b>public</b> <b>fun</b> <a href="transaction_context.md#0x1_transaction_context_is_automation_registration">is_automation_registration</a>(): bool
</code></pre>



<details>
<summary>Implementation</summary>


<pre><code><b>public</b> <b>fun</b> <a href="transaction_context.md#0x1_transaction_context_is_automation_registration">is_automation_registration</a>(): bool {
<b>assert</b>!(<a href="../../aptos-stdlib/../move-stdlib/doc/features.md#0x1_features_transaction_context_extension_enabled">features::transaction_context_extension_enabled</a>(), <a href="../../aptos-stdlib/../move-stdlib/doc/error.md#0x1_error_invalid_state">error::invalid_state</a>(<a href="transaction_context.md#0x1_transaction_context_ETRANSACTION_CONTEXT_EXTENSION_NOT_ENABLED">ETRANSACTION_CONTEXT_EXTENSION_NOT_ENABLED</a>));
<a href="transaction_context.md#0x1_transaction_context_is_automation_registration_internal">is_automation_registration_internal</a>()
}
</code></pre>



</details>

<a id="0x1_transaction_context_is_automation_registration_internal"></a>

## Function `is_automation_registration_internal`



<pre><code><b>fun</b> <a href="transaction_context.md#0x1_transaction_context_is_automation_registration_internal">is_automation_registration_internal</a>(): bool
</code></pre>



<details>
<summary>Implementation</summary>


<pre><code><b>native</b> <b>fun</b> <a href="transaction_context.md#0x1_transaction_context_is_automation_registration_internal">is_automation_registration_internal</a>(): bool;
</code></pre>



</details>

<a id="0x1_transaction_context_txn_app_hash"></a>
Expand Down
Loading

0 comments on commit 01c361f

Please sign in to comment.