-
Notifications
You must be signed in to change notification settings - Fork 34
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
feat(contracts): validator gating #1127
Changes from 2 commits
49555f7
5697ee4
2e7761c
43ee469
d961819
05d1095
9f00973
7102a78
92a1dc4
2cec1a7
374a7df
6057dd8
e89c069
4c6000c
3dcacd0
9388b24
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Large diffs are not rendered by default.
Large diffs are not rendered by default.
Large diffs are not rendered by default.
Large diffs are not rendered by default.
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,14 @@ | ||
// SPDX-License-Identifier: MIT OR Apache-2.0 | ||
pragma solidity ^0.8.23; | ||
|
||
import {SubnetID} from "../structs/Subnet.sol"; | ||
|
||
/// @title Validator Gater interface | ||
/// This interface introduces the ability to intercept validator power updates before it's executed. Power updates could | ||
/// come from staking, unstaking, and explicit validator membership adjustments (federated membership). With this interface, | ||
/// it introduces an extra layer of checking to directly allow or deny the action, according to a user-defined policy. | ||
interface IValidatorGater { | ||
/// This intercepts the power update call. | ||
/// @notice This method should revert if the power update is not allowed. | ||
function interceptPowerDelta(SubnetID memory id, address validator, uint256 prevPower, uint256 newPower) external; | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -5,14 +5,17 @@ import {VALIDATOR_SECP256K1_PUBLIC_KEY_LENGTH} from "../constants/Constants.sol" | |
import {ERR_PERMISSIONED_AND_BOOTSTRAPPED} from "../errors/IPCErrors.sol"; | ||
import {NotEnoughGenesisValidators, DuplicatedGenesisValidator, NotOwnerOfPublicKey, MethodNotAllowed} from "../errors/IPCErrors.sol"; | ||
import {IGateway} from "../interfaces/IGateway.sol"; | ||
import {Validator, ValidatorSet, PermissionMode} from "../structs/Subnet.sol"; | ||
import {IValidatorGater} from "../interfaces/IValidatorGater.sol"; | ||
import {Validator, ValidatorSet, PermissionMode, SubnetID} from "../structs/Subnet.sol"; | ||
import {SubnetActorModifiers} from "../lib/LibSubnetActorStorage.sol"; | ||
import {LibValidatorSet, LibStaking} from "../lib/LibStaking.sol"; | ||
import {EnumerableSet} from "@openzeppelin/contracts/utils/structs/EnumerableSet.sol"; | ||
import {LibSubnetActorStorage, SubnetActorStorage} from "./LibSubnetActorStorage.sol"; | ||
import {SubnetIDHelper} from "../lib/SubnetIDHelper.sol"; | ||
|
||
library LibSubnetActor { | ||
using EnumerableSet for EnumerableSet.AddressSet; | ||
using SubnetIDHelper for SubnetID; | ||
|
||
event SubnetBootstrapped(Validator[]); | ||
|
||
|
@@ -38,6 +41,39 @@ library LibSubnetActor { | |
return; | ||
} | ||
|
||
/// @notice Performs validator gating, i.e. checks if the validator power update is actually allowed. | ||
function validatorGating(address validator, uint256 powerDelta, bool isIncrease) internal { | ||
cryptoAtwill marked this conversation as resolved.
Show resolved
Hide resolved
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do we absolutely need these two variants? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I have updated slightly to accommodate different usages. |
||
SubnetActorStorage storage s = LibSubnetActorStorage.appStorage(); | ||
|
||
// zero address means no gating needed | ||
if (s.validatorGater == address(0)) { | ||
return; | ||
} | ||
|
||
uint256 oldPower = LibStaking.getPower(validator); | ||
uint256 newPower = 0; | ||
if (isIncrease) { | ||
newPower = oldPower + powerDelta; | ||
} else { | ||
newPower = oldPower - powerDelta; | ||
} | ||
|
||
SubnetID memory id = s.parentId.createSubnetId(address(this)); | ||
IValidatorGater(s.validatorGater).interceptPowerDelta(id, validator, oldPower, newPower); | ||
} | ||
|
||
/// @notice Performs validator gating, i.e. checks if the validator power update is actually allowed. | ||
function validatorGating(SubnetID memory id, address validator, uint256 prevPower, uint256 newPower) internal { | ||
cryptoAtwill marked this conversation as resolved.
Show resolved
Hide resolved
|
||
SubnetActorStorage storage s = LibSubnetActorStorage.appStorage(); | ||
|
||
// zero address means no gating needed | ||
if (s.validatorGater == address(0)) { | ||
return; | ||
} | ||
|
||
IValidatorGater(s.validatorGater).interceptPowerDelta(id, validator, prevPower, newPower); | ||
} | ||
|
||
/// @dev This function is used to bootstrap the subnet, | ||
/// if its total collateral is greater than minimum activation collateral. | ||
function bootstrapSubnetIfNeeded() internal { | ||
|
@@ -76,6 +112,7 @@ library LibSubnetActor { | |
uint256[] calldata powers | ||
) internal { | ||
SubnetActorStorage storage s = LibSubnetActorStorage.appStorage(); | ||
SubnetID memory subnet = s.parentId.createSubnetId(address(this)); | ||
|
||
uint256 length = validators.length; | ||
|
||
|
@@ -96,6 +133,7 @@ library LibSubnetActor { | |
revert DuplicatedGenesisValidator(); | ||
} | ||
|
||
LibSubnetActor.validatorGating(subnet, validators[i], 0, powers[i]); | ||
LibStaking.setMetadataWithConfirm(validators[i], publicKeys[i]); | ||
LibStaking.setFederatedPowerWithConfirm(validators[i], powers[i]); | ||
|
||
|
@@ -124,13 +162,24 @@ library LibSubnetActor { | |
uint256[] calldata powers | ||
) internal { | ||
uint256 length = validators.length; | ||
|
||
SubnetActorStorage storage s = LibSubnetActorStorage.appStorage(); | ||
SubnetID memory subnet = s.parentId.createSubnetId(address(this)); | ||
|
||
for (uint256 i; i < length; ) { | ||
// check addresses | ||
address convertedAddress = publicKeyToAddress(publicKeys[i]); | ||
if (convertedAddress != validators[i]) { | ||
revert NotOwnerOfPublicKey(); | ||
} | ||
|
||
LibSubnetActor.validatorGating( | ||
subnet, | ||
validators[i], | ||
LibStaking.getPower(validators[i]), | ||
powers[i] | ||
); | ||
|
||
// no need to do deduplication as set directly set the power, there wont be any addition of | ||
// federated power. | ||
LibStaking.setFederatedPower({validator: validators[i], metadata: publicKeys[i], amount: powers[i]}); | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think this contract should be in an examples directory. |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,51 @@ | ||
// SPDX-License-Identifier: MIT OR Apache-2.0 | ||
pragma solidity ^0.8.23; | ||
|
||
import {IValidatorGater} from "../interfaces/IValidatorGater.sol"; | ||
import {InvalidSubnet, NotAuthorized, PowerChangeRequestNotApproved} from "../errors/IPCErrors.sol"; | ||
import {SubnetID} from "../structs/Subnet.sol"; | ||
import {SubnetIDHelper} from "../lib/SubnetIDHelper.sol"; | ||
import {Ownable} from "@openzeppelin/contracts/access/Ownable.sol"; | ||
|
||
/// This is a simple implementation of `IValidatorGater`. It makes sure the exact power change | ||
/// request is approved. This is a very strict requirement. | ||
contract SubnetValidatorGater is IValidatorGater, Ownable { | ||
using SubnetIDHelper for SubnetID; | ||
|
||
address public caller; | ||
SubnetID public subnet; | ||
|
||
mapping(bytes32 => bool) public allowed; | ||
|
||
constructor(address _caller, SubnetID memory _subnet) Ownable(msg.sender) { | ||
caller = _caller; | ||
subnet = _subnet; | ||
} | ||
|
||
/// Only owner can approve the validator join request | ||
function approve(address validator, uint256 prevPower, uint256 newPower) external onlyOwner { | ||
allowed[genKey(validator, prevPower, newPower)] = true; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think it would be generally more useful if in this standard implementation, you approve validators to hold a power between a min and a max, instead of explicitly approving every delta. Note that in this implementation, the allowed map can only grow infinitely. With my proposal, it would be a static configuration. If you agree, make sure you handle deletions (e.g. when min and max is 0 for a validator). There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah, I think the current one might be too strict. |
||
} | ||
|
||
function interceptPowerDelta(SubnetID memory id, address validator, uint256 prevPower, uint256 newPower) external override { | ||
if (msg.sender != caller) { | ||
revert NotAuthorized(msg.sender); | ||
} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We should be having a helper method to verify that a given caller matches the subnet actor given a subnet ID. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Would we need to store the caller if we had that method? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. good call, because the subnet actor is the only one that should be calling it. |
||
|
||
if (id.equals(subnet)) { | ||
revert InvalidSubnet(); | ||
} | ||
|
||
bytes32 key = genKey(validator, prevPower, newPower); | ||
if (!allowed[key]) { | ||
revert PowerChangeRequestNotApproved(); | ||
} | ||
|
||
// remove the approved request | ||
delete allowed[key]; | ||
} | ||
|
||
function genKey(address validator, uint256 prevPower, uint256 newPower) internal pure returns(bytes32) { | ||
return keccak256(abi.encode(validator, prevPower, newPower)); | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.