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

Require non-zero delegation fee #521

Closed
wants to merge 14 commits into from
Closed
Show file tree
Hide file tree
Changes from 10 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

Large diffs are not rendered by default.

Large diffs are not rendered by default.

7 changes: 6 additions & 1 deletion contracts/staking/ERC20TokenStakingManager.sol
Original file line number Diff line number Diff line change
Expand Up @@ -88,16 +88,21 @@ contract ERC20TokenStakingManager is
* @param stakeAmount The amount to be staked.
* @param nodeID The node ID of the validator being registered.
* @param registrationExpiry The Unix timestamp after which the reigistration is no longer valid on the P-Chain.
* @param delegationFeeRate The fee charged to delegators by the validator.
* @param blsPublicKey The BLS public key of the validator.
*/
function initializeValidatorRegistration(
uint256 stakeAmount,
bytes32 nodeID,
uint64 registrationExpiry,
uint256 delegationFeeRate,
bytes memory blsPublicKey
) external returns (bytes32 validationID) {
iansuvak marked this conversation as resolved.
Show resolved Hide resolved
uint64 weight = _processStake(stakeAmount);
return _initializeValidatorRegistration(nodeID, weight, registrationExpiry, blsPublicKey);
validationID =
_initializeValidatorRegistration(nodeID, weight, registrationExpiry, blsPublicKey);
_setDelegationFeeRate(validationID, delegationFeeRate);
return validationID;
}

/**
Expand Down
9 changes: 7 additions & 2 deletions contracts/staking/NativeTokenStakingManager.sol
Original file line number Diff line number Diff line change
Expand Up @@ -51,15 +51,20 @@ contract NativeTokenStakingManager is
* @notice Begins the validator registration process. Locks the provided native asset in the contract as the stake.
* @param nodeID The node ID of the validator being registered.
* @param registrationExpiry The Unix timestamp after which the registration is no longer valid on the P-Chain.
* @param delegationFeeRate The fee rate in basis points charged by this validator for delegations.
* @param blsPublicKey The BLS public key of the validator.
*/
function initializeValidatorRegistration(
bytes32 nodeID,
uint64 registrationExpiry,
uint256 delegationFeeRate,
bytes memory blsPublicKey
) external payable returns (bytes32) {
) external payable returns (bytes32 validationID) {
uint64 weight = _processStake(msg.value);
return _initializeValidatorRegistration(nodeID, weight, registrationExpiry, blsPublicKey);
validationID =
_initializeValidatorRegistration(nodeID, weight, registrationExpiry, blsPublicKey);
_setDelegationFeeRate(validationID, delegationFeeRate);
return validationID;
}

