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-24T10:57:43Z

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.

Hi @udityadav-supraoracles , in that case we should use the guidelines and avoid nested structs.

sjoshisupra · 2026-02-27T12:38:14Z

+
+library LibDiamond {
+    // 32 bytes keccak hash of a string to use as a diamond storage location.
+    bytes32 constant DIAMOND_STORAGE_POSITION = keccak256("diamond.standard.diamond.storage");


Shall we modify the string passed to keccak256? It looks like this came from template and to ensure the storage at random location, we should use a unique string. @udityadav-supraoracles

Yes, this is from the standard Diamond implementation. Existing string "diamond.standard.diamond.storage" also creates a unique storage slot for our Diamond contract.

The concept of having dedicated slot for the shared storage is a standard, but I think we can use automation registry as part of the string defining the unique position, such as diamond.registry.automation.storage.

-Resolved PR comments

- removed setErc20Supra, setVmSigner

- added mapping for authorised addresses with mint/burn capabilities

…mation_registry

sjoshisupra · 2026-04-06T15:12:24Z

+        // Check caller is VM Signer
+        msg.sender.enforceIsVmSigner(s.vmSigner);
+
+        LibCore.handleTaskRemoval(_taskIndex);


Do we need a method for bulk removal? Also, shouldn't Vm signer also pass some diagnosic message for removal which should be emitted as an event?

I'll add a parameter for diagnostic message. @aregng please let me know your opinion if we require a method for bulk removal?

Yes bulk task removal will be preferable, in similar fashion as tasks are processed in bulk in scope of processTasks

sjoshisupra · 2026-04-06T15:15:12Z

    );

+    /// @notice Emitted when a task is removed due to predicate validation failure.
+    event TaskRemovedAsPredicateFailed(


We may want to remove the task for multiple reasons, right? 1) Predicate is malformed due to invalid bytecode 2) Predicate is exceeding gas limits imposed, we should have a uniform mechanism for VM signer to convey the reason of failure from layer below @aregng @udityadav-supraoracles

@udityadav-supraoracles , can a view funcion have a failing assert? The questions is should we remove a predicate that fails an assert. @aregng

I'll add a string parameter which can be used to convey the reason for failure.

It is possible for a view function to have an assertion. Let's say it can only be called by whitelisted addresses, in that case it'll fail.

@udityadav-supraoracles typically view functions are never access controlled, since they are just meant for viewing, so either it is view method or a private method. WHat use case do we have to have access control on view?

Agree with you Dr @sjoshisupra , largely they're not access controlled. I only mentioned it to depict a scenario where view function can revert.

sjoshisupra · 2026-04-06T15:42:17Z

+
+password=""
+if [ -n ${PASSWORD} ]; then
+  password="--password ${PASSWORD}"


not sure what is being done here, if the variable is not set, how does "--passowrd ${PASSWORD}" help?

If PASSWORD is set then that value will be utilised as password else forge will prompt to input the password.

sjoshisupra

Let's use 20M for user and system registry gas reserve, 1M is too low to be of any practical use.

sjoshisupra · 2026-04-08T13:56:08Z

    );

+    /// @notice Emitted when a task is removed due to predicate validation failure.
+    event TaskRemovedAsPredicateFailed(


@udityadav-supraoracles typically view functions are never access controlled, since they are just meant for viewing, so either it is view method or a private method. WHat use case do we have to have access control on view?

- changed cli script to python while using cast - fixed test cases

aregng · 2026-04-21T07:56:42Z

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.

Mapping point option is preferable and is currently implemented.

aregng · 2026-04-21T08:18:06Z

+
+    // Address of the VM Signer: SUP0
+    address constant VM_SIGNER = address(0x53555000);


Here we have VM_SIGNER constant, but also we specify it as input parameter during initialization. Besides we also specify the manually vm_signer when checking condition of sender to be VM_SIGNER.
Suggestion: instead of specifying it VM_SINGER as input param, let's instead use the hardcoded constant. As mentioned by Dr. @sjoshisupra it is not going to be change at all.

aregng · 2026-04-21T15:11:35Z

+
+library LibDiamond {
+    // 32 bytes keccak hash of a string to use as a diamond storage location.
+    bytes32 constant DIAMOND_STORAGE_POSITION = keccak256("diamond.standard.diamond.storage");


The concept of having dedicated slot for the shared storage is a standard, but I think we can use automation registry as part of the string defining the unique position, such as diamond.registry.automation.storage.

aregng · 2026-04-21T15:31:36Z

+        // Check caller is VM Signer
+        msg.sender.enforceIsVmSigner(s.vmSigner);
+
+        LibCore.handleTaskRemoval(_taskIndex);


Yes bulk task removal will be preferable, in similar fashion as tasks are processed in bulk in scope of processTasks

udityadav-supraoracles · 2026-04-22T08:55:12Z

s.cycleState shows the current state of the registry, so if it is READY and automation feature flag is enabled, then we move to STARTED state. One thing that can be done to improve user experience in case if state is not READY, to revert the call with error indicating inconsistent state.

@aregng So, you mean if cycle state is not READY then enableAutomation should revert? In that case
testProcessTasksWhenCycleStateSuspendedAutomationEnabled fails for automation enabled condition

aregng · 2026-04-22T12:08:52Z

s.cycleState shows the current state of the registry, so if it is READY and automation feature flag is enabled, then we move to STARTED state. One thing that can be done to improve user experience in case if state is not READY, to revert the call with error indicating inconsistent state.

@aregng So, you mean if cycle state is not READY then enableAutomation should revert? In that case testProcessTasksWhenCycleStateSuspendedAutomationEnabled fails for automation enabled condition

Oh, I see, I missed the use-case when automation-feature is being disabled during transition state.
The current implementation is good then.

aregng · 2026-04-22T19:40:36Z

+
+                // Refund only for UST
+                if (!isGst) {
+                    bool result = LibAccounting.safeDepositRefund(


According to discussion related to task removal on predicate failure documented in this ticket: https://github.com/Entropy-Foundation/smr-moonshot/issues/2648
the tasks with failed/invalid predicate will the removed and the same refund logic will be applied to the tasks as if they have been stopped by user.
Which implies full deposit refund and refund of the half of automation cycle fee for the remaining cycle time.

Dr. @sjoshisupra , @udityadav-supraoracles , was the requirement incorrectly documented or it has been changed over the time ?

@aregng Actually during the Meet you mentioned that user will be refunded as if they've cancelled the task which is in PENDING state. So, based on that I had implemented refund logic.

aregng · 2026-04-23T12:10:17Z

+        if (!s.ifTransitionStateExists) { revert InvalidRegistryState(); }
+
+        if (isTransitionFinalized()) {
+            if (!s.automationEnabled && s.cycleState == LibCommon.CycleState.FINISHED) {


Usecase:

Cycle is in transition state and transition is in progress

Automation Feature is disabled

Cycle transition ends

Here we attempt to move to suspend state which will revert as transition state exists and it is in progress.

Described scenario is a valid one.
In this case transition to SUSPENED state should happen via STARTED state, which means first here we should transition to STARTED state always when cycle transition is finalized, i.e update cycle index by one and set cycle state to STARTED and only after that check if automation-feature flag is false, directly transition to suspended state.

I will update the ticket describing the transition table.
https://github.com/Entropy-Foundation/smr-moonshot/issues/2523#issuecomment-3677617421

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

udityadav-supraoracles added 2 commits February 24, 2026 14:13

resolved PR comments

48a1683

fixed cancel task and stop task

b1f4511

udityadav-supraoracles requested a review from sjoshisupra February 24, 2026 10:29

udityadav-supraoracles added 2 commits February 24, 2026 16:35

updated run.sh for cance task

d420c76

updated AppStorage: using mapping in place of inner structs

6f92967

sjoshisupra requested changes Feb 27, 2026

View reviewed changes

udityadav-supraoracles added 2 commits March 13, 2026 17:22

-Separated conversion logic to ERC20SupraHandler from ERC20Supra

3806953

-Resolved PR comments

- renamed getUserTasks => getTasksByAddress

3d3e1a2

- removed setErc20Supra, setVmSigner

udityadav-supraoracles requested a review from sjoshisupra March 26, 2026 12:37

aregng reviewed Mar 26, 2026

View reviewed changes

Comment thread solidity/supra_contracts/src/ERC20SupraHandler.sol

sjoshisupra requested changes Mar 27, 2026

View reviewed changes

udityadav-supraoracles added 3 commits April 3, 2026 12:08

- removed nativToErc20SupraWithAllowance

0629bbf

- added mapping for authorised addresses with mint/burn capabilities

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

c25145f

…mation_registry

Added changes for predicate

780b895

udityadav-supraoracles requested review from aregng and sjoshisupra April 6, 2026 10:13

sjoshisupra requested changes Apr 6, 2026

View reviewed changes

resolved PR comments

b0cfc7a

udityadav-supraoracles requested a review from sjoshisupra April 8, 2026 13:22

sjoshisupra reviewed Apr 8, 2026

View reviewed changes

aregng reviewed Apr 10, 2026

View reviewed changes

Comment thread solidity/supra_contracts/src/SupraContractsBindings.sol

aregng reviewed Apr 10, 2026

View reviewed changes

Comment thread solidity/supra_contracts/src/upgradeInitializers/DiamondInit.sol

udityadav-supraoracles added 6 commits April 10, 2026 16:38

- resolved PR comments

8f19471

- changed cli script to python while using cast - fixed test cases

- renamed nativeToErc20Supra -> deposit, erc20SupraToNative -> withdraw

bee712c

bug fixes

077eeab

renamed burn to burnFrom

b698680

added getCycleStateDetails to coreFacet

d6898da

added burn in ERC20Supra and its test cases

58c31ec

udityadav-supraoracles requested review from aregng and sjoshisupra April 21, 2026 06:40

aregng reviewed Apr 21, 2026

View reviewed changes

resolved pr comments

7d725fc

aregng reviewed Apr 22, 2026

View reviewed changes

aregng reviewed Apr 23, 2026

View reviewed changes


		// Address of the VM Signer: SUP0
		address constant VM_SIGNER = address(0x53555000);

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!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

sjoshisupra commented Feb 24, 2026

Uh oh!

Uh oh!

Uh oh!

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!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

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!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sjoshisupra left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

aregng commented Apr 21, 2026

Uh oh!

Choose a reason for hiding this comment

udityadav-supraoracles commented Feb 17, 2026 •

edited

Loading

udityadav-supraoracles Apr 23, 2026 •

edited

Loading