feat(ethexe): Support slash commitments in router #4403

ukint-vs · 2024-12-13T12:00:01Z

Resolves #4383 .

implement IMiddleware
implement request/execute slash commitment in Router
add test in POC

@reviewer-or-team

StackOverflowExcept1on · 2024-12-26T13:08:24Z

ethexe/contracts/script/Deployment.s.sol

@@ -34,6 +36,7 @@ contract DeploymentScript is Script {

        address mirrorAddress = vm.computeCreateAddress(deployerAddress, vm.getNonce(deployerAddress) + 2);
        address mirrorProxyAddress = vm.computeCreateAddress(deployerAddress, vm.getNonce(deployerAddress) + 3);
+        address middlewareAddress = vm.computeCreateAddress(deployerAddress, vm.getNonce(deployerAddress) + 4);


vm.computeCreateAddress(deployerAddress, deployerNonce) is used to compute the future contract address. after mirrorProxy = new MirrorProxy(address(router)) you need to create middleware to make this address valid.

StackOverflowExcept1on · 2024-12-26T13:09:23Z

ethexe/ethereum/src/lib.rs

@@ -154,6 +154,7 @@ impl Ethereum {
                    _mirror: mirror_address,
                    _mirrorProxy: mirror_proxy_address,
                    _wrappedVara: wvara_address,
+                    _middleware: Address::default(),


this place should be the same as ethexe/contracts/script/Deployment.s.sol

I think you can do it in the next PR such as #4403 for the Rust part, and do the Solidity part in this PR

StackOverflowExcept1on · 2024-12-26T13:13:10Z

ethexe/contracts/src/IRouter.sol

+    event RequestSlashCommitmentProcessed(bytes32 indexed commitmentHash, IMiddleware.SlashData[] slashData);
+
+    /**
+     * @dev Emitted when a slash request is executed by the middleware.
+     * @param slashId An array of SlashIdentifier structures containing details of the slash execution.
+     */
+    event ExecuteSlashCommitmentProcessed(bytes32 indexed commitmentHash, IMiddleware.SlashIdentifier[] slashId);


indexed is not needed, because the observer is watching many events at once

Suggested change

event RequestSlashCommitmentProcessed(bytes32 indexed commitmentHash, IMiddleware.SlashData[] slashData);

/**

* @dev Emitted when a slash request is executed by the middleware.

* @param slashId An array of SlashIdentifier structures containing details of the slash execution.

*/

event ExecuteSlashCommitmentProcessed(bytes32 indexed commitmentHash, IMiddleware.SlashIdentifier[] slashId);

event RequestSlashCommitmentProcessed(bytes32 commitmentHash, IMiddleware.SlashData[] slashData);

/**

* @dev Emitted when a slash request is executed by the middleware.

* @param slashId An array of SlashIdentifier structures containing details of the slash execution.

*/

event ExecuteSlashCommitmentProcessed(bytes32 commitmentHash, IMiddleware.SlashIdentifier[] slashId);

grishasobol · 2024-12-26T11:56:07Z

ethexe/contracts/test/POC.t.sol

        vm.startPrank(admin);
        {
            middleware.changeSlashRequester(address(router));
            middleware.changeSlashExecutor(address(router));
        }
        vm.stopPrank();
+
+        // For understanding where symbiotic ecosystem is using
+        sym = POCBaseTest(address(this));


grishasobol · 2024-12-26T11:56:14Z

ethexe/contracts/test/POC.t.sol

 import {MessageHashUtils} from "@openzeppelin/contracts/utils/cryptography/MessageHashUtils.sol";
 import {Upgrades} from "openzeppelin-foundry-upgrades/Upgrades.sol";
 import {EnumerableMap} from "@openzeppelin/contracts/utils/structs/EnumerableMap.sol";
 import "forge-std/Test.sol";

+import {POCBaseTest} from "symbiotic-core/test/POCBase.t.sol";


grishasobol · 2024-12-26T12:42:18Z

ethexe/contracts/src/libraries/Gear.sol

@@ -101,6 +103,16 @@ library Gear {
        Message[] messages;
    }

+    struct RequestSlashCommitment {


Looks like better to use naming SlashRequestCommitment, SlashExecuteCommitment and the same in other places. Because when read function names like requestSlashCommitmentHash - this understands like a calling some function which send a request for hash.

grishasobol · 2024-12-26T12:51:54Z

ethexe/contracts/src/Router.sol

+        uint256 currentEraIndex = (block.timestamp - router.genesisBlock.timestamp) / router.timelines.era;
+
+        // Validate the era index
+        require(commitment.eraIndex == currentEraIndex, "commitment era index is invalid");


Is not possible to sent a slash for the previous era? for example we found that validator from the previous era did something wrong - why we cannot to sent a slash for the funds it had in the previous era?

Ah now I can see. I think era param should be removed from commitment. Cannot see why do we need it. Slashes have already all informations about timestamps.

grishasobol · 2024-12-26T12:52:33Z

ethexe/contracts/src/Router.sol

+        uint256 eraStart = router.genesisBlock.timestamp + router.timelines.era * currentEraIndex;
+        require(block.timestamp >= eraStart, "current era has not started yet");


This is mathematically impossible case

grishasobol · 2024-12-26T12:55:28Z

ethexe/contracts/src/Router.sol

+
+        // Ensure middleware is set
+        address middlewareAddress = router.implAddresses.middleware;
+        require(middlewareAddress != address(0), "Middleware address not set");


Not important: Looks like over-checking. This must be checked in constructor

grishasobol · 2024-12-26T12:56:53Z

ethexe/contracts/src/Router.sol

+        // Call the middleware's `requestSlash` function
+        middlewareInstance.requestSlash(commitment.slashes);
+
+        emit RequestSlashCommitmentProcessed(commitmentHash, commitment.slashes);


I think is enough to sent a commitmentHash. This is cheaper. And validators could save their slashes in db by hash and access them if they need in future.

grishasobol · 2024-12-26T16:47:42Z

ethexe/contracts/test/POC.t.sol

+        uint256 currentEraIndex = (block.timestamp - router.genesisTimestamp()) / eraDuration;
+        uint256 nextEraIndex = currentEraIndex + 1;
+
+        // Move to the election period for the next era


Your are not going to election period

ukint-vs added 2 commits December 13, 2024 18:28

feat(ethexe): Impl Middleware interface

fe85580

Impl requestSlash test

275c6a1

ukint-vs added the A1-inprogress Issue is in progress or PR draft is not ready to be reviewed label Dec 13, 2024

ukint-vs self-assigned this Dec 13, 2024

ukint-vs added 4 commits December 16, 2024 17:56

impl executeSlash

2d0fce0

include signatures

b0fad1d

Update test

6f18dec

update events

41fbb4f

ukint-vs changed the title ~~feat(ethexe): Support slash in router~~ feat(ethexe): Support slash commitments in router Dec 17, 2024

ukint-vs added 2 commits December 18, 2024 16:57

setup router

c440d7f

update json

8a55708

ukint-vs marked this pull request as ready for review December 18, 2024 09:59

ukint-vs added A0-pleasereview PR is ready to be reviewed by the team C1-feature Feature request and removed A1-inprogress Issue is in progress or PR draft is not ready to be reviewed labels Dec 18, 2024

ukint-vs requested review from grishasobol and StackOverflowExcept1on December 19, 2024 06:42

grishasobol added 3 commits December 26, 2024 13:09

Merge branch 'master' into vs/ethexe-slashrequest

a6f60ad

update jsons after merge

4fb714d

Merge branch 'master' into vs/ethexe-slashrequest

c4d0482

StackOverflowExcept1on reviewed Dec 26, 2024

View reviewed changes

grishasobol reviewed Dec 26, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(ethexe): Support slash commitments in router #4403

feat(ethexe): Support slash commitments in router #4403

ukint-vs commented Dec 13, 2024 •

edited

Loading

StackOverflowExcept1on Dec 26, 2024

StackOverflowExcept1on Dec 26, 2024

StackOverflowExcept1on Dec 26, 2024

StackOverflowExcept1on Dec 26, 2024

grishasobol Dec 26, 2024

grishasobol Dec 26, 2024

grishasobol Dec 26, 2024

grishasobol Dec 26, 2024

grishasobol Dec 26, 2024

grishasobol Dec 26, 2024

grishasobol Dec 26, 2024

grishasobol Dec 26, 2024

grishasobol Dec 26, 2024

		uint256 eraStart = router.genesisBlock.timestamp + router.timelines.era * currentEraIndex;
		require(block.timestamp >= eraStart, "current era has not started yet");

feat(ethexe): Support slash commitments in router #4403

Are you sure you want to change the base?

feat(ethexe): Support slash commitments in router #4403

Conversation

ukint-vs commented 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

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

ukint-vs commented Dec 13, 2024 •

edited

Loading