diff --git a/contracts/src/arbitration/dispute-kits/DisputeKitClassicBase.sol b/contracts/src/arbitration/dispute-kits/DisputeKitClassicBase.sol index fa8548a6b..9ed66cdfe 100644 --- a/contracts/src/arbitration/dispute-kits/DisputeKitClassicBase.sol +++ b/contracts/src/arbitration/dispute-kits/DisputeKitClassicBase.sol @@ -334,13 +334,13 @@ abstract contract DisputeKitClassicBase is IDisputeKit, Initializable, UUPSProxi { (uint96 courtID, , , , ) = core.disputes(_coreDisputeID); (, bool hiddenVotes, , , , ) = core.courts(courtID); - bytes32 actualVoteHash = hashVote(_choice, _salt, _justification); + if (hiddenVotes) { + _verifyHiddenVoteCommitments(localDisputeID, localRoundID, _voteIDs, _choice, _justification, _salt); + } // Save the votes. for (uint256 i = 0; i < _voteIDs.length; i++) { if (round.votes[_voteIDs[i]].account != _juror) revert JurorHasToOwnTheVote(); - if (hiddenVotes && _getExpectedVoteHash(localDisputeID, localRoundID, _voteIDs[i]) != actualVoteHash) - revert HashDoesNotMatchHiddenVoteCommitment(); if (round.votes[_voteIDs[i]].voted) revert VoteAlreadyCast(); round.votes[_voteIDs[i]].choice = _choice; round.votes[_voteIDs[i]].voted = true; @@ -711,17 +711,26 @@ abstract contract DisputeKitClassicBase is IDisputeKit, Initializable, UUPSProxi // * Internal * // // ************************************* // - /// @notice Returns the expected vote hash for a given vote. + /// @notice Verifies that revealed choice and justification match the hidden vote commitments. /// @param _localDisputeID The ID of the dispute in the Dispute Kit. /// @param _localRoundID The ID of the round in the Dispute Kit. - /// @param _voteID The ID of the vote. - /// @return The expected vote hash. - function _getExpectedVoteHash( + /// @param _voteIDs The IDs of the votes. + /// @param _choice The choice. + /// @param _justification The justification. + /// @param _salt The salt. + function _verifyHiddenVoteCommitments( uint256 _localDisputeID, uint256 _localRoundID, - uint256 _voteID - ) internal view virtual returns (bytes32) { - return disputes[_localDisputeID].rounds[_localRoundID].votes[_voteID].commit; + uint256[] calldata _voteIDs, + uint256 _choice, + string memory _justification, + uint256 _salt + ) internal view virtual { + bytes32 actualVoteHash = hashVote(_choice, _salt, _justification); + for (uint256 i = 0; i < _voteIDs.length; i++) { + if (disputes[_localDisputeID].rounds[_localRoundID].votes[_voteIDs[i]].commit != actualVoteHash) + revert ChoiceCommitmentMismatch(); + } } /// @notice Checks that the chosen address satisfies certain conditions for being drawn. @@ -764,7 +773,7 @@ abstract contract DisputeKitClassicBase is IDisputeKit, Initializable, UUPSProxi error NotVotePeriod(); error EmptyVoteIDs(); error ChoiceOutOfBounds(); - error HashDoesNotMatchHiddenVoteCommitment(); + error ChoiceCommitmentMismatch(); error VoteAlreadyCast(); error NotAppealPeriod(); error NotAppealPeriodForLoser(); diff --git a/contracts/src/arbitration/dispute-kits/DisputeKitGatedShutter.sol b/contracts/src/arbitration/dispute-kits/DisputeKitGatedShutter.sol index f5835e2a5..38b3a66e3 100644 --- a/contracts/src/arbitration/dispute-kits/DisputeKitGatedShutter.sol +++ b/contracts/src/arbitration/dispute-kits/DisputeKitGatedShutter.sol @@ -37,8 +37,8 @@ contract DisputeKitGatedShutter is DisputeKitClassicBase { // ************************************* // mapping(address token => bool supported) public supportedTokens; // Whether the token is supported or not. - mapping(uint256 localDisputeID => mapping(uint256 localRoundID => mapping(uint256 voteID => bytes32 recoveryCommitment))) - public recoveryCommitments; + mapping(uint256 localDisputeID => mapping(uint256 localRoundID => mapping(uint256 voteID => bytes32 justificationCommitment))) + public justificationCommitments; // ************************************* // // * Transient Storage * // @@ -53,15 +53,15 @@ contract DisputeKitGatedShutter is DisputeKitClassicBase { /// @dev Emitted when a vote is cast. /// @param _coreDisputeID The identifier of the dispute in the Arbitrator contract. /// @param _juror The address of the juror casting the vote commitment. - /// @param _commit The commitment hash. - /// @param _recoveryCommit The commitment hash without the justification. + /// @param _choiceCommit The commitment hash without the justification. + /// @param _justificationCommit The commitment hash for the justification. /// @param _identity The Shutter identity used for encryption. /// @param _encryptedVote The Shutter encrypted vote. event CommitCastShutter( uint256 indexed _coreDisputeID, address indexed _juror, - bytes32 indexed _commit, - bytes32 _recoveryCommit, + bytes32 indexed _choiceCommit, + bytes32 _justificationCommit, bytes32 _identity, bytes _encryptedVote ); @@ -135,30 +135,37 @@ contract DisputeKitGatedShutter is DisputeKitClassicBase { /// /// @param _coreDisputeID The ID of the dispute in Kleros Core. /// @param _voteIDs The IDs of the votes. - /// @param _commit The commitment hash including the justification. - /// @param _recoveryCommit The commitment hash without the justification. + /// @param _choiceCommit The commitment hash without the justification. + /// @param _justificationCommit The commitment hash for justification. /// @param _identity The Shutter identity used for encryption. /// @param _encryptedVote The Shutter encrypted vote. function castCommitShutter( uint256 _coreDisputeID, uint256[] calldata _voteIDs, - bytes32 _commit, - bytes32 _recoveryCommit, + bytes32 _choiceCommit, + bytes32 _justificationCommit, bytes32 _identity, bytes calldata _encryptedVote ) external { - if (_recoveryCommit == bytes32(0)) revert EmptyRecoveryCommit(); + if (_justificationCommit == bytes32(0)) revert EmptyJustificationCommit(); uint256 localDisputeID = coreDisputeIDToLocal[_coreDisputeID]; Dispute storage dispute = disputes[localDisputeID]; uint256 localRoundID = dispute.rounds.length - 1; for (uint256 i = 0; i < _voteIDs.length; i++) { - recoveryCommitments[localDisputeID][localRoundID][_voteIDs[i]] = _recoveryCommit; + justificationCommitments[localDisputeID][localRoundID][_voteIDs[i]] = _justificationCommit; } // `_castCommit()` ensures that the caller owns the vote and that dispute is active - _castCommit(_coreDisputeID, _voteIDs, _commit); - emit CommitCastShutter(_coreDisputeID, msg.sender, _commit, _recoveryCommit, _identity, _encryptedVote); + _castCommit(_coreDisputeID, _voteIDs, _choiceCommit); + emit CommitCastShutter( + _coreDisputeID, + msg.sender, + _choiceCommit, + _justificationCommit, + _identity, + _encryptedVote + ); } /// @notice Version of the `castVote` function designed specifically for Shutter. @@ -190,24 +197,12 @@ contract DisputeKitGatedShutter is DisputeKitClassicBase { // * Public Views * // // ************************************* // - /// @notice Computes the hash of a vote using ABI encoding - /// @param _choice The choice being voted for + /// @notice Computes the hash of a justification using ABI encoding /// @param _salt A random salt for commitment /// @param _justification The justification for the vote - /// @return bytes32 The hash of the encoded vote parameters - function hashVote( - uint256 _choice, - uint256 _salt, - string memory _justification - ) public view override returns (bytes32) { - if (callerIsJuror) { - // Caller is the juror, hash without `_justification` to facilitate recovery. - return keccak256(abi.encodePacked(_choice, _salt)); - } else { - // Caller is not the juror, hash with `_justification`. - bytes32 justificationHash = keccak256(bytes(_justification)); - return keccak256(abi.encode(_choice, _salt, justificationHash)); - } + /// @return bytes32 The hash of the encoded justification + function hashJustification(uint256 _salt, string memory _justification) public pure returns (bytes32) { + return keccak256(abi.encode(_salt, keccak256(bytes(_justification)))); } // ************************************* // @@ -215,15 +210,23 @@ contract DisputeKitGatedShutter is DisputeKitClassicBase { // ************************************* // /// @inheritdoc DisputeKitClassicBase - function _getExpectedVoteHash( + function _verifyHiddenVoteCommitments( uint256 _localDisputeID, uint256 _localRoundID, - uint256 _voteID - ) internal view override returns (bytes32) { - if (callerIsJuror) { - return recoveryCommitments[_localDisputeID][_localRoundID][_voteID]; - } else { - return disputes[_localDisputeID].rounds[_localRoundID].votes[_voteID].commit; + uint256[] calldata _voteIDs, + uint256 _choice, + string memory _justification, + uint256 _salt + ) internal view override { + super._verifyHiddenVoteCommitments(_localDisputeID, _localRoundID, _voteIDs, _choice, _justification, _salt); + + // The juror is allowed to reveal without verifying the justification commitment for recovery purposes. + if (callerIsJuror) return; + + bytes32 actualJustificationHash = hashJustification(_salt, _justification); + for (uint256 i = 0; i < _voteIDs.length; i++) { + if (justificationCommitments[_localDisputeID][_localRoundID][_voteIDs[i]] != actualJustificationHash) + revert JustificationCommitmentMismatch(); } } @@ -283,5 +286,6 @@ contract DisputeKitGatedShutter is DisputeKitClassicBase { // ************************************* // error TokenNotSupported(address tokenGate); - error EmptyRecoveryCommit(); + error EmptyJustificationCommit(); + error JustificationCommitmentMismatch(); } diff --git a/contracts/src/arbitration/dispute-kits/DisputeKitShutter.sol b/contracts/src/arbitration/dispute-kits/DisputeKitShutter.sol index fe2c0eb7b..53f89d895 100644 --- a/contracts/src/arbitration/dispute-kits/DisputeKitShutter.sol +++ b/contracts/src/arbitration/dispute-kits/DisputeKitShutter.sol @@ -18,8 +18,8 @@ contract DisputeKitShutter is DisputeKitClassicBase { // * Storage * // // ************************************* // - mapping(uint256 localDisputeID => mapping(uint256 localRoundID => mapping(uint256 voteID => bytes32 recoveryCommitment))) - public recoveryCommitments; + mapping(uint256 localDisputeID => mapping(uint256 localRoundID => mapping(uint256 voteID => bytes32 justificationCommitment))) + public justificationCommitments; // ************************************* // // * Transient Storage * // @@ -34,15 +34,15 @@ contract DisputeKitShutter is DisputeKitClassicBase { /// @notice Emitted when a vote is cast. /// @param _coreDisputeID The identifier of the dispute in the Arbitrator contract. /// @param _juror The address of the juror casting the vote commitment. - /// @param _commit The commitment hash. - /// @param _recoveryCommit The commitment hash without the justification. + /// @param _choiceCommit The commitment hash without the justification. + /// @param _justificationCommit The commitment hash for the justification. /// @param _identity The Shutter identity used for encryption. /// @param _encryptedVote The Shutter encrypted vote. event CommitCastShutter( uint256 indexed _coreDisputeID, address indexed _juror, - bytes32 indexed _commit, - bytes32 _recoveryCommit, + bytes32 indexed _choiceCommit, + bytes32 _justificationCommit, bytes32 _identity, bytes _encryptedVote ); @@ -91,30 +91,37 @@ contract DisputeKitShutter is DisputeKitClassicBase { /// /// @param _coreDisputeID The ID of the dispute in Kleros Core. /// @param _voteIDs The IDs of the votes. - /// @param _commit The commitment hash including the justification. - /// @param _recoveryCommit The commitment hash without the justification. + /// @param _choiceCommit The commitment hash without the justification. + /// @param _justificationCommit The commitment hash for justification. /// @param _identity The Shutter identity used for encryption. /// @param _encryptedVote The Shutter encrypted vote. function castCommitShutter( uint256 _coreDisputeID, uint256[] calldata _voteIDs, - bytes32 _commit, - bytes32 _recoveryCommit, + bytes32 _choiceCommit, + bytes32 _justificationCommit, bytes32 _identity, bytes calldata _encryptedVote ) external { - if (_recoveryCommit == bytes32(0)) revert EmptyRecoveryCommit(); + if (_justificationCommit == bytes32(0)) revert EmptyJustificationCommit(); uint256 localDisputeID = coreDisputeIDToLocal[_coreDisputeID]; Dispute storage dispute = disputes[localDisputeID]; uint256 localRoundID = dispute.rounds.length - 1; for (uint256 i = 0; i < _voteIDs.length; i++) { - recoveryCommitments[localDisputeID][localRoundID][_voteIDs[i]] = _recoveryCommit; + justificationCommitments[localDisputeID][localRoundID][_voteIDs[i]] = _justificationCommit; } // `_castCommit()` ensures that the caller owns the vote and that dispute is active - _castCommit(_coreDisputeID, _voteIDs, _commit); - emit CommitCastShutter(_coreDisputeID, msg.sender, _commit, _recoveryCommit, _identity, _encryptedVote); + _castCommit(_coreDisputeID, _voteIDs, _choiceCommit); + emit CommitCastShutter( + _coreDisputeID, + msg.sender, + _choiceCommit, + _justificationCommit, + _identity, + _encryptedVote + ); } /// @notice Version of `castVote` function designed specifically for Shutter. @@ -146,44 +153,36 @@ contract DisputeKitShutter is DisputeKitClassicBase { // * Public Views * // // ************************************* // - /// @notice Computes the hash of a vote using ABI encoding - /// @param _choice The choice being voted for + /// @notice Computes the hash of a justification using ABI encoding /// @param _salt A random salt for commitment /// @param _justification The justification for the vote - /// @return bytes32 The hash of the encoded vote parameters - function hashVote( - uint256 _choice, - uint256 _salt, - string memory _justification - ) public view override returns (bytes32) { - if (callerIsJuror) { - // Caller is the juror, hash without `_justification` to facilitate recovery. - return keccak256(abi.encodePacked(_choice, _salt)); - } else { - // Caller is not the juror, hash with `_justification`. - bytes32 justificationHash = keccak256(bytes(_justification)); - return keccak256(abi.encode(_choice, _salt, justificationHash)); - } + /// @return bytes32 The hash of the encoded justification + function hashJustification(uint256 _salt, string memory _justification) public pure returns (bytes32) { + return keccak256(abi.encode(_salt, keccak256(bytes(_justification)))); } // ************************************* // // * Internal * // // ************************************* // - /// @notice Returns the expected vote hash for a given vote. - /// @param _localDisputeID The ID of the dispute in the Dispute Kit. - /// @param _localRoundID The ID of the round in the Dispute Kit. - /// @param _voteID The ID of the vote. - /// @return The expected vote hash. - function _getExpectedVoteHash( + /// @inheritdoc DisputeKitClassicBase + function _verifyHiddenVoteCommitments( uint256 _localDisputeID, uint256 _localRoundID, - uint256 _voteID - ) internal view override returns (bytes32) { - if (callerIsJuror) { - return recoveryCommitments[_localDisputeID][_localRoundID][_voteID]; - } else { - return disputes[_localDisputeID].rounds[_localRoundID].votes[_voteID].commit; + uint256[] calldata _voteIDs, + uint256 _choice, + string memory _justification, + uint256 _salt + ) internal view override { + super._verifyHiddenVoteCommitments(_localDisputeID, _localRoundID, _voteIDs, _choice, _justification, _salt); + + // The juror is allowed to reveal without verifying the justification commitment for recovery purposes. + if (callerIsJuror) return; + + bytes32 actualJustificationHash = hashJustification(_salt, _justification); + for (uint256 i = 0; i < _voteIDs.length; i++) { + if (justificationCommitments[_localDisputeID][_localRoundID][_voteIDs[i]] != actualJustificationHash) + revert JustificationCommitmentMismatch(); } } @@ -191,5 +190,6 @@ contract DisputeKitShutter is DisputeKitClassicBase { // * Errors * // // ************************************* // - error EmptyRecoveryCommit(); + error EmptyJustificationCommit(); + error JustificationCommitmentMismatch(); } diff --git a/contracts/test/arbitration/dispute-kit-gated-shutter.ts b/contracts/test/arbitration/dispute-kit-gated-shutter.ts index 3078c2319..2ea2950fd 100644 --- a/contracts/test/arbitration/dispute-kit-gated-shutter.ts +++ b/contracts/test/arbitration/dispute-kit-gated-shutter.ts @@ -15,7 +15,6 @@ import { testCommitPhase, testNormalFlowBotReveals, testRecoveryFlowJurorReveals, - testHashFunctionBehavior, testEdgeCasesAndSecurity, ShutterTestContext, } from "./helpers/dispute-kit-shutter-common"; @@ -68,7 +67,6 @@ describe("DisputeKitGatedShutter", async () => { testCommitPhase(() => shutterContext); testNormalFlowBotReveals(() => shutterContext); testRecoveryFlowJurorReveals(() => shutterContext); - testHashFunctionBehavior(() => shutterContext); testEdgeCasesAndSecurity(() => shutterContext); }); }); diff --git a/contracts/test/arbitration/dispute-kit-shutter.ts b/contracts/test/arbitration/dispute-kit-shutter.ts index facf932e3..5bfee9ef7 100644 --- a/contracts/test/arbitration/dispute-kit-shutter.ts +++ b/contracts/test/arbitration/dispute-kit-shutter.ts @@ -3,7 +3,6 @@ import { testCommitPhase, testNormalFlowBotReveals, testRecoveryFlowJurorReveals, - testHashFunctionBehavior, testEdgeCasesAndSecurity, ShutterTestContext, } from "./helpers/dispute-kit-shutter-common"; @@ -33,6 +32,5 @@ describe("DisputeKitShutter", async () => { testCommitPhase(() => context); testNormalFlowBotReveals(() => context); testRecoveryFlowJurorReveals(() => context); - testHashFunctionBehavior(() => context); testEdgeCasesAndSecurity(() => context); }); diff --git a/contracts/test/arbitration/helpers/dispute-kit-shutter-common.ts b/contracts/test/arbitration/helpers/dispute-kit-shutter-common.ts index 5bb98e204..699968f9c 100644 --- a/contracts/test/arbitration/helpers/dispute-kit-shutter-common.ts +++ b/contracts/test/arbitration/helpers/dispute-kit-shutter-common.ts @@ -71,20 +71,20 @@ const thousandPNK = (amount: BigNumberish) => toBigInt(amount) * ONE_THOUSAND_PN export const encodeExtraData = (courtId: BigNumberish, minJurors: BigNumberish, disputeKitId: number) => ethers.AbiCoder.defaultAbiCoder().encode(["uint256", "uint256", "uint256"], [courtId, minJurors, disputeKitId]); -// Helper function to generate full and recovery commitments +// Helper function to generate choice and justification commitments export const generateCommitments = (choice: bigint, salt: bigint, justification: string) => { - // Recovery commitment: hash(choice, salt) - no justification - const recoveryCommit = ethers.keccak256( - ethers.AbiCoder.defaultAbiCoder().encode(["uint256", "uint256"], [choice, salt]) + // Choice commitment: hash(choice, salt) + const justificationHash = ethers.keccak256(ethers.toUtf8Bytes(justification)); + // Justification commitment: hash(salt, justificationHash) + const justificationCommit = ethers.keccak256( + ethers.AbiCoder.defaultAbiCoder().encode(["uint256", "bytes32"], [salt, justificationHash]) ); - // Full commitment: hash(choice, salt, justificationHash) - const justificationHash = ethers.keccak256(ethers.toUtf8Bytes(justification)); - const fullCommit = ethers.keccak256( - ethers.AbiCoder.defaultAbiCoder().encode(["uint256", "uint256", "bytes32"], [choice, salt, justificationHash]) + const choiceCommit = ethers.keccak256( + ethers.AbiCoder.defaultAbiCoder().encode(["uint256", "uint256"], [choice, salt]) ); - return { fullCommit, recoveryCommit }; + return { choiceCommit, justificationCommit }; }; // Helper to create dispute and draw jurors @@ -329,7 +329,7 @@ export async function setupShutterTest(config: ShutterTestConfig): Promise ShutterTestContext) { describe("Commit Phase - castCommitShutter()", () => { describe("Successful commits", () => { - it("Should allow juror to commit vote with recovery commitment", async () => { + it("Should allow juror to commit vote with justification commitment", async () => { const ctx = context(); const disputeId = await createDisputeAndDraw(ctx, ctx.shutterCourtID, 3, ctx.shutterDKID); await advanceToCommitPeriod(ctx, disputeId); @@ -337,20 +337,20 @@ export function testCommitPhase(context: () => ShutterTestContext) { const voteIDs = await getVoteIDsForJuror(ctx, disputeId, ctx.juror1); expect(voteIDs.length).to.be.greaterThan(0); - const { fullCommit, recoveryCommit } = generateCommitments(ctx.choice, ctx.salt, ctx.justification); + const { choiceCommit, justificationCommit } = generateCommitments(ctx.choice, ctx.salt, ctx.justification); await expect( ctx.disputeKit .connect(ctx.juror1) - .castCommitShutter(disputeId, voteIDs, fullCommit, recoveryCommit, ctx.identity, ctx.encryptedVote) + .castCommitShutter(disputeId, voteIDs, choiceCommit, justificationCommit, ctx.identity, ctx.encryptedVote) ) .to.emit(ctx.disputeKit, "CommitCastShutter") - .withArgs(disputeId, ctx.juror1.address, fullCommit, recoveryCommit, ctx.identity, ctx.encryptedVote); + .withArgs(disputeId, ctx.juror1.address, choiceCommit, justificationCommit, ctx.identity, ctx.encryptedVote); // Verify recovery commitment was stored const localDisputeId = await ctx.disputeKit.coreDisputeIDToLocal(disputeId); - const storedRecoveryCommit = await ctx.disputeKit.recoveryCommitments(localDisputeId, 0, voteIDs[0]); - expect(storedRecoveryCommit).to.equal(recoveryCommit); + const storedRecoveryCommit = await ctx.disputeKit.justificationCommitments(localDisputeId, 0, voteIDs[0]); + expect(storedRecoveryCommit).to.equal(justificationCommit); }); it("Should allow juror to update commitment multiple times", async () => { @@ -361,47 +361,51 @@ export function testCommitPhase(context: () => ShutterTestContext) { const voteIDs = await getVoteIDsForJuror(ctx, disputeId, ctx.juror1); // First commitment - const { fullCommit: commit1, recoveryCommit: recovery1 } = generateCommitments(1n, 111n, "First justification"); + const { choiceCommit: commit1, justificationCommit: justification1 } = generateCommitments( + 1n, + 111n, + "First justification" + ); await ctx.disputeKit .connect(ctx.juror1) - .castCommitShutter(disputeId, voteIDs, commit1, recovery1, ctx.identity, ctx.encryptedVote); + .castCommitShutter(disputeId, voteIDs, commit1, justification1, ctx.identity, ctx.encryptedVote); // Second commitment (overwrites first) - const { fullCommit: commit2, recoveryCommit: recovery2 } = generateCommitments( + const { choiceCommit: commit2, justificationCommit: justification2 } = generateCommitments( 2n, 222n, "Second justification" ); await ctx.disputeKit .connect(ctx.juror1) - .castCommitShutter(disputeId, voteIDs, commit2, recovery2, ctx.identity, ctx.encryptedVote); + .castCommitShutter(disputeId, voteIDs, commit2, justification2, ctx.identity, ctx.encryptedVote); // Verify only the second commitment is stored const localDisputeId = await ctx.disputeKit.coreDisputeIDToLocal(disputeId); - const storedRecoveryCommit = await ctx.disputeKit.recoveryCommitments(localDisputeId, 0, voteIDs[0]); - expect(storedRecoveryCommit).to.equal(recovery2); + const storedRecoveryCommit = await ctx.disputeKit.justificationCommitments(localDisputeId, 0, voteIDs[0]); + expect(storedRecoveryCommit).to.equal(justification2); }); }); describe("Failed commits", () => { - it("Should revert if recovery commitment is empty", async () => { + it("Should revert if justification commitment is empty", async () => { const ctx = context(); const disputeId = await createDisputeAndDraw(ctx, ctx.shutterCourtID, 3, ctx.shutterDKID); await advanceToCommitPeriod(ctx, disputeId); const voteIDs = await getVoteIDsForJuror(ctx, disputeId, ctx.juror1); - const { fullCommit } = generateCommitments(ctx.choice, ctx.salt, ctx.justification); + const { choiceCommit } = generateCommitments(ctx.choice, ctx.salt, ctx.justification); await expect( ctx.disputeKit.connect(ctx.juror1).castCommitShutter( disputeId, voteIDs, - fullCommit, - ethers.ZeroHash, // Empty recovery commit + choiceCommit, + ethers.ZeroHash, // Empty justification commit ctx.identity, ctx.encryptedVote ) - ).to.be.revertedWithCustomError(ctx.disputeKit, "EmptyRecoveryCommit"); + ).to.be.revertedWithCustomError(ctx.disputeKit, "EmptyJustificationCommit"); }); it("Should revert if not in commit period", async () => { @@ -410,12 +414,12 @@ export function testCommitPhase(context: () => ShutterTestContext) { // Still in evidence period const voteIDs = await getVoteIDsForJuror(ctx, disputeId, ctx.juror1); - const { fullCommit, recoveryCommit } = generateCommitments(ctx.choice, ctx.salt, ctx.justification); + const { choiceCommit, justificationCommit } = generateCommitments(ctx.choice, ctx.salt, ctx.justification); await expect( ctx.disputeKit .connect(ctx.juror1) - .castCommitShutter(disputeId, voteIDs, fullCommit, recoveryCommit, ctx.identity, ctx.encryptedVote) + .castCommitShutter(disputeId, voteIDs, choiceCommit, justificationCommit, ctx.identity, ctx.encryptedVote) ).to.be.revertedWithCustomError(ctx.disputeKit, "NotCommitPeriod"); }); @@ -425,14 +429,14 @@ export function testCommitPhase(context: () => ShutterTestContext) { await advanceToCommitPeriod(ctx, disputeId); const voteIDs = await getVoteIDsForJuror(ctx, disputeId, ctx.juror1); - const { fullCommit, recoveryCommit } = generateCommitments(ctx.choice, ctx.salt, ctx.justification); + const { choiceCommit, justificationCommit } = generateCommitments(ctx.choice, ctx.salt, ctx.justification); await expect( ctx.disputeKit.connect(ctx.juror2).castCommitShutter( disputeId, voteIDs, // Using juror1's vote IDs - fullCommit, - recoveryCommit, + choiceCommit, + justificationCommit, ctx.identity, ctx.encryptedVote ) @@ -451,12 +455,12 @@ export function testNormalFlowBotReveals(context: () => ShutterTestContext) { await advanceToCommitPeriod(ctx, disputeId); const voteIDs = await getVoteIDsForJuror(ctx, disputeId, ctx.juror1); - const { fullCommit, recoveryCommit } = generateCommitments(ctx.choice, ctx.salt, ctx.justification); + const { choiceCommit, justificationCommit } = generateCommitments(ctx.choice, ctx.salt, ctx.justification); // Juror commits await ctx.disputeKit .connect(ctx.juror1) - .castCommitShutter(disputeId, voteIDs, fullCommit, recoveryCommit, ctx.identity, ctx.encryptedVote); + .castCommitShutter(disputeId, voteIDs, choiceCommit, justificationCommit, ctx.identity, ctx.encryptedVote); await advanceToVotePeriod(ctx, disputeId); @@ -481,11 +485,11 @@ export function testNormalFlowBotReveals(context: () => ShutterTestContext) { await advanceToCommitPeriod(ctx, disputeId); const voteIDs = await getVoteIDsForJuror(ctx, disputeId, ctx.juror1); - const { fullCommit, recoveryCommit } = generateCommitments(ctx.choice, ctx.salt, ctx.justification); + const { choiceCommit, justificationCommit } = generateCommitments(ctx.choice, ctx.salt, ctx.justification); await ctx.disputeKit .connect(ctx.juror1) - .castCommitShutter(disputeId, voteIDs, fullCommit, recoveryCommit, ctx.identity, ctx.encryptedVote); + .castCommitShutter(disputeId, voteIDs, choiceCommit, justificationCommit, ctx.identity, ctx.encryptedVote); await advanceToVotePeriod(ctx, disputeId); @@ -498,7 +502,7 @@ export function testNormalFlowBotReveals(context: () => ShutterTestContext) { ctx.salt, ctx.justification ) - ).to.be.revertedWithCustomError(ctx.disputeKit, "HashDoesNotMatchHiddenVoteCommitment"); + ).to.be.revertedWithCustomError(ctx.disputeKit, "ChoiceCommitmentMismatch"); }); it("Should revert if wrong salt provided", async () => { @@ -507,11 +511,11 @@ export function testNormalFlowBotReveals(context: () => ShutterTestContext) { await advanceToCommitPeriod(ctx, disputeId); const voteIDs = await getVoteIDsForJuror(ctx, disputeId, ctx.juror1); - const { fullCommit, recoveryCommit } = generateCommitments(ctx.choice, ctx.salt, ctx.justification); + const { choiceCommit, justificationCommit } = generateCommitments(ctx.choice, ctx.salt, ctx.justification); await ctx.disputeKit .connect(ctx.juror1) - .castCommitShutter(disputeId, voteIDs, fullCommit, recoveryCommit, ctx.identity, ctx.encryptedVote); + .castCommitShutter(disputeId, voteIDs, choiceCommit, justificationCommit, ctx.identity, ctx.encryptedVote); await advanceToVotePeriod(ctx, disputeId); @@ -524,7 +528,7 @@ export function testNormalFlowBotReveals(context: () => ShutterTestContext) { wrongSalt, // Wrong salt ctx.justification ) - ).to.be.revertedWithCustomError(ctx.disputeKit, "HashDoesNotMatchHiddenVoteCommitment"); + ).to.be.revertedWithCustomError(ctx.disputeKit, "ChoiceCommitmentMismatch"); }); it("Should revert if wrong justification provided", async () => { @@ -533,11 +537,11 @@ export function testNormalFlowBotReveals(context: () => ShutterTestContext) { await advanceToCommitPeriod(ctx, disputeId); const voteIDs = await getVoteIDsForJuror(ctx, disputeId, ctx.juror1); - const { fullCommit, recoveryCommit } = generateCommitments(ctx.choice, ctx.salt, ctx.justification); + const { choiceCommit, justificationCommit } = generateCommitments(ctx.choice, ctx.salt, ctx.justification); await ctx.disputeKit .connect(ctx.juror1) - .castCommitShutter(disputeId, voteIDs, fullCommit, recoveryCommit, ctx.identity, ctx.encryptedVote); + .castCommitShutter(disputeId, voteIDs, choiceCommit, justificationCommit, ctx.identity, ctx.encryptedVote); await advanceToVotePeriod(ctx, disputeId); @@ -550,7 +554,7 @@ export function testNormalFlowBotReveals(context: () => ShutterTestContext) { ctx.salt, wrongJustification // Wrong justification ) - ).to.be.revertedWithCustomError(ctx.disputeKit, "HashDoesNotMatchHiddenVoteCommitment"); + ).to.be.revertedWithCustomError(ctx.disputeKit, "JustificationCommitmentMismatch"); }); it("Should revert if vote already cast", async () => { @@ -559,11 +563,11 @@ export function testNormalFlowBotReveals(context: () => ShutterTestContext) { await advanceToCommitPeriod(ctx, disputeId); const voteIDs = await getVoteIDsForJuror(ctx, disputeId, ctx.juror1); - const { fullCommit, recoveryCommit } = generateCommitments(ctx.choice, ctx.salt, ctx.justification); + const { choiceCommit, justificationCommit } = generateCommitments(ctx.choice, ctx.salt, ctx.justification); await ctx.disputeKit .connect(ctx.juror1) - .castCommitShutter(disputeId, voteIDs, fullCommit, recoveryCommit, ctx.identity, ctx.encryptedVote); + .castCommitShutter(disputeId, voteIDs, choiceCommit, justificationCommit, ctx.identity, ctx.encryptedVote); await advanceToVotePeriod(ctx, disputeId); @@ -583,19 +587,19 @@ export function testNormalFlowBotReveals(context: () => ShutterTestContext) { export function testRecoveryFlowJurorReveals(context: () => ShutterTestContext) { describe("Recovery Flow - Juror Reveals", () => { - describe("Successful recovery reveals", () => { + describe("Successful justification reveals", () => { it("Should allow juror to recover vote without justification", async () => { const ctx = context(); const disputeId = await createDisputeAndDraw(ctx, ctx.shutterCourtID, 3, ctx.shutterDKID); await advanceToCommitPeriod(ctx, disputeId); const voteIDs = await getVoteIDsForJuror(ctx, disputeId, ctx.juror1); - const { fullCommit, recoveryCommit } = generateCommitments(ctx.choice, ctx.salt, ctx.justification); + const { choiceCommit, justificationCommit } = generateCommitments(ctx.choice, ctx.salt, ctx.justification); // Juror commits await ctx.disputeKit .connect(ctx.juror1) - .castCommitShutter(disputeId, voteIDs, fullCommit, recoveryCommit, ctx.identity, ctx.encryptedVote); + .castCommitShutter(disputeId, voteIDs, choiceCommit, justificationCommit, ctx.identity, ctx.encryptedVote); await advanceToVotePeriod(ctx, disputeId); @@ -619,17 +623,17 @@ export function testRecoveryFlowJurorReveals(context: () => ShutterTestContext) expect(voteInfo[2]).to.equal(ctx.choice); // choice is at index 2 }); - it("Should validate against recovery commitment when juror reveals", async () => { + it("Should validate against justification commitment when juror reveals", async () => { const ctx = context(); const disputeId = await createDisputeAndDraw(ctx, ctx.shutterCourtID, 3, ctx.shutterDKID); await advanceToCommitPeriod(ctx, disputeId); const voteIDs = await getVoteIDsForJuror(ctx, disputeId, ctx.juror1); - const { fullCommit, recoveryCommit } = generateCommitments(ctx.choice, ctx.salt, ctx.justification); + const { choiceCommit, justificationCommit } = generateCommitments(ctx.choice, ctx.salt, ctx.justification); await ctx.disputeKit .connect(ctx.juror1) - .castCommitShutter(disputeId, voteIDs, fullCommit, recoveryCommit, ctx.identity, ctx.encryptedVote); + .castCommitShutter(disputeId, voteIDs, choiceCommit, justificationCommit, ctx.identity, ctx.encryptedVote); await advanceToVotePeriod(ctx, disputeId); @@ -654,11 +658,11 @@ export function testRecoveryFlowJurorReveals(context: () => ShutterTestContext) await advanceToCommitPeriod(ctx, disputeId); const voteIDs = await getVoteIDsForJuror(ctx, disputeId, ctx.juror1); - const { fullCommit, recoveryCommit } = generateCommitments(ctx.choice, ctx.salt, ctx.justification); + const { choiceCommit, justificationCommit } = generateCommitments(ctx.choice, ctx.salt, ctx.justification); await ctx.disputeKit .connect(ctx.juror1) - .castCommitShutter(disputeId, voteIDs, fullCommit, recoveryCommit, ctx.identity, ctx.encryptedVote); + .castCommitShutter(disputeId, voteIDs, choiceCommit, justificationCommit, ctx.identity, ctx.encryptedVote); await advanceToVotePeriod(ctx, disputeId); @@ -671,7 +675,7 @@ export function testRecoveryFlowJurorReveals(context: () => ShutterTestContext) ctx.salt, "" ) - ).to.be.revertedWithCustomError(ctx.disputeKit, "HashDoesNotMatchHiddenVoteCommitment"); + ).to.be.revertedWithCustomError(ctx.disputeKit, "ChoiceCommitmentMismatch"); }); it("Should revert if wrong salt in recovery", async () => { @@ -680,11 +684,11 @@ export function testRecoveryFlowJurorReveals(context: () => ShutterTestContext) await advanceToCommitPeriod(ctx, disputeId); const voteIDs = await getVoteIDsForJuror(ctx, disputeId, ctx.juror1); - const { fullCommit, recoveryCommit } = generateCommitments(ctx.choice, ctx.salt, ctx.justification); + const { choiceCommit, justificationCommit } = generateCommitments(ctx.choice, ctx.salt, ctx.justification); await ctx.disputeKit .connect(ctx.juror1) - .castCommitShutter(disputeId, voteIDs, fullCommit, recoveryCommit, ctx.identity, ctx.encryptedVote); + .castCommitShutter(disputeId, voteIDs, choiceCommit, justificationCommit, ctx.identity, ctx.encryptedVote); await advanceToVotePeriod(ctx, disputeId); @@ -697,7 +701,7 @@ export function testRecoveryFlowJurorReveals(context: () => ShutterTestContext) wrongSalt, // Wrong salt "" ) - ).to.be.revertedWithCustomError(ctx.disputeKit, "HashDoesNotMatchHiddenVoteCommitment"); + ).to.be.revertedWithCustomError(ctx.disputeKit, "ChoiceCommitmentMismatch"); }); it("Should revert if non-juror tries to reveal without correct full commitment", async () => { @@ -706,11 +710,11 @@ export function testRecoveryFlowJurorReveals(context: () => ShutterTestContext) await advanceToCommitPeriod(ctx, disputeId); const voteIDs = await getVoteIDsForJuror(ctx, disputeId, ctx.juror1); - const { fullCommit, recoveryCommit } = generateCommitments(ctx.choice, ctx.salt, ctx.justification); + const { choiceCommit, justificationCommit } = generateCommitments(ctx.choice, ctx.salt, ctx.justification); await ctx.disputeKit .connect(ctx.juror1) - .castCommitShutter(disputeId, voteIDs, fullCommit, recoveryCommit, ctx.identity, ctx.encryptedVote); + .castCommitShutter(disputeId, voteIDs, choiceCommit, justificationCommit, ctx.identity, ctx.encryptedVote); await advanceToVotePeriod(ctx, disputeId); @@ -723,59 +727,12 @@ export function testRecoveryFlowJurorReveals(context: () => ShutterTestContext) ctx.salt, "" // No justification - would work for juror but not for others ) - ).to.be.revertedWithCustomError(ctx.disputeKit, "HashDoesNotMatchHiddenVoteCommitment"); + ).to.be.revertedWithCustomError(ctx.disputeKit, "JustificationCommitmentMismatch"); }); }); }); } -export function testHashFunctionBehavior(context: () => ShutterTestContext) { - describe("Hash Function Behavior", () => { - it("Should compute different hashes for juror recovery vs non-juror normal flow", async () => { - const ctx = context(); - - // Test 1: Verify hashVote matches generateCommitments for non-juror case - const { fullCommit, recoveryCommit } = generateCommitments(ctx.choice, ctx.salt, ctx.justification); - const nonJurorHash = await ctx.disputeKit.hashVote(ctx.choice, ctx.salt, ctx.justification); - expect(nonJurorHash).to.equal(fullCommit, "Non-juror hash should match full commitment"); - - // Test 2: Verify the two commitment types are different - expect(fullCommit).to.not.equal(recoveryCommit, "Full and recovery commitments should differ"); - - // Test 3: Calculate what the juror hash would be and verify it matches recovery commitment - const jurorExpectedHash = ethers.keccak256( - ethers.AbiCoder.defaultAbiCoder().encode(["uint256", "uint256"], [ctx.choice, ctx.salt]) - ); - expect(jurorExpectedHash).to.equal(recoveryCommit, "Juror hash calculation should match recovery commitment"); - - // Test 4: Verify that changing justification affects non-juror hash but not juror hash - const differentJustification = "Different justification"; - const { fullCommit: newFullCommit } = generateCommitments(ctx.choice, ctx.salt, differentJustification); - const newNonJurorHash = await ctx.disputeKit.hashVote(ctx.choice, ctx.salt, differentJustification); - - expect(newNonJurorHash).to.equal(newFullCommit, "New non-juror hash should match new full commitment"); - expect(newNonJurorHash).to.not.equal(nonJurorHash, "Non-juror hash should change with justification"); - // Note: juror hash would remain the same (recoveryCommit) regardless of justification - }); - - it("Should correctly compute hash for normal flow", async () => { - const ctx = context(); - // Test hashVote function directly - const justificationHash = ethers.keccak256(ethers.toUtf8Bytes(ctx.justification)); - const expectedHash = ethers.keccak256( - ethers.AbiCoder.defaultAbiCoder().encode( - ["uint256", "uint256", "bytes32"], - [ctx.choice, ctx.salt, justificationHash] - ) - ); - - // When called by non-juror (normal case), should include justification - const computedHash = await ctx.disputeKit.hashVote(ctx.choice, ctx.salt, ctx.justification); - expect(computedHash).to.equal(expectedHash); - }); - }); -} - export function testEdgeCasesAndSecurity(context: () => ShutterTestContext) { describe("Edge Cases and Security", () => { it("Should handle mixed normal and recovery reveals in same dispute", async () => { @@ -786,17 +743,25 @@ export function testEdgeCasesAndSecurity(context: () => ShutterTestContext) { const voteIDsJuror1 = await getVoteIDsForJuror(ctx, disputeId, ctx.juror1); const voteIDsJuror2 = await getVoteIDsForJuror(ctx, disputeId, ctx.juror2); - const { fullCommit: commit1, recoveryCommit: recovery1 } = generateCommitments(1n, 111n, "Juror 1 justification"); - const { fullCommit: commit2, recoveryCommit: recovery2 } = generateCommitments(2n, 222n, "Juror 2 justification"); + const { choiceCommit: commit1, justificationCommit: justification1 } = generateCommitments( + 1n, + 111n, + "Juror 1 justification" + ); + const { choiceCommit: commit2, justificationCommit: justification2 } = generateCommitments( + 2n, + 222n, + "Juror 2 justification" + ); // Both jurors commit await ctx.disputeKit .connect(ctx.juror1) - .castCommitShutter(disputeId, voteIDsJuror1, commit1, recovery1, ctx.identity, ctx.encryptedVote); + .castCommitShutter(disputeId, voteIDsJuror1, commit1, justification1, ctx.identity, ctx.encryptedVote); await ctx.disputeKit .connect(ctx.juror2) - .castCommitShutter(disputeId, voteIDsJuror2, commit2, recovery2, ctx.identity, ctx.encryptedVote); + .castCommitShutter(disputeId, voteIDsJuror2, commit2, justification2, ctx.identity, ctx.encryptedVote); await advanceToVotePeriod(ctx, disputeId); @@ -834,16 +799,23 @@ export function testEdgeCasesAndSecurity(context: () => ShutterTestContext) { await advanceToCommitPeriod(ctx, disputeId); const voteIDsJuror1 = await getVoteIDsForJuror(ctx, disputeId, ctx.juror1); - const { fullCommit, recoveryCommit } = generateCommitments(ctx.choice, ctx.salt, ctx.justification); + const { choiceCommit, justificationCommit } = generateCommitments(ctx.choice, ctx.salt, ctx.justification); await ctx.disputeKit .connect(ctx.juror1) - .castCommitShutter(disputeId, voteIDsJuror1, fullCommit, recoveryCommit, ctx.identity, ctx.encryptedVote); + .castCommitShutter( + disputeId, + voteIDsJuror1, + choiceCommit, + justificationCommit, + ctx.identity, + ctx.encryptedVote + ); // Juror2 commits with a different choice const differentChoice = 2n; const voteIDsJuror2 = await getVoteIDsForJuror(ctx, disputeId, ctx.juror2); - const { fullCommit: commit2, recoveryCommit: recovery2 } = generateCommitments( + const { choiceCommit: commit2, justificationCommit: justification2 } = generateCommitments( differentChoice, ctx.salt, ctx.justification @@ -851,7 +823,7 @@ export function testEdgeCasesAndSecurity(context: () => ShutterTestContext) { await ctx.disputeKit .connect(ctx.juror2) - .castCommitShutter(disputeId, voteIDsJuror2, commit2, recovery2, ctx.identity, ctx.encryptedVote); + .castCommitShutter(disputeId, voteIDsJuror2, commit2, justification2, ctx.identity, ctx.encryptedVote); await advanceToVotePeriod(ctx, disputeId); @@ -874,7 +846,7 @@ export function testEdgeCasesAndSecurity(context: () => ShutterTestContext) { ctx.salt, ctx.justification ) - ).to.be.revertedWithCustomError(ctx.disputeKit, "HashDoesNotMatchHiddenVoteCommitment"); + ).to.be.revertedWithCustomError(ctx.disputeKit, "ChoiceCommitmentMismatch"); }); }); } diff --git a/contracts/test/foundry/KlerosCore_Voting.t.sol b/contracts/test/foundry/KlerosCore_Voting.t.sol index 689df3a7d..5a7b4c091 100644 --- a/contracts/test/foundry/KlerosCore_Voting.t.sol +++ b/contracts/test/foundry/KlerosCore_Voting.t.sol @@ -114,11 +114,11 @@ contract KlerosCore_VotingTest is KlerosCore_TestBase { // Check the require with the wrong choice and then with the wrong salt vm.prank(staker1); - vm.expectRevert(DisputeKitClassicBase.HashDoesNotMatchHiddenVoteCommitment.selector); + vm.expectRevert(DisputeKitClassicBase.ChoiceCommitmentMismatch.selector); disputeKit.castVote(disputeID, voteIDs, 2, salt, "XYZ"); vm.prank(staker1); - vm.expectRevert(DisputeKitClassicBase.HashDoesNotMatchHiddenVoteCommitment.selector); + vm.expectRevert(DisputeKitClassicBase.ChoiceCommitmentMismatch.selector); disputeKit.castVote(disputeID, voteIDs, YES, salt - 1, "XYZ"); vm.prank(staker1);