Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

reward address change and unit tests #204

Merged
merged 6 commits into from
Sep 21, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
45 changes: 38 additions & 7 deletions contracts/PermissionedNodeRegistry.sol
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@ contract PermissionedNodeRegistry is
//mapping of operator Id and nextQueuedValidatorIndex
mapping(uint256 => uint256) public override nextQueuedValidatorIndexByOperatorId;
mapping(uint256 => uint256) public socializingPoolStateChangeBlock;
mapping(uint256 => address) public proposedRewardAddressByOperatorId;

/// @custom:oz-upgrades-unsafe-allow constructor
constructor() {
Expand Down Expand Up @@ -390,19 +391,49 @@ contract PermissionedNodeRegistry is
}

/**
* @notice update the name and reward address of an operator
* @notice propose the new reward address of an operator
* @dev only the existing reward address (msg.sender) can propose
* @param _operatorAddress operator address
* @param _newRewardAddress new reward address
*/
function proposeRewardAddress(address _operatorAddress, address _newRewardAddress) external override {
UtilLib.checkNonZeroAddress(_newRewardAddress);
uint256 _operatorId = operatorIDByAddress[_operatorAddress];
sanjay-staderlabs marked this conversation as resolved.
Show resolved Hide resolved
address existingRewardAddress = operatorStructById[_operatorId].operatorRewardAddress;
if (msg.sender != existingRewardAddress) {
revert CallerNotExistingRewardAddress();
}
proposedRewardAddressByOperatorId[_operatorId] = _newRewardAddress;
emit RewardAddressProposed(_operatorAddress, _newRewardAddress);
}

/**
* @notice confirms and sets the new reward address of an operator
* @dev only the new reward address (msg.sender) can confirm
* @param _operatorAddress operator address
*/
function confirmRewardAddressChange(address _operatorAddress) external override {
uint256 _operatorId = operatorIDByAddress[_operatorAddress];
sanjay-staderlabs marked this conversation as resolved.
Show resolved Hide resolved
if (msg.sender != proposedRewardAddressByOperatorId[_operatorId]) {
revert CallerNotNewRewardAddress();
}
delete proposedRewardAddressByOperatorId[_operatorId];

operatorStructById[_operatorId].operatorRewardAddress = payable(msg.sender);
emit OperatorRewardAddressUpdated(_operatorAddress, msg.sender);
}

