Diamond Proxy pattern based smart contracts by udityadav-supraoracles · Pull Request #16 · Entropy-Foundation/revm

udityadav-supraoracles · 2026-02-17T06:21:48Z

This PR includes changes to accommodate use of Diamond Proxy pattern for EVM Automation Registry smart contracts.

-includes:
facets: ConfigFacet. RegistryFacet and CoreFacet.
DiamondInit: This contract is used to initialise the state of Diamond, its not added as facet. So, we don't have a flag to check for initialisation. Once Diamond is deployed, this contract is supposed to be called as part of diamondCut.

…mation_registry

udityadav-supraoracles · 2026-02-19T11:50:20Z

Currently the AppStorage have inner structs which doesn't allow the inner structs to be expanded later. Either we can have all variables in AppStorage struct or have a mapping pointing to inner struct as structs pointing by mapping are expandable.

Referring to: https://eip2535diamonds.substack.com/p/diamond-upgrades
Do not put structs directly in structs unless you don’t plan on ever adding more state variables to the inner structs. You won't be able to add new state variables to inner structs in upgrades without overwriting existing state variables.

Please let me know your opinion Dr @sjoshisupra @aregng.

sjoshisupra · 2026-02-20T10:13:56Z

solidity/supra_contracts/src/facets/ConfigFacet.sol

+    function setVmSigner(address _vmSigner) external {
+        LibDiamond.enforceIsContractOwner();
+
+        LibUtils.validateAddress(_vmSigner);
+
+        address oldVmSigner = s.vmSigner;
+        s.vmSigner = _vmSigner;
+
+        emit VmSignerUpdated(oldVmSigner, _vmSigner);
+    }


@udityadav-supraoracles, @aregng, I do not foresee any circumstances for VM_SIGNER to change since this is hard coded in rust layer. Is this method only to initialize after contract deployment or do we foresee this to be useful? If not, perhaps we should not have this API (if this is initialized with appropriate VM_SIGNER address at the time of deployment)

The VM_SIGNER and ERC20Supra both are initialised at the time of deployment. We can remove their respective API's to update the addresses if we do not plan to change them later. Please let me know your opinion @aregng .

sjoshisupra · 2026-02-20T10:16:45Z

solidity/supra_contracts/src/facets/ConfigFacet.sol

+
+    /// @notice Function to update the ERC20Supra address.
+    /// @param _erc20Supra New address for ERC20Supra.
+    function setErc20Supra(address _erc20Supra) external {


@udityadav-supraoracles, is this to update ERC 20 contract address? Or to update the address of the registry itself where it stores ERC20 tokens collected from fee? It is not clear from the comments.
If it is the former, why does registry contract need to know ERC20 contract address?

This function is meant to update the address of ERC20Supra contract in registry, as the registry holds ERC20Supra address. The registry needs to know ERC20Supra's address in order to call transferFrom on it while charging fees.

Regarding fee collection, fee charged in terms of ERC20Supra tokens is stored at the registry's address(ERC20 standard contract inherited by ERC20Supra maintains mapping _balances where it stores the registry's balance).

sjoshisupra · 2026-02-20T10:23:25Z

solidity/supra_contracts/src/facets/ConfigFacet.sol

+        uint256 balance = IERC20(s.erc20Supra).balanceOf(address(this));
+
+        if (balance < _amount) { revert InsufficientBalance(); }
+        if (balance - _amount < s.registryState.cycleLockedFees + s.registryState.totalDepositedAutomationFees) { revert RequestExceedsLockedBalance(); }


@udityadav-supraoracles, something seems off. I believe we want to withdraw from s.registryState.totalDepostedAutomationFees, meaning that shouldn't the expression be
balance - _amount < s.registryState.cycleLockedFees? does totalDepositedAutomationFees refer to fees or does it refer to the caution that we take to deter DOS via cancellation?

sjoshisupra · 2026-02-20T10:24:59Z

solidity/supra_contracts/src/facets/ConfigFacet.sol

+        if (s.registryState.gasCommittedForNextCycle > _registryMaxGasCap) { revert UnacceptableRegistryMaxGasCap(); }
+        if (s.registryState.sysGasCommittedForNextCycle > _sysRegistryMaxGasCap) { revert UnacceptableSysRegistryMaxGasCap(); }


@udityadav-supraoracles, shouldn't these two checks also be part of LibCommon.validateConfigParameters method? This seems like validation checks only.

sjoshisupra · 2026-02-20T10:27:47Z

solidity/supra_contracts/src/facets/CoreFacet.sol

