Permissioned Validators by ksatyarth2 · Pull Request #141 · PufferFinance/puffer-contracts

ksatyarth2 · 2026-02-04T10:55:56Z

What this PR does / why we need it:

Which issue(s) does this PR fixes:

Additional comments:

…-7002

…upport

eladiosch

I left a few comments. I haven't dived deeply into tests or scripts. I want to validate the approach and flows first

mainnet-contracts/src/PermissionedOracle.sol

eladiosch · 2026-02-16T16:57:27Z

mainnet-contracts/src/PufferModuleManager.sol

+     * @dev Permissioned module beacon address (stored in contract storage for upgradeability)
+     * keccak256(abi.encode(uint256(keccak256("PufferModuleManager.permissionedModuleBeacon")) - 1)) & ~bytes32(uint256(0xff))
+     */
+    bytes32 private constant _PERMISSIONED_MODULE_BEACON_SLOT =


Any reason to follow this approach to store the beacon? Why not follow the same approach with the regular puffer module beacon? It's a bit strage to have different approaches for pretty much the same thing in the same contract

PUFFER_MODULE_BEACON is immutable and fixed at deployment of this contract, but _PERMISSIONED_MODULE_BEACON_SLOT uses storage for post deployment upgradability

Yes, I see that. However, since both are really similar (and now also the beacon for NRWC). Using the current approach, when deploying we need to also send 2 transactions to set these beacons (right now those are not part of the deployment flow). I think it might be easy to miss and complicated the deployment flow.

If we went with the constructor arg approach it will be simpler and if we need to upgrade any of the beacons we can upgrade the PMM just deploying a new implementation instead of sending the transactions to set the beacons. It would also mean a simpler code since we remove the setter functions and the storage slots, etc

mainnet-contracts/src/PermissionedModule.sol

eladiosch · 2026-02-16T17:16:31Z

mainnet-contracts/src/PermissionedModule.sol

+        // Create EigenPod for restaked validators
+        $.eigenPod = IEigenPod(address(EIGEN_POD_MANAGER.createPod()));
+        // Deploy NonRestakingWithdrawalCredentials for non-restaked validators
+        $.nonRestakingWithdrawalCredentials = new NonRestakingWithdrawalCredentials(address(this), initialAuthority);


Not sure about this way of deploying NonRestakingWithdralCredentials. If we need to change the withdrawal credentials contract, we have no way of upgrading the it, maybe it should be upgradeable. Imagine we found a bug in there, or a new EIP comes with new functionality to interact with the beacon deposit contract, we don't even have custom external call (EDIT: I see we do have a call function). I think it should be upgradeable

And if we have a lot of permissioned modules in the future, it would be very tedious to upgrade the contracts one by one. I think we should use a beacon proxy, just like with the modules

make sense. made NRWC as beacon upgradable

LGTM. See my other comment related to how to manage the beacons

mainnet-contracts/src/PufferVaultV5.sol

eladiosch · 2026-02-17T10:43:39Z

mainnet-contracts/src/PermissionedOracle.sol

+    /**
+     * @inheritdoc IPermissionedOracle
+     */
+    function adjustLockedEth(bytes32 moduleName, uint256 reductionAmount) external restricted {


I'm unsure about this oracle approach.Since non-restaked validators don't get their rewards in the withdrawal credentials automatically (unless they get to the max 2048 ETH), the rewards will be compounding in the validator in the beaconchain. That extra ETH is not represented in our protocol in any way at the moment, so I think we should add another function here that increases the locked ETH amount that would be the auto-compounding of the rewards in the beaconchain. Otherwise, the accounting is not being totally accurate. A backend service would need to check beaconchain periodically and call this function

as mentioned in natspec now, In oracle, adjustLockedEth is only for slashing/inactivity penalties, not rewards
Extra ETH (rewards) flows to module automatically, or keep compounding. They are not the part of the exchange rate ever - unless we plan to transfer the ETH to the vault. And for that we already have withdrawNonRestakedETH in PufferModuleManager that moves ETH from NRWC -> PermissionedModule.
And then we can call transferPermissionedModuleETHToVault , that moves ETH to the vault - increasing the exchange rate - which is intentional and if we plan to profit pufETH holders.
Otherwise we can add another function in PMM to utilize call() of PermissionedModule to transfer ETH to another recipient like external EOA/Multisig/Partner

i'll add another function to transfer rewards to arbitrary EOA/multisig

Ok I get it. Since the auto compounding rewards might or might not go into the vault, it shouldn't be reflected in the exchange rate, even if 32 ETH where used for a validator that currently it's at say 1k ETH in the beacon chain

yes, that's the whole point of having permissioned validator - the rewards are revenue to the protocol or partner who's provided the ETH/ running the validator. And it's their decision whether they want to provide benefit to pufETH holders or keep the revenue to themselves.

eladiosch · 2026-02-17T12:32:55Z

mainnet-contracts/src/PufferProtocol.sol

+     *      If withdrawalAmount < stakeAmount, a slashing event is emitted for transparency.
+     *      If withdrawalAmount > stakeAmount, extra is considered rewards (oracle only deducts stake).
+     */
+    function handlePermissionedValidatorExit(bytes32 moduleName, uint256 validatorIndex, uint256 withdrawalAmount)


So if I understand this correctly, this function is only to adjust the eth locked in the new oracle and remove them from the storage, right?

However this only allows to adjust for the initial stake amount (or minus penalties). However, how are compounding rewards managed in non-restaking validators?

(This is related to my comment in the oracle)

Are we going to withdraw the compounding rewards from 0x02 validators periodically, as if they were 0x01? I think the auto compounding is a very interesting part of pectra validators and maybe we should use it properly, allowing for rewards to auto compound, updating the locked eth amount in the oracle, and allowing to reduce the eth of a 0x02 validator up to 32 ETH (we would need to be careful dividing the amount to the noop and the vault).

Maybe we need to give it some thought... what do you think?

improved the natspec to describe the flow of rewards better, let me know if this is more clearer

mainnet-contracts/script/SetupAccess.s.sol

…ffer-contracts into feat/no-vt-bond

eladiosch · 2026-02-19T11:36:56Z

mainnet-contracts/script/DeployPufferProtocolImplementation.s.sol

-                beaconDepositContract: _getBeaconDepositContract()
-            })
+                beaconDepositContract: _getBeaconDepositContract(),
+                permissionedOracle: IPermissionedOracle(address(0)) // TODO: set actual address


Should add _getPermissionedOracle() function, like we do with puffer oracle

eladiosch · 2026-02-19T11:37:28Z

mainnet-contracts/script/DeployPufferVault.s.sol

-            revenueDepositor: IPufferRevenueDepositor(_getRevenueDepositor())
-        });
+            revenueDepositor: IPufferRevenueDepositor(_getRevenueDepositor()),
+            permissionedOracle: IPermissionedOracle(address(0)) // TODO: set actual address


Like mentioned in other comment, should call a _getPermissionedOracle() function

eladiosch · 2026-02-19T11:39:31Z

mainnet-contracts/script/DeployPuffer.s.sol

-                beaconDepositContract: getStakingContract()
-            });
+                beaconDepositContract: getStakingContract(),
+                permissionedOracle: IPermissionedOracle(address(0)) // Will be set in upgrade


I think permissionedOracle should be passed as a param, just like with basic oracle

eladiosch · 2026-02-19T11:56:57Z

mainnet-contracts/script/DeployPuffer.s.sol

            enclaveVerifier: guardiansDeployment.enclaveVerifier,
            beacon: address(pufferModuleBeacon),
            restakingOperatorBeacon: address(restakingOperatorBeacon),
+            permissionedModuleBeacon: address(0), // Set during permissioned module deployment


I don't think this is the right approach. These beacons (and the permissioned oracle) should be deployed here and assigned to the PufferProtocolDeployment. And the beacons should be set to the PMM, calling the setPermissionedModuleBeacon and setNRWCBeacon. Actually, I still think they should be set in the constructor, just like the PM beacon. Having to send different transactions for that seems inefficient and it can be easy to miss. I'll leave a comment in the PR about this subject

ah, i see your point now. i'll move it to the constructor. my bad

eladiosch · 2026-02-19T12:01:23Z

mainnet-contracts/script/UpgradePufETH.s.sol

            IPufferOracleV2(pufferOracle),
-            IPufferRevenueDepositor(revenueDepositor)
+            IPufferRevenueDepositor(revenueDepositor),
+            IPermissionedOracle(address(0))


I think it should be passed as a param

eladiosch · 2026-02-19T12:03:45Z

mainnet-contracts/script/SetupAccess.s.sol

        selectors[4] = PufferModuleManager.callRegisterOperatorToAVS.selector;
        selectors[5] = PufferModuleManager.callDeregisterOperatorFromAVS.selector;
        selectors[6] = PufferModuleManager.customExternalCall.selector;
+        selectors[7] = PufferModuleManager.transferPermissionedModuleETH.selector;


There are some restricted functions in PMM not reflected here

eladiosch · 2026-02-19T12:09:20Z

mainnet-contracts/src/interface/IPermissionedModule.sol

+     * @param withdrawalCredentials The NRWC contract address
+     */
+    event NonRestakingWithdrawalCredentialsSet(
+        address indexed permissionedModule, address indexed withdrawalCredentials


Isn't the permissionedModule param here the address of the contract that emits the event? We could remove it in that case since it would be redundant

eladiosch and others added 24 commits October 22, 2025 14:38

Implemented triggerValidatorExit in PM and PMM

72f8a26

Added function to exit validator from PufferProtocol, and more checks

34a1764

Reverted old fmt, improved natspec and removed role

9a6652d

Fixed codespell in dependecy

737d390

Merge commit '4eeb307f174bd83d131e5b1a49ba6c021145ae42' into feat/eip…

5ac7b07

…-7002

Fixed edge case in testing where caller is broadcaster

57c4cdc

Implemented PMM tests

2d9089b

Added access control to the new flows

957b7d1

Fixes access control, added tests for PufferProtocol

9f70b7e

Implemented extra tests

efe3eb2

Added new role for validator ejection

6aa3c9a

feat: init designing of permissioned validator

6fed931

feat: add NonRestakingWithdrawalCredentials contract

e99bdce

feat: add IPermissionedModule interface

9fd53a5

feat: add PermissionedModule contract

051c2ff

feat: add permissioned module and oracle

3ee87ed

feat: remove consolidation

9d15c5d

feat: partial withdrawals, skip provisioning and permissionedModule s…

42ac37d

…upport

fix: storage keccak

2f76042

feat: add interfaces

7267bf6

fix: compile issue due to constructor arg

6e7ce63

feat: add permissionedModule test

a09ca85

fix: oracle handling and index issue

8f65740

feat: add fork test for permissioned validator flow

4a2c8ab

ksatyarth2 requested a review from eladiosch February 4, 2026 10:56

ksatyarth2 and others added 2 commits February 4, 2026 10:59

forge fmt

bf4e747

feat: edge case fuzz test

93466dd

eladiosch reviewed Feb 17, 2026

View reviewed changes

ksatyarth2 added 2 commits February 18, 2026 14:06

feat: make NRWC upgradable

e93a79c

feat: PermissionedModule storage and beacon deploy

a941fbc

ksatyarth2 and others added 7 commits February 18, 2026 14:34

fix: improve natspec

d25ddbe

forge fmt

14323ed

fix: poc to have beacon

68e66c2

Merge branch 'feat/no-vt-bond' of https://github.com/PufferFinance/pu…

fe949d3

…ffer-contracts into feat/no-vt-bond

fix: pufferVault natspec

6ff2b08

fix: NRWC beacon deploy

f708ad8

forge fmt

b223400

ksatyarth2 requested a review from eladiosch February 18, 2026 10:56

ksatyarth2 and others added 4 commits February 18, 2026 17:34

feat: transfer rewards to the vault or recipient

7b7ad6e

Merge branch 'feat/no-vt-bond' of https://github.com/PufferFinance/pu…

aa2e98c

…ffer-contracts into feat/no-vt-bond

forge fmt

d51cf6d

fix: error names in PMM

55a8439

eladiosch reviewed Feb 19, 2026

View reviewed changes

Comments

Conversation

ksatyarth2 commented Feb 4, 2026

What this PR does / why we need it:

Which issue(s) does this PR fixes:

Additional comments:

Uh oh!

eladiosch 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!

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!

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!

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