diff --git a/contracts/src/arbitration/dispute-kits/DisputeKitGatedShutter.sol b/contracts/src/arbitration/dispute-kits/DisputeKitGatedShutter.sol index dbcde618c..f65564fed 100644 --- a/contracts/src/arbitration/dispute-kits/DisputeKitGatedShutter.sol +++ b/contracts/src/arbitration/dispute-kits/DisputeKitGatedShutter.sol @@ -107,15 +107,15 @@ 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 _justificationHash Hash of the justification of the choice. /// @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 _justificationHash, bytes32 _identity, bytes calldata _encryptedVote ) external notJumped(_coreDisputeID) { @@ -127,10 +127,11 @@ contract DisputeKitGatedShutter is DisputeKitClassicBase { for (uint256 i = 0; i < _voteIDs.length; i++) { recoveryCommitments[localDisputeID][localRoundID][_voteIDs[i]] = _recoveryCommit; } - + // Construct Shutter commit out of recovery commit + justification. + bytes32 commit = keccak256(abi.encode(_recoveryCommit, _justificationHash)); // `_castCommit()` ensures that the caller owns the vote - _castCommit(_coreDisputeID, _voteIDs, _commit); - emit CommitCastShutter(_coreDisputeID, msg.sender, _commit, _recoveryCommit, _identity, _encryptedVote); + _castCommit(_coreDisputeID, _voteIDs, commit); + emit CommitCastShutter(_coreDisputeID, msg.sender, commit, _recoveryCommit, _identity, _encryptedVote); } /// @notice Version of the `castVote` function designed specifically for Shutter. @@ -178,7 +179,7 @@ contract DisputeKitGatedShutter is DisputeKitClassicBase { } else { // Caller is not the juror, hash with `_justification`. bytes32 justificationHash = keccak256(bytes(_justification)); - return keccak256(abi.encode(_choice, _salt, justificationHash)); + return keccak256(abi.encode(keccak256(abi.encodePacked(_choice, _salt)), justificationHash)); } } diff --git a/contracts/src/arbitration/dispute-kits/DisputeKitShutter.sol b/contracts/src/arbitration/dispute-kits/DisputeKitShutter.sol index 0ea8bdb41..0fcb3ac23 100644 --- a/contracts/src/arbitration/dispute-kits/DisputeKitShutter.sol +++ b/contracts/src/arbitration/dispute-kits/DisputeKitShutter.sol @@ -91,15 +91,15 @@ 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 _justification Justification of the choice. /// @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, + string memory _justification, bytes32 _identity, bytes calldata _encryptedVote ) external notJumped(_coreDisputeID) { @@ -111,10 +111,12 @@ contract DisputeKitShutter is DisputeKitClassicBase { for (uint256 i = 0; i < _voteIDs.length; i++) { recoveryCommitments[localDisputeID][localRoundID][_voteIDs[i]] = _recoveryCommit; } - + // Construct Shutter commit out of recovery commit + justification. + bytes32 justificationHash = keccak256(bytes(_justification)); + bytes32 commit = keccak256(abi.encode(_recoveryCommit, justificationHash)); // `_castCommit()` ensures that the caller owns the vote - _castCommit(_coreDisputeID, _voteIDs, _commit); - emit CommitCastShutter(_coreDisputeID, msg.sender, _commit, _recoveryCommit, _identity, _encryptedVote); + _castCommit(_coreDisputeID, _voteIDs, commit); + emit CommitCastShutter(_coreDisputeID, msg.sender, commit, _recoveryCommit, _identity, _encryptedVote); } /// @notice Version of `castVote` function designed specifically for Shutter. @@ -162,7 +164,7 @@ contract DisputeKitShutter is DisputeKitClassicBase { } else { // Caller is not the juror, hash with `_justification`. bytes32 justificationHash = keccak256(bytes(_justification)); - return keccak256(abi.encode(_choice, _salt, justificationHash)); + return keccak256(abi.encode(keccak256(abi.encodePacked(_choice, _salt)), justificationHash)); } } diff --git a/contracts/test/arbitration/dispute-kit-shutter.ts b/contracts/test/arbitration/dispute-kit-shutter.ts index aab7efa58..5852ff1ab 100644 --- a/contracts/test/arbitration/dispute-kit-shutter.ts +++ b/contracts/test/arbitration/dispute-kit-shutter.ts @@ -101,7 +101,7 @@ describe("DisputeKitShutter", async () => { // 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]) + ethers.AbiCoder.defaultAbiCoder().encode(["bytes32", "bytes32"], [recoveryCommit, justificationHash]) ); return { fullCommit, recoveryCommit }; @@ -221,7 +221,7 @@ describe("DisputeKitShutter", async () => { await expect( disputeKitShutter .connect(juror1) - .castCommitShutter(disputeId, voteIDs, fullCommit, recoveryCommit, identity, encryptedVote) + .castCommitShutter(disputeId, voteIDs, recoveryCommit, justification, identity, encryptedVote) ) .to.emit(disputeKitShutter, "CommitCastShutter") .withArgs(disputeId, juror1.address, fullCommit, recoveryCommit, identity, encryptedVote); @@ -243,7 +243,7 @@ describe("DisputeKitShutter", async () => { const { fullCommit: commit1, recoveryCommit: recovery1 } = generateCommitments(1n, 111n, "First justification"); await disputeKitShutter .connect(juror1) - .castCommitShutter(disputeId, voteIDs, commit1, recovery1, identity, encryptedVote); + .castCommitShutter(disputeId, voteIDs, recovery1, "First justification", identity, encryptedVote); // Second commitment (overwrites first) const { fullCommit: commit2, recoveryCommit: recovery2 } = generateCommitments( @@ -253,7 +253,7 @@ describe("DisputeKitShutter", async () => { ); await disputeKitShutter .connect(juror1) - .castCommitShutter(disputeId, voteIDs, commit2, recovery2, identity, encryptedVote); + .castCommitShutter(disputeId, voteIDs, recovery2, "Second justification", identity, encryptedVote); // Verify only the second commitment is stored const localDisputeId = await disputeKitShutter.coreDisputeIDToLocal(disputeId); @@ -269,14 +269,13 @@ describe("DisputeKitShutter", async () => { await advanceToCommitPeriod(disputeId); const voteIDs = await getVoteIDsForJuror(disputeId, juror1); - const { fullCommit } = generateCommitments(choice, salt, justification); await expect( disputeKitShutter.connect(juror1).castCommitShutter( disputeId, voteIDs, - fullCommit, ethers.ZeroHash, // Empty recovery commit + justification, identity, encryptedVote ) @@ -294,7 +293,7 @@ describe("DisputeKitShutter", async () => { await expect( disputeKitShutter .connect(juror1) - .castCommitShutter(disputeId, voteIDs, fullCommit, recoveryCommit, identity, encryptedVote) + .castCommitShutter(disputeId, voteIDs, recoveryCommit, justification, identity, encryptedVote) ).to.be.revertedWithCustomError(disputeKitShutter, "NotCommitPeriod"); }); @@ -310,8 +309,8 @@ describe("DisputeKitShutter", async () => { disputeKitShutter.connect(juror2).castCommitShutter( disputeId, voteIDs, // Using juror1's vote IDs - fullCommit, recoveryCommit, + justification, identity, encryptedVote ) @@ -333,7 +332,7 @@ describe("DisputeKitShutter", async () => { // Juror commits await disputeKitShutter .connect(juror1) - .castCommitShutter(disputeId, voteIDs, fullCommit, recoveryCommit, identity, encryptedVote); + .castCommitShutter(disputeId, voteIDs, recoveryCommit, justification, identity, encryptedVote); await advanceToVotePeriod(disputeId); @@ -360,7 +359,7 @@ describe("DisputeKitShutter", async () => { await disputeKitShutter .connect(juror1) - .castCommitShutter(disputeId, voteIDs, fullCommit, recoveryCommit, identity, encryptedVote); + .castCommitShutter(disputeId, voteIDs, recoveryCommit, justification, identity, encryptedVote); await advanceToVotePeriod(disputeId); @@ -386,7 +385,7 @@ describe("DisputeKitShutter", async () => { await disputeKitShutter .connect(juror1) - .castCommitShutter(disputeId, voteIDs, fullCommit, recoveryCommit, identity, encryptedVote); + .castCommitShutter(disputeId, voteIDs, recoveryCommit, justification, identity, encryptedVote); await advanceToVotePeriod(disputeId); @@ -412,7 +411,7 @@ describe("DisputeKitShutter", async () => { await disputeKitShutter .connect(juror1) - .castCommitShutter(disputeId, voteIDs, fullCommit, recoveryCommit, identity, encryptedVote); + .castCommitShutter(disputeId, voteIDs, recoveryCommit, justification, identity, encryptedVote); await advanceToVotePeriod(disputeId); @@ -438,7 +437,7 @@ describe("DisputeKitShutter", async () => { await disputeKitShutter .connect(juror1) - .castCommitShutter(disputeId, voteIDs, fullCommit, recoveryCommit, identity, encryptedVote); + .castCommitShutter(disputeId, voteIDs, recoveryCommit, justification, identity, encryptedVote); await advanceToVotePeriod(disputeId); @@ -466,7 +465,7 @@ describe("DisputeKitShutter", async () => { // Juror commits await disputeKitShutter .connect(juror1) - .castCommitShutter(disputeId, voteIDs, fullCommit, recoveryCommit, identity, encryptedVote); + .castCommitShutter(disputeId, voteIDs, recoveryCommit, justification, identity, encryptedVote); await advanceToVotePeriod(disputeId); @@ -500,7 +499,7 @@ describe("DisputeKitShutter", async () => { await disputeKitShutter .connect(juror1) - .castCommitShutter(disputeId, voteIDs, fullCommit, recoveryCommit, identity, encryptedVote); + .castCommitShutter(disputeId, voteIDs, recoveryCommit, justification, identity, encryptedVote); await advanceToVotePeriod(disputeId); @@ -529,7 +528,7 @@ describe("DisputeKitShutter", async () => { await disputeKitShutter .connect(juror1) - .castCommitShutter(disputeId, voteIDs, fullCommit, recoveryCommit, identity, encryptedVote); + .castCommitShutter(disputeId, voteIDs, recoveryCommit, justification, identity, encryptedVote); await advanceToVotePeriod(disputeId); @@ -555,7 +554,7 @@ describe("DisputeKitShutter", async () => { await disputeKitShutter .connect(juror1) - .castCommitShutter(disputeId, voteIDs, fullCommit, recoveryCommit, identity, encryptedVote); + .castCommitShutter(disputeId, voteIDs, recoveryCommit, justification, identity, encryptedVote); await advanceToVotePeriod(disputeId); @@ -581,7 +580,7 @@ describe("DisputeKitShutter", async () => { await disputeKitShutter .connect(juror1) - .castCommitShutter(disputeId, voteIDs, fullCommit, recoveryCommit, identity, encryptedVote); + .castCommitShutter(disputeId, voteIDs, recoveryCommit, justification, identity, encryptedVote); await advanceToVotePeriod(disputeId); @@ -609,7 +608,7 @@ describe("DisputeKitShutter", async () => { await disputeKitShutter .connect(juror1) - .castCommitShutter(disputeId, voteIDs, fullCommit, recoveryCommit, identity, encryptedVote); + .castCommitShutter(disputeId, voteIDs, recoveryCommit, justification, identity, encryptedVote); await advanceToVotePeriod(disputeId); @@ -624,8 +623,12 @@ describe("DisputeKitShutter", async () => { it("Should correctly compute hash for normal flow", async () => { // Test hashVote function directly const justificationHash = ethers.keccak256(ethers.toUtf8Bytes(justification)); + const recoveryCommit = ethers.keccak256( + ethers.AbiCoder.defaultAbiCoder().encode(["uint256", "uint256"], [choice, salt]) + ); + const expectedHash = ethers.keccak256( - ethers.AbiCoder.defaultAbiCoder().encode(["uint256", "uint256", "bytes32"], [choice, salt, justificationHash]) + ethers.AbiCoder.defaultAbiCoder().encode(["bytes32", "bytes32"], [recoveryCommit, justificationHash]) ); // When called by non-juror (normal case), should include justification @@ -648,11 +651,11 @@ describe("DisputeKitShutter", async () => { // Both jurors commit await disputeKitShutter .connect(juror1) - .castCommitShutter(disputeId, voteIDsJuror1, commit1, recovery1, identity, encryptedVote); + .castCommitShutter(disputeId, voteIDsJuror1, recovery1, "Juror 1 justification", identity, encryptedVote); await disputeKitShutter .connect(juror2) - .castCommitShutter(disputeId, voteIDsJuror2, commit2, recovery2, identity, encryptedVote); + .castCommitShutter(disputeId, voteIDsJuror2, recovery2, "Juror 2 justification", identity, encryptedVote); await advanceToVotePeriod(disputeId); @@ -693,7 +696,7 @@ describe("DisputeKitShutter", async () => { await disputeKitShutter .connect(juror1) - .castCommitShutter(disputeId, voteIDsJuror1, fullCommit, recoveryCommit, identity, encryptedVote); + .castCommitShutter(disputeId, voteIDsJuror1, recoveryCommit, justification, identity, encryptedVote); // Juror2 commits with a different choice const differentChoice = 2n; @@ -706,7 +709,7 @@ describe("DisputeKitShutter", async () => { await disputeKitShutter .connect(juror2) - .castCommitShutter(disputeId, voteIDsJuror2, commit2, recovery2, identity, encryptedVote); + .castCommitShutter(disputeId, voteIDsJuror2, recovery2, justification, identity, encryptedVote); await advanceToVotePeriod(disputeId);