+    /// @param _taskIndexes Array of task index to be processed.
+    function processTasks(uint64 _cycleIndex, uint64[] memory _taskIndexes) external {
+        // Check caller is VM Signer
+        if (msg.sender != s.vmSigner) { revert CallerNotVmSigner(); }


@udityadav-supraoracles, it is better to have a common method enforceIsVmSigner similar to enforceIsContractOwner.

sjoshisupra · 2026-02-20T11:03:08Z

solidity/supra_contracts/src/libraries/LibAccounting.sol

+            uint256 thresholdScaledAsFraction = thresholdPercentageScaled / 100;    // DECIMAL-scaled fraction
+            uint256 surplusClipped = thresholdScaledAsFraction + surplusScaled > DECIMAL ? DECIMAL - thresholdScaledAsFraction : surplusScaled;
+
+            uint256 baseScaled = DECIMAL + surplusClipped;  // (1 + base)


take the exponential computation out as a separate private method. @udityadav-supraoracles

sjoshisupra · 2026-02-20T11:04:23Z

solidity/supra_contracts/src/libraries/LibAccounting.sol

+        return uint128(automationFeeForInterval / DECIMAL);
+    }
+
+    // :::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::: INTERNAL FUNCTIONS ::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::


I think there are methods above this line which are also internal, right? @udityadav-supraoracles

sjoshisupra · 2026-02-20T11:08:58Z

solidity/supra_contracts/src/libraries/LibCommon.sol

+    /// @notice Helper function to sort an array.
+    /// @param arr Input array to sort.
+    /// @return Returns the sorted array. 
+    function sortUint64(uint64[] memory arr) internal pure returns (uint64[] memory) {


use quick / heap sort, perhaps there might be existing library in solidity which does this. current sorting is highly inefficient @udityadav-supraoracles

sjoshisupra · 2026-02-20T11:09:19Z

solidity/supra_contracts/src/libraries/LibCommon.sol

+    /// @notice Helper function to sort an array.
+    /// @param arr Input array to sort.
+    /// @return Returns the sorted array. 
+    function sortUint256(uint256[] memory arr) internal pure returns (uint256[] memory) {


same suggestion, find library or use quick/heap sort @udityadav-supraoracles

sjoshisupra · 2026-02-20T11:12:45Z

solidity/supra_contracts/src/libraries/LibCore.sol

+    /// @return intermediateState Returns the intermediate state.
+    function dropOrChargeTasks(
+        uint64[] memory _taskIndexes
+    ) private returns (LibCommon.IntermediateStateOfCycleChange memory intermediateState) {


would this be private? I believe this need to be called by VM_SIGNER right? In fact, multiple places I believe methods are marked as private which may not work with evm and they need to be restricted to be called by VM_SIGNER only. @udityadav-supraoracles @aregng

It is called from onCycleTransition defined as internal in the same library which furthur gets called by VM_SIGNER facing function processTasks. So, there's no issue with this function being marked as private as its not directly called by VM_SIGNER.

VM_SIGNER only calls monitorCycleEnd and processTasks which're marked as external and other helper functions are marked as internal or private.

refactored to use diamond proxy

2dff29c

udityadav-supraoracles requested review from aregng and sjoshisupra February 17, 2026 06:21

udityadav-supraoracles self-assigned this Feb 17, 2026

udityadav-supraoracles added 4 commits February 17, 2026 14:40

minor fixes

1c0b743

refactored code

cb855fe

updated scripts

3992962

refactored

3f6f2df

udityadav-supraoracles marked this pull request as ready for review February 19, 2026 08:43

udityadav-supraoracles added 2 commits February 19, 2026 15:35

fix to remove task if insufficient allowance while bookkeeping

cc73404

Merge branch 'feature/evm_automation' into feature/diamond_proxy_auto…

93d76cc

…mation_registry

udityadav-supraoracles added 2 commits February 20, 2026 11:53

updated unlockDepositAndCycleFee

7a844dd

updated Diamond to not receive native tokens

35ae8f6

sjoshisupra requested changes Feb 20, 2026

View reviewed changes

		if (s.registryState.gasCommittedForNextCycle > _registryMaxGasCap) { revert UnacceptableRegistryMaxGasCap(); }
		if (s.registryState.sysGasCommittedForNextCycle > _sysRegistryMaxGasCap) { revert UnacceptableSysRegistryMaxGasCap(); }

Comments

Conversation

udityadav-supraoracles commented Feb 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

udityadav-supraoracles commented Feb 19, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

udityadav-supraoracles commented Feb 17, 2026 •

edited

Loading