diff --git a/contracts/Errors.sol b/contracts/Errors.sol new file mode 100644 index 0000000..7c40b2c --- /dev/null +++ b/contracts/Errors.sol @@ -0,0 +1,23 @@ +// SPDX-License-Identifier: UNLICENSED +pragma solidity ^0.8.9; + +interface ISFCErrors { + error TransfersNotAllowed(); + + // pubkeys + error PubkeyExists(); + error NotMainNet(); + error MalformedPubkey(); + error NotLegacyValidator(); + error ValidatorDoesNotExist(); + error SamePubkey(); + error PubkeyAllowedOnlyOnce(); + + // redirections + error SameRedirectionAuthorizer(); + error NotAuthorized(); + error AlreadyCompleted(); + error SameAddress(); + error ZeroAddress(); + error NoRequest(); +} \ No newline at end of file diff --git a/contracts/ownership/Ownable.sol b/contracts/ownership/Ownable.sol index 65537ea..2efd354 100644 --- a/contracts/ownership/Ownable.sol +++ b/contracts/ownership/Ownable.sol @@ -2,6 +2,8 @@ pragma solidity ^0.8.9; import {Initializable} from "../common/Initializable.sol"; +import {ZeroAddress} from "../Errors.sol"; + /** * @dev Contract module which provides a basic access control mechanism, where * there is an account (an owner) that can be granted exclusive access to @@ -19,11 +21,6 @@ contract Ownable is Initializable { */ error NotOwner(); - /** - * @dev Given zero address. - */ - error ZeroAddress(); - event OwnershipTransferred(address indexed previousOwner, address indexed newOwner); /** diff --git a/contracts/sfc/SFC.sol b/contracts/sfc/SFC.sol index a063d56..51a5465 100644 --- a/contracts/sfc/SFC.sol +++ b/contracts/sfc/SFC.sol @@ -40,12 +40,14 @@ contract SFC is SFCBase, Version { // solhint-disable-next-line no-complex-fallback fallback() external payable { - require(msg.data.length != 0, "transfers not allowed"); + if (msg.data.length == 0) { + revert TransfersNotAllowed(); + } _delegate(libAddress); } receive() external payable { - revert("transfers not allowed"); + revert TransfersNotAllowed(); } /* @@ -131,24 +133,38 @@ contract SFC is SFCBase, Version { for (uint256 vid = start; vid < end; vid++) { bytes memory pubkey = getValidatorPubkey[vid]; if (pubkey.length > 0 && pubkeyHashToValidatorID[keccak256(pubkey)] != vid) { - require(pubkeyHashToValidatorID[keccak256(pubkey)] == 0, "already exists"); + if (pubkeyHashToValidatorID[keccak256(pubkey)] != 0) { + revert PubkeyExists(); + } pubkeyHashToValidatorID[keccak256(pubkey)] = vid; } } } function updateValidatorPubkey(bytes calldata pubkey) external { - require(getValidator[1].auth == 0x541E408443A592C38e01Bed0cB31f9De8c1322d0, "not mainnet"); - require(pubkey.length == 66 && pubkey[0] == 0xc0, "malformed pubkey"); + // TODO: Valid requirements? + if (getValidator[1].auth != 0x541E408443A592C38e01Bed0cB31f9De8c1322d0) { + revert NotMainNet(); + } + if (pubkey.length != 66 || pubkey[0] != 0xc0) { + revert MalformedPubkey(); + } uint256 validatorID = getValidatorID[msg.sender]; - require(validatorID <= 59 || validatorID == 64, "not legacy validator"); - require(_validatorExists(validatorID), "validator doesn't exist"); - require(keccak256(pubkey) != keccak256(getValidatorPubkey[validatorID]), "same pubkey"); - require(pubkeyHashToValidatorID[keccak256(pubkey)] == 0, "already used"); - require( - validatorPubkeyChanges[validatorID] == 0 || validatorID == 64 || validatorID <= 12, - "allowed only once" - ); + if (validatorID > 59 && validatorID != 64) { + revert NotLegacyValidator(); + } + if (!_validatorExists(validatorID)) { + revert ValidatorDoesNotExist(); + } + if (keccak256(pubkey) == keccak256(getValidatorPubkey[validatorID])) { + revert SamePubkey(); + } + if (pubkeyHashToValidatorID[keccak256(pubkey)] != 0) { + revert PubkeyExists(); + } + if (validatorPubkeyChanges[validatorID] != 0 && validatorID != 64 && validatorID > 12) { + revert PubkeyAllowedOnlyOnce(); + } validatorPubkeyChanges[validatorID]++; pubkeyHashToValidatorID[keccak256(pubkey)] = validatorID; @@ -157,7 +173,9 @@ contract SFC is SFCBase, Version { } function setRedirectionAuthorizer(address v) external onlyOwner { - require(redirectionAuthorizer != v, "same"); + if (redirectionAuthorizer == v) { + revert SameRedirectionAuthorizer(); + } redirectionAuthorizer = v; } @@ -168,16 +186,26 @@ contract SFC is SFCBase, Version { } function initiateRedirection(address from, address to) external { - require(msg.sender == redirectionAuthorizer, "not authorized"); - require(getRedirection[from] != to, "already complete"); - require(from != to, "same address"); + if (msg.sender != redirectionAuthorizer) { + revert NotAuthorized(); + } + if (getRedirection[from] == to) { + revert AlreadyCompleted(); + } + if (from == to) { + revert SameAddress(); + } getRedirectionRequest[from] = to; } function redirect(address to) external { address from = msg.sender; - require(to != address(0), "zero address"); - require(getRedirectionRequest[from] == to, "no request"); + if (to == address(0)) { + revert ZeroAddress(); + } + if (getRedirectionRequest[from] != to) { + revert NoRequest(); + } getRedirection[from] = to; getRedirectionRequest[from] = address(0); } diff --git a/contracts/sfc/SFCBase.sol b/contracts/sfc/SFCBase.sol index d76acf1..0c4ada1 100644 --- a/contracts/sfc/SFCBase.sol +++ b/contracts/sfc/SFCBase.sol @@ -3,6 +3,7 @@ pragma solidity ^0.8.9; import {Decimal} from "../common/Decimal.sol"; import {SFCState} from "./SFCState.sol"; +import "../Errors.sol"; contract SFCBase is SFCState { uint256 internal constant OK_STATUS = 0; diff --git a/contracts/test/UnitTestSFC.sol b/contracts/test/UnitTestSFC.sol index e2306b4..0f15f35 100644 --- a/contracts/test/UnitTestSFC.sol +++ b/contracts/test/UnitTestSFC.sol @@ -5,6 +5,7 @@ import {Decimal} from "../common/Decimal.sol"; import {SFC} from "../sfc/SFC.sol"; import {SFCBase} from "../sfc/SFCBase.sol"; import {SFCLib} from "../sfc/SFCLib.sol"; +import {ISFCErrors} from "../Errors.sol"; import {NodeDriverAuth} from "../sfc/NodeDriverAuth.sol"; import {NodeDriver} from "../sfc/NodeDriver.sol"; import {UnitTestConstantsManager} from "./UnitTestConstantsManager.sol"; @@ -113,7 +114,7 @@ contract UnitTestNetworkInitializer { } } -interface SFCUnitTestI { +interface SFCUnitTestI is ISFCErrors { function currentSealedEpoch() external view returns (uint256); function getEpochSnapshot( diff --git a/test/SFC.ts b/test/SFC.ts index 201c88c..07bdd0e 100644 --- a/test/SFC.ts +++ b/test/SFC.ts @@ -52,7 +52,7 @@ describe('SFC', () => { to: this.sfc, value: 1, }), - ).to.revertedWith('transfers not allowed'); + ).to.revertedWithCustomError(this.sfc, 'TransfersNotAllowed'); }); describe('Genesis validator', () => {