Skip to content

Commit f9cb5a6

Browse files
jaybuidljaybuidl
authored
Merge pull request #1765 from kleros/feat/foundry-tests
feat(KC): add foundry test file
2 parents 6ccef13 + 0220cc2 commit f9cb5a6

18 files changed

+2878
-42
lines changed

‎.github/workflows/contracts-testing.yml

+18-22
Original file line numberDiff line numberDiff line change
@@ -40,14 +40,16 @@ jobs:
4040
54.185.253.63:443
4141
4242
- name: Setup Node.js environment
43-
uses: actions/setup-node@v4
43+
uses: actions/setup-node@39370e3970a6d050c480ffad4ff0ed4d3fdee5af # v4.1.0
4444
with:
4545
node-version: 18.x
4646

47-
- uses: actions/checkout@v4
47+
- uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2
48+
with:
49+
submodules: recursive
4850

4951
- name: Cache node modules
50-
uses: actions/cache@v4
52+
uses: actions/cache@1bd1e32a3bdc45362d1e726936510720a7c30a57 # v4.2.0
5153
env:
5254
cache-name: cache-node-modules
5355
with:
@@ -57,28 +59,22 @@ jobs:
5759
key: ${{ runner.os }}-build-${{ secrets.CACHE_VERSION }}-${{ env.cache-name }}-${{ hashFiles('**/package-lock.json', '**/yarn.lock') }}
5860
restore-keys: |
5961
${{ runner.os }}-build-${{ secrets.CACHE_VERSION }}-${{ env.cache-name }}-
60-
61-
#- name: Install parent dependencies
62-
# run: |
63-
# echo "current dir: $PWD"
64-
# yarn install
65-
62+
6663
- name: Install contracts dependencies
67-
run: |
68-
yarn workspace @kleros/kleros-v2-contracts install
69-
70-
- name: Compile
71-
run: |
72-
yarn hardhat compile
73-
working-directory: contracts
74-
75-
- name: Test with coverage
76-
run: |
77-
yarn hardhat coverage --solcoverjs ./.solcover.js --temp artifacts --testfiles './test/**/*.ts' --show-stack-traces
78-
working-directory: contracts
64+
run: yarn workspace @kleros/kleros-v2-contracts install
65+
66+
- name: Install Foundry
67+
uses: foundry-rs/foundry-toolchain@8f1998e9878d786675189ef566a2e4bf24869773 # v1.2.0
7968

69+
- name: Install lcov
70+
run: sudo apt-get install -y lcov
71+
72+
- name: Run Hardhat and Foundry tests with coverage
73+
run: yarn coverage
74+
working-directory: contracts
75+
8076
- name: Upload a build artifact
81-
uses: actions/upload-artifact@v4
77+
uses: actions/upload-artifact@b4b15b8c7c6ac21ea08fcf65892d2ee8f75cf882 # v4.4.3
8278
with:
8379
name: code-coverage-report
8480
path: contracts/coverage

‎contracts/.solcover.js

+2-2
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ const shell = require("shelljs");
55
// The environment variables are loaded in hardhat.config.ts
66

77
module.exports = {
8-
istanbulReporter: ["html"],
8+
istanbulReporter: ["lcov"],
99
onCompileComplete: async function (_config) {
1010
await run("typechain");
1111
},
@@ -14,7 +14,7 @@ module.exports = {
1414
shell.rm("-rf", "./artifacts");
1515
shell.rm("-rf", "./typechain");
1616
},
17-
skipFiles: ["mocks", "test"],
17+
skipFiles: ["test", "token", "kleros-v1", "proxy/mock", "gateway/mock", "rng/mock"],
1818
mocha: {
1919
timeout: 20000,
2020
grep: "@skip-on-coverage", // Find everything with this tag

‎contracts/lib/forge-std

Submodule forge-std updated 43 files

‎contracts/package.json

+1
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@
2222
"clean": "hardhat clean",
2323
"check": "hardhat check",
2424
"test": "TS_NODE_TRANSPILE_ONLY=1 hardhat test",
25+
"coverage": "scripts/coverage.sh",
2526
"start": "hardhat node --tags nop",
2627
"start-local": "hardhat node --tags Arbitration,HomeArbitrable --hostname 0.0.0.0",
2728
"deploy": "hardhat deploy",

‎contracts/scripts/coverage.sh

+62
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,62 @@
1+
#!/usr/bin/env bash
2+
3+
set -e # exit on error
4+
5+
rm -rf coverage
6+
mkdir -p coverage
7+
8+
# Generate the Forge coverage report
9+
forge clean
10+
if [ "$CI" != "true" ]; then
11+
forge coverage --report summary --report lcov --report-file coverage/lcov-forge.info
12+
else
13+
# FIXME: Temporarily workaround a CI issue
14+
touch coverage/lcov-forge.info
15+
fi
16+
17+
# Generate the Hardhat coverage report
18+
yarn clean
19+
yarn hardhat coverage --solcoverjs ./.solcover.js --temp artifacts --show-stack-traces --testfiles "test/**/*.ts"
20+
mv coverage/lcov.info coverage/lcov-hardhat.info
21+
22+
# Make the Hardhat report paths relative for consistency with Forge coverage report
23+
sed -i -e 's/\/.*\/kleros-v2\/contracts\///g' coverage/lcov-hardhat.info
24+
25+
# Merge the two reports
26+
lcov \
27+
--ignore-errors format \
28+
--ignore-errors inconsistent \
29+
--ignore-errors empty \
30+
--rc max_message_count=3 \
31+
--rc derive_function_end_line=0 \
32+
--rc branch_coverage=1 \
33+
--add-tracefile coverage/lcov-hardhat.info \
34+
--add-tracefile coverage/lcov-forge.info \
35+
--output-file coverage/merged-lcov.info
36+
37+
# Filter out unnecessary contracts from the report
38+
lcov \
39+
--ignore-errors format \
40+
--ignore-errors inconsistent \
41+
--ignore-errors empty \
42+
--ignore-errors unused \
43+
--rc max_message_count=3 \
44+
--rc branch_coverage=1 \
45+
--rc derive_function_end_line=0 \
46+
--remove coverage/merged-lcov.info \
47+
--output-file coverage/filtered-lcov.info \
48+
"../node_modules" "src/test" "src/token" "src/kleros-v1" "src/proxy/mock" "src/gateway/mock" "src/rng/mock"
49+
50+
# Open more granular breakdown in browser
51+
if [ "$CI" != "true" ]; then
52+
# Generate the HTML report
53+
genhtml coverage/filtered-lcov.info \
54+
--ignore-errors format \
55+
--ignore-errors inconsistent \
56+
--ignore-errors empty \
57+
--ignore-errors category \
58+
--rc branch_coverage=1 \
59+
--rc max_message_count=3 \
60+
-o coverage
61+
open coverage/index.html
62+
fi

‎contracts/src/arbitration/KlerosCoreBase.sol

+10-8
Original file line numberDiff line numberDiff line change
@@ -113,7 +113,7 @@ abstract contract KlerosCoreBase is IArbitratorV2 {
113113
event AppealDecision(uint256 indexed _disputeID, IArbitrableV2 indexed _arbitrable);
114114
event Draw(address indexed _address, uint256 indexed _disputeID, uint256 _roundID, uint256 _voteID);
115115
event CourtCreated(
116-
uint256 indexed _courtID,
116+
uint96 indexed _courtID,
117117
uint96 indexed _parent,
118118
bool _hiddenVotes,
119119
uint256 _minStake,
@@ -237,16 +237,18 @@ abstract contract KlerosCoreBase is IArbitratorV2 {
237237

238238
sortitionModule.createTree(bytes32(uint256(GENERAL_COURT)), _sortitionExtraData);
239239

240+
uint256[] memory supportedDisputeKits = new uint256[](1);
241+
supportedDisputeKits[0] = DISPUTE_KIT_CLASSIC;
240242
emit CourtCreated(
241-
1,
243+
GENERAL_COURT,
242244
court.parent,
243245
_hiddenVotes,
244246
_courtParameters[0],
245247
_courtParameters[1],
246248
_courtParameters[2],
247249
_courtParameters[3],
248250
_timesPerPeriod,
249-
new uint256[](0)
251+
supportedDisputeKits
250252
);
251253
_enableDisputeKit(GENERAL_COURT, DISPUTE_KIT_CLASSIC, true);
252254
}
@@ -351,7 +353,7 @@ abstract contract KlerosCoreBase is IArbitratorV2 {
351353
if (_supportedDisputeKits[i] == 0 || _supportedDisputeKits[i] >= disputeKits.length) {
352354
revert WrongDisputeKitIndex();
353355
}
354-
court.supportedDisputeKits[_supportedDisputeKits[i]] = true;
356+
_enableDisputeKit(uint96(courtID), _supportedDisputeKits[i], true);
355357
}
356358
// Check that Classic DK support was added.
357359
if (!court.supportedDisputeKits[DISPUTE_KIT_CLASSIC]) revert MustSupportDisputeKitClassic();
@@ -370,7 +372,7 @@ abstract contract KlerosCoreBase is IArbitratorV2 {
370372
// Update the parent.
371373
courts[_parent].children.push(courtID);
372374
emit CourtCreated(
373-
courtID,
375+
uint96(courtID),
374376
_parent,
375377
_hiddenVotes,
376378
_minStake,
@@ -1061,7 +1063,7 @@ abstract contract KlerosCoreBase is IArbitratorV2 {
10611063
bool _alreadyTransferred,
10621064
OnError _onError
10631065
) internal returns (bool) {
1064-
if (_courtID == FORKING_COURT || _courtID > courts.length) {
1066+
if (_courtID == FORKING_COURT || _courtID >= courts.length) {
10651067
_stakingFailed(_onError, StakingResult.CannotStakeInThisCourt); // Staking directly into the forking court is not allowed.
10661068
return false;
10671069
}
@@ -1102,6 +1104,7 @@ abstract contract KlerosCoreBase is IArbitratorV2 {
11021104
if (_result == StakingResult.CannotStakeInMoreCourts) revert StakingInTooManyCourts();
11031105
if (_result == StakingResult.CannotStakeInThisCourt) revert StakingNotPossibeInThisCourt();
11041106
if (_result == StakingResult.CannotStakeLessThanMinStake) revert StakingLessThanCourtMinStake();
1107+
if (_result == StakingResult.CannotStakeZeroWhenNoStake) revert StakingZeroWhenNoStake();
11051108
}
11061109

11071110
/// @dev Gets a court ID, the minimum number of jurors and an ID of a dispute kit from a specified extra data bytes array.
@@ -1147,13 +1150,11 @@ abstract contract KlerosCoreBase is IArbitratorV2 {
11471150
error SortitionModuleOnly();
11481151
error UnsuccessfulCall();
11491152
error InvalidDisputKitParent();
1150-
error DepthLevelMax();
11511153
error MinStakeLowerThanParentCourt();
11521154
error UnsupportedDisputeKit();
11531155
error InvalidForkingCourtAsParent();
11541156
error WrongDisputeKitIndex();
11551157
error CannotDisableClassicDK();
1156-
error ArraysLengthMismatch();
11571158
error StakingInTooManyCourts();
11581159
error StakingNotPossibeInThisCourt();
11591160
error StakingLessThanCourtMinStake();
@@ -1177,4 +1178,5 @@ abstract contract KlerosCoreBase is IArbitratorV2 {
11771178
error TransferFailed();
11781179
error WhenNotPausedOnly();
11791180
error WhenPausedOnly();
1181+
error StakingZeroWhenNoStake();
11801182
}

‎contracts/src/arbitration/SortitionModuleBase.sol

+8-4
Original file line numberDiff line numberDiff line change
@@ -206,13 +206,14 @@ abstract contract SortitionModuleBase is ISortitionModule {
206206
DelayedStake storage delayedStake = delayedStakes[i];
207207
// Delayed stake could've been manually removed already. In this case simply move on to the next item.
208208
if (delayedStake.account != address(0)) {
209+
// Nullify the index so the delayed stake won't get deleted before its own execution.
210+
delete latestDelayedStakeIndex[delayedStake.account][delayedStake.courtID];
209211
core.setStakeBySortitionModule(
210212
delayedStake.account,
211213
delayedStake.courtID,
212214
delayedStake.stake,
213215
delayedStake.alreadyTransferred
214216
);
215-
delete latestDelayedStakeIndex[delayedStake.account][delayedStake.courtID];
216217
delete delayedStakes[i];
217218
}
218219
}
@@ -265,13 +266,16 @@ abstract contract SortitionModuleBase is ISortitionModule {
265266
uint256 currentStake = stakeOf(_account, _courtID);
266267

267268
uint256 nbCourts = juror.courtIDs.length;
268-
if (_newStake == 0 && (nbCourts >= MAX_STAKE_PATHS || currentStake == 0)) {
269+
if (currentStake == 0 && nbCourts >= MAX_STAKE_PATHS) {
269270
return (0, 0, StakingResult.CannotStakeInMoreCourts); // Prevent staking beyond MAX_STAKE_PATHS but unstaking is always allowed.
270271
}
271272

272-
if (phase != Phase.staking) {
273-
pnkWithdrawal = _deleteDelayedStake(_courtID, _account);
273+
if (currentStake == 0 && _newStake == 0) {
274+
return (0, 0, StakingResult.CannotStakeZeroWhenNoStake); // Forbid staking 0 amount when current stake is 0 to avoid flaky behaviour.
275+
}
274276

277+
pnkWithdrawal = _deleteDelayedStake(_courtID, _account);
278+
if (phase != Phase.staking) {
275279
// Store the stake change as delayed, to be applied when the phase switches back to Staking.
276280
DelayedStake storage delayedStake = delayedStakes[++delayedStakeWriteIndex];
277281
delayedStake.account = _account;

‎contracts/src/arbitration/dispute-kits/DisputeKitClassic.sol

+4-1
Original file line numberDiff line numberDiff line change
@@ -238,7 +238,6 @@ contract DisputeKitClassic is IDisputeKit, Initializable, UUPSProxiable {
238238
(uint96 courtID, , , , ) = core.disputes(_coreDisputeID);
239239
bytes32 key = bytes32(uint256(courtID)); // Get the ID of the tree.
240240

241-
// TODO: Handle the situation when no one has staked yet.
242241
drawnAddress = sortitionModule.draw(key, _coreDisputeID, _nonce);
243242

244243
if (_postDrawCheck(_coreDisputeID, drawnAddress)) {
@@ -603,6 +602,10 @@ contract DisputeKitClassic is IDisputeKit, Initializable, UUPSProxiable {
603602
/// @param _coreDisputeID ID of the dispute in the core contract.
604603
/// @param _juror Chosen address.
605604
/// @return Whether the address can be drawn or not.
605+
/// Note that we don't check the minStake requirement here because of the implicit staking in parent courts.
606+
/// minStake is checked directly during staking process however it's possible for the juror to get drawn
607+
/// while having < minStake if it is later increased by governance.
608+
/// This issue is expected and harmless since we check for insolvency anyway.
606609
function _postDrawCheck(uint256 _coreDisputeID, address _juror) internal view returns (bool) {
607610
(uint96 courtID, , , , ) = core.disputes(_coreDisputeID);
608611
uint256 lockedAmountPerJuror = core

‎contracts/src/arbitration/dispute-kits/DisputeKitSybilResistant.sol

+4-1
Original file line numberDiff line numberDiff line change
@@ -255,7 +255,6 @@ contract DisputeKitSybilResistant is IDisputeKit, Initializable, UUPSProxiable {
255255
(uint96 courtID, , , , ) = core.disputes(_coreDisputeID);
256256
bytes32 key = bytes32(uint256(courtID)); // Get the ID of the tree.
257257

258-
// TODO: Handle the situation when no one has staked yet.
259258
drawnAddress = sortitionModule.draw(key, _coreDisputeID, _nonce);
260259

261260
if (_postDrawCheck(_coreDisputeID, drawnAddress) && !round.alreadyDrawn[drawnAddress]) {
@@ -621,6 +620,10 @@ contract DisputeKitSybilResistant is IDisputeKit, Initializable, UUPSProxiable {
621620
/// @param _coreDisputeID ID of the dispute in the core contract.
622621
/// @param _juror Chosen address.
623622
/// @return Whether the address can be drawn or not.
623+
/// Note that we don't check the minStake requirement here because of the implicit staking in parent courts.
624+
/// minStake is checked directly during staking process however it's possible for the juror to get drawn
625+
/// while having < minStake if it is later increased by governance.
626+
/// This issue is expected and harmless since we check for insolvency anyway.
624627
function _postDrawCheck(uint256 _coreDisputeID, address _juror) internal view returns (bool) {
625628
(uint96 courtID, , , , ) = core.disputes(_coreDisputeID);
626629
uint256 lockedAmountPerJuror = core

‎contracts/src/arbitration/interfaces/IDisputeKit.sol

+1
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,7 @@ interface IDisputeKit {
4141
/// @param _coreDisputeID The ID of the dispute in Kleros Core, not in the Dispute Kit.
4242
/// @param _numberOfChoices Number of choices of the dispute
4343
/// @param _extraData Additional info about the dispute, for possible use in future dispute kits.
44+
/// @param _nbVotes Maximal number of votes this dispute can get. DEPRECATED as we don't need to pass it now. KC handles the count.
4445
function createDispute(
4546
uint256 _coreDisputeID,
4647
uint256 _numberOfChoices,

‎contracts/src/libraries/Constants.sol

+3-2
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ uint96 constant FORKING_COURT = 0; // Index of the forking court.
99
uint96 constant GENERAL_COURT = 1; // Index of the default (general) court.
1010

1111
// Dispute Kits
12-
uint256 constant NULL_DISPUTE_KIT = 0; // Null pattern to indicate a top-level DK which has no parent.
12+
uint256 constant NULL_DISPUTE_KIT = 0; // Null pattern to indicate a top-level DK which has no parent. DEPRECATED, as its main purpose was to accommodate forest structure which is not used now.
1313
uint256 constant DISPUTE_KIT_CLASSIC = 1; // Index of the default DK. 0 index is skipped.
1414

1515
// Sortition Module
@@ -33,5 +33,6 @@ enum StakingResult {
3333
CannotStakeInThisCourt,
3434
CannotStakeLessThanMinStake,
3535
CannotStakeMoreThanMaxStakePerJuror,
36-
CannotStakeMoreThanMaxTotalStaked
36+
CannotStakeMoreThanMaxTotalStaked,
37+
CannotStakeZeroWhenNoStake
3738
}

‎contracts/src/test/KlerosCoreMock.sol

+25
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
// SPDX-License-Identifier: MIT
2+
3+
/// @custom:authors: [@unknownunknown1]
4+
/// @custom:reviewers: []
5+
/// @custom:auditors: []
6+
/// @custom:bounties: []
7+
/// @custom:deployments: []
8+
9+
pragma solidity 0.8.24;
10+
11+
import "../arbitration/KlerosCore.sol";
12+
13+
/// @title KlerosCoreMock
14+
/// KlerosCore with view functions to use in Foundry tests.
15+
contract KlerosCoreMock is KlerosCore {
16+
function getCourtChildren(uint256 _courtId) external view returns (uint256[] memory children) {
17+
children = courts[_courtId].children;
18+
}
19+
20+
function extraDataToCourtIDMinJurorsDisputeKit(
21+
bytes memory _extraData
22+
) external view returns (uint96 courtID, uint256 minJurors, uint256 disputeKitID) {
23+
(courtID, minJurors, disputeKitID) = _extraDataToCourtIDMinJurorsDisputeKit(_extraData);
24+
}
25+
}

‎contracts/src/test/SortitionModuleMock.sol

+23
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
// SPDX-License-Identifier: MIT
2+
3+
/**
4+
* @custom:authors: [unknownunknown1]
5+
* @custom:reviewers: []
6+
* @custom:auditors: []
7+
* @custom:bounties: []
8+
* @custom:deployments: []
9+
*/
10+
11+
pragma solidity 0.8.24;
12+
13+
import "../arbitration/SortitionModule.sol";
14+
15+
/// @title SortitionModuleMock
16+
/// @dev Adds getter functions to sortition module for Foundry tests.
17+
contract SortitionModuleMock is SortitionModule {
18+
function getSortitionProperties(bytes32 _key) external view returns (uint256 K, uint256 nodeLength) {
19+
SortitionSumTree storage tree = sortitionSumTrees[_key];
20+
K = tree.K;
21+
nodeLength = tree.nodes.length;
22+
}
23+
}

‎contracts/test/arbitration/index.ts

+1-1
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@ describe("DisputeKitClassic", async () => {
3030
expect(events2[0].args._feeForJuror).to.equal(ethers.parseUnits("0.1", 18));
3131
expect(events2[0].args._jurorsForCourtJump).to.equal(256);
3232
expect(events2[0].args._timesPerPeriod).to.deep.equal([0, 0, 0, 10]);
33-
expect(events2[0].args._supportedDisputeKits).to.deep.equal([]);
33+
expect(events2[0].args._supportedDisputeKits).to.deep.equal([1]);
3434

3535
const events3 = await core.queryFilter(core.filters.DisputeKitEnabled());
3636
expect(events3.length).to.equal(1);

‎contracts/test/foundry/Contract.t.sol

Whitespace-only changes.

0 commit comments

Comments
 (0)