From 5ca0c590c8e76366a1d2ea4af58c730f147a7669 Mon Sep 17 00:00:00 2001
From: Aregnaz Harutyunyan <>
Date: Wed, 18 Dec 2024 19:48:06 +0400
Subject: [PATCH] Added couple of unit-tests for automation-registry-state
---
.../doc/automation_registry_state.md | 29 +++-
.../sources/automation_registry.move | 5 +-
.../sources/automation_registry_state.move | 145 ++++++++++++++++--
3 files changed, 161 insertions(+), 18 deletions(-)
diff --git a/aptos-move/framework/supra-framework/doc/automation_registry_state.md b/aptos-move/framework/supra-framework/doc/automation_registry_state.md
index e380eaa6aed4d..66f3c1a99c4f0 100644
--- a/aptos-move/framework/supra-framework/doc/automation_registry_state.md
+++ b/aptos-move/framework/supra-framework/doc/automation_registry_state.md
@@ -247,6 +247,17 @@ Gas amount does not go beyond upper cap limit
+
+
+Upon new epoch entry failed to propertly calculated committed gas for the next epoch.
+It is greater than current epoch committed gas.
+
+
+const EINVALID_COMMITTED_GAS_CALCULATION: u64 = 5;
+
+
+
+
Registry Id not found
@@ -267,6 +278,16 @@ Unauthorized access: the caller is not the owner of the task
+
+
+Conversion factor between microseconds and second
+
+
+const MICROSECS_CONVERSION_FACTOR: u64 = 1000000;
+
+
+
+
## Function `task_expiry_time`
@@ -328,7 +349,7 @@ Unauthorized access: the caller is not the owner of the task
-public(friend) fun on_new_epoch(epoch_interval: u64)
+public(friend) fun on_new_epoch(epoch_interval_micro: u64)
@@ -337,10 +358,11 @@ Unauthorized access: the caller is not the owner of the task
Implementation
-public(friend) fun on_new_epoch(epoch_interval: u64) acquires AutomationRegistryState {
+public(friend) fun on_new_epoch(epoch_interval_micro: u64) acquires AutomationRegistryState {
let state = borrow_global_mut<AutomationRegistryState>(@supra_framework);
let ids = enumerable_map::get_map_list(&state.tasks);
+ let epoch_interval_secs = epoch_interval_micro / MICROSECS_CONVERSION_FACTOR;
let current_time = timestamp::now_seconds();
let expired_task_gas = 0;
@@ -349,7 +371,7 @@ Unauthorized access: the caller is not the owner of the task
let task = enumerable_map::get_value_mut(&mut state.tasks, id);
// Tasks that are active during this new epoch but will be already expired for the next epoch
- if (task.expiry_time <= (current_time + epoch_interval)) {
+ if (task.expiry_time <= (current_time + epoch_interval_secs)) {
expired_task_gas = expired_task_gas + task.max_gas_amount;
};
@@ -359,6 +381,7 @@ Unauthorized access: the caller is not the owner of the task
task.is_active = true;
}
});
+ assert!(expired_task_gas <= state.gas_committed_for_next_epoch, EINVALID_COMMITTED_GAS_CALCULATION);
// Adjust the gas committed for the next epoch by subtracting the gas amount of the expired task
state.gas_committed_for_next_epoch = state.gas_committed_for_next_epoch - expired_task_gas;
diff --git a/aptos-move/framework/supra-framework/sources/automation_registry.move b/aptos-move/framework/supra-framework/sources/automation_registry.move
index d2d64d915b20d..044fe5de93f76 100644
--- a/aptos-move/framework/supra-framework/sources/automation_registry.move
+++ b/aptos-move/framework/supra-framework/sources/automation_registry.move
@@ -248,9 +248,8 @@ module supra_framework::automation_registry {
#[view]
/// Ge gas committed for next epoch
- public fun get_gas_committed_for_next_epoch(): u64 acquires AutomationRegistry {
- let automation_task_metadata = borrow_global(@supra_framework);
- automation_task_metadata.gas_committed_for_next_epoch
+ public fun get_gas_committed_for_next_epoch(): u64 {
+ automation_registry_state::get_gas_committed_for_next_epoch()
}
#[test_only]
diff --git a/aptos-move/framework/supra-framework/sources/automation_registry_state.move b/aptos-move/framework/supra-framework/sources/automation_registry_state.move
index e1b07ffa3e57b..0fe2a4bdef97c 100644
--- a/aptos-move/framework/supra-framework/sources/automation_registry_state.move
+++ b/aptos-move/framework/supra-framework/sources/automation_registry_state.move
@@ -11,6 +11,8 @@ module supra_framework::automation_registry_state {
use supra_framework::system_addresses;
use supra_framework::timestamp;
+ #[test_only]
+ use supra_framework::account;
friend supra_framework::automation_registry;
friend supra_framework::block;
@@ -23,7 +25,12 @@ module supra_framework::automation_registry_state {
const EAUTOMATION_TASK_NOT_EXIST: u64 = 3;
/// Unauthorized access: the caller is not the owner of the task
const EUNAUTHORIZED_TASK_OWNER: u64 = 4;
+ /// Upon new epoch entry failed to propertly calculated committed gas for the next epoch.
+ /// It is greater than current epoch committed gas.
+ const EINVALID_COMMITTED_GAS_CALCULATION: u64 = 5;
+ /// Conversion factor between microseconds and second
+ const MICROSECS_CONVERSION_FACTOR: u64 = 1_000_000;
/// It tracks entries both pending and completed, organized by unique indices.
struct AutomationRegistryState has key, store {
/// A collection of automation task entries that are active state.
@@ -85,10 +92,11 @@ module supra_framework::automation_registry_state {
automation_gas_limit
})
}
- public(friend) fun on_new_epoch(epoch_interval: u64) acquires AutomationRegistryState {
+ public(friend) fun on_new_epoch(epoch_interval_micro: u64) acquires AutomationRegistryState {
let state = borrow_global_mut(@supra_framework);
let ids = enumerable_map::get_map_list(&state.tasks);
+ let epoch_interval_secs = epoch_interval_micro / MICROSECS_CONVERSION_FACTOR;
let current_time = timestamp::now_seconds();
let expired_task_gas = 0;
@@ -97,7 +105,7 @@ module supra_framework::automation_registry_state {
let task = enumerable_map::get_value_mut(&mut state.tasks, id);
// Tasks that are active during this new epoch but will be already expired for the next epoch
- if (task.expiry_time <= (current_time + epoch_interval)) {
+ if (task.expiry_time <= (current_time + epoch_interval_secs)) {
expired_task_gas = expired_task_gas + task.max_gas_amount;
};
@@ -107,6 +115,7 @@ module supra_framework::automation_registry_state {
task.is_active = true;
}
});
+ assert!(expired_task_gas <= state.gas_committed_for_next_epoch, EINVALID_COMMITTED_GAS_CALCULATION);
// Adjust the gas committed for the next epoch by subtracting the gas amount of the expired task
state.gas_committed_for_next_epoch = state.gas_committed_for_next_epoch - expired_task_gas;
@@ -150,16 +159,16 @@ module supra_framework::automation_registry_state {
/// Remove Automatioon task entry.
public (friend) fun remove_task(owner: &signer, id: u64): AutomationTaskMetaData acquires AutomationRegistryState {
- let automation_registry = borrow_global_mut(@supra_framework);
- assert!(enumerable_map::contains(&automation_registry.tasks, id), EAUTOMATION_TASK_NOT_EXIST);
+ let state = borrow_global_mut(@supra_framework);
+ assert!(enumerable_map::contains(&state.tasks, id), EAUTOMATION_TASK_NOT_EXIST);
- let automation_task_metadata = enumerable_map::get_value(&automation_registry.tasks, id);
+ let automation_task_metadata = enumerable_map::get_value(&state.tasks, id);
assert!(automation_task_metadata.owner == signer::address_of(owner), EUNAUTHORIZED_TASK_OWNER);
- enumerable_map::remove_value(&mut automation_registry.tasks, id);
+ enumerable_map::remove_value(&mut state.tasks, id);
// Adjust the gas committed for the next epoch by subtracting the gas amount of the expired task
- automation_registry.gas_committed_for_next_epoch = automation_registry.gas_committed_for_next_epoch - automation_task_metadata.max_gas_amount;
+ state.gas_committed_for_next_epoch = state.gas_committed_for_next_epoch - automation_task_metadata.max_gas_amount;
event::emit(RemoveAutomationTask { id: automation_task_metadata.id });
// todo : return refund amount to user
@@ -173,21 +182,21 @@ module supra_framework::automation_registry_state {
) acquires AutomationRegistryState {
system_addresses::assert_supra_framework(supra_framework);
- let automation_registry = borrow_global_mut(@supra_framework);
- automation_registry.automation_gas_limit = automation_gas_limit;
+ let state = borrow_global_mut(@supra_framework);
+ state.automation_gas_limit = automation_gas_limit;
event::emit(UpdateAutomationGasLimit { automation_gas_limit });
}
/// List all the automation task ids
public(friend) fun get_active_task_ids(): vector acquires AutomationRegistryState {
- let automation_registry = borrow_global(@supra_framework);
+ let state = borrow_global(@supra_framework);
let active_task_ids = vector[];
- let ids = enumerable_map::get_map_list(&automation_registry.tasks);
+ let ids = enumerable_map::get_map_list(&state.tasks);
vector::for_each(ids, |id| {
- let task = enumerable_map::get_value(&automation_registry.tasks, id);
+ let task = enumerable_map::get_value(&state.tasks, id);
if (task.is_active) {
vector::push_back(&mut active_task_ids, id);
};
@@ -221,4 +230,116 @@ module supra_framework::automation_registry_state {
state.current_index
}
+ /// Ge gas committed for next epoch
+ public(friend) fun get_gas_committed_for_next_epoch(): u64 acquires AutomationRegistryState {
+ let state = borrow_global(@supra_framework);
+ state.gas_committed_for_next_epoch
+ }
+
+ #[test_only]
+ fun initialize_registry_state_test(framework: &signer) {
+ timestamp::set_time_has_started_for_testing(framework);
+ initialize(framework, 100);
+ }
+
+ #[test(framework = @supra_framework)]
+ fun check_task_registration(framework: signer) acquires AutomationRegistryState {
+ initialize_registry_state_test(&framework);
+
+ let account = account::create_account_for_test(@0x123456);
+ register(&account,
+ vector[0, 1, 2, 3, 4],
+ 100,
+ 10,
+ 20,
+ 1,
+ vector[0, 1, 2 ,3],
+ );
+ assert!(1 == get_next_task_index(), 1);
+ assert!(10 == get_gas_committed_for_next_epoch(), 1)
+ }
+
+ #[test(framework = @supra_framework)]
+ #[expected_failure(abort_code = 2)]
+ fun check_registration_with_overflow_gas_limit(framework: signer) acquires AutomationRegistryState {
+ initialize_registry_state_test(&framework);
+
+ let account = account::create_account_for_test(@0x123456);
+ register(&account,
+ vector[0, 1, 2, 3, 4],
+ 100,
+ 70,
+ 20,
+ 1,
+ vector[0, 1, 2 ,3],
+ );
+ assert!(1 == get_next_task_index(), 1);
+ assert!(70 == get_gas_committed_for_next_epoch(), 1);
+ register(&account,
+ vector[0, 1, 2, 3, 4],
+ 100,
+ 70,
+ 20,
+ 1,
+ vector[0, 1, 2 ,3],
+ );
+ }
+
+ #[test(framework = @supra_framework)]
+ fun check_task_activation_on_new_epoch(framework: signer) acquires AutomationRegistryState {
+ initialize_registry_state_test(&framework);
+ let payload_tx = vector[0, 1, 2, 3, 4];
+ let txn_hash = vector[1, 2, 3, 4, 5];
+
+ let account = account::create_account_for_test(@0x123456);
+ register(&account,
+ payload_tx,
+ 100,
+ 10,
+ 20,
+ 1,
+ txn_hash,
+ );
+ // When moving to next epoch this task will be considered as expired
+ register(&account,
+ payload_tx,
+ 25,
+ 10,
+ 20,
+ 1,
+ txn_hash,
+ );
+ register(&account,
+ payload_tx,
+ 150,
+ 10,
+ 20,
+ 1,
+ txn_hash,
+ );
+ // When moving to next epoch this task will be considered as expired for the updcoming new epoch
+ register(&account,
+ payload_tx,
+ 75,
+ 10,
+ 20,
+ 1,
+ txn_hash,
+ );
+ // No active task and committed gas for the next epoch is total of the all registered tasks
+ assert!(40 == get_gas_committed_for_next_epoch(), 1);
+ let active_task_ids = get_active_task_ids();
+ assert!(active_task_ids == vector[], 1);
+
+ timestamp::update_global_time_for_test_secs(50);
+ on_new_epoch(30 * MICROSECS_CONVERSION_FACTOR);
+ // Committed gas for the next epoch only for 2 tasks 0 and 2, the task 3 will not be active during upcoming epoch
+ assert!(20 == get_gas_committed_for_next_epoch(), 1);
+ let active_task_ids = get_active_task_ids();
+ // But here task 3 is in the active list as it is still active in this new epoch.
+ let expected_ids = vector[0, 2, 3];
+ vector::for_each(active_task_ids, |id| {
+ assert!(vector::contains(&expected_ids, &id), 1);
+ })
+ }
}