Skip to content

Commit bbf156c

Browse files
xdaniortegaxdaniortega
committed
style: Improved tests description.
1 parent 5ead28d commit bbf156c

File tree

5 files changed

+123
-279
lines changed

5 files changed

+123
-279
lines changed

‎src/ValidatorFactory.sol‎

Lines changed: 0 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -226,18 +226,6 @@ contract ValidatorFactory is ReentrancyGuard {
226226
return ValidatorLogic(proxy).getStakeAmount();
227227
}
228228

229-
/**
230-
* @dev Check if validator can unstake (bonding period expired)
231-
* @param validator Validator address
232-
* @return canUnstake Whether validator can unstake
233-
*/
234-
function canUnstake(address validator) external view returns (bool) {
235-
if (!isValidator[validator]) return false;
236-
address proxy = validatorToProxy[validator];
237-
uint256 bondingBlock = ValidatorLogic(proxy).getBondingBlock();
238-
return block.number > bondingBlock;
239-
}
240-
241229
/**
242230
* @dev Compute proxy address
243231
* @param validator Validator address

‎src/ValidatorLogic.sol‎

Lines changed: 10 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -23,16 +23,6 @@ contract ValidatorLogic {
2323
event StakingPositionClosed(address indexed owner, uint256 positionId, uint256 amount);
2424
event StakingPositionDecreased(address indexed owner, uint256 positionId, uint256 amount, uint256 newAmount);
2525

26-
bytes32 private constant VALIDATOR_OWNER_SLOT = keccak256("validator.owner"); // address
27-
bytes32 private constant TOKEN_SLOT = keccak256("token"); // address
28-
bytes32 private constant STAKE_AMOUNT_SLOT = keccak256("validator.stake.amount"); // uint256
29-
bytes32 private constant IS_ACTIVE_SLOT = keccak256("validator.is.active"); // bool
30-
bytes32 private constant BONDING_BLOCK_SLOT = keccak256("validator.bonding.block"); // uint256
31-
bytes32 private constant POSITION_COUNTER_SLOT = keccak256("position.counter"); // uint256
32-
bytes32 private constant STAKING_POSITIONS_SLOT = keccak256("validator.stakingPositions"); // mapping(uint256 => StakingPosition)
33-
bytes32 private constant TOTAL_POSITIONS_SLOT = keccak256("validator.totalPositions"); //uint256
34-
35-
// Custom errors
3626
error ValidatorNotActive();
3727
error NotValidatorOwner();
3828
error NotFactory();
@@ -41,6 +31,13 @@ contract ValidatorLogic {
4131
error InsufficientStakeAmount();
4232
error InvalidAmount();
4333

34+
bytes32 private constant VALIDATOR_OWNER_SLOT = keccak256("validator.owner"); // address
35+
bytes32 private constant TOKEN_SLOT = keccak256("token"); // address
36+
bytes32 private constant VALIDATOR_FACTORY_SLOT = keccak256("validator.factory.address"); // address
37+
bytes32 private constant STAKE_AMOUNT_SLOT = keccak256("validator.stake.amount"); // uint256
38+
bytes32 private constant STAKING_POSITIONS_SLOT = keccak256("validator.stakingPositions"); // mapping(uint256 => StakingPosition)
39+
bytes32 private constant TOTAL_POSITIONS_SLOT = keccak256("validator.totalPositions"); //uint256
40+
4441
struct StakingPosition {
4542
uint256 id;
4643
uint256 amount;
@@ -58,7 +55,7 @@ contract ValidatorLogic {
5855
}
5956

6057
modifier onlyFactory() {
61-
if (msg.sender != StorageSlot.getAddressSlot(keccak256("genlayer.factory.address")).value) {
58+
if (msg.sender != StorageSlot.getAddressSlot(VALIDATOR_FACTORY_SLOT).value) {
6259
revert NotFactory();
6360
}
6461
_;
@@ -85,9 +82,7 @@ contract ValidatorLogic {
8582
StorageSlot.getAddressSlot(VALIDATOR_OWNER_SLOT).value = _owner;
8683
StorageSlot.getAddressSlot(TOKEN_SLOT).value = _token;
8784
StorageSlot.getUint256Slot(STAKE_AMOUNT_SLOT).value = _stakeAmount;
88-
StorageSlot.getBooleanSlot(IS_ACTIVE_SLOT).value = true;
89-
StorageSlot.getUint256Slot(BONDING_BLOCK_SLOT).value = block.number;
90-
StorageSlot.getAddressSlot(keccak256("genlayer.factory.address")).value = msg.sender;
85+
StorageSlot.getAddressSlot(VALIDATOR_FACTORY_SLOT).value = msg.sender;
9186

9287
// Create initial staking position for the owner (not address(this))
9388
_createStakingPosition(_owner, _stakeAmount, "Initial validator stake");
@@ -102,7 +97,6 @@ contract ValidatorLogic {
10297
*/
10398
function stake(uint256 amount) external onlyFactory returns (uint256 stakedAmount) {
10499
if (amount == 0) revert InvalidAmount();
105-
if (!StorageSlot.getBooleanSlot(IS_ACTIVE_SLOT).value) revert ValidatorNotActive();
106100

107101
// Create new position
108102
uint256 positionId = _createStakingPosition(
@@ -235,30 +229,14 @@ contract ValidatorLogic {
235229
total = StorageSlot.getUint256Slot(TOTAL_POSITIONS_SLOT).value;
236230
}
237231

238-
/**
239-
* @dev Get bonding block
240-
* @return bondingBlock Block when validator was bonded
241-
*/
242-
function getBondingBlock() external view returns (uint256) {
243-
return StorageSlot.getUint256Slot(BONDING_BLOCK_SLOT).value;
244-
}
245-
246232
/**
247233
* @dev Get validator information
248234
* @return owner Validator owner address
249235
* @return stake Current stake amount
250-
* @return isActive Whether validator is active
251-
* @return bondingBlock Block when validator was bonded
252236
*/
253-
function getValidatorInfo()
254-
external
255-
view
256-
returns (address owner, uint256 stake, bool isActive, uint256 bondingBlock)
257-
{
237+
function getValidatorInfo() external view returns (address owner, uint256 stake) {
258238
owner = StorageSlot.getAddressSlot(VALIDATOR_OWNER_SLOT).value;
259239
stake = StorageSlot.getUint256Slot(STAKE_AMOUNT_SLOT).value;
260-
isActive = StorageSlot.getBooleanSlot(IS_ACTIVE_SLOT).value;
261-
bondingBlock = StorageSlot.getUint256Slot(BONDING_BLOCK_SLOT).value;
262240
}
263241

264242
/**
@@ -277,14 +255,6 @@ contract ValidatorLogic {
277255
return StorageSlot.getUint256Slot(STAKE_AMOUNT_SLOT).value;
278256
}
279257

280-
/**
281-
* @dev Check if validator is active
282-
* @return isActive Whether validator is active
283-
*/
284-
function isActive() external view returns (bool) {
285-
return StorageSlot.getBooleanSlot(IS_ACTIVE_SLOT).value;
286-
}
287-
288258
// ---------------------------------- INTERNAL FUNCTIONS ----------------------------------
289259

290260
/**

‎test/MockLLMOracle.t.sol‎

Lines changed: 28 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,29 @@ import "forge-std/Test.sol";
55
import "../src/oracles/MockLLMOracle.sol";
66
import "@openzeppelin/contracts/access/Ownable.sol";
77

8+
/**
9+
* @title MockLLMOracle Test Suite
10+
* @notice Test suite for MockLLMOracle contract used in optimistic consensus
11+
* @dev This test suite covers the mock LLM validation functionality including:
12+
* - Transaction validation using deterministic hash-based logic
13+
* (e.g. test_ValidateTransaction, test_ValidateTransactionWithEvent)
14+
* - Oracle enablement/disablement controls
15+
* (e.g. test_SetValidationEnabled, test_DisabledOracle)
16+
* - Ownership management and access control
17+
* (e.g. test_TransferOwnership, test_RevertWhen_TransferOwnership_NotOwner)
18+
* - Event emission for validation results
19+
* (e.g. test_ValidateTransactionWithEvent with proper event testing)
20+
* - Batch validation capabilities
21+
* (e.g. test_Statistics showing multiple validations)
22+
* - Statistical tracking and consistency verification
23+
* (e.g. test_ConsistentResults, test_Statistics with counters)
24+
*
25+
* Key Test Categories:
26+
* - Core Validation: test_ValidateTransaction, test_RevertWhen_ValidateTransaction_EmptyString
27+
* - Access Control: test_RevertWhen_SetValidationEnabled_NotOwner, test_RevertWhen_TransferOwnership_InvalidAddress
28+
* - State Management: test_SetValidationEnabled, test_DisabledOracle
29+
* - Consistency: test_ConsistentResults, test_Statistics
30+
*/
831
contract MockLLMOracleTest is Test {
932
MockLLMOracle public oracle;
1033

@@ -64,7 +87,7 @@ contract MockLLMOracleTest is Test {
6487
oracle.validateTransaction("Test");
6588
}
6689

67-
function test_SetValidationEnabled_NotOwner() public {
90+
function test_RevertWhen_SetValidationEnabled_NotOwner() public {
6891
vm.prank(user);
6992
vm.expectRevert(abi.encodeWithSelector(Ownable.OwnableUnauthorizedAccount.selector, user));
7093
oracle.setValidationEnabled(false);
@@ -78,24 +101,24 @@ contract MockLLMOracleTest is Test {
78101
assertEq(oracle.owner(), user);
79102
}
80103

81-
function test_TransferOwnership_InvalidAddress() public {
104+
function test_RevertWhen_TransferOwnership_InvalidAddress() public {
82105
vm.prank(owner);
83106
vm.expectRevert(abi.encodeWithSelector(Ownable.OwnableInvalidOwner.selector, address(0)));
84107
oracle.transferOwnership(address(0));
85108
}
86109

87-
function test_TransferOwnership_NotOwner() public {
110+
function test_RevertWhen_TransferOwnership_NotOwner() public {
88111
vm.prank(user);
89112
vm.expectRevert(abi.encodeWithSelector(Ownable.OwnableUnauthorizedAccount.selector, user));
90113
oracle.transferOwnership(address(0x2));
91114
}
92115

93-
function test_ValidateTransaction_EmptyString() public {
116+
function test_RevertWhen_ValidateTransaction_EmptyString() public {
94117
vm.expectRevert(MockLLMOracle.EmptyTransaction.selector);
95118
oracle.validateTransaction("");
96119
}
97120

98-
function test_ValidateTransactionWithEvent_EmptyString() public {
121+
function test_RevertWhen_ValidateTransactionWithEvent_EmptyString() public {
99122
vm.expectRevert(MockLLMOracle.EmptyTransaction.selector);
100123
oracle.validateTransactionWithEvent("");
101124
}

‎test/TransactionManager.t.sol‎

Lines changed: 36 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,32 @@ import "../src/ValidatorFactory.sol";
77
import "../src/oracles/MockLLMOracle.sol";
88
import "../test/mock/ERC20TokenMock.sol";
99

10+
/**
11+
* @title TransactionManager Test Suite
12+
* @dev This test suite covers the optimistic consensus including:
13+
* - Proposal submission and validation
14+
* (e.g. test_SubmitProposal, test_RevertWhen_SubmitProposal_DuplicateProposal)
15+
* - LLM-based transaction validation (mocked)
16+
* (e.g. test_TestLLMValidation, test_OptimisticApproval_RequiresLLMValidation)
17+
* - Validator signature collection for optimistic approval
18+
* (e.g. test_SignProposal, test_RevertWhen_SignProposal_InvalidSignature)
19+
* - Challenge mechanisms and dispute resolution
20+
* (e.g. test_ChallengeProposal, test_ResolveChallenge_Approved)
21+
* - Voting periods and consensus resolution
22+
* (e.g. test_SubmitVote, test_IsInVotingPeriod, test_VotingPeriodExpiry)
23+
* - Slashing mechanisms for false challenges
24+
* (e.g. test_ResolveChallenge_Rejected with validator slashing)
25+
*
26+
* Test Strategy:
27+
* 1. Unit Tests: Individual function behavior and state changes
28+
* 2. Integration Tests: Complete proposal lifecycle from submission to finalization
29+
* (e.g. test_CompleteProposalLifecycle)
30+
* 3. Edge Cases: Empty validator sets, expired periods, invalid signatures
31+
* (e.g. test_EdgeCase_EmptyValidatorSet, test_RevertWhen_ChallengeProposal_ExpiredChallengePeriod)
32+
* 4. Security Tests: Access control, reentrancy protection, slashing mechanics
33+
* 5. Event Testing: All state changes emit appropriate events
34+
*
35+
*/
1036
contract TransactionManagerTest is Test {
1137
// Contracts
1238
TransactionManager public transactionManager;
@@ -159,13 +185,13 @@ contract TransactionManagerTest is Test {
159185
assertEq(transactionManager.proposalCount(), 1);
160186
}
161187

162-
function test_SubmitProposal_EmptyTransaction() public {
188+
function test_RevertWhen_SubmitProposal_EmptyTransaction() public {
163189
vm.prank(proposer);
164190
vm.expectRevert("Empty transaction");
165191
transactionManager.submitProposal("");
166192
}
167193

168-
function test_SubmitProposal_DuplicateProposal() public {
194+
function test_RevertWhen_SubmitProposal_DuplicateProposal() public {
169195
vm.startPrank(proposer);
170196

171197
transactionManager.submitProposal(TEST_TRANSACTION);
@@ -225,7 +251,7 @@ contract TransactionManagerTest is Test {
225251
assertEq(signers[0], validator1);
226252
}
227253

228-
function test_SignProposal_InvalidSignature() public {
254+
function test_RevertWhen_SignProposal_InvalidSignature() public {
229255
vm.prank(proposer);
230256
bytes32 proposalId = transactionManager.submitProposal(TEST_TRANSACTION);
231257

@@ -237,7 +263,7 @@ contract TransactionManagerTest is Test {
237263
transactionManager.signProposal(proposalId, invalidSignature);
238264
}
239265

240-
function test_SignProposal_AlreadySigned() public {
266+
function test_RevertWhen_SignProposal_AlreadySigned() public {
241267
vm.prank(proposer);
242268
bytes32 proposalId = transactionManager.submitProposal(TEST_TRANSACTION);
243269

@@ -326,7 +352,7 @@ contract TransactionManagerTest is Test {
326352
assertFalse(executed); // Should revert optimistic execution
327353
}
328354

329-
function test_ChallengeProposal_InvalidState() public {
355+
function test_RevertWhen_ChallengeProposal_InvalidState() public {
330356
vm.prank(proposer);
331357
bytes32 proposalId = transactionManager.submitProposal(TEST_TRANSACTION);
332358
// Don't get optimistic approval
@@ -336,7 +362,7 @@ contract TransactionManagerTest is Test {
336362
transactionManager.challengeProposal(proposalId);
337363
}
338364

339-
function test_ChallengeProposal_ExpiredChallengePeriod() public {
365+
function test_RevertWhen_ChallengeProposal_ExpiredChallengePeriod() public {
340366
vm.prank(proposer);
341367
bytes32 proposalId = transactionManager.submitProposal(TEST_TRANSACTION);
342368
_signProposalWithValidators(proposalId, TEST_TRANSACTION, 3);
@@ -400,7 +426,7 @@ contract TransactionManagerTest is Test {
400426
assertEq(noVotes, 0);
401427
}
402428

403-
function test_SubmitVote_InvalidVotingState() public {
429+
function test_RevertWhen_SubmitVote_InvalidVotingState() public {
404430
vm.prank(proposer);
405431
bytes32 proposalId = transactionManager.submitProposal(TEST_TRANSACTION);
406432

@@ -418,7 +444,7 @@ contract TransactionManagerTest is Test {
418444
transactionManager.submitVote(proposalId, support, voteSignature);
419445
}
420446

421-
function test_SubmitVote_AlreadyVoted() public {
447+
function test_RevertWhen_SubmitVote_AlreadyVoted() public {
422448
// Setup challenged proposal
423449
vm.prank(proposer);
424450
bytes32 proposalId = transactionManager.submitProposal(TEST_TRANSACTION);
@@ -569,7 +595,7 @@ contract TransactionManagerTest is Test {
569595
assertTrue(executed);
570596
}
571597

572-
function test_FinalizeProposal_ChallengePeriodNotEnded() public {
598+
function test_RevertWhen_FinalizeProposal_ChallengePeriodNotEnded() public {
573599
vm.prank(proposer);
574600
bytes32 proposalId = transactionManager.submitProposal(TEST_TRANSACTION);
575601
_signProposalWithValidators(proposalId, TEST_TRANSACTION, 3);
@@ -716,7 +742,7 @@ contract TransactionManagerTest is Test {
716742
transactionManager.resolveChallenge(proposalId);
717743
}
718744

719-
function test_FinalizeProposal_InvalidState() public {
745+
function test_RevertWhen_FinalizeProposal_InvalidState() public {
720746
vm.prank(proposer);
721747
bytes32 proposalId = transactionManager.submitProposal(TEST_TRANSACTION);
722748

0 commit comments

Comments
 (0)