Skip to content

Commit

Permalink
[AN-Issue-1409] Handled automation-registry state update on new epoch (
Browse files Browse the repository at this point in the history
…#144)

* Handled automation-registry state update on new epoch

- split automation registry external API and internal state to avoid circular dependencies
- enabled automation-registry-state update on new epoch from block module.

* Addressed review comments

* Added couple of unit-tests for automation-registry-state

* Updated automation-registry to accept registration transaction hash as argument

- Moved basic parameters checks of registration to automation_registry_state
- Added and updated unit tests of automation_registry_state

* Removed redundant imports.

* Addressed review comments

---------

Co-authored-by: Aregnaz Harutyunyan <>
  • Loading branch information
aregng authored Dec 24, 2024
1 parent bb6fb1b commit a978ff6
Show file tree
Hide file tree
Showing 20 changed files with 1,775 additions and 715 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -270,7 +270,6 @@ crate::gas_schedule::macros::define_gas_parameters!(

[transaction_context_get_txn_hash_base: InternalGas, { 10.. => "transaction_context.get_txn_hash.base" }, 735],
[transaction_context_get_script_hash_base: InternalGas, "transaction_context.get_script_hash.base", 735],
[transaction_context_get_txn_app_hash_base: InternalGas, "transaction_context.get_txn_app_hash.base", 735],
// Based on SHA3-256's cost
[transaction_context_generate_unique_address_base: InternalGas, { 10.. => "transaction_context.generate_unique_address.base" }, 14704],
[transaction_context_sender_base: InternalGas, {RELEASE_V1_12.. => "transaction_context.sender.base"}, 735],
Expand Down
5 changes: 4 additions & 1 deletion aptos-move/aptos-vm/src/aptos_vm.rs
Original file line number Diff line number Diff line change
Expand Up @@ -921,6 +921,7 @@ impl AptosVM {
traversal_context,
txn_data.sender(),
registration_params,
txn_data
)
})?;

Expand Down Expand Up @@ -958,6 +959,7 @@ impl AptosVM {
traversal_context: &mut TraversalContext,
sender: AccountAddress,
registration_params: &RegistrationParams,
txn_metadata: &TransactionMetadata,
) -> 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
Expand All @@ -972,7 +974,8 @@ impl AptosVM {
[(module_id.address(), module_id.name())],
)?;
}
let args = registration_params.serialized_args_with_sender(sender);
let args = registration_params
.serialized_args_with_sender_and_parent_hash(sender, txn_metadata.txn_app_hash.clone());

session.execute_function_bypass_visibility(
registration_params.module_id(),
Expand Down
1 change: 0 additions & 1 deletion aptos-move/aptos-vm/src/transaction_metadata.rs
Original file line number Diff line number Diff line change
Expand Up @@ -173,7 +173,6 @@ impl TransactionMetadata {
self.gas_unit_price.into(),
self.chain_id.id(),
payload_type_reference,
self.txn_app_hash.clone(),
)
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -130,7 +130,7 @@ fn check_successful_registration() {
let result = view_output.values.expect("Valid result");
assert_eq!(result.len(), 1);
let next_task_id = bcs::from_bytes::<u64>(&result[0]).unwrap();
assert_eq!(next_task_id, 2);
assert_eq!(next_task_id, 1);
}

#[test]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -147,12 +147,19 @@ pub enum EntryFunctionCall {
cap_update_table: Vec<u8>,
},

