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);
+        })
+    }
 }