fix: signature utils #1015

0xClandestine · 2025-01-11T00:29:20Z

Changes:

Dynamic Domain Separator: SignatureUtils.domainSeparator() is now recomputed for each signature verification. This eliminates the need for storing initial values in storage or as immutables, which is important for beacon proxy support.
Version Bump Command: Introduced make bump-version VERSION=2, which automatically updates the version function's return values.
Version Fn + Constructor Param: Adds an immutable oz ShortString that's set in the constructor.

ypatil12

Can we sanity check that the domain separator of the DM/RC/AVSD is based on the proxy and not the implementation

0xClandestine · 2025-01-11T01:01:13Z

Can we sanity check that the domain separator of the DM/RC/AVSD is based on the proxy and not the implementation

Oops, forgot that. Will do.

ypatil12

Comments regarding version number

bin/bump-version.sh

src/contracts/core/AVSDirectory.sol

src/contracts/core/DelegationManager.sol

src/test/unit/RewardsCoordinatorUnit.t.sol

0xClandestine · 2025-01-13T15:27:12Z

Check storage layout ci failing due to SignatureUtils -> SignatureUtilsMixin.

src/contracts/core/StrategyManager.sol

src/contracts/mixins/SignatureUtilsMixin.sol

src/contracts/core/AVSDirectory.sol

Makefile

script/configs/devnet/deploy_from_scratch.anvil.config.json

ypatil12

So far looks good, we should also add SemverMixin to the EigenPodManager and StrategyFactory.

We may want to also add to all Strategies and EigenPods too (can talk in standup about this one).

script/configs/mainnet/mainnet-addresses.config.json

ypatil12 · 2025-01-22T14:46:30Z

script/releases/Env.sol

