mirror of
https://github.com/datahaven-xyz/datahaven
synced 2026-05-24 09:50:01 +00:00
fix: 🩹 map validator address improvements (#460)
## Summary This is a follow up PR of #441, adding some proper error handling and removing inconsistent checks. ## Changes ### DataHavenServiceManager.sol 1. **updateSolochainAddressForValidator()** - Removed the unclear `existingEthOperator == msg.sender` check - Removed the `oldSolochainAddress != address(0)` guard on the `delete` (since `onlyValidator` guarantees the caller is registered and thus always has a solochain address set). - Added `require(oldSolochainAddress != solochainAddress, SolochainAddressAlreadyAssigned())` to reject no-op updates. - The delete of the old reverse mapping is now unconditional (safe because `oldSolochainAddress` is always non-zero for a registered validator). 2. **registerOperator** - Added `require(validatorEthAddressToSolochainAddress[operator] == address(0), OperatorAlreadyRegistered())` to prevent re-registration. - Removed the old "update if already registered" logic (the `existingEthOperator == operator` branch and the `oldSolochainAddress` cleanup). - Simplified the solochain collision check to just `require(existingEthOperator == address(0), ...)`. 3. **deregisterOperator** - Added `require(validatorEthAddressToSolochainAddress[operator] != address(0), OperatorNotRegistered())`. - The delete of the reverse mapping is now unconditional (safe because the above check guarantees `oldSolochainAddress` is non-zero). 4. Added **OperatorAlreadyRegistered** and **OperatorNotRegistered** errors 5. Removed **_ethOperatorFromSolochain** helper — unknown solochain addresses no longer revert; they are now silently skipped via a `continue`. 6. **Rewards**: unknown operators are filtered out of the submission array 7. **Slashing**: unknown addresses in slashing requests are skipped instead of reverting ### Tests - Replaced `test_registerOperator_replacesSolochainAndClearsOldReverseMapping` (which tested the now-removed re-registration path) with `test_registerOperator_revertsIfAlreadyRegistered`. - Added `test_deregisterOperator_clearsBothMappings` — verifies both mappings are wiped on deregistration. - Added `test_deregisterOperator_revertsIfNotRegistered` — verifies the new OperatorNotRegistered guard. - Added `test_updateSolochainAddressForValidator_revertsIfSameAddress` — verifies the new same-address revert. - Replaced `revertsIfUnknownSolochainAddress` with `skipsUnknownSolochainAddress` variants that assert the skip-and-emit behavior.
This commit is contained in:
parent
1a9e213322
commit
e04ca0f6b5
8 changed files with 917 additions and 115 deletions
|
|
@ -1 +1 @@
|
|||
9c861e3e1d290888127bc6d772fb1a3422bdf8b3
|
||||
74018d5581304551932388025aebb9508b907e22
|
||||
File diff suppressed because one or more lines are too long
|
|
@ -295,16 +295,13 @@ contract DataHavenServiceManager is OwnableUpgradeable, IAVSRegistrar, IDataHave
|
|||
) external onlyValidator {
|
||||
require(solochainAddress != address(0), ZeroAddress());
|
||||
|
||||
address existingEthOperator = validatorSolochainAddressToEthAddress[solochainAddress];
|
||||
require(
|
||||
existingEthOperator == address(0) || existingEthOperator == msg.sender,
|
||||
SolochainAddressAlreadyAssigned()
|
||||
);
|
||||
|
||||
address oldSolochainAddress = validatorEthAddressToSolochainAddress[msg.sender];
|
||||
if (oldSolochainAddress != address(0) && oldSolochainAddress != solochainAddress) {
|
||||
delete validatorSolochainAddressToEthAddress[oldSolochainAddress];
|
||||
}
|
||||
require(oldSolochainAddress != solochainAddress, SolochainAddressAlreadyAssigned());
|
||||
|
||||
address existingEthOperator = validatorSolochainAddressToEthAddress[solochainAddress];
|
||||
require(existingEthOperator == address(0), SolochainAddressAlreadyAssigned());
|
||||
|
||||
delete validatorSolochainAddressToEthAddress[oldSolochainAddress];
|
||||
|
||||
validatorEthAddressToSolochainAddress[msg.sender] = solochainAddress;
|
||||
validatorSolochainAddressToEthAddress[solochainAddress] = msg.sender;
|
||||
|
|
@ -338,18 +335,16 @@ contract DataHavenServiceManager is OwnableUpgradeable, IAVSRegistrar, IDataHave
|
|||
require(operatorSetIds.length == 1, CantRegisterToMultipleOperatorSets());
|
||||
require(operatorSetIds[0] == VALIDATORS_SET_ID, InvalidOperatorSetId());
|
||||
require(validatorsAllowlist[operator], OperatorNotInAllowlist());
|
||||
|
||||
address solochainAddress = _toAddress(data);
|
||||
address existingEthOperator = validatorSolochainAddressToEthAddress[solochainAddress];
|
||||
require(
|
||||
existingEthOperator == address(0) || existingEthOperator == operator,
|
||||
SolochainAddressAlreadyAssigned()
|
||||
validatorEthAddressToSolochainAddress[operator] == address(0),
|
||||
OperatorAlreadyRegistered()
|
||||
);
|
||||
|
||||
address oldSolochainAddress = validatorEthAddressToSolochainAddress[operator];
|
||||
if (oldSolochainAddress != address(0) && oldSolochainAddress != solochainAddress) {
|
||||
delete validatorSolochainAddressToEthAddress[oldSolochainAddress];
|
||||
}
|
||||
address solochainAddress = _toAddress(data);
|
||||
require(
|
||||
validatorSolochainAddressToEthAddress[solochainAddress] == address(0),
|
||||
SolochainAddressAlreadyAssigned()
|
||||
);
|
||||
|
||||
validatorEthAddressToSolochainAddress[operator] = solochainAddress;
|
||||
validatorSolochainAddressToEthAddress[solochainAddress] = operator;
|
||||
|
|
@ -366,12 +361,13 @@ contract DataHavenServiceManager is OwnableUpgradeable, IAVSRegistrar, IDataHave
|
|||
require(avsAddress == address(this), IncorrectAVSAddress());
|
||||
require(operatorSetIds.length == 1, CantDeregisterFromMultipleOperatorSets());
|
||||
require(operatorSetIds[0] == VALIDATORS_SET_ID, InvalidOperatorSetId());
|
||||
require(
|
||||
validatorEthAddressToSolochainAddress[operator] != address(0), OperatorNotRegistered()
|
||||
);
|
||||
|
||||
address oldSolochainAddress = validatorEthAddressToSolochainAddress[operator];
|
||||
delete validatorEthAddressToSolochainAddress[operator];
|
||||
if (oldSolochainAddress != address(0)) {
|
||||
delete validatorSolochainAddressToEthAddress[oldSolochainAddress];
|
||||
}
|
||||
delete validatorSolochainAddressToEthAddress[oldSolochainAddress];
|
||||
|
||||
emit OperatorDeregistered(operator, operatorSetIds[0]);
|
||||
}
|
||||
|
|
@ -490,13 +486,33 @@ contract DataHavenServiceManager is OwnableUpgradeable, IAVSRegistrar, IDataHave
|
|||
) external override onlyRewardsInitiator {
|
||||
IRewardsCoordinatorTypes.OperatorDirectedRewardsSubmission memory translatedSubmission =
|
||||
submission;
|
||||
|
||||
uint256 len = translatedSubmission.operatorRewards.length;
|
||||
IRewardsCoordinatorTypes.OperatorReward[] memory translated =
|
||||
new IRewardsCoordinatorTypes.OperatorReward[](len);
|
||||
uint256 totalAmount = 0;
|
||||
for (uint256 i = 0; i < translatedSubmission.operatorRewards.length; i++) {
|
||||
translatedSubmission.operatorRewards[i].operator =
|
||||
_ethOperatorFromSolochain(translatedSubmission.operatorRewards[i].operator);
|
||||
totalAmount += translatedSubmission.operatorRewards[i].amount;
|
||||
uint256 resolvedCount = 0;
|
||||
for (uint256 i = 0; i < len; i++) {
|
||||
address ethOp = validatorSolochainAddressToEthAddress[
|
||||
translatedSubmission.operatorRewards[i].operator
|
||||
];
|
||||
if (ethOp == address(0)) continue;
|
||||
translated[resolvedCount] = translatedSubmission.operatorRewards[i];
|
||||
translated[resolvedCount].operator = ethOp;
|
||||
totalAmount += translated[resolvedCount].amount;
|
||||
resolvedCount++;
|
||||
}
|
||||
|
||||
// Resize to the number of successfully resolved operators
|
||||
assembly {
|
||||
mstore(translated, resolvedCount)
|
||||
}
|
||||
translatedSubmission.operatorRewards = translated;
|
||||
|
||||
emit RewardsSubmitted(totalAmount, resolvedCount);
|
||||
|
||||
if (resolvedCount == 0) return;
|
||||
|
||||
_sortOperatorRewards(translatedSubmission.operatorRewards);
|
||||
|
||||
submission.token.safeIncreaseAllowance(address(_REWARDS_COORDINATOR), totalAmount);
|
||||
|
|
@ -509,8 +525,6 @@ contract DataHavenServiceManager is OwnableUpgradeable, IAVSRegistrar, IDataHave
|
|||
_REWARDS_COORDINATOR.createOperatorDirectedOperatorSetRewardsSubmission(
|
||||
operatorSet, submissions
|
||||
);
|
||||
|
||||
emit RewardsSubmitted(totalAmount, submission.operatorRewards.length);
|
||||
}
|
||||
|
||||
/// @inheritdoc IDataHavenServiceManager
|
||||
|
|
@ -554,7 +568,8 @@ contract DataHavenServiceManager is OwnableUpgradeable, IAVSRegistrar, IDataHave
|
|||
SlashingRequest[] calldata slashings
|
||||
) external onlyRewardsInitiator {
|
||||
for (uint256 i = 0; i < slashings.length; i++) {
|
||||
address ethOperator = _ethOperatorFromSolochain(slashings[i].operator);
|
||||
address ethOperator = validatorSolochainAddressToEthAddress[slashings[i].operator];
|
||||
if (ethOperator == address(0)) continue;
|
||||
IAllocationManagerTypes.SlashingParams memory slashingParams =
|
||||
IAllocationManagerTypes.SlashingParams({
|
||||
operator: ethOperator,
|
||||
|
|
@ -627,16 +642,4 @@ contract DataHavenServiceManager is OwnableUpgradeable, IAVSRegistrar, IDataHave
|
|||
}
|
||||
return opA < opB;
|
||||
}
|
||||
|
||||
/**
|
||||
* @notice Returns the EigenLayer operator address for a Solochain validator address
|
||||
* @dev Reverts if the Solochain address has not been mapped to an operator
|
||||
*/
|
||||
function _ethOperatorFromSolochain(
|
||||
address solochainAddress
|
||||
) internal view returns (address) {
|
||||
address ethOperator = validatorSolochainAddressToEthAddress[solochainAddress];
|
||||
require(ethOperator != address(0), UnknownSolochainAddress());
|
||||
return ethOperator;
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -45,6 +45,12 @@ interface IDataHavenServiceManagerErrors {
|
|||
|
||||
/// @notice Thrown when a strategy is not registered in the operator set
|
||||
error StrategyNotInOperatorSet();
|
||||
|
||||
/// @notice Thrown when an operator attempts to register but is already registered
|
||||
error OperatorAlreadyRegistered();
|
||||
|
||||
/// @notice Thrown when an operation requires the operator to be registered but it is not
|
||||
error OperatorNotRegistered();
|
||||
}
|
||||
|
||||
/**
|
||||
|
|
|
|||
|
|
@ -123,36 +123,63 @@ contract OperatorAddressMappingsTest is AVSDeployer {
|
|||
);
|
||||
}
|
||||
|
||||
function test_registerOperator_replacesSolochainAndClearsOldReverseMapping() public {
|
||||
address soloOld = address(0xBEEF);
|
||||
address soloNew = address(0xCAFE);
|
||||
function test_registerOperator_revertsIfAlreadyRegistered() public {
|
||||
address solo1 = address(0xBEEF);
|
||||
address solo2 = address(0xCAFE);
|
||||
|
||||
_registerOperator(operator1, soloOld);
|
||||
_registerOperator(operator1, solo1);
|
||||
|
||||
// simulate allocationManager registering operator1 again with a new solochain address
|
||||
// operator1 cannot register again, even with a different solochain address
|
||||
uint32[] memory operatorSetIds = new uint32[](1);
|
||||
operatorSetIds[0] = serviceManager.VALIDATORS_SET_ID();
|
||||
|
||||
vm.prank(address(allocationManager));
|
||||
vm.expectRevert(abi.encodeWithSignature("OperatorAlreadyRegistered()"));
|
||||
serviceManager.registerOperator(
|
||||
operator1, address(serviceManager), operatorSetIds, abi.encodePacked(soloNew)
|
||||
operator1, address(serviceManager), operatorSetIds, abi.encodePacked(solo2)
|
||||
);
|
||||
}
|
||||
|
||||
function test_deregisterOperator_clearsBothMappings() public {
|
||||
address solo1 = address(0xBEEF);
|
||||
|
||||
_registerOperator(operator1, solo1);
|
||||
|
||||
uint32[] memory operatorSetIds = new uint32[](1);
|
||||
operatorSetIds[0] = serviceManager.VALIDATORS_SET_ID();
|
||||
|
||||
vm.prank(address(allocationManager));
|
||||
serviceManager.deregisterOperator(operator1, address(serviceManager), operatorSetIds);
|
||||
|
||||
assertEq(
|
||||
serviceManager.validatorEthAddressToSolochainAddress(operator1),
|
||||
soloNew,
|
||||
"forward mapping should update"
|
||||
);
|
||||
assertEq(
|
||||
serviceManager.validatorSolochainAddressToEthAddress(soloNew),
|
||||
operator1,
|
||||
"reverse mapping should update"
|
||||
);
|
||||
assertEq(
|
||||
serviceManager.validatorSolochainAddressToEthAddress(soloOld),
|
||||
address(0),
|
||||
"old reverse mapping should be cleared"
|
||||
"forward mapping should be cleared"
|
||||
);
|
||||
assertEq(
|
||||
serviceManager.validatorSolochainAddressToEthAddress(solo1),
|
||||
address(0),
|
||||
"reverse mapping should be cleared"
|
||||
);
|
||||
}
|
||||
|
||||
function test_deregisterOperator_revertsIfNotRegistered() public {
|
||||
uint32[] memory operatorSetIds = new uint32[](1);
|
||||
operatorSetIds[0] = serviceManager.VALIDATORS_SET_ID();
|
||||
|
||||
vm.prank(address(allocationManager));
|
||||
vm.expectRevert(abi.encodeWithSignature("OperatorNotRegistered()"));
|
||||
serviceManager.deregisterOperator(operator1, address(serviceManager), operatorSetIds);
|
||||
}
|
||||
|
||||
function test_updateSolochainAddressForValidator_revertsIfSameAddress() public {
|
||||
address solo1 = address(0xBEEF);
|
||||
|
||||
_registerOperator(operator1, solo1);
|
||||
|
||||
vm.prank(operator1);
|
||||
vm.expectRevert(abi.encodeWithSignature("SolochainAddressAlreadyAssigned()"));
|
||||
serviceManager.updateSolochainAddressForValidator(solo1);
|
||||
}
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -348,13 +348,15 @@ contract RewardsSubmitterTest is AVSDeployer {
|
|||
serviceManager.submitRewards(submission);
|
||||
}
|
||||
|
||||
function test_submitRewards_revertsIfUnknownSolochainAddress() public {
|
||||
function test_submitRewards_skipsUnknownSolochainAddress() public {
|
||||
address unknownSolochainOperator = address(0xDEAD);
|
||||
IRewardsCoordinatorTypes.OperatorDirectedRewardsSubmission memory submission =
|
||||
_buildSubmission(1000e18, unknownSolochainOperator);
|
||||
|
||||
// Unknown solochain address is silently skipped; RewardsSubmitted is emitted with zero amount and zero operator count
|
||||
vm.prank(snowbridgeAgent);
|
||||
vm.expectRevert(abi.encodeWithSignature("UnknownSolochainAddress()"));
|
||||
vm.expectEmit();
|
||||
emit IDataHavenServiceManagerEvents.RewardsSubmitted(0, 0);
|
||||
serviceManager.submitRewards(submission);
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -144,7 +144,7 @@ contract SlashingTest is AVSDeployer {
|
|||
serviceManager.slashValidatorsOperator(slashings);
|
||||
}
|
||||
|
||||
function test_fulfilSlashingRequest_revertsIfUnknownSolochainAddress() public {
|
||||
function test_fulfilSlashingRequest_skipsUnknownSolochainAddress() public {
|
||||
// Configure the rewards initiator (because only the reward agent can submit slashing request)
|
||||
vm.prank(avsOwner);
|
||||
serviceManager.setRewardsInitiator(snowbridgeAgent);
|
||||
|
|
@ -159,8 +159,10 @@ contract SlashingTest is AVSDeployer {
|
|||
"Testing unknown solochain operator"
|
||||
);
|
||||
|
||||
// Unknown solochain address is silently skipped; SlashingComplete is still emitted
|
||||
vm.prank(snowbridgeAgent);
|
||||
vm.expectRevert(abi.encodeWithSignature("UnknownSolochainAddress()"));
|
||||
vm.expectEmit();
|
||||
emit IDataHavenServiceManagerEvents.SlashingComplete();
|
||||
serviceManager.slashValidatorsOperator(slashings);
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -2671,7 +2671,9 @@ export const dataHavenServiceManagerAbi = [
|
|||
{ type: 'error', inputs: [], name: 'OnlyAllocationManager' },
|
||||
{ type: 'error', inputs: [], name: 'OnlyRewardsInitiator' },
|
||||
{ type: 'error', inputs: [], name: 'OnlyValidatorSetSubmitter' },
|
||||
{ type: 'error', inputs: [], name: 'OperatorAlreadyRegistered' },
|
||||
{ type: 'error', inputs: [], name: 'OperatorNotInAllowlist' },
|
||||
{ type: 'error', inputs: [], name: 'OperatorNotRegistered' },
|
||||
{ type: 'error', inputs: [], name: 'SolochainAddressAlreadyAssigned' },
|
||||
{ type: 'error', inputs: [], name: 'StrategyNotInOperatorSet' },
|
||||
{ type: 'error', inputs: [], name: 'UnknownSolochainAddress' },
|
||||
|
|
|
|||
Loading…
Reference in a new issue