Skip to content

[OZ][N-11] Code Cohesion and Readability #120

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.

Sign up for GitHub

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

Merged
Mike-CZ merged 6 commits into from
Jan 31, 2025
Merged

[OZ][N-11] Code Cohesion and Readability #120

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
cf70ffc
Improve code cohesion and readability
Mike-CZ Dec 13, 2024
48b8ceb
Merge branch 'main' into mike/oz-n-11
Mike-CZ Jan 31, 2025
401fcab
Remove unused events
Mike-CZ Jan 31, 2025
cba9a84
Remove unused method
Mike-CZ Jan 31, 2025
d6467ca
Merge branch 'main' into mike/oz-n-11
Mike-CZ Jan 31, 2025
395cb5a
Remove unnecessary virtual methods
Mike-CZ Jan 31, 2025
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
20 changes: 10 additions & 10 deletions contracts/sfc/ConstantsManager.sol
View file
Open in desktop
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
Open in desktop
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
Open in desktop
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