/// Remove Automatioon task entry.
AutomationRegistryRemoveTask {
registry_id: u64,
/// Cancel Automation task with specified id.
/// Only existing task, which is PENDING or ACTIVE, can be cancled and only by task onwer.
/// If the task is
/// - active, its state is updated to be CANCELLED.
/// - pending, it is removed form the list.
/// - cancelled, an error is reported
/// Committed gas-limit is updated by reducing it with the max-gas-amount of the cancelled task.
AutomationRegistryCancelTask {
id: u64,
},

/// Update Automation gas limit
/// Update Automation gas limit.
/// If the committed gas amount for the next epoch is greater then the new gas limit, then error is reported.
AutomationRegistryUpdateAutomationGasLimit {
automation_gas_limit: u64,
},
Expand All @@ -162,12 +169,6 @@ pub enum EntryFunctionCall {
duration_upper_limit: u64,
},

/// Withdraw accumulated automation task fees from the resource account - access by admin
AutomationRegistryWithdrawAutomationTaskFees {
to: AccountAddress,
amount: u64,
},

/// Same as `publish_package` but as an entry function which can be called as a transaction. Because
/// of current restrictions for txn parameters, the metadata needs to be passed in serialized form.
CodePublishPackageTxn {
Expand Down Expand Up @@ -1202,18 +1203,13 @@ impl EntryFunctionCall {
new_public_key_bytes,
cap_update_table,
),
AutomationRegistryRemoveTask { registry_id } => {
automation_registry_remove_task(registry_id)
},
AutomationRegistryCancelTask { id } => automation_registry_cancel_task(id),
AutomationRegistryUpdateAutomationGasLimit {
automation_gas_limit,
} => automation_registry_update_automation_gas_limit(automation_gas_limit),
AutomationRegistryUpdateDurationUpperLimit {
duration_upper_limit,
} => automation_registry_update_duration_upper_limit(duration_upper_limit),
AutomationRegistryWithdrawAutomationTaskFees { to, amount } => {
automation_registry_withdraw_automation_task_fees(to, amount)
},
CodePublishPackageTxn {
metadata_serialized,
code,
Expand Down Expand Up @@ -2150,8 +2146,14 @@ pub fn account_rotate_authentication_key_with_rotation_capability(
))
}

/// Remove Automatioon task entry.
pub fn automation_registry_remove_task(registry_id: u64) -> TransactionPayload {
/// Cancel Automation task with specified id.
/// Only existing task, which is PENDING or ACTIVE, can be cancled and only by task onwer.
/// If the task is
/// - active, its state is updated to be CANCELLED.
/// - pending, it is removed form the list.
/// - cancelled, an error is reported
/// Committed gas-limit is updated by reducing it with the max-gas-amount of the cancelled task.
pub fn automation_registry_cancel_task(id: u64) -> TransactionPayload {
TransactionPayload::EntryFunction(EntryFunction::new(
ModuleId::new(
AccountAddress::new([
Expand All @@ -2160,13 +2162,14 @@ pub fn automation_registry_remove_task(registry_id: u64) -> TransactionPayload {
]),
ident_str!("automation_registry").to_owned(),
),
ident_str!("remove_task").to_owned(),
ident_str!("cancel_task").to_owned(),
vec![],
vec![bcs::to_bytes(&registry_id).unwrap()],
vec![bcs::to_bytes(&id).unwrap()],
))
}

/// Update Automation gas limit
/// Update Automation gas limit.
/// If the committed gas amount for the next epoch is greater then the new gas limit, then error is reported.
pub fn automation_registry_update_automation_gas_limit(
automation_gas_limit: u64,
) -> TransactionPayload {
Expand Down Expand Up @@ -2202,25 +2205,6 @@ pub fn automation_registry_update_duration_upper_limit(
))
}

/// Withdraw accumulated automation task fees from the resource account - access by admin
pub fn automation_registry_withdraw_automation_task_fees(
to: AccountAddress,
amount: 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!("withdraw_automation_task_fees").to_owned(),
vec![],
vec![bcs::to_bytes(&to).unwrap(), bcs::to_bytes(&amount).unwrap()],
))
}

/// Same as `publish_package` but as an entry function which can be called as a transaction. Because
/// of current restrictions for txn parameters, the metadata needs to be passed in serialized form.
pub fn code_publish_package_txn(
Expand Down Expand Up @@ -5381,12 +5365,12 @@ mod decoder {
}
}

pub fn automation_registry_remove_task(
pub fn automation_registry_cancel_task(
payload: &TransactionPayload,
) -> Option<EntryFunctionCall> {
if let TransactionPayload::EntryFunction(script) = payload {
Some(EntryFunctionCall::AutomationRegistryRemoveTask {
registry_id: bcs::from_bytes(script.args().get(0)?).ok()?,
Some(EntryFunctionCall::AutomationRegistryCancelTask {
id: bcs::from_bytes(script.args().get(0)?).ok()?,
})
} else {
None
Expand Down Expand Up @@ -5421,21 +5405,6 @@ mod decoder {
}
}

pub fn automation_registry_withdraw_automation_task_fees(
payload: &TransactionPayload,
) -> Option<EntryFunctionCall> {
if let TransactionPayload::EntryFunction(script) = payload {
Some(
EntryFunctionCall::AutomationRegistryWithdrawAutomationTaskFees {
to: bcs::from_bytes(script.args().get(0)?).ok()?,
amount: bcs::from_bytes(script.args().get(1)?).ok()?,
},
)
} else {
None
}
}

pub fn code_publish_package_txn(payload: &TransactionPayload) -> Option<EntryFunctionCall> {
if let TransactionPayload::EntryFunction(script) = payload {
Some(EntryFunctionCall::CodePublishPackageTxn {
Expand Down Expand Up @@ -7290,8 +7259,8 @@ static SCRIPT_FUNCTION_DECODER_MAP: once_cell::sync::Lazy<EntryFunctionDecoderMa
Box::new(decoder::account_rotate_authentication_key_with_rotation_capability),
);
map.insert(
"automation_registry_remove_task".to_string(),
Box::new(decoder::automation_registry_remove_task),
"automation_registry_cancel_task".to_string(),
Box::new(decoder::automation_registry_cancel_task),
);
map.insert(
"automation_registry_update_automation_gas_limit".to_string(),
Expand All @@ -7301,10 +7270,6 @@ static SCRIPT_FUNCTION_DECODER_MAP: once_cell::sync::Lazy<EntryFunctionDecoderMa
"automation_registry_update_duration_upper_limit".to_string(),
Box::new(decoder::automation_registry_update_duration_upper_limit),
);
map.insert(
"automation_registry_withdraw_automation_task_fees".to_string(),
Box::new(decoder::automation_registry_withdraw_automation_task_fees),
);
map.insert(
"code_publish_package_txn".to_string(),
Box::new(decoder::code_publish_package_txn),
Expand Down
26 changes: 0 additions & 26 deletions aptos-move/framework/src/natives/transaction_context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -238,31 +238,6 @@ fn native_chain_id_internal(
}
}

/***************************************************************************************************
* native fun get_txn_app_hash
*
* gas cost: base_cost
*
**************************************************************************************************/
fn native_txn_app_hash_internal(
context: &mut SafeNativeContext,
_ty_args: Vec<Type>,
_args: VecDeque<Value>,
) -> SafeNativeResult<SmallVec<[Value; 1]>> {
context.charge(TRANSACTION_CONTEXT_GET_TXN_APP_HASH_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::vector_u8(
transaction_context.txn_app_hash()
)])
} else {
Err(SafeNativeError::Abort {
abort_code: error::invalid_state(abort_codes::ETRANSACTION_CONTEXT_NOT_AVAILABLE),
})
}
}

fn create_option_some_value(value: Value) -> Value {
Value::struct_(Struct::pack(vec![create_singleton_vector(value)]))
}
Expand Down Expand Up @@ -431,7 +406,6 @@ pub fn make_all(
"multisig_payload_internal",
native_multisig_payload_internal,
),
("txn_app_hash_internal", native_txn_app_hash_internal),
];

builder.make_named_natives(natives)
Expand Down
Loading

0 comments on commit a978ff6

Please sign in to comment.