Includes smart contracts, libraries, scripts and tests required by automation registry#5
Conversation
| /// Otherwise suspention is postponed until the end of the transition state. | ||
| /// Nothing will be done if automation cycle was already suspended, i.e. in READY state. | ||
| /// If native automation feature is enabled and automation lifecycle has been in READY state, then lifecycle is restarted. | ||
| function onNewCycle() public { |
There was a problem hiding this comment.
On Move Automation side as chain feature flags where applied from epoch to epoch, the feature suspension was done in the expected natural way by implementing and calling on_new_epoch function during chain reconfiguration which was causing epoch-change as well.
With Evm Automation we do not have this option available. So I think feature total suspension should be rather immediate action run by governance.
In that case this function should be updated to be guarded by onlyOwner and also can be named as toggleAutomationFeature(bool enable)
There was a problem hiding this comment.
What would be the use of bool enable argument? We already have functions to enable/disable automation. How this function would be different?
There was a problem hiding this comment.
What is the main reason of having controller and registry separate.
There was a problem hiding this comment.
Ethereum contracts have size limit of 24KB. The idea was to avoid exceeding the size limit by bifurcating the contracts.
…e state -created bash script for deployment and interacting to registry contracts
0bcdd7d to
11d5782
Compare
variable name fixes
-added priority and task type in parameters -updated test cases
…omationCore updated scripts
|
|
||
| /// @notice Tracks per-cycle automation state and task indexes. | ||
| struct RegistryState { | ||
| uint256 cycleLockedFees; |
There was a problem hiding this comment.
cycleLockedFees is not used in scope of automation registry directly its value is mainly used by AutomationCore. Besides it will be more appropriate to keep billing related information together, so my suggestion here will be to have cycleLockedFees and totalDepositedAutomationFees together and maybe part of the AutomationCore
|
|
||
| /// @notice Checks the cycle end and emit an event on it. Does nothing if SUPRA_NATIVE_AUTOMATION or SUPRA_AUTOMATION_V2 is disabled. | ||
| function monitorCycleEnd() external { | ||
| if (tx.origin != IAutomationCore(automationCore).getVmSigner()) { revert CallerNotVmSigner(); } |
There was a problem hiding this comment.
I can see that vm Signer is not utilized by AutomationCore, so it can be property of the controller property instead.
| (_taskIndex, _lockedFeeForNextCycle) | ||
| ) | ||
| ); | ||
| require(unlocked, UnlockLockedDepositFailed()); |
There was a problem hiding this comment.
As this is happening in scope of system transaction (AutomationRecord) execution, if unlock fails we do not fail the transaction, we simply emit an error only. Error will be addressed later by other means.
| using LibController for *; | ||
|
|
||
| /// @dev State variables | ||
| LibController.AutomationCycleInfo cycleInfo; |
There was a problem hiding this comment.
Currently AutomationCycleInfo does not only hold only cycle related info, it also hold automation-feature flag, so my suggestion would be to rename AutomationCycleInfo to AutomationControllerState which will include automation_feature_flag and AutomationCycleDetails/Info
|
|
||
| /// @notice Function to ensure that AutomationController contract is the caller. | ||
| function onlyController() private view { | ||
| if(msg.sender != regConfig.automationController()) { revert CallerNotController(); } |
There was a problem hiding this comment.
There are some functions that are guarded by this function and only call internal function.
Will having this condition extended to support sender as controller or this will help to avoid simple function re-directions and contract size reduction.
e.g
/// @notice Internally calls _calculateTaskFee, reverts if caller is not AutomationController.
function calculateTaskFee(
CommonUtils.TaskState _state,
uint64 _expiryTime,
uint128 _maxGasAmount,
uint64 _potentialFeeTimeframe,
uint64 _currentTime,
uint128 _automationFeePerSec
) external view returns (uint128) {
onlyController();
return _calculateTaskFee(
_state,
_expiryTime,
_maxGasAmount,
_potentialFeeTimeframe,
_currentTime,
_automationFeePerSec
);
}
instead _calculateTaskFee can be a public function and can be called both by this and controller contract
| function safeFeeRefund( | ||
| uint64 _taskIndex, | ||
| address _taskOwner, | ||
| uint256 _cycleLockedFees, |
There was a problem hiding this comment.
Referring this fix :Entropy-Foundation/aptos-core#329
refund amount can differ from the amount that needs to be unlocked when task is stopped.
Please updated the logic here according to the fix in move-automation.
| regConfig.setGasCommittedForNextCycle(gasCommitted); | ||
| } else { | ||
| regConfig.setSysGasCommittedForNextCycle(gasCommitted); | ||
| } |
There was a problem hiding this comment.
this function can also include deposit-fees update if task is UST and registration passed validation and to avoid having incTotalDepositedAutomationFees function.
| function register( | ||
| bytes memory _payloadTx, | ||
| uint64 _expiryTime, | ||
| bytes32 _txHash, |
There was a problem hiding this comment.
_txHash should not be as input, as user does not now it util txn is constructed.
There is a precompile implemented in revm-supra version which helps to retrieve it.
Example code:
contract TxHashReader {
address public constant TX_HASH_PRECOMPILE = 0x0000000000000000000000000000000053555001;
/// Read tx hash via precompile. Reverts if precompile missing / fails.
function readTxHash() external view {
(bool ok, bytes memory out) = TX_HASH_PRECOMPILE.staticcall("");
require(ok, "txhash precompile call failed");
require(out.length == 32, "txhash bad len");
}
}
There was a problem hiding this comment.
Could you please share the exact function signature to call on the precompile? or a link to relevant code?
| function registerSystemTask( | ||
| bytes memory _payloadTx, | ||
| uint64 _expiryTime, | ||
| bytes32 _txHash, |
-removed access control for external view functions -updated lockedFeeForNextCycle to depositFee
This PR includes smart contracts, libraries, scripts and tests required by automation registry related to Entropy-Foundation/smr-moonshot#2523