From 33c161ca5a57cdbb495e646cc3bc0521e5c2277c Mon Sep 17 00:00:00 2001 From: taek Date: Tue, 24 Feb 2026 12:47:51 +0900 Subject: [PATCH 1/2] chore: replace string literal requires/reverts with custom errors --- src/policies/CallerPolicy.sol | 7 +++++-- src/policies/TimelockPolicy.sol | 8 +++++--- src/signers/ECDSASigner.sol | 6 ++++-- src/signers/WeightedECDSASigner.sol | 24 ++++++++++++++++-------- src/validators/ECDSAValidator.sol | 3 ++- 5 files changed, 32 insertions(+), 16 deletions(-) diff --git a/src/policies/CallerPolicy.sol b/src/policies/CallerPolicy.sol index c10e734..2aa8fdc 100644 --- a/src/policies/CallerPolicy.sol +++ b/src/policies/CallerPolicy.sol @@ -28,6 +28,9 @@ import { * If you need to validate who signed, use a signer module instead. */ contract CallerPolicy is PolicyBase, IStatelessValidatorWithSender { + error PolicyAlreadyInstalled(); + error PolicyNotLive(); + mapping(bytes32 id => mapping(address => Status)) public status; /// @notice Maps policy ID => requesting protocol => wallet => whether protocol is allowed mapping(bytes32 id => mapping(address caller => mapping(address wallet => bool))) public allowedCaller; @@ -82,7 +85,7 @@ contract CallerPolicy is PolicyBase, IStatelessValidatorWithSender { } function _policyOninstall(bytes32 id, bytes calldata _data) internal override { - require(status[id][msg.sender] == Status.NA, "Already installed"); + require(status[id][msg.sender] == Status.NA, PolicyAlreadyInstalled()); address[] memory callers = abi.decode(_data, (address[])); require(callers.length > 0, "Empty callers array"); for (uint256 i = 0; i < callers.length; i++) { @@ -93,7 +96,7 @@ contract CallerPolicy is PolicyBase, IStatelessValidatorWithSender { } function _policyOnUninstall(bytes32 id, bytes calldata _data) internal override { - require(status[id][msg.sender] == Status.Live); + require(status[id][msg.sender] == Status.Live, PolicyNotLive()); status[id][msg.sender] = Status.Deprecated; } diff --git a/src/policies/TimelockPolicy.sol b/src/policies/TimelockPolicy.sol index 5bb6ed1..1a98b49 100644 --- a/src/policies/TimelockPolicy.sol +++ b/src/policies/TimelockPolicy.sol @@ -68,6 +68,8 @@ contract TimelockPolicy is PolicyBase, IStatelessValidator, IStatelessValidatorW error ProposalNotPending(); error OnlyAccount(); error ParametersTooLarge(); + error SignatureValidationNotSupported(); + error StatelessValidationNotSupported(); /** * @notice Install the timelock policy @@ -326,7 +328,7 @@ contract TimelockPolicy is PolicyBase, IStatelessValidator, IStatelessValidatorW * @dev TimelockPolicy does not support ERC-1271 signature validation - always reverts */ function checkSignaturePolicy(bytes32, address, bytes32, bytes calldata) external pure override returns (uint256) { - revert("TimelockPolicy: signature validation not supported"); + revert SignatureValidationNotSupported(); } function validateSignatureWithData(bytes32, bytes calldata, bytes calldata) @@ -335,7 +337,7 @@ contract TimelockPolicy is PolicyBase, IStatelessValidator, IStatelessValidatorW override(IStatelessValidator) returns (bool) { - revert("TimelockPolicy: stateless signature validation not supported"); + revert StatelessValidationNotSupported(); } function validateSignatureWithDataWithSender(address, bytes32, bytes calldata, bytes calldata) @@ -344,7 +346,7 @@ contract TimelockPolicy is PolicyBase, IStatelessValidator, IStatelessValidatorW override(IStatelessValidatorWithSender) returns (bool) { - revert("TimelockPolicy: stateless signature validation not supported"); + revert StatelessValidationNotSupported(); } // ==================== Internal Shared Logic ==================== diff --git a/src/signers/ECDSASigner.sol b/src/signers/ECDSASigner.sol index 7193480..b65cdfd 100644 --- a/src/signers/ECDSASigner.sol +++ b/src/signers/ECDSASigner.sol @@ -17,6 +17,8 @@ import { contract ECDSASigner is SignerBase, IStatelessValidator, IStatelessValidatorWithSender { error InvalidDataLength(); error ZeroAddressSigner(); + error SignerAlreadySet(); + error SignerNotSet(); mapping(bytes32 id => mapping(address wallet => address)) public signer; @@ -61,7 +63,7 @@ contract ECDSASigner is SignerBase, IStatelessValidator, IStatelessValidatorWith } function _signerOninstall(bytes32 id, bytes calldata _data) internal override { - require(signer[id][msg.sender] == address(0), "Already installed"); + require(signer[id][msg.sender] == address(0), SignerAlreadySet()); if (_data.length != 20) revert InvalidDataLength(); address signerAddr = address(bytes20(_data[0:20])); if (signerAddr == address(0)) revert ZeroAddressSigner(); @@ -69,7 +71,7 @@ contract ECDSASigner is SignerBase, IStatelessValidator, IStatelessValidatorWith } function _signerOnUninstall(bytes32 id, bytes calldata) internal override { - require(signer[id][msg.sender] != address(0)); + require(signer[id][msg.sender] != address(0), SignerNotSet()); delete signer[id][msg.sender]; } diff --git a/src/signers/WeightedECDSASigner.sol b/src/signers/WeightedECDSASigner.sol index b4852e8..ff36df4 100644 --- a/src/signers/WeightedECDSASigner.sol +++ b/src/signers/WeightedECDSASigner.sol @@ -36,6 +36,14 @@ contract WeightedECDSASigner is EIP712, SignerBase, IStatelessValidator, IStatel keccak256("Proposal(address account,bytes32 id,bytes callData,uint256 nonce)"); error ZeroWeightSigner(); + error LengthMismatch(); + error EmptyGuardians(); + error ZeroThreshold(); + error GuardianCannotBeSelf(); + error ZeroAddressGuardian(); + error ZeroWeight(); + error GuardianAlreadyEnabled(); + error SignersNotSorted(); mapping(bytes32 id => mapping(address kernel => WeightedECDSASignerStorage)) public weightedStorage; mapping(address guardian => mapping(bytes32 id => mapping(address kernel => GuardianStorage))) public guardian; @@ -53,16 +61,16 @@ contract WeightedECDSASigner is EIP712, SignerBase, IStatelessValidator, IStatel (address[] memory _guardians, uint24[] memory _weights, uint24 _threshold) = abi.decode(_data, (address[], uint24[], uint24)); - require(_guardians.length == _weights.length, "Length mismatch"); - require(_guardians.length > 0, "No guardians"); - require(_threshold > 0, "Zero threshold"); + require(_guardians.length == _weights.length, LengthMismatch()); + require(_guardians.length > 0, EmptyGuardians()); + require(_threshold > 0, ZeroThreshold()); weightedStorage[id][msg.sender].firstGuardian = msg.sender; for (uint256 i = 0; i < _guardians.length; i++) { - require(_guardians[i] != msg.sender, "Guardian cannot be self"); - require(_guardians[i] != address(0), "Guardian cannot be 0"); - require(_weights[i] != 0, "Weight cannot be 0"); - require(guardian[_guardians[i]][id][msg.sender].weight == 0, "Guardian already enabled"); + require(_guardians[i] != msg.sender, GuardianCannotBeSelf()); + require(_guardians[i] != address(0), ZeroAddressGuardian()); + require(_weights[i] != 0, ZeroWeight()); + require(guardian[_guardians[i]][id][msg.sender].weight == 0, GuardianAlreadyEnabled()); guardian[_guardians[i]][id][msg.sender] = GuardianStorage({weight: _weights[i], nextGuardian: weightedStorage[id][msg.sender].firstGuardian}); weightedStorage[id][msg.sender].firstGuardian = _guardians[i]; @@ -188,7 +196,7 @@ contract WeightedECDSASigner is EIP712, SignerBase, IStatelessValidator, IStatel signer = ECDSA.tryRecoverCalldata(proposalHash, sig[i * 65:(i + 1) * 65]); // Enforce sorted order to prevent signature reuse - require(signer > lastSigner, "Signers not sorted"); + require(signer > lastSigner, SignersNotSorted()); lastSigner = signer; proposalSigners[i] = signer; diff --git a/src/validators/ECDSAValidator.sol b/src/validators/ECDSAValidator.sol index c2f8547..24c58aa 100644 --- a/src/validators/ECDSAValidator.sol +++ b/src/validators/ECDSAValidator.sol @@ -32,6 +32,7 @@ contract ECDSAValidator is IValidator, IHook, IStatelessValidator, IStatelessVal error InvalidDataLength(); error ZeroAddressOwner(); + error SenderNotOwner(); function onInstall(bytes calldata _data) external payable override { if (_isInitialized(msg.sender)) revert AlreadyInitialized(msg.sender); @@ -106,7 +107,7 @@ contract ECDSAValidator is IValidator, IHook, IStatelessValidator, IStatelessVal } function preCheck(address msgSender, uint256, bytes calldata) external payable override returns (bytes memory) { - require(msgSender == ecdsaValidatorStorage[msg.sender].owner, "ECDSAValidator: sender is not owner"); + require(msgSender == ecdsaValidatorStorage[msg.sender].owner, SenderNotOwner()); return hex""; } From a7614bef3ce0c26de2283eeb562cd55bfaf51a38 Mon Sep 17 00:00:00 2001 From: taek Date: Tue, 24 Feb 2026 12:47:58 +0900 Subject: [PATCH 2/2] test: update TimelockPolicy tests for custom error selectors --- test/btt/Timelock.t.sol | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/test/btt/Timelock.t.sol b/test/btt/Timelock.t.sol index b0f1dda..0e122f4 100644 --- a/test/btt/Timelock.t.sol +++ b/test/btt/Timelock.t.sol @@ -503,7 +503,7 @@ contract TimelockTest is Test { function test_GivenInitialized_WhenCallingCheckSignaturePolicy() external whenCallingCheckSignaturePolicy { // it should revert (TOB-KERNEL-20: signature validation not supported) vm.prank(WALLET); - vm.expectRevert("TimelockPolicy: signature validation not supported"); + vm.expectRevert(TimelockPolicy.SignatureValidationNotSupported.selector); timelockPolicy.checkSignaturePolicy(POLICY_ID, address(0), bytes32(0), ""); } @@ -511,7 +511,7 @@ contract TimelockTest is Test { // it should revert (TOB-KERNEL-20: signature validation not supported) address uninitWallet = address(0xcccc); vm.prank(uninitWallet); - vm.expectRevert("TimelockPolicy: signature validation not supported"); + vm.expectRevert(TimelockPolicy.SignatureValidationNotSupported.selector); timelockPolicy.checkSignaturePolicy(POLICY_ID, address(0), bytes32(0), ""); } @@ -524,7 +524,7 @@ contract TimelockTest is Test { function test_GivenDelayAndExpirationAreNonzero() external whenCallingValidateSignatureWithData { // it should revert (TOB-KERNEL-20: stateless signature validation not supported) bytes memory data = abi.encode(uint48(1 hours), uint48(1 days)); - vm.expectRevert("TimelockPolicy: stateless signature validation not supported"); + vm.expectRevert(TimelockPolicy.StatelessValidationNotSupported.selector); timelockPolicy.validateSignatureWithData(bytes32(0), "", data); } @@ -534,7 +534,7 @@ contract TimelockTest is Test { { // it should revert (TOB-KERNEL-20: stateless signature validation not supported) bytes memory data = abi.encode(uint48(0), uint48(1 days)); - vm.expectRevert("TimelockPolicy: stateless signature validation not supported"); + vm.expectRevert(TimelockPolicy.StatelessValidationNotSupported.selector); timelockPolicy.validateSignatureWithData(bytes32(0), "", data); } @@ -544,7 +544,7 @@ contract TimelockTest is Test { { // it should revert (TOB-KERNEL-20: stateless signature validation not supported) bytes memory data = abi.encode(uint48(1 hours), uint48(0)); - vm.expectRevert("TimelockPolicy: stateless signature validation not supported"); + vm.expectRevert(TimelockPolicy.StatelessValidationNotSupported.selector); timelockPolicy.validateSignatureWithData(bytes32(0), "", data); } @@ -560,7 +560,7 @@ contract TimelockTest is Test { { // it should revert (TOB-KERNEL-20: stateless signature validation not supported) bytes memory data = abi.encode(uint48(1 hours), uint48(1 days)); - vm.expectRevert("TimelockPolicy: stateless signature validation not supported"); + vm.expectRevert(TimelockPolicy.StatelessValidationNotSupported.selector); timelockPolicy.validateSignatureWithDataWithSender(address(0), bytes32(0), "", data); } @@ -570,7 +570,7 @@ contract TimelockTest is Test { { // it should revert (TOB-KERNEL-20: stateless signature validation not supported) bytes memory data = abi.encode(uint48(0), uint48(1 days)); - vm.expectRevert("TimelockPolicy: stateless signature validation not supported"); + vm.expectRevert(TimelockPolicy.StatelessValidationNotSupported.selector); timelockPolicy.validateSignatureWithDataWithSender(address(0), bytes32(0), "", data); } @@ -580,7 +580,7 @@ contract TimelockTest is Test { { // it should revert (TOB-KERNEL-20: stateless signature validation not supported) bytes memory data = abi.encode(uint48(1 hours), uint48(0)); - vm.expectRevert("TimelockPolicy: stateless signature validation not supported"); + vm.expectRevert(TimelockPolicy.StatelessValidationNotSupported.selector); timelockPolicy.validateSignatureWithDataWithSender(address(0), bytes32(0), "", data); }