@@ -53,6 +53,10 @@ library Env {
     * env
     */

+    function version() internal view returns (string memory) {
+        return _envString("version");


Can you coordinate with @jbrower95 to make sure this is a parameter in the Zeus config that the upgrade scripts can access

in progress

To add to this, but maybe overkill, we may want to have zeus config to have semver for each contract. Some of our upgrades will touch just a few contracts so the versions will be different at a point in time

Ahh that's a good point, so rather than a single config param, there's one per contract?

Ya that would make sense to me

Just noting we decided against this...

Ahh that's a good point, so rather than a single config param, there's one per contract?

src/contracts/core/DelegationManager.sol

src/contracts/mixins/SemVerMixin.sol

src/contracts/interfaces/IRewardsCoordinator.sol

src/contracts/interfaces/ISignatureUtilsMixin.sol

8sunyuan · 2025-01-22T16:10:39Z

src/contracts/strategies/EigenStrategy.sol

Yeah not entirely sure if we want to change EigenStrategy here. I figure the Eigen/EigenStrategy changes will take place in its own separate upgrade.

We upgraded the EigenStrategy in the initial upgrade, so we should be good. Eigen can be separate:

eigenlayer-contracts/script/releases/v1.0.0-slashing/1-deployContracts.s.sol

Lines 153 to 159 in 7a6f8a6

deployImpl({

name: type(EigenStrategy).name,

deployedTo: address(new EigenStrategy({

_strategyManager: Env.proxy.strategyManager(),

_pauserRegistry: Env.impl.pauserRegistry()

}))

});

ypatil12

LGTM, approving. Just need to coordinate with @jbrower95 on the Zeus changes

ypatil12 · 2025-01-22T20:14:29Z

Oh, can we also update the storage diff for sanity?

ypatil12 · 2025-01-22T22:15:27Z

LGTM, approving. Just need to coordinate with @jbrower95 on the Zeus changes

Spoke with @jbrower95, we're just going to add a Zeus helper solidity script to tick the version. Will be done in a separate PR

ypatil12

Blocking for now - let's wait to merge until we have all audit fixes in

ypatil12

LGTM, one comment

script/releases/Env.sol

ypatil12

LGTM

refactor: review changes refactor: review changes feat: remaining semver feat: remaining semver refactor: review changes refactor: review changes fix: ci refactor: remove make cmd fix: ci fix: ci feat: add `SemVerMixin` (#1034) * feat: add `SemVerMixin` * refactor: add `_majorVersion` helper for `SignatureUtils` * test: passing * test: passing * refactor: rename EIP712_VERSION * refactor: review changes chore: forge fmt refactor: natspec improvements chore: forge fmt fix: eip712 typehash refactor: remove version overrides refactor: remove make bump-version refactor: immutable version refactor: remove unused import chore: `make bump-version VERSION=1` refactor: `SignatureUtils` -> `SignatureUtilsMixin` fix: bump-version.sh chore: `make bump-version VERSION=1.0.3` feat: add `make bump-version` feat: add version fn refactor: cleanup fix: signature utils

ypatil12 · 2025-02-14T18:21:38Z

script/releases/v1.0.3-slashing/1-eoa.s.sol

@@ -13,6 +13,8 @@ import "@openzeppelin/contracts/token/ERC20/extensions/IERC20Metadata.sol";
 contract Deploy is EOADeployer {
    using Env for *;

+    string internal constant VERSION = "v1.0.3-slashing";


Spoke with Justin, he's going to add a way to get inject version of the new upgrade in Zeus params. This is a placeholder till that is up :)

Layr-Labs/zeus#28 (review)

### Changes: - *Dynamic Domain Separator:* `SignatureUtils.domainSeparator()` is now recomputed for each signature verification. This eliminates the need for storing initial values in storage or as immutables, which is important for beacon proxy support. - ~*Version Bump Command:* Introduced `make bump-version VERSION=2`, which automatically updates the version function's return values.~ - *Version Fn + Constructor Param:* Adds an immutable oz `ShortString` that's set in the constructor.

- *Dynamic Domain Separator:* `SignatureUtils.domainSeparator()` is now recomputed for each signature verification. This eliminates the need for storing initial values in storage or as immutables, which is important for beacon proxy support. - ~*Version Bump Command:* Introduced `make bump-version VERSION=2`, which automatically updates the version function's return values.~ - *Version Fn + Constructor Param:* Adds an immutable oz `ShortString` that's set in the constructor.

### Changes: - *Dynamic Domain Separator:* `SignatureUtils.domainSeparator()` is now recomputed for each signature verification. This eliminates the need for storing initial values in storage or as immutables, which is important for beacon proxy support. - ~*Version Bump Command:* Introduced `make bump-version VERSION=2`, which automatically updates the version function's return values.~ - *Version Fn + Constructor Param:* Adds an immutable oz `ShortString` that's set in the constructor.

- *Dynamic Domain Separator:* `SignatureUtils.domainSeparator()` is now recomputed for each signature verification. This eliminates the need for storing initial values in storage or as immutables, which is important for beacon proxy support. - ~*Version Bump Command:* Introduced `make bump-version VERSION=2`, which automatically updates the version function's return values.~ - *Version Fn + Constructor Param:* Adds an immutable oz `ShortString` that's set in the constructor.

@8sunyuan

fix(slashing): upgrade script part 4 (#953) fix: patch (#956) feat: bindings (#960) fix: remove numtocomplete interface (#966) feat: add share helpers (#964) * feat: add share helpers * fix: add deposit scaling factor * fix: rebase fix: slashable window boundaries (#965) * fix: slashable window boundaries * test: regression for alm * test: update withdrawal delay not passed reversion * test: burning indices * refactor: switch conditionals * fix: added unit tests * test: assert slashable shares in queue * fix: typos --------- Co-authored-by: Yash Patil <ypatil12@gmail.com> refactor: small cleanup (#959) refactor small cleanup chore: `forge fmt` fix: `getQueuedWithdrawals` + test fix: add constructor back test: `totalQueued` > `withdrawal.strategies.length` test(wip): `completeQueuedWithdrawals` currently failing fix: effectBlock test(wip): @8sunyuan patch fix: one flaky test fix: second flaky test feat: slashing patch upgrade script (#967) * feat: initial deploy * feat: slashing patch fix non-present upgrade.json fix: try catch out of gas edge case (#971) chore: slashing consolidated script (#972) test: more slashing integration todos (#961) * test(wip): todos * fix: dealloc issue * fix: remaining * fix: forktest upgrade issue * test: add `check_Withdrawal_AsShares_State_AfterSlash` * refactor: cleanup * fix: ci * refactor: review changes docs: wip slashing docs (#925) * docs: add slashing docs * chore: bindings * docs: fixed commenting and updated queue withdrawal docs * docs: minor cleanup --------- Co-authored-by: Nadir Akhtar <nadirakhtar123@gmail.com> refactor: scaled shares accounting (#975) * fix: correct expected share calc * chore: bindings * fix: rounding on failing unit test refactor: final slashing cleanup (#982) * chore: clean comments and naming in dm * refactor: simplify undelegate method * feat: removed 0 address check because 0 stakers cant be delegated * feat: condensed non-staker caller logic * refactor: remove unnecessary check * feat: use checks-effects-interactions when completing withdrawals * feat: remove implicit public method for queuedWithdrawals and impl dedicated getter * feat: deprecate withdrawer field * chore: make bindings and clean compile errors * refactor: redelegate reuses delegateTo and undelegate * fix: broken integration test * docs: update to reflect deprecated field * feat: add getter for stakers withdrawal roots fix: integration test initialization params (#978) * fix: initialization params * fix: roll blocks usage fix: `SignatureUtils` construction (#990) * fix: integration test initialization params (#978) * fix: initialization params * fix: roll blocks usage * fix: `SignatureUtils` construction --------- Co-authored-by: Yash Patil <40046473+ypatil12@users.noreply.github.com> Co-authored-by: davidironblocks <david@ironblocks.com> feat: slashing 1.0.3 upgrade script (#995) * feat: add step 1 * feat: step 1 & 2 complete; pending step 3 sanity * test: add `_validateProxyDomainSeparators` * feat: add rc validation --------- Co-authored-by: clandestine.eth <96172957+0xClandestine@users.noreply.github.com> refactor: async burning (#1001) * refactor: burning * chore: fmt * chore: update storage report * chore: update readme * refactor: add burnableShares for epm storage * chore: update storage report docs: finish delegation manager docs (#1004) * docs: finish delegation manager docs * docs: update docs readme * docs: permission controller * fix: small typos * docs: address feedback * docs: nit --------- Co-authored-by: Michael Sun <michaelsun97@gmail.com> docs: Strategy Manager slashing updates (#999) * docs: update StrategyManager docs with slashing delta * docs: remove references to thirdPartyTransfersForbidden * docs: update strategy docs to latest * also various edits to docs and natspec * chore: fmt and make bindings --------- Co-authored-by: wadealexc <pragma-services@proton.me> test: enable shared setups for integration tests (#1036) * test: improve integration invariants * also removes unneeded fork logic * adds checks to some invariants * fixes some broken tests * test(integration): enable shared setups fix: remove token param from Deposit event and related APIs (#1013) * fix: remove token param from Deposit event and related APIs * fix: forge fmt * fix: rebase * fix: update EigenPodManager and test * fix: update tests * fix: update eigenpodmanager tests * fix: update StrategyManagerMock * feat: add bindings * fix: update docs feat: changing burnableShares to EnumerableMap (#1028) * feat: changing burnableShares to EnumerableMap * style: linter * docs: storage docs * style: natspec and import * style: lint * feat: adding view function for cronjob and moving functions * fix: updating storage gap * docs: storage slots comment * feat: new bindings * docs: updating StrategyManager doc * docs: bindings --------- Co-authored-by: Michael <michael@Michaels-MacBook-Pro.local> feat: add `getAllocatedStake` and update `getMinimumSlashableStake` (#1037) * chore: add view functions for isOperatorSlashable * feat: add `getAllocatedStake` func * test: getAllocatedStake * test: add getMinimumSlashableStake tests * chore: format * chore: fmt interface * chore: remove unnecessary test * chore: update comment * chore: bindings --------- Co-authored-by: Yash Patil <40046473+ypatil12@users.noreply.github.com> test(integration): implement registration and allocation invariants (#1042) * chore: fix forge nightly release breaking two tests * test: fix outdated alm tests fix: delegate shares (#1045) **Motivation:** Fixes an issue where stakers delegating Beacon Chain ETH from slashed Eigen Pods were able to delegate more shares than they should. Specifically, operators now are delegated a staker's `withdrawableShares` rather than their `depositShares`. **Modifications:** - Changed accounting logic on delegation in `DelegationManger.sol` - `DepositScalingFactor` now resets when a staker withdraws all their shares, whether through undelegation, redelegation, or a simple withdrawal - Changes in `StrategyManager.sol`, `IShareManager.sol`, `SlashingLib.sol`, and `EigenPodManager.sol` to accommodate new accounting - New test files and changes to others to reflect new accounting and invariants - Updated `docs/SharesAccounting.md` **Result:** System is now robust to stakers with arbitrary EigenPod states --------- Co-authored-by: Michael <michael@Michaels-MacBook-Pro.local> Co-authored-by: Michael Sun <michaelsun97@gmail.com> Co-authored-by: wadealexc <pragma-services@proton.me> Co-authored-by: Yash Patil <40046473+ypatil12@users.noreply.github.com> Co-authored-by: clandestine.eth <96172957+0xClandestine@users.noreply.github.com> fix: signature utils (#1015) - *Dynamic Domain Separator:* `SignatureUtils.domainSeparator()` is now recomputed for each signature verification. This eliminates the need for storing initial values in storage or as immutables, which is important for beacon proxy support. - ~*Version Bump Command:* Introduced `make bump-version VERSION=2`, which automatically updates the version function's return values.~ - *Version Fn + Constructor Param:* Adds an immutable oz `ShortString` that's set in the constructor. feat: require avs register metadata in allocation manager (#1025) require avs register metadata in allocation manager before they can create operatorset --------- Co-authored-by: clandestine.eth <96172957+0xClandestine@users.noreply.github.com>

0xClandestine mentioned this pull request Jan 11, 2025

refactor: signature utils #1009

Closed

ypatil12 reviewed Jan 11, 2025

View reviewed changes

bin/bump-version.sh Outdated Show resolved Hide resolved

src/contracts/core/AVSDirectory.sol Outdated Show resolved Hide resolved

src/contracts/core/DelegationManager.sol Outdated Show resolved Hide resolved

src/test/unit/RewardsCoordinatorUnit.t.sol Outdated Show resolved Hide resolved

wadealexc force-pushed the slashing-magnitudes-fixes branch from 1427a85 to 403e0a1 Compare January 13, 2025 19:58

0xClandestine force-pushed the fix/signature-utils branch 2 times, most recently from 4189591 to 6d5d06b Compare January 16, 2025 19:23

8sunyuan reviewed Jan 16, 2025

View reviewed changes

src/contracts/core/StrategyManager.sol Outdated Show resolved Hide resolved

8sunyuan reviewed Jan 16, 2025

View reviewed changes

src/contracts/mixins/SignatureUtilsMixin.sol Outdated Show resolved Hide resolved

8sunyuan reviewed Jan 16, 2025

View reviewed changes

src/contracts/core/AVSDirectory.sol Show resolved Hide resolved

0xClandestine mentioned this pull request Jan 17, 2025

feat: add SemVerMixin #1034

Merged

0xClandestine force-pushed the fix/signature-utils branch from 943d7ee to 0aa946b Compare January 21, 2025 21:18

0xClandestine commented Jan 22, 2025

View reviewed changes

Makefile Outdated Show resolved Hide resolved

0xClandestine commented Jan 22, 2025

View reviewed changes

script/configs/devnet/deploy_from_scratch.anvil.config.json Outdated Show resolved Hide resolved

0xClandestine requested review from ypatil12 and 8sunyuan January 22, 2025 14:31

ypatil12 requested changes Jan 22, 2025

View reviewed changes

0xClandestine mentioned this pull request Jan 22, 2025

feat: add remaining semver #1041

Merged

0xClandestine force-pushed the fix/signature-utils branch from dbd8143 to 85f7bf1 Compare January 22, 2025 15:49

8sunyuan reviewed Jan 22, 2025

View reviewed changes

0xClandestine force-pushed the fix/signature-utils branch from 2ff7718 to 38d2ad8 Compare January 22, 2025 16:13

ypatil12 approved these changes Jan 22, 2025

View reviewed changes

ypatil12 requested changes Jan 27, 2025

View reviewed changes

0xClandestine force-pushed the fix/signature-utils branch from 38d2ad8 to a09d514 Compare January 29, 2025 13:34

0xClandestine added the ⚖️ Audit Fix Audit-related fixes. label Feb 13, 2025

0xClandestine force-pushed the fix/signature-utils branch from 9ceb4d8 to 0a8c4eb Compare February 13, 2025 22:51

0xClandestine requested a review from 8sunyuan February 13, 2025 22:52

ypatil12 reviewed Feb 14, 2025

View reviewed changes

script/releases/Env.sol Outdated Show resolved Hide resolved

ypatil12 approved these changes Feb 14, 2025

View reviewed changes

0xClandestine added 4 commits February 14, 2025 12:36

chore: forge fmt

79ee18e

refactor: review changes

901ea39

refactor: review changes

7669e08

0xClandestine force-pushed the fix/signature-utils branch from 4a87e78 to 7669e08 Compare February 14, 2025 17:37

ypatil12 reviewed Feb 14, 2025

View reviewed changes

0xClandestine requested a review from wadealexc February 14, 2025 18:40

wadealexc approved these changes Feb 14, 2025

View reviewed changes

0xClandestine merged commit 067010c into slashing-magnitudes-fixes Feb 14, 2025
10 of 11 checks passed

0xClandestine deleted the fix/signature-utils branch February 14, 2025 19:46

wadealexc mentioned this pull request Feb 18, 2025

feat: slashing audit fixes #1046

Merged

NaijaCoderGirl mentioned this pull request Feb 25, 2025

docs: update DelegationManager doc SignatureUtils.sol -> SignatureUtilsMixin.sol #1166

Closed

	deployImpl({
	name: type(EigenStrategy).name,
	deployedTo: address(new EigenStrategy({
	_strategyManager: Env.proxy.strategyManager(),
	_pauserRegistry: Env.impl.pauserRegistry()
	}))
	});

fix: signature utils #1015

fix: signature utils #1015

Uh oh!

Conversation

0xClandestine commented Jan 11, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Changes:

Uh oh!

ypatil12 left a comment

Choose a reason for hiding this comment

Uh oh!

0xClandestine commented Jan 11, 2025

Uh oh!

ypatil12 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

0xClandestine commented Jan 13, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

ypatil12 left a comment

Choose a reason for hiding this comment

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!

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!

ypatil12 left a comment

Choose a reason for hiding this comment

Uh oh!

ypatil12 commented Jan 22, 2025

Uh oh!

ypatil12 commented Jan 22, 2025

Uh oh!

ypatil12 left a comment

Choose a reason for hiding this comment

Uh oh!

ypatil12 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

ypatil12 left a comment

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!

0xClandestine commented Jan 11, 2025 •

edited

Loading

0xClandestine commented Jan 13, 2025 •

edited

Loading