Skip to content

Commit

Permalink
[OZ][N-11] Code Cohesion and Readability (#120)
Browse files Browse the repository at this point in the history
  • Loading branch information
@Mike-CZ
Mike-CZ authored Jan 31, 2025
1 parent 23beff7 commit 2520ec9
Show file tree
Hide file tree
Showing 3 changed files with 20 additions and 39 deletions.
20 changes: 10 additions & 10 deletions contracts/sfc/ConstantsManager.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ contract ConstantsManager is Ownable {
minSelfStake = v;
}

function updateMaxDelegatedRatio(uint256 v) external virtual onlyOwner {
function updateMaxDelegatedRatio(uint256 v) external onlyOwner {
if (v < Decimal.unit()) {
revert ValueTooSmall();
}
Expand All @@ -71,28 +71,28 @@ contract ConstantsManager is Ownable {
maxDelegatedRatio = v;
}

function updateValidatorCommission(uint256 v) external virtual onlyOwner {
function updateValidatorCommission(uint256 v) external onlyOwner {
if (v > Decimal.unit() / 2) {
revert ValueTooLarge();
}
validatorCommission = v;
}

function updateBurntFeeShare(uint256 v) external virtual onlyOwner {
function updateBurntFeeShare(uint256 v) external onlyOwner {
if (v + treasuryFeeShare > Decimal.unit()) {
revert ValueTooLarge();
}
burntFeeShare = v;
}

function updateTreasuryFeeShare(uint256 v) external virtual onlyOwner {
function updateTreasuryFeeShare(uint256 v) external onlyOwner {
if (v + burntFeeShare > Decimal.unit()) {
revert ValueTooLarge();
}
treasuryFeeShare = v;
}

function updateWithdrawalPeriodEpochs(uint256 v) external virtual onlyOwner {
function updateWithdrawalPeriodEpochs(uint256 v) external onlyOwner {
if (v < 2) {
revert ValueTooSmall();
}
Expand All @@ -102,7 +102,7 @@ contract ConstantsManager is Ownable {
withdrawalPeriodEpochs = v;
}

function updateWithdrawalPeriodTime(uint256 v) external virtual onlyOwner {
function updateWithdrawalPeriodTime(uint256 v) external onlyOwner {
if (v < 86400) {
revert ValueTooSmall();
}
Expand All @@ -129,7 +129,7 @@ contract ConstantsManager is Ownable {
offlinePenaltyThresholdTime = v;
}

function updateOfflinePenaltyThresholdBlocksNum(uint256 v) external virtual onlyOwner {
function updateOfflinePenaltyThresholdBlocksNum(uint256 v) external onlyOwner {
if (v < 100) {
revert ValueTooSmall();
}
Expand All @@ -139,7 +139,7 @@ contract ConstantsManager is Ownable {
offlinePenaltyThresholdBlocksNum = v;
}

function updateAverageUptimeEpochWindow(uint32 v) external virtual onlyOwner {
function updateAverageUptimeEpochWindow(uint32 v) external onlyOwner {
if (v < 10) {
// needs to be long enough to allow permissible downtime for validators maintenance
revert ValueTooSmall();
Expand All @@ -150,14 +150,14 @@ contract ConstantsManager is Ownable {
averageUptimeEpochWindow = v;
}

function updateMinAverageUptime(uint64 v) external virtual onlyOwner {
function updateMinAverageUptime(uint64 v) external onlyOwner {
if (v > ((Decimal.unit() * 9) / 10)) {
revert ValueTooLarge();
}
minAverageUptime = v;
}

function updateIssuedTokensRecipient(address v) external virtual onlyOwner {
function updateIssuedTokensRecipient(address v) external onlyOwner {
issuedTokensRecipient = v;
}
}
35 changes: 8 additions & 27 deletions contracts/sfc/SFC.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -162,7 +162,6 @@ contract SFC is OwnableUpgradeable, UUPSUpgradeable, Version {
// redirections
error AlreadyRedirected();
error SameRedirectionAuthorizer();
error Redirected();

// validators
error ValidatorNotExists();
Expand Down Expand Up @@ -220,12 +219,11 @@ contract SFC is OwnableUpgradeable, UUPSUpgradeable, Version {
event RestakedRewards(address indexed delegator, uint256 indexed toValidatorID, uint256 rewards);
event BurntNativeTokens(uint256 amount);
event UpdatedSlashingRefundRatio(uint256 indexed validatorID, uint256 refundRatio);
event RefundedSlashedLegacyDelegation(address indexed delegator, uint256 indexed validatorID, uint256 amount);
event AnnouncedRedirection(address indexed from, address indexed to);
event TreasuryFeesResolved(uint256 amount);

modifier onlyDriver() {
if (!_isNode(msg.sender)) {
if (!_isNodeDriverAuth(msg.sender)) {
revert NotDriverAuth();
}
_;
Expand Down Expand Up @@ -363,11 +361,8 @@ contract SFC is OwnableUpgradeable, UUPSUpgradeable, Version {
auth,
validatorID,
pubkey,
OK_STATUS,
0, // createdEpoch
createdTime,
0, // deactivatedEpoch - not deactivated
0 // deactivatedTime - not deactivated
createdTime
);
if (validatorID > lastValidatorID) {
lastValidatorID = validatorID;
Expand Down Expand Up @@ -631,7 +626,7 @@ contract SFC is OwnableUpgradeable, UUPSUpgradeable, Version {
}

/// Check if an address is the NodeDriverAuth contract.
function _isNode(address addr) internal view virtual returns (bool) {
function _isNodeDriverAuth(address addr) internal view virtual returns (bool) {
return addr == address(node);
}

Expand Down Expand Up @@ -843,11 +838,6 @@ contract SFC is OwnableUpgradeable, UUPSUpgradeable, Version {
}
}

/// Check if an address is redirected.
function _redirected(address addr) internal view returns (bool) {
return getRedirection[addr] != address(0);
}

/// Get address which should receive rewards and withdrawn stake for the given delegator.
/// The delegator is usually the receiver, unless a redirection is created.
function _receiverOf(address addr) internal view returns (address payable) {
Expand Down Expand Up @@ -1039,40 +1029,31 @@ contract SFC is OwnableUpgradeable, UUPSUpgradeable, Version {
/// Create a new validator.
function _createValidator(address auth, bytes calldata pubkey) internal {
uint256 validatorID = ++lastValidatorID;
_rawCreateValidator(auth, validatorID, pubkey, OK_STATUS, currentEpoch(), _now(), 0, 0);
_rawCreateValidator(auth, validatorID, pubkey, currentEpoch(), _now());
}

/// Create a new validator without incrementing lastValidatorID.
function _rawCreateValidator(
address auth,
uint256 validatorID,
bytes calldata pubkey,
uint256 status,
uint256 createdEpoch,
uint256 createdTime,
uint256 deactivatedEpoch,
uint256 deactivatedTime
uint256 createdTime
) internal {
if (getValidatorID[auth] != 0) {
revert ValidatorExists();
}
getValidatorID[auth] = validatorID;
getValidator[validatorID].status = status;
getValidator[validatorID].status = OK_STATUS;
getValidator[validatorID].createdEpoch = createdEpoch;
getValidator[validatorID].createdTime = createdTime;
getValidator[validatorID].deactivatedTime = deactivatedTime;
getValidator[validatorID].deactivatedEpoch = deactivatedEpoch;
getValidator[validatorID].deactivatedTime = 0;
getValidator[validatorID].deactivatedEpoch = 0;
getValidator[validatorID].auth = auth;
getValidatorPubkey[validatorID] = pubkey;
pubkeyAddressToValidatorID[_pubkeyToAddress(pubkey)] = validatorID;

emit CreatedValidator(validatorID, auth, createdEpoch, createdTime);
if (deactivatedEpoch != 0) {
emit DeactivatedValidator(validatorID, deactivatedEpoch, deactivatedTime);
}
if (status != 0) {
emit ChangedValidatorStatus(validatorID, status);
}
}

/// Calculate raw validator epoch transaction reward.
Expand Down
4 changes: 2 additions & 2 deletions contracts/test/UnitTestSFC.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -40,11 +40,11 @@ contract UnitTestSFC is SFC {
return time;
}

function _isNode(address addr) internal view override returns (bool) {
function _isNodeDriverAuth(address addr) internal view override returns (bool) {
if (allowedNonNodeCalls) {
return true;
}
return SFC._isNode(addr);
return SFC._isNodeDriverAuth(addr);
}

function syncValidator(uint256 validatorID, bool syncPubkey) public {
Expand Down

0 comments on commit 2520ec9

Please sign in to comment.