Skip to content

Commit

Permalink
Added couple of unit-tests for automation-registry-state
Browse files Browse the repository at this point in the history
  • Loading branch information
Aregnaz Harutyunyan committed Dec 18, 2024
1 parent 58781a8 commit 5ca0c59
Show file tree
Hide file tree
Showing 3 changed files with 161 additions and 18 deletions.
29 changes: 26 additions & 3 deletions aptos-move/framework/supra-framework/doc/automation_registry_state.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -247,6 +247,17 @@ Gas amount does not go beyond upper cap limit



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

Upon new epoch entry failed to propertly calculated committed gas for the next epoch.
It is greater than current epoch committed gas.


<pre><code><b>const</b> <a href="automation_registry_state.md#0x1_automation_registry_state_EINVALID_COMMITTED_GAS_CALCULATION">EINVALID_COMMITTED_GAS_CALCULATION</a>: u64 = 5;
</code></pre>



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

Registry Id not found
Expand All @@ -267,6 +278,16 @@ Unauthorized access: the caller is not the owner of the task



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

Conversion factor between microseconds and second


<pre><code><b>const</b> <a href="automation_registry_state.md#0x1_automation_registry_state_MICROSECS_CONVERSION_FACTOR">MICROSECS_CONVERSION_FACTOR</a>: u64 = 1000000;
</code></pre>



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

## Function `task_expiry_time`
Expand Down Expand Up @@ -328,7 +349,7 @@ Unauthorized access: the caller is not the owner of the task



<pre><code><b>public</b>(<b>friend</b>) <b>fun</b> <a href="automation_registry_state.md#0x1_automation_registry_state_on_new_epoch">on_new_epoch</a>(epoch_interval: u64)
<pre><code><b>public</b>(<b>friend</b>) <b>fun</b> <a href="automation_registry_state.md#0x1_automation_registry_state_on_new_epoch">on_new_epoch</a>(epoch_interval_micro: u64)
</code></pre>


Expand All @@ -337,10 +358,11 @@ Unauthorized access: the caller is not the owner of the task
<summary>Implementation</summary>


<pre><code><b>public</b>(<b>friend</b>) <b>fun</b> <a href="automation_registry_state.md#0x1_automation_registry_state_on_new_epoch">on_new_epoch</a>(epoch_interval: u64) <b>acquires</b> <a href="automation_registry_state.md#0x1_automation_registry_state_AutomationRegistryState">AutomationRegistryState</a> {
<pre><code><b>public</b>(<b>friend</b>) <b>fun</b> <a href="automation_registry_state.md#0x1_automation_registry_state_on_new_epoch">on_new_epoch</a>(epoch_interval_micro: u64) <b>acquires</b> <a href="automation_registry_state.md#0x1_automation_registry_state_AutomationRegistryState">AutomationRegistryState</a> {
<b>let</b> state = <b>borrow_global_mut</b>&lt;<a href="automation_registry_state.md#0x1_automation_registry_state_AutomationRegistryState">AutomationRegistryState</a>&gt;(@supra_framework);
<b>let</b> ids = <a href="../../supra-stdlib/doc/enumerable_map.md#0x1_enumerable_map_get_map_list">enumerable_map::get_map_list</a>(&state.tasks);

<b>let</b> epoch_interval_secs = epoch_interval_micro / <a href="automation_registry_state.md#0x1_automation_registry_state_MICROSECS_CONVERSION_FACTOR">MICROSECS_CONVERSION_FACTOR</a>;
<b>let</b> current_time = <a href="timestamp.md#0x1_timestamp_now_seconds">timestamp::now_seconds</a>();
<b>let</b> expired_task_gas = 0;

Expand All @@ -349,7 +371,7 @@ Unauthorized access: the caller is not the owner of the task
<b>let</b> task = <a href="../../supra-stdlib/doc/enumerable_map.md#0x1_enumerable_map_get_value_mut">enumerable_map::get_value_mut</a>(&<b>mut</b> state.tasks, id);

// Tasks that are active during this new epoch but will be already expired for the next epoch
<b>if</b> (task.expiry_time &lt;= (current_time + epoch_interval)) {
<b>if</b> (task.expiry_time &lt;= (current_time + epoch_interval_secs)) {
expired_task_gas = expired_task_gas + task.max_gas_amount;
};

Expand All @@ -359,6 +381,7 @@ Unauthorized access: the caller is not the owner of the task
task.is_active = <b>true</b>;
}
});
<b>assert</b>!(expired_task_gas &lt;= state.gas_committed_for_next_epoch, <a href="automation_registry_state.md#0x1_automation_registry_state_EINVALID_COMMITTED_GAS_CALCULATION">EINVALID_COMMITTED_GAS_CALCULATION</a>);

// 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;
Expand Down
5 changes: 2 additions & 3 deletions aptos-move/framework/supra-framework/sources/automation_registry.move
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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<AutomationRegistry>(@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]
Expand Down
145 changes: 133 additions & 12 deletions aptos-move/framework/supra-framework/sources/automation_registry_state.move
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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.
Expand Down Expand Up @@ -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<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;

Expand All @@ -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;
};

Expand All @@ -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;
Expand Down Expand Up @@ -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<AutomationRegistryState>(@supra_framework);
assert!(enumerable_map::contains(&automation_registry.tasks, id), EAUTOMATION_TASK_NOT_EXIST);
let state = borrow_global_mut<AutomationRegistryState>(@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
Expand All @@ -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<AutomationRegistryState>(@supra_framework);
automation_registry.automation_gas_limit = automation_gas_limit;
let state = borrow_global_mut<AutomationRegistryState>(@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<u64> acquires AutomationRegistryState {
let automation_registry = borrow_global<AutomationRegistryState>(@supra_framework);
let state = borrow_global<AutomationRegistryState>(@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);
};
Expand Down Expand Up @@ -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<AutomationRegistryState>(@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<u8>[0, 1, 2, 3, 4];
let txn_hash = vector<u8>[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<u64>[0, 2, 3];
vector::for_each(active_task_ids, |id| {
assert!(vector::contains(&expected_ids, &id), 1);
})
}
}

0 comments on commit 5ca0c59

Please sign in to comment.