feat: slasher templates / examples #310
Conversation
| @@ -22,6 +23,8 @@ library SignatureCheckerLib { | |||
| bytes32 digestHash, | |||
| bytes memory signature | |||
| ) external view { | |||
| EIP1271SignatureUtils.checkSignature_EIP1271(signer, digestHash, signature); | |||
| if (!SignatureCheckerUpgradeable.isValidSignatureNow(signer, digestHash, signature)) { | |||
There was a problem hiding this comment.
we should use require syntax with new solc version
There was a problem hiding this comment.
yeah lets bump to 0.8.27
| @@ -491,7 +491,8 @@ contract StakeRegistry is StakeRegistryStorage { | |||
| uint256 stratsLength = strategyParamsLength(quorumNumber); | |||
| StrategyParams memory strategyAndMultiplier; | |||
|
|
|||
| uint256[] memory strategyShares = delegation.getOperatorShares(operator, strategiesPerQuorum[quorumNumber]); | |||
| uint256[] memory strategyShares; | |||
| // = delegation.getDelegatableShares(operator, strategiesPerQuorum[quorumNumber]); | |||
There was a problem hiding this comment.
The interface changed just commented out temporarily. It was initially more annoying to deal with because the type returned changed and then the function signature changed
| string memory description | ||
| ) internal virtual { | ||
|
|
||
| IAllocationManagerTypes.SlashingParams memory params = IAllocationManagerTypes.SlashingParams({ |
There was a problem hiding this comment.
have them pass in the SlashingParams
| wadToSlash, | ||
| description | ||
| ); | ||
| emit OperatorSlashed(requestId, operator, operatorSetId, strategies, wadToSlash, description); |
There was a problem hiding this comment.
the core will emit this event
| SlashingStatus status; | ||
| } | ||
|
|
||
| uint256 public constant VETO_PERIOD = 3 days; |
There was a problem hiding this comment.
constant feels fine imho. what do you view the benefit of the immutable vs constant?
|
|
||
| contract VetoableSlashing is SlasherBase { | ||
| struct SlashingRequest { | ||
| address operator; |
There was a problem hiding this comment.
abstract a SlashingParams in here
| IStrategy[] memory strategies, | ||
| uint256 wadToSlash, | ||
| string memory description | ||
| ) external virtual { |
There was a problem hiding this comment.
oh you mean access control. yeah i made this virtual so some type of access control could be added via inheritance
| IStrategy[] memory strategies, | ||
| uint256 wadToSlash, | ||
| string memory description | ||
| ) external virtual { |
| _queueSlashingRequest(operator, operatorSetId, strategies, wadToSlash, description); | ||
| } | ||
|
|
||
| function cancelSlashingRequest(uint256 requestId) external virtual onlyVetoCommittee { |
There was a problem hiding this comment.
Request, Slash, and Cancel are more generic. Cancellation is just happening through a veto committee in this case
| request.description | ||
| ); | ||
|
|
||
| emit OperatorSlashed(requestId, request.operator, request.operatorSetId, request.strategies, request.wadToSlash, request.description); |
There was a problem hiding this comment.
we should have a cohesive theory around events
There was a problem hiding this comment.
I agree; what do you view as incohesive?
There are 3 event categories.
Requested
Slashed <- This is duplicated from core so that we can have a cohesive requestId tied to all 3
Cancelled
| * @param newSlasher The new slasher address | ||
| * @dev only callable by the owner | ||
| */ | ||
| function setSlasher(address newSlasher) external onlyOwner { |
There was a problem hiding this comment.
Do we plan on changing this because this is a bit concerning how the slasher can just be updated without any significant delay or additional security measures. Not sure if we want to have something in place by default or leave it up to AVSs because there are trust assumptions that operators have done their due diligence on AVSs they are securing.
There was a problem hiding this comment.
Sure we can add a delay
|
|
||
| emit OperatorSlashed(requestId, request.operator, request.operatorSetId, request.strategies, request.wadToSlash, request.description); | ||
|
|
||
| delete slashingRequests[requestId]; |
There was a problem hiding this comment.
Lets just change SlashingStatus to Completed and leave in storage
There was a problem hiding this comment.
nice catch, i think i added the enum after writing this
No description provided.