Skip to content

Commit 13f3708

Browse files
committed
fix: coderabbit review, fixed a test not making any assertions
1 parent cefa0cb commit 13f3708

File tree

4 files changed

+23
-18
lines changed

4 files changed

+23
-18
lines changed

contracts/src/test/DisputeKitGatedMock.sol

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,8 +4,8 @@ pragma solidity ^0.8.24;
44

55
import "../arbitration/dispute-kits/DisputeKitGated.sol";
66

7-
/// @title KlerosCoreMock
8-
/// KlerosCore with view functions to use in Foundry tests.
7+
/// @title DisputeKitGatedMock
8+
/// DisputeKitGated with view functions to use in the tests.
99
contract DisputeKitGatedMock is DisputeKitGated {
1010
function extraDataToTokenInfo(
1111
bytes memory _extraData

contracts/src/test/DisputeKitGatedShutterMock.sol

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ pragma solidity ^0.8.24;
55
import "../arbitration/dispute-kits/DisputeKitGatedShutter.sol";
66

77
/// @title DisputeKitGatedShutterMock
8-
/// DisputeKitGatedShutter with view functions to use in Foundry tests.
8+
/// DisputeKitGatedShutter with view functions to use in the tests.
99
contract DisputeKitGatedShutterMock is DisputeKitGatedShutter {
1010
function extraDataToTokenInfo(
1111
bytes memory _extraData

contracts/test/arbitration/helpers/dispute-kit-gated-common.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -229,7 +229,7 @@ export async function setupTokenGatedTest(config: TokenGatedTestConfig): Promise
229229
PNK: PNK_AMOUNT,
230230
};
231231

232-
// Whitelist all tokens by default to make existing tests work
232+
// Whitelist all tokens by default
233233
await whitelistTokens(context, [dai.target, nft721.target, nft1155.target], true);
234234

235235
return context;
@@ -390,7 +390,7 @@ export function testERC20Gating(context: () => TokenGatedTestContext) {
390390

391391
it("Should draw only the jurors who have some DAI balance", async () => {
392392
const ctx = context();
393-
ctx.dai.transfer(ctx.juror1.address, 1);
393+
await ctx.dai.transfer(ctx.juror1.address, 1);
394394

395395
const nbOfJurors = 15n;
396396
const tx = await stakeAndDraw(

contracts/test/arbitration/helpers/dispute-kit-shutter-common.ts

Lines changed: 18 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -731,26 +731,31 @@ export function testRecoveryFlowJurorReveals(context: () => ShutterTestContext)
731731

732732
export function testHashFunctionBehavior(context: () => ShutterTestContext) {
733733
describe("Hash Function Behavior", () => {
734-
it("Should return different hashes for juror vs non-juror callers", async () => {
734+
it("Should compute different hashes for juror recovery vs non-juror normal flow", async () => {
735735
const ctx = context();
736-
const disputeId = await createDisputeAndDraw(ctx, ctx.shutterCourtID, 3, ctx.shutterDKID);
737-
await advanceToCommitPeriod(ctx, disputeId);
738736

739-
const voteIDs = await getVoteIDsForJuror(ctx, disputeId, ctx.juror1);
737+
// Test 1: Verify hashVote matches generateCommitments for non-juror case
740738
const { fullCommit, recoveryCommit } = generateCommitments(ctx.choice, ctx.salt, ctx.justification);
739+
const nonJurorHash = await ctx.disputeKit.hashVote(ctx.choice, ctx.salt, ctx.justification);
740+
expect(nonJurorHash).to.equal(fullCommit, "Non-juror hash should match full commitment");
741741

742-
await ctx.disputeKit
743-
.connect(ctx.juror1)
744-
.castCommitShutter(disputeId, voteIDs, fullCommit, recoveryCommit, ctx.identity, ctx.encryptedVote);
742+
// Test 2: Verify the two commitment types are different
743+
expect(fullCommit).to.not.equal(recoveryCommit, "Full and recovery commitments should differ");
745744

746-
await advanceToVotePeriod(ctx, disputeId);
745+
// Test 3: Calculate what the juror hash would be and verify it matches recovery commitment
746+
const jurorExpectedHash = ethers.keccak256(
747+
ethers.AbiCoder.defaultAbiCoder().encode(["uint256", "uint256"], [ctx.choice, ctx.salt])
748+
);
749+
expect(jurorExpectedHash).to.equal(recoveryCommit, "Juror hash calculation should match recovery commitment");
747750

748-
// During castVoteShutter, the contract should use different hash logic
749-
// For juror: hash(choice, salt)
750-
// For non-juror: hash(choice, salt, justificationHash)
751+
// Test 4: Verify that changing justification affects non-juror hash but not juror hash
752+
const differentJustification = "Different justification";
753+
const { fullCommit: newFullCommit } = generateCommitments(ctx.choice, ctx.salt, differentJustification);
754+
const newNonJurorHash = await ctx.disputeKit.hashVote(ctx.choice, ctx.salt, differentJustification);
751755

752-
// This is tested implicitly by the recovery flow tests above
753-
// The juror can reveal with any justification, while non-juror must provide exact justification
756+
expect(newNonJurorHash).to.equal(newFullCommit, "New non-juror hash should match new full commitment");
757+
expect(newNonJurorHash).to.not.equal(nonJurorHash, "Non-juror hash should change with justification");
758+
// Note: juror hash would remain the same (recoveryCommit) regardless of justification
754759
});
755760

756761
it("Should correctly compute hash for normal flow", async () => {

0 commit comments

Comments
 (0)