/**
* @notice update the name of an operator
* @dev only operator msg.sender can update
* @param _operatorName new Name of the operator
* @param _rewardAddress new reward address
* @param _operatorName new name of the operator
*/
function updateOperatorDetails(string calldata _operatorName, address payable _rewardAddress) external override {
function updateOperatorName(string calldata _operatorName) external override {
IPoolUtils(staderConfig.getPoolUtils()).onlyValidName(_operatorName);
UtilLib.checkNonZeroAddress(_rewardAddress);
onlyActiveOperator(msg.sender);
uint256 operatorId = operatorIDByAddress[msg.sender];
operatorStructById[operatorId].operatorName = _operatorName;
operatorStructById[operatorId].operatorRewardAddress = _rewardAddress;
emit UpdatedOperatorDetails(msg.sender, _operatorName, _rewardAddress);
emit UpdatedOperatorName(msg.sender, _operatorName);
}

/**
Expand Down
45 changes: 38 additions & 7 deletions contracts/PermissionlessNodeRegistry.sol
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,7 @@ contract PermissionlessNodeRegistry is
mapping(uint256 => uint256) public socializingPoolStateChangeBlock;
//mapping of operator address with nodeELReward vault address
mapping(uint256 => address) public override nodeELRewardVaultByOperatorId;
mapping(uint256 => address) public proposedRewardAddressByOperatorId;

/// @custom:oz-upgrades-unsafe-allow constructor
constructor() {
Expand Down Expand Up @@ -358,19 +359,49 @@ contract PermissionlessNodeRegistry is
}

/**
* @notice update the name and reward address of an operator
* @dev only node operator can update
* @notice propose the new reward address of an operator
* @dev only the existing reward address (msg.sender) can propose
* @param _operatorAddress operator address
* @param _newRewardAddress new reward address
*/
function proposeRewardAddress(address _operatorAddress, address _newRewardAddress) external override {
UtilLib.checkNonZeroAddress(_newRewardAddress);
uint256 _operatorId = operatorIDByAddress[_operatorAddress];
manoj9april marked this conversation as resolved.
Show resolved Hide resolved
address existingRewardAddress = operatorStructById[_operatorId].operatorRewardAddress;
if (msg.sender != existingRewardAddress) {
revert CallerNotExistingRewardAddress();
}
proposedRewardAddressByOperatorId[_operatorId] = _newRewardAddress;
emit RewardAddressProposed(_operatorAddress, _newRewardAddress);
}

/**
* @notice confirms and sets the new reward address of an operator
* @dev only the new reward address (msg.sender) can confirm
* @param _operatorAddress operator address
*/
function confirmRewardAddressChange(address _operatorAddress) external override {
uint256 _operatorId = operatorIDByAddress[_operatorAddress];
if (msg.sender != proposedRewardAddressByOperatorId[_operatorId]) {
revert CallerNotNewRewardAddress();
}
delete proposedRewardAddressByOperatorId[_operatorId];

operatorStructById[_operatorId].operatorRewardAddress = payable(msg.sender);
emit OperatorRewardAddressUpdated(_operatorAddress, msg.sender);
}

/**
* @notice update the name of an operator
* @dev only operator msg.sender can update
* @param _operatorName new name of the operator
* @param _rewardAddress new reward address
*/
function updateOperatorDetails(string calldata _operatorName, address payable _rewardAddress) external override {
function updateOperatorName(string calldata _operatorName) external override {
manoj9april marked this conversation as resolved.
Show resolved Hide resolved
IPoolUtils(staderConfig.getPoolUtils()).onlyValidName(_operatorName);
UtilLib.checkNonZeroAddress(_rewardAddress);
onlyActiveOperator(msg.sender);
uint256 operatorId = operatorIDByAddress[msg.sender];
operatorStructById[operatorId].operatorName = _operatorName;
operatorStructById[operatorId].operatorRewardAddress = _rewardAddress;
emit UpdatedOperatorDetails(msg.sender, _operatorName, _rewardAddress);
emit UpdatedOperatorName(msg.sender, _operatorName);
}

/**
Expand Down
6 changes: 5 additions & 1 deletion contracts/interfaces/INodeRegistry.sol
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,8 @@ interface INodeRegistry {
error NotEnoughSDCollateral();
error TooManyVerifiedKeysReported();
error TooManyWithdrawnKeysReported();
error CallerNotExistingRewardAddress();
error CallerNotNewRewardAddress();

// Events
event AddedValidatorKey(address indexed nodeOperator, bytes pubkey, uint256 validatorId);
Expand All @@ -49,7 +51,9 @@ interface INodeRegistry {
event UpdatedMaxNonTerminalKeyPerOperator(uint64 maxNonTerminalKeyPerOperator);
event UpdatedInputKeyCountLimit(uint256 batchKeyDepositLimit);
event UpdatedStaderConfig(address staderConfig);
event UpdatedOperatorDetails(address indexed nodeOperator, string operatorName, address rewardAddress);
event RewardAddressProposed(address indexed nodeOperator, address indexed rewardAddress);
event OperatorRewardAddressUpdated(address indexed nodeOperator, address indexed rewardAddress);
event UpdatedOperatorName(address indexed nodeOperator, string operatorName);
event IncreasedTotalActiveValidatorCount(uint256 totalActiveValidatorCount);
event UpdatedVerifiedKeyBatchSize(uint256 verifiedKeysBatchSize);
event UpdatedWithdrawnKeyBatchSize(uint256 withdrawnKeysBatchSize);
Expand Down
6 changes: 5 additions & 1 deletion contracts/interfaces/IPermissionedNodeRegistry.sol
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,11 @@ interface IPermissionedNodeRegistry {

function updateInputKeyCountLimit(uint16 _inputKeyCountLimit) external;

function updateOperatorDetails(string calldata _operatorName, address payable _rewardAddress) external;
function proposeRewardAddress(address _operatorAddress, address _newRewardAddress) external;

function confirmRewardAddressChange(address _operatorAddress) external;

function updateOperatorName(string calldata _operatorName) external;

function pause() external;

Expand Down
6 changes: 5 additions & 1 deletion contracts/interfaces/IPermissionlessNodeRegistry.sol
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,11 @@ interface IPermissionlessNodeRegistry {

function updateMaxNonTerminalKeyPerOperator(uint64 _maxNonTerminalKeyPerOperator) external;

function updateOperatorDetails(string calldata _operatorName, address payable _rewardAddress) external;
function proposeRewardAddress(address _operatorAddress, address _newRewardAddress) external;

function confirmRewardAddressChange(address _operatorAddress) external;

function updateOperatorName(string calldata _operatorName) external;

function changeSocializingPoolState(bool _optInForSocializingPool)
external
Expand Down
112 changes: 74 additions & 38 deletions test/foundry_tests/PermissionedNodeRegistry.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -454,76 +454,112 @@ contract PermissionedNodeRegistryTest is Test {
assertEq(address(nodeRegistry.staderConfig()), newStaderConfig);
}

function test_updateOperatorDetails(
string calldata _operatorName,
uint64 __opAddrSeed,
uint64 _opRewardAddrSeed,
uint64 _newOPRewardAddrSeed
) public {
function test_updateOperatorRewardAddress(string calldata _operatorName, uint64 __opAddrSeed) public {
vm.assume(bytes(_operatorName).length > 0 && bytes(_operatorName).length < 255);
vm.assume(__opAddrSeed > 0);
vm.assume(_opRewardAddrSeed > 0);
vm.assume(_newOPRewardAddrSeed > 0);
address operatorAddr = vm.addr(__opAddrSeed);
address payable opRewardAddr = payable(vm.addr(_opRewardAddrSeed));
address payable newOPRewardAddr = payable(vm.addr(_newOPRewardAddrSeed));
address payable opRewardAddr = payable(vm.addr(456));
address payable newOPRewardAddr = payable(vm.addr(567));
whitelistOperator(operatorAddr);
vm.startPrank(operatorAddr);

vm.prank(operatorAddr);
nodeRegistry.onboardNodeOperator(_operatorName, opRewardAddr);

uint256 operatorId = nodeRegistry.operatorIDByAddress(operatorAddr);
string memory newOpName = string(abi.encodePacked(_operatorName, 'test'));
nodeRegistry.updateOperatorDetails(newOpName, newOPRewardAddr);
(, , string memory operatorName, address payable operatorRewardAddress, ) = nodeRegistry.operatorStructById(
operatorId
);
assertEq(operatorName, newOpName);

// propose new reward addr
vm.expectRevert(INodeRegistry.CallerNotExistingRewardAddress.selector);
vm.prank(operatorAddr);
nodeRegistry.proposeRewardAddress(operatorAddr, newOPRewardAddr);

// passed wrong new reward address by mistake
vm.prank(opRewardAddr);
nodeRegistry.proposeRewardAddress(operatorAddr, vm.addr(666));

vm.prank(opRewardAddr);
nodeRegistry.proposeRewardAddress(operatorAddr, newOPRewardAddr);

address pendingRewardAddress = nodeRegistry.proposedRewardAddressByOperatorId(operatorId);
assertEq(pendingRewardAddress, newOPRewardAddr);

// confirm new reward address
vm.expectRevert(INodeRegistry.CallerNotNewRewardAddress.selector);
vm.prank(opRewardAddr);
nodeRegistry.confirmRewardAddressChange(operatorAddr);

vm.expectRevert(INodeRegistry.CallerNotNewRewardAddress.selector);
vm.prank(operatorAddr);
nodeRegistry.confirmRewardAddressChange(operatorAddr);

vm.prank(newOPRewardAddr);
nodeRegistry.confirmRewardAddressChange(operatorAddr);

(, , , address payable operatorRewardAddress, ) = nodeRegistry.operatorStructById(operatorId);
assertEq(operatorRewardAddress, newOPRewardAddr);
vm.stopPrank();
}

function test_updateOperatorDetailWithInActiveOperator(string calldata _operatorName, uint64 _opRewardAddrSeed)
function test_updateOperatorRewardAddressWithInvalidOperatorAddress() public {
address operatorAddr = vm.addr(778);
address payable opRewardAddr = payable(vm.addr(456));
address payable newOPRewardAddr = payable(vm.addr(567));

vm.expectRevert(INodeRegistry.CallerNotExistingRewardAddress.selector);
vm.prank(opRewardAddr);
nodeRegistry.proposeRewardAddress(operatorAddr, newOPRewardAddr);

// it will pass if caller is address(0), which is not possible
vm.prank(address(0));
nodeRegistry.proposeRewardAddress(operatorAddr, newOPRewardAddr);
}

function test_updateOperatorRewardAddressWithZeroRewardAddr(string calldata _operatorName, uint64 __opAddrSeed)
public
{
vm.assume(bytes(_operatorName).length > 0 && bytes(_operatorName).length < 255);
vm.assume(_opRewardAddrSeed > 0);
address payable opRewardAddr = payable(vm.addr(_opRewardAddrSeed));
string memory newOpName = string(abi.encodePacked(_operatorName, 'test'));
vm.expectRevert(INodeRegistry.OperatorNotOnBoarded.selector);
nodeRegistry.updateOperatorDetails(newOpName, payable(opRewardAddr));
vm.assume(__opAddrSeed > 0);
address operatorAddr = vm.addr(__opAddrSeed);
whitelistOperator(operatorAddr);
vm.prank(operatorAddr);
nodeRegistry.onboardNodeOperator(_operatorName, payable(operatorAddr));

vm.expectRevert(UtilLib.ZeroAddress.selector);
vm.prank(operatorAddr);
nodeRegistry.proposeRewardAddress(operatorAddr, payable(address(0)));
}

function test_updateOperatorDetailWithZeroRewardAddr(string calldata _operatorName, uint64 __opAddrSeed) public {
function test_updateOperatorName(string calldata _operatorName, uint64 __opAddrSeed) public {
vm.assume(bytes(_operatorName).length > 0 && bytes(_operatorName).length < 255);
vm.assume(__opAddrSeed > 0);
address operatorAddr = vm.addr(__opAddrSeed);
whitelistOperator(operatorAddr);
vm.startPrank(operatorAddr);
nodeRegistry.onboardNodeOperator(_operatorName, payable(operatorAddr));
uint256 operatorId = nodeRegistry.operatorIDByAddress(operatorAddr);
string memory newOpName = string(abi.encodePacked(_operatorName, 'test'));
vm.expectRevert(UtilLib.ZeroAddress.selector);
nodeRegistry.updateOperatorDetails(newOpName, payable(address(0)));
nodeRegistry.updateOperatorName(newOpName);
(, , string memory operatorName, , ) = nodeRegistry.operatorStructById(operatorId);
assertEq(operatorName, newOpName);
vm.stopPrank();
}

function test_updateOperatorDetailWithInvalidName(
string calldata _operatorName,
uint64 __opAddrSeed,
uint64 _opRewardAddrSeed,
uint64 _newOPRewardAddrSeed
) public {
function test_updateOperatorNameWithInActiveOperator(string calldata _operatorName) public {
vm.assume(bytes(_operatorName).length > 0 && bytes(_operatorName).length < 255);
string memory newOpName = string(abi.encodePacked(_operatorName, 'test'));
vm.expectRevert(INodeRegistry.OperatorNotOnBoarded.selector);
nodeRegistry.updateOperatorName(newOpName);
}

function test_updateOperatorNameWithInvalidName(string calldata _operatorName, uint64 __opAddrSeed) public {
vm.assume(bytes(_operatorName).length > 0 && bytes(_operatorName).length < 255);
vm.assume(__opAddrSeed > 0);
vm.assume(_opRewardAddrSeed > 0);
vm.assume(_newOPRewardAddrSeed > 0);
address operatorAddr = vm.addr(__opAddrSeed);
address payable opRewardAddr = payable(vm.addr(_opRewardAddrSeed));
address payable newOPRewardAddr = payable(vm.addr(_newOPRewardAddrSeed));
whitelistOperator(operatorAddr);
vm.startPrank(operatorAddr);
nodeRegistry.onboardNodeOperator(_operatorName, opRewardAddr);
nodeRegistry.onboardNodeOperator(_operatorName, payable(operatorAddr));
string memory newOpName = string(abi.encodePacked(''));
vm.expectRevert(PoolUtilsMockForDepositFlow.EmptyNameString.selector);
nodeRegistry.updateOperatorDetails(newOpName, newOPRewardAddr);
nodeRegistry.updateOperatorName(newOpName);
}

function test_increaseTotalActiveValidatorCount(uint256 _count) public {
Expand Down
Loading
Loading