Supra automation registry smart contract implementation within supra framework #119

nizam-supraoracles · 2024-11-07T10:03:59Z

No description provided.

aptos-move/framework/supra-framework/sources/automation_registry.move

sjoshisupra · 2024-11-21T06:03:30Z

aptos-move/framework/supra-framework/sources/automation_registry.move

@@ -0,0 +1,89 @@
+/// Supra Automation Registry
+///


public fun get_active_tasks() : vector<AutomationMetaData>

on_new_epoch() should perform clean up and updation of state

Do we want to restrict only for entry function or should we allow scripts as well?

aptos-move/framework/supra-framework/sources/automation_registry.move

aregng · 2024-11-26T08:38:24Z

aptos-move/framework/supra-framework/sources/automation_registry.move

+
+        move_to(supra_framework, AutomationRegistry {
+            current_index: 0,
+            automation_gas_limit: DEFAULT_AUTOMATION_GAS_LIMIT,


I think there should be separate type for config holding these values, and it should be read during initialization.

Yes, it depends on how we are upgrading the automation_registry with the SupraFramework.
If we are initializing through multisig, we will need to pass the parameter in that case. I will update accordingly once we finalize it.

aptos-move/framework/supra-framework/sources/automation_registry.move

aregng · 2024-11-28T09:45:02Z

aptos-move/framework/supra-framework/sources/automation_registry.move

+        // todo : pre-paid amount collect from the user
+        // todo : duration/expiry in seconds
+        // Expiry time does not go beyond upper cap duration set by admin/governance
+        assert!(expiry_time < registry_data.duration_upper_limit, EEXPIRY_TIME_UPPER);


I think check should be (expiry_time - now) < registry_data.duration_upper_limit

I assume that the client will provide the expiration time in seconds, indicating how many days or months from now they need the execution to occur.
For example, if they want the execution to be valid for only one day, they would provide 86,400 seconds as an argument.

Please let me know if we should handle it in the manner you’ve outlined.
CC: Dr. @sjoshisupra

Based on the last discussion I remember that we decided to have exact exipration-time in seconds.
Now can be different in scope of different nodes, so global/absolute time is better than relative one. and this value also will be used filter out already expired automated-transactions in scope of SMR absolute time is preferable.
So I think even here we should take block-creation-time or predicted_next_epoch_start_time to check the actual duration, instead of current-time

aregng · 2024-11-28T09:46:45Z

aptos-move/framework/supra-framework/sources/automation_registry.move

+        // todo : gas_price_cap should not below chain minimum
+
+        registry_data.current_index = registry_data.current_index + 1;
+


deposit_from_owner(owner) - where automation fee calculation and withdrawal from owner account is done. should be done after all verification and before increasing index and registering the task.

…updates

…xt_epoch` field, add more assersion in `register` function, also `get_active_task_ids` function will return only active task ids

…s_limit` and `update_duration_upper_limit`

…price_cap` should not be zero also collect static registry fee from user

…o include missing events

aregng · 2024-12-13T06:43:23Z

aptos-move/framework/supra-framework/sources/automation_registry.move

+    }
+
+    /// Withdraw user's deposited fee from the resource account
+    entry fun withdraw_fee(supra_framework: &signer, to: address, amount: u64) acquires AutomationRegistry {


should this function really be entry one? Isn't it utilized only in scope of this module in scope of remove_task. If that's the case then here again registry_fee_address_signer_cap can be passed as parameter to this function.

but here I have one question: @sjoshisupra how the accumulated coins will be later distributed.

The withdraw_fee function is invoked by the Supra Framework, enabling the framework admin to withdraw the automation task fees paid by users. However, this function is not utilized in the remove_task process and directly access from outside of the SC

aregng · 2024-12-13T06:53:01Z

aptos-move/framework/supra-framework/sources/automation_registry.move

+    }
+
+    /// Calculate and collect registry charge from user
+    fun collect_from_owner(owner: &signer, _expiry_time: u64, _max_gas_amount: u64) {


This function will be called in scope of register function where we already have registry information, I think in order not to calculate registry fee address here again, it can be passed as parameter to this function.

yeah, we can do that

aregng · 2024-12-13T06:55:23Z

aptos-move/framework/supra-framework/sources/automation_registry.move

+        /// It's resource address which is use to deposit user automation fee
+        registry_fee_address: address,
+        /// Resource account signature capability
+        registry_fee_address_signer_cap: SignerCapability,


we should have automation_unit_price configuration property here which will define price per second or AutomationFeeParameters struct were unit-price, threshold, etc. will be included.

aregng · 2024-12-13T07:08:29Z

aptos-move/framework/supra-framework/sources/automation_registry.move

+    /// Calculate and collect registry charge from user
+    fun collect_from_owner(owner: &signer, _expiry_time: u64, _max_gas_amount: u64) {
+        // todo : calculate and collect pre-paid amount from the user
+        let static_amount = 100000000; // 1 Aptos


for the time being we can avoid applying dynamic charging, and we can collect flat rate based on the expiry-time and automation_unit_price.

aregng · 2024-12-13T07:13:29Z

aptos-move/framework/supra-framework/sources/automation_registry.move

+        // Perform clean up and updation of state
+        vector::for_each(ids, |id| {
+            let task = enumerable_map::get_value_mut(&mut automation_registry.tasks, id);
+            if (task.expiry_time < current_time) {


Here we should take into account also the tasks that are active during this new epoch but will be already expired for the next epoch. so we need to check (task.expire_time < current_time + epoch_timeframe)

I assume that if a task does not have sufficient time duration to run within the epoch, and it expires in the middle of the epoch, such a task will not be considered for execution. Is that correct?

yes, true it will not be considered expired during that epoch.

aregng · 2024-12-13T07:15:42Z

aptos-move/framework/supra-framework/sources/automation_registry.move

+    }
+
+    /// Calculate and collect registry charge from user
+    fun collect_from_owner(owner: &signer, _expiry_time: u64, _max_gas_amount: u64) {


can you please remind how the max_gas_amount will be utilized in automation fee calculation.

Ohh, I see max_gas_amount will be utilize for this purpose only

aregng · 2024-12-16T06:23:28Z

aptos-move/framework/supra-framework/sources/automation_registry.move

+                expired_task_gas = expired_task_gas + task.max_gas_amount;
+            };
+
+            if (task.expiry_time < current_time) {


here should be <= and the negative check in the else branch can be removed.

aregng · 2024-12-16T06:23:36Z

aptos-move/framework/supra-framework/sources/automation_registry.move

+    }
+
+    /// Withdraw accumulated automation task fees from the resource account
+    entry fun withdraw_automation_task_fees(


maybe we can call this function refund?

Actually I have replaced withdraw_fee function to withdraw_automation_task_fees which is only access by supra framework admin, it's not used for refund

aregng · 2024-12-16T06:29:11Z

aptos-move/framework/supra-framework/sources/automation_registry.move

+        assert!(expiry_time > (last_epoch_time + epoch_interval), EEXPIRY_BEFORE_NEXT_EPOCH);
+
+        registry_data.gas_committed_for_next_epoch = registry_data.gas_committed_for_next_epoch + max_gas_amount;
+        assert!(registry_data.gas_committed_for_next_epoch < registry_data.automation_gas_limit, EGAS_AMOUNT_UPPER);


I think we should provide means to query gas_committed_for_next_epoch in order for user to avoid task registration and being charged for the failed transaction.

…nsfer_fee_to_account_internal`

nizam-supraoracles requested a review from sjoshisupra November 7, 2024 10:03

nizam-supraoracles self-assigned this Nov 7, 2024

sjoshisupra requested changes Nov 21, 2024

View reviewed changes

nizam-supraoracles requested a review from aregng November 21, 2024 11:51

aregng reviewed Nov 25, 2024

View reviewed changes

aptos-move/framework/supra-framework/sources/automation_registry.move Outdated Show resolved Hide resolved

aregng reviewed Nov 26, 2024

View reviewed changes

aregng reviewed Nov 28, 2024

View reviewed changes

nizam-supraoracles changed the base branch from dev to feature/automation-networks December 6, 2024 11:37

nizam-supraoracles force-pushed the supra_automation_registry branch from 26bc075 to 2392127 Compare December 6, 2024 11:40

nizam-supraoracles added 7 commits December 9, 2024 15:09

feat - initial commite for supra automation registry smart contract

cdbbbe0

restructe automation registry contract

30b8ee8

registry contract refactor functions and the struct

3c97e47

chore - registry sc, commenting correction

c2243b0

introduce supra-lib enumerable_map

e80b25d

supra automation - replace table with enumerable_map and did related …

58b70e0

…updates

move enumerable_map file into supra-stdlib, add `gas_committed_for_ne…

d737095

…xt_epoch` field, add more assersion in `register` function, also `get_active_task_ids` function will return only active task ids

nizam-supraoracles force-pushed the supra_automation_registry branch from 2392127 to d737095 Compare December 9, 2024 09:39

nizam-supraoracles added 4 commits December 11, 2024 14:46

automation registry - implement on_new_epoch, `update_automation_ga…

39bb59f

…s_limit` and `update_duration_upper_limit`

registry function modify as get tx_hash from txn_app_hash and `gas_…

af3f115

…price_cap` should not be zero also collect static registry fee from user

add test_registry unit testcase in supra automation registry

060e853

automation_registry - implement withdraw_fee and remove_task, als…

1414818

…o include missing events

aregng reviewed Dec 13, 2024

View reviewed changes

address PR comments

39a216a

nizam-supraoracles marked this pull request as ready for review December 13, 2024 14:00

nizam-supraoracles requested review from sjoshisupra and aregng December 13, 2024 14:00

aregng reviewed Dec 16, 2024

View reviewed changes

nizam-supraoracles added 2 commits December 17, 2024 13:32

create refund_automation_task_fee function and common function `tra…

8e5f874

…nsfer_fee_to_account_internal`

create view function get_gas_committed_for_next_epoch

afbbb92

aregng approved these changes Dec 17, 2024

View reviewed changes

nizam-supraoracles merged commit 4917e1e into feature/automation-networks Dec 17, 2024
1 check failed

nizam-supraoracles deleted the supra_automation_registry branch December 17, 2024 08:14

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Supra automation registry smart contract implementation within supra framework #119

Supra automation registry smart contract implementation within supra framework #119

nizam-supraoracles commented Nov 7, 2024

sjoshisupra Nov 21, 2024

aregng Nov 26, 2024

nizam-supraoracles Nov 26, 2024

aregng Nov 28, 2024

nizam-supraoracles Dec 5, 2024

aregng Dec 5, 2024

aregng Nov 28, 2024

aregng Dec 13, 2024

nizam-supraoracles Dec 13, 2024

aregng Dec 13, 2024

nizam-supraoracles Dec 13, 2024

aregng Dec 13, 2024

aregng Dec 13, 2024

aregng Dec 13, 2024

nizam-supraoracles Dec 13, 2024

aregng Dec 16, 2024

aregng Dec 13, 2024

nizam-supraoracles Dec 13, 2024 •

edited

Loading

aregng Dec 16, 2024

aregng Dec 16, 2024

nizam-supraoracles Dec 17, 2024

aregng Dec 16, 2024

nizam-supraoracles Dec 17, 2024

		// todo : gas_price_cap should not below chain minimum

		registry_data.current_index = registry_data.current_index + 1;

Supra automation registry smart contract implementation within supra framework #119

Supra automation registry smart contract implementation within supra framework #119

Conversation

nizam-supraoracles commented Nov 7, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nizam-supraoracles Dec 13, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nizam-supraoracles Dec 13, 2024 •

edited

Loading