/**
Expand Down
23 changes: 23 additions & 0 deletions contracts/staking/PoSValidatorManager.sol
Original file line number Diff line number Diff line change
Expand Up @@ -26,8 +26,12 @@ abstract contract PoSValidatorManager is IPoSValidatorManager, ValidatorManager
uint256 _maximumStakeAmount;
/// @notice The minimum amount of time a validator must be staked for.
uint64 _minimumStakeDuration;
/// @notice The minimum fee rate in basis points charged to the delegators by the validator.
uint256 _minimumDelegationFeeRate;
/// @notice The reward calculator for this validator manager.
IRewardCalculator _rewardCalculator;
/// @notice Maps the validationID to the delegation fee rate.
mapping(bytes32 validationID => uint256) _validatorDelegationFeeRates;
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we want to track this here or in Validator? This makes it more specific to the PoS and maintains less state by not adding it to the delegation. The tradeoff that it's an additional thing that needs to be cleaned up once all delegators have exited.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I do like keeping the staking specific settings in the staking contract. We should think about if this is also the simplest solution given the PoA -> PoS upgrade potential, which I think it does? If we allow Validators to set their fee rate to 0 to disable delegation, then we could automatically get the property of PoA validators not being able to be delegated to, I think?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think that I love overloading the function of the fee rate, but this might be the cheapest/cleanest way of doing this. I will give it a try and consider alternatives.

/// @notice Maps the delegationID to the delegator information.
mapping(bytes32 delegationID => Delegator) _delegatorStakes;
/// @notice Maps the delegationID to pending register delegator messages.
Expand All @@ -42,6 +46,9 @@ abstract contract PoSValidatorManager is IPoSValidatorManager, ValidatorManager
bytes32 private constant _POS_VALIDATOR_MANAGER_STORAGE_LOCATION =
0x4317713f7ecbdddd4bc99e95d903adedaa883b2e7c2551610bd13e2c7e473d00;

// The maximum delegation fee rate in basis points. This is 99.99%
uint256 public constant MAXIMUM_DELEGATION_FEE_RATE = 1e4 - 1;

// solhint-disable ordering
function _getPoSValidatorManagerStorage()
private
Expand All @@ -64,6 +71,7 @@ abstract contract PoSValidatorManager is IPoSValidatorManager, ValidatorManager
settings.minimumStakeAmount,
settings.maximumStakeAmount,
settings.minimumStakeDuration,
settings.minimumDelegationFeeRate,
settings.rewardCalculator
);
}
Expand All @@ -73,8 +81,13 @@ abstract contract PoSValidatorManager is IPoSValidatorManager, ValidatorManager
uint256 minimumStakeAmount,
uint256 maximumStakeAmount,
uint64 minimumStakeDuration,
uint256 minimumDelegationFeeRate,
IRewardCalculator rewardCalculator
) internal onlyInitializing {
require(
minimumDelegationFeeRate > 0 && minimumDelegationFeeRate < MAXIMUM_DELEGATION_FEE_RATE,
"PoSValidatorManager: invalid minimum delegation fee rate"
);
PoSValidatorManagerStorage storage $ = _getPoSValidatorManagerStorage();
$._minimumStakeAmount = minimumStakeAmount;
$._maximumStakeAmount = maximumStakeAmount;
Expand All @@ -95,6 +108,16 @@ abstract contract PoSValidatorManager is IPoSValidatorManager, ValidatorManager
_initializeEndValidation(validationID);
}

function _setDelegationFeeRate(bytes32 validationID, uint256 feeRate) internal {
PoSValidatorManagerStorage storage $ = _getPoSValidatorManagerStorage();
require(
feeRate >= $._minimumDelegationFeeRate && feeRate <= MAXIMUM_DELEGATION_FEE_RATE,
"PoSValidatorManager: invalid delegation fee rate"
);
$._validatorDelegationFeeRates[validationID] = feeRate;
emit DelegationFeeRateSet(validationID, feeRate);
}
Comment on lines +110 to +118
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should add documentation to this function regarding when it can/should be called


function _getUptime(bytes32 validationID, uint32 messageIndex) internal view returns (uint64) {
(WarpMessage memory warpMessage, bool valid) =
WARP_MESSENGER.getVerifiedWarpMessage(messageIndex);
Expand Down
1 change: 1 addition & 0 deletions contracts/staking/ValidatorManager.sol
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,7 @@ abstract contract ValidatorManager is
/**
* @notice Begins the validator registration process, and sets the initial weight for the validator.
* @param nodeID The node ID of the validator being registered.
* @param weight The weight of the validator being registered.
* @param registrationExpiry The Unix timestamp after which the reigistration is no longer valid on the P-Chain.
* @param blsPublicKey The BLS public key of the validator.
*/
Expand Down
3 changes: 3 additions & 0 deletions contracts/staking/interfaces/IERC20TokenStakingManager.sol
Original file line number Diff line number Diff line change
Expand Up @@ -10,14 +10,17 @@ import {IPoSValidatorManager} from "./IPoSValidatorManager.sol";
interface IERC20TokenStakingManager is IPoSValidatorManager {
/**
* @notice Begins the validator registration process. Locks the {stakeAmount} of the managers specified ERC20 token.
* @param stakeAmount The amount of the ERC20 token to stake.
* @param nodeID The node ID of the validator being registered.
* @param registrationExpiry The Unix timestamp after which the reigistration is no longer valid on the P-Chain.
* @param delegationFeeRate The fee rate in basis points charged by this validator for delegations.
* @param blsPublicKey The BLS public key of the validator.
*/
function initializeValidatorRegistration(
uint256 stakeAmount,
bytes32 nodeID,
uint64 registrationExpiry,
uint256 delegationFeeRate,
bytes memory blsPublicKey
) external returns (bytes32 validationID);

Expand Down
2 changes: 2 additions & 0 deletions contracts/staking/interfaces/INativeTokenStakingManager.sol
Original file line number Diff line number Diff line change
Expand Up @@ -12,11 +12,13 @@ interface INativeTokenStakingManager is IPoSValidatorManager {
* @notice Begins the validator registration process. Locks the provided native asset in the contract as the stake.
* @param nodeID The node ID of the validator being registered.
* @param registrationExpiry The Unix timestamp after which the reigistration is no longer valid on the P-Chain.
* @param delegationFeeRate The fee rate in basis points charged by this validator for delegations.
* @param blsPublicKey The BLS public key of the validator.
*/
function initializeValidatorRegistration(
bytes32 nodeID,
uint64 registrationExpiry,
uint256 delegationFeeRate,
bytes memory blsPublicKey
) external payable returns (bytes32 validationID);

Expand Down
8 changes: 8 additions & 0 deletions contracts/staking/interfaces/IPoSValidatorManager.sol
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ struct PoSValidatorManagerSettings {
uint256 minimumStakeAmount;
uint256 maximumStakeAmount;
uint64 minimumStakeDuration;
uint256 minimumDelegationFeeRate;
IRewardCalculator rewardCalculator;
}

Expand All @@ -36,6 +37,13 @@ struct Delegator {
}

interface IPoSValidatorManager is IValidatorManager {
/**
* @notice Event emitted when a validator's delegation fee is set. This is only emitted at the start of a validation
* @param validationID The ID of the validation period
* @param delegationFeeRate Fee rate in basis points charged by this validator its delegators.
*/
event DelegationFeeRateSet(bytes32 indexed validationID, uint256 delegationFeeRate);

/**
* @notice Event emitted when a delegator registration is initiated
* @param delegationID The ID of the delegation
Expand Down
4 changes: 3 additions & 1 deletion contracts/staking/tests/ERC20TokenStakingManagerTests.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -37,10 +37,12 @@ contract ERC20TokenStakingManagerTest is PoSValidatorManagerTest {
minimumStakeAmount: DEFAULT_MINIMUM_STAKE,
maximumStakeAmount: DEFAULT_MAXIMUM_STAKE,
minimumStakeDuration: DEFAULT_MINIMUM_STAKE_DURATION,
minimumDelegationFeeRate: DEFAULT_MINIMUM_DELEGATION_FEE_RATE,
rewardCalculator: IRewardCalculator(address(0))
}),
token
);
delegationFeeRate = DEFAULT_MINIMUM_DELEGATION_FEE_RATE;
validatorManager = app;
posValidatorManager = app;
}
Expand All @@ -52,7 +54,7 @@ contract ERC20TokenStakingManagerTest is PoSValidatorManagerTest {
uint64 weight
) internal virtual override returns (bytes32) {
return app.initializeValidatorRegistration(
app.weightToValue(weight), nodeID, registrationExpiry, signature
app.weightToValue(weight), nodeID, registrationExpiry, delegationFeeRate, signature
);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ contract ExampleRewardCalculatorTest is Test {

uint256 public constant DEFAULT_STAKE_AMOUNT = 1e12;
uint64 public constant DEFAULT_START_TIME = 1000;
uint64 public constant DEFAULT_END_TIME = 31537000; // a year + 1000 seonds
uint64 public constant DEFAULT_END_TIME = 31537000; // a year + 1000 seconds
uint64 public constant DEFAULT_REWARD_BASIS_POINTS = 42;

function setUp() public {
Expand Down
4 changes: 3 additions & 1 deletion contracts/staking/tests/NativeTokenStakingManagerTests.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -30,9 +30,11 @@ contract NativeTokenStakingManagerTest is PoSValidatorManagerTest {
minimumStakeAmount: DEFAULT_MINIMUM_STAKE,
maximumStakeAmount: DEFAULT_MAXIMUM_STAKE,
minimumStakeDuration: DEFAULT_MINIMUM_STAKE_DURATION,
minimumDelegationFeeRate: DEFAULT_MINIMUM_DELEGATION_FEE_RATE,
rewardCalculator: IRewardCalculator(address(0))
})
);
delegationFeeRate = DEFAULT_MINIMUM_DELEGATION_FEE_RATE;
validatorManager = app;
posValidatorManager = app;
}
Expand All @@ -45,7 +47,7 @@ contract NativeTokenStakingManagerTest is PoSValidatorManagerTest {
uint64 weight
) internal virtual override returns (bytes32) {
return app.initializeValidatorRegistration{value: app.weightToValue(weight)}(
nodeID, registrationExpiry, signature
nodeID, registrationExpiry, delegationFeeRate, signature
);
}

Expand Down
4 changes: 4 additions & 0 deletions contracts/staking/tests/PoSValidatorManagerTests.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,10 @@ abstract contract PoSValidatorManagerTest is ValidatorManagerTest {
address public constant DEFAULT_DELEGATOR_ADDRESS =
address(0x1234123412341234123412341234123412341234);

// This is the rate that will be passed into the child contract `initializeValidatorRegistration` calls
// It is set here to avoid having to pass irrelevant initializers to the parent contract.
uint256 public delegationFeeRate;
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not set on this stateful solution being the cleanest way to pass in delegationFeeRate to child test contracts. Happy to hear alternatives.


PoSValidatorManager public posValidatorManager;

event DelegatorAdded(
Expand Down
1 change: 1 addition & 0 deletions contracts/staking/tests/ValidatorManagerTests.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ abstract contract ValidatorManagerTest is Test {
uint64 public constant DEFAULT_EXPIRY = 1000;
uint64 public constant DEFAULT_REGISTRATION_TIMESTAMP = 1000;
uint64 public constant DEFAULT_COMPLETION_TIMESTAMP = 2000;
uint256 public constant DEFAULT_MINIMUM_DELEGATION_FEE_RATE = 100; // 1%

ValidatorManager public validatorManager;

Expand Down
9 changes: 5 additions & 4 deletions tests/flows/staking/poa_to_pos.go
Original file line number Diff line number Diff line change
Expand Up @@ -151,10 +151,11 @@ func PoAMigrationToPoS(network interfaces.LocalNetwork) {
SubnetID: subnetAInfo.SubnetID,
MaximumHourlyChurn: 0,
},
MinimumStakeAmount: big.NewInt(0).SetUint64(1e6),
MaximumStakeAmount: big.NewInt(0).SetUint64(10e6),
MinimumStakeDuration: uint64(24 * time.Hour),
RewardCalculator: common.Address{},
MinimumStakeAmount: big.NewInt(0).SetUint64(1e6),
MaximumStakeAmount: big.NewInt(0).SetUint64(10e6),
MinimumStakeDuration: uint64(24 * time.Hour),
MinimumDelegationFeeRate: big.NewInt(0).SetUint64(100),
RewardCalculator: common.Address{},
},
)
Expect(err).Should(BeNil())
Expand Down
20 changes: 12 additions & 8 deletions tests/utils/staking.go
Original file line number Diff line number Diff line change
Expand Up @@ -71,10 +71,11 @@ func DeployAndInitializeNativeTokenStakingManager(
SubnetID: subnet.SubnetID,
MaximumHourlyChurn: 0,
},
MinimumStakeAmount: big.NewInt(0).SetUint64(1e6),
MaximumStakeAmount: big.NewInt(0).SetUint64(10e6),
MinimumStakeDuration: uint64(24 * time.Hour),
RewardCalculator: common.Address{},
MinimumStakeAmount: big.NewInt(0).SetUint64(1e6),
MaximumStakeAmount: big.NewInt(0).SetUint64(10e6),
MinimumStakeDuration: uint64(24 * time.Hour),
MinimumDelegationFeeRate: big.NewInt(0).SetUint64(100),
RewardCalculator: common.Address{},
},
)
Expect(err).Should(BeNil())
Expand Down Expand Up @@ -131,10 +132,11 @@ func DeployAndInitializeERC20TokenStakingManager(
SubnetID: subnet.SubnetID,
MaximumHourlyChurn: 0,
},
MinimumStakeAmount: big.NewInt(0).SetUint64(1e6),
MaximumStakeAmount: big.NewInt(0).SetUint64(10e6),
MinimumStakeDuration: uint64(24 * time.Hour),
RewardCalculator: common.Address{},
MinimumStakeAmount: big.NewInt(1e6),
MaximumStakeAmount: big.NewInt(10e6),
MinimumStakeDuration: uint64(24 * time.Hour),
MinimumDelegationFeeRate: big.NewInt(100),
RewardCalculator: common.Address{},
},
erc20Address,
)
Expand Down Expand Up @@ -213,6 +215,7 @@ func InitializeNativeValidatorRegistration(
opts,
nodeID,
uint64(time.Now().Add(24*time.Hour).Unix()),
big.NewInt(100),
blsPublicKey[:],
)
Expect(err).Should(BeNil())
Expand Down Expand Up @@ -252,6 +255,7 @@ func InitializeERC20ValidatorRegistration(
stakeAmount,
nodeID,
uint64(time.Now().Add(24*time.Hour).Unix()),
big.NewInt(100),
blsPublicKey[:],
)
Expect(err).Should(BeNil())
Expand Down