diff --git a/contracts/DisputeManager.sol b/contracts/DisputeManager.sol index eedc3cd76..73661b6f7 100644 --- a/contracts/DisputeManager.sol +++ b/contracts/DisputeManager.sol @@ -211,12 +211,14 @@ contract DisputeManager is Governed { } /** - * @dev Get the hash of encoded message to use as disputeID - * @notice Return the disputeID for a particular receipt - * @param _receipt Receipt returned by index node and submitted by fisherman - * @return Hash of encoded message used as disputeID + * @dev Get the message hash that an indexer used to sign the receipt. + * Encodes a receipt using a domain separator, as described on + * https://github.com/ethereum/EIPs/blob/master/EIPS/eip-712.md#specification. + * @notice Return the message hash used to sign the receipt + * @param _receipt Receipt returned by indexer and submitted by fisherman + * @return Message hash used to sign the receipt */ - function getDisputeID(bytes memory _receipt) public view returns (bytes32) { + function encodeHashReceipt(bytes memory _receipt) public view returns (bytes32) { return keccak256( abi.encodePacked( @@ -291,7 +293,7 @@ contract DisputeManager is Governed { // Make sure the token is the caller of this function require(msg.sender == address(token), "Caller is not the GRT token contract"); - // Create a dispute using the received payload + // Create a dispute using the received attestation _createDispute(_data, _from, _value); return true; @@ -397,17 +399,18 @@ contract DisputeManager is Governed { // Decode attestation Attestation memory attestation = _parseAttestation(_attestationData); - // Obtain the hash of the fully-encoded message, per EIP-712 encoding - bytes memory receipt = abi.encode( - attestation.requestCID, - attestation.responseCID, - attestation.subgraphID - ); - bytes32 disputeID = getDisputeID(receipt); + // Get attestation signer + address indexNode = _recoverAttestationSigner(attestation); - // Obtain the signer of the fully-encoded EIP-712 message hash - // NOTE: The signer of the attestation is the indexNode that served the request - address indexNode = _recover(disputeID, attestation.v, attestation.r, attestation.s); + // Create a disputeID + bytes32 disputeID = keccak256( + abi.encodePacked( + attestation.requestCID, + attestation.responseCID, + attestation.subgraphID, + indexNode + ) + ); // This also validates that index node exists require(staking.hasStake(indexNode), "Dispute has no stake by the index node"); @@ -431,6 +434,25 @@ contract DisputeManager is Governed { ); } + /** + * @dev Recover the signer address of the `_attestation` + * @param _attestation The attestation struct + * @return Signer address + */ + function _recoverAttestationSigner(Attestation memory _attestation) private view returns (address) { + // Obtain the hash of the fully-encoded message, per EIP-712 encoding + bytes memory receipt = abi.encode( + _attestation.requestCID, + _attestation.responseCID, + _attestation.subgraphID + ); + bytes32 messageHash = encodeHashReceipt(receipt); + + // Obtain the signer of the fully-encoded EIP-712 message hash + // NOTE: The signer of the attestation is the indexer that served the request + return _recover(messageHash, _attestation.v, _attestation.r, _attestation.s); + } + /** * @dev Get the running network chain ID * @return The chain ID @@ -455,9 +477,9 @@ contract DisputeManager is Governed { // Decode signature // Signature is expected to be in the order defined in the Attestation struct - uint8 v = toUint8(_data, RECEIPT_SIZE_BYTES); - bytes32 r = toBytes32(_data, RECEIPT_SIZE_BYTES + 1); - bytes32 s = toBytes32(_data, RECEIPT_SIZE_BYTES + 33); + uint8 v = _toUint8(_data, RECEIPT_SIZE_BYTES); + bytes32 r = _toBytes32(_data, RECEIPT_SIZE_BYTES + 1); + bytes32 s = _toBytes32(_data, RECEIPT_SIZE_BYTES + 33); return Attestation(requestCID, responseCID, subgraphID, v, r, s); } @@ -496,7 +518,7 @@ contract DisputeManager is Governed { * @dev Parse a uint8 from `_bytes` starting at offset `_start` * @return uint8 value */ - function toUint8(bytes memory _bytes, uint256 _start) internal pure returns (uint8) { + function _toUint8(bytes memory _bytes, uint256 _start) internal pure returns (uint8) { require(_bytes.length >= (_start + 1), "Bytes: out of bounds"); uint8 tempUint; @@ -511,7 +533,7 @@ contract DisputeManager is Governed { * @dev Parse a bytes32 from `_bytes` starting at offset `_start` * @return bytes32 value */ - function toBytes32(bytes memory _bytes, uint256 _start) internal pure returns (bytes32) { + function _toBytes32(bytes memory _bytes, uint256 _start) internal pure returns (bytes32) { require(_bytes.length >= (_start + 32), "Bytes: out of bounds"); bytes32 tempBytes32; diff --git a/test/disputes.test.js b/test/disputes.test.js index 08cf498e6..caf417f25 100644 --- a/test/disputes.test.js +++ b/test/disputes.test.js @@ -12,9 +12,13 @@ const { defaults } = require('./lib/testHelpers') const MAX_PPM = 1000000 const NON_EXISTING_DISPUTE_ID = '0x0' -contract('Disputes', ([me, other, governor, arbitrator, indexNode, fisherman]) => { +contract('Disputes', ([me, other, governor, arbitrator, indexNode, fisherman, otherIndexNode]) => { beforeEach(async function() { + // Private key for account #4 this.indexNodePrivKey = '0xadd53f9a7e588d003326d1cbf9e4a43c061aadd9bc938c843a79e7b4fd2ad743' + // Private key for account #6 + this.otherIndexNodePrivKey = + '0xe485d098507f54e7733a205420dfddbe58db035fa577fc294ebd14db90767a52' // Deploy epoch contract this.epochManager = await deployment.deployEpochManagerContract(governor, { from: me }) @@ -198,36 +202,38 @@ contract('Disputes', ([me, other, governor, arbitrator, indexNode, fisherman]) = }) describe('dispute lifecycle', function() { - before(async function() { + beforeEach(async function() { // Defaults this.tokensForIndexNode = helpers.graphTokenConstants.tokensMintedForStaker this.tokensForFisherman = helpers.graphTokenConstants.tokensMintedForStaker this.indexNodeStake = this.tokensForIndexNode - // Create a subgraphId - this.subgraphId = helpers.randomSubgraphIdHex0x() + // Create a dispute + const receipt = { + requestCID: web3.utils.randomHex(32), + responseCID: web3.utils.randomHex(32), + subgraphID: helpers.randomSubgraphIdHex0x(), + } + this.dispute = await attestation.createDispute( + receipt, + this.disputeManager.address, + this.indexNodePrivKey, + ) }) - context('when stake does not exist', function() { + context('> when stake does not exist', function() { it('reject create a dispute', async function() { // Give some funds to the fisherman await this.graphToken.mint(fisherman, this.tokensForFisherman, { from: governor, }) - // Get index node signed attestation - const dispute = await attestation.createDisputePayload( - this.subgraphId, - this.disputeManager.address, - this.indexNodePrivKey, - ) - // Create dispute await expectRevert( this.graphToken.transferToTokenReceiver( this.disputeManager.address, this.tokensForFisherman, - dispute.attestation, + this.dispute.attestation, { from: fisherman }, ), 'Dispute has no stake by the index node', @@ -235,26 +241,28 @@ contract('Disputes', ([me, other, governor, arbitrator, indexNode, fisherman]) = }) }) - context('when stake does exist', function() { + context('> when stake does exist', function() { beforeEach(async function() { // Dispute manager is allowed to slash await this.staking.setSlasher(this.disputeManager.address, true, { from: governor, }) - // Give some funds to the indexNode - await this.graphToken.mint(indexNode, this.tokensForIndexNode, { - from: governor, - }) + // Stake + for (const indexer of [indexNode, otherIndexNode]) { + // Give some funds to the indexer + await this.graphToken.mint(indexer, this.tokensForIndexNode, { + from: governor, + }) - // Index node stake funds - const data = '0x00' + this.subgraphId.substring(2) - await this.graphToken.transferToTokenReceiver( - this.staking.address, - this.indexNodeStake, - data, - { from: indexNode }, - ) + // Indexer stake funds + await this.graphToken.transferToTokenReceiver( + this.staking.address, + this.indexNodeStake, + '0x0', + { from: indexer }, + ) + } }) describe('reward calculation', function() { @@ -270,64 +278,59 @@ contract('Disputes', ([me, other, governor, arbitrator, indexNode, fisherman]) = }) }) - describe('create dispute', function() { - beforeEach(async function() { - // Get index node signed attestation - this.dispute = await attestation.createDisputePayload( - this.subgraphId, - this.disputeManager.address, - this.indexNodePrivKey, - ) - }) + context('> when dispute is not created', function() { + describe('create dispute', function() { + it('reject fisherman deposit below minimum required', async function() { + // Give some funds to the fisherman + await this.graphToken.mint(fisherman, this.tokensForFisherman, { + from: governor, + }) - it('reject fisherman deposit below minimum required', async function() { - // Give some funds to the fisherman - await this.graphToken.mint(fisherman, this.tokensForFisherman, { - from: governor, + // Minimum deposit a fisherman is required to do should be >= reward + const minimumDeposit = await this.disputeManager.minimumDeposit() + const belowMinimumDeposit = minimumDeposit.sub(web3.utils.toBN(1)) + + // Create invalid dispute as deposit is below minimum + await expectRevert( + this.graphToken.transferToTokenReceiver( + this.disputeManager.address, + belowMinimumDeposit, + this.dispute.attestation, + { from: fisherman }, + ), + 'Dispute deposit under minimum required', + ) }) - // Minimum deposit a fisherman is required to do should be >= reward - const minimumDeposit = await this.disputeManager.minimumDeposit() - const belowMinimumDeposit = minimumDeposit.sub(web3.utils.toBN(1)) + it('should create a dispute', async function() { + // Give some funds to the fisherman + await this.graphToken.mint(fisherman, this.tokensForFisherman, { + from: governor, + }) - // Create invalid dispute as deposit is below minimum - await expectRevert( - this.graphToken.transferToTokenReceiver( + // Create dispute + const { tx } = await this.graphToken.transferToTokenReceiver( this.disputeManager.address, - belowMinimumDeposit, + this.tokensForFisherman, this.dispute.attestation, { from: fisherman }, - ), - 'Dispute deposit under minimum required', - ) - }) - - it('should create a dispute', async function() { - // Give some funds to the fisherman - await this.graphToken.mint(fisherman, this.tokensForFisherman, { - from: governor, - }) - - // Create dispute - const { tx } = await this.graphToken.transferToTokenReceiver( - this.disputeManager.address, - this.tokensForFisherman, - this.dispute.attestation, - { from: fisherman }, - ) + ) - // Event emitted - expectEvent.inTransaction(tx, this.disputeManager.constructor, 'DisputeCreated', { - disputeID: this.dispute.messageHash, - subgraphID: this.subgraphId, - indexNode: indexNode, - fisherman: fisherman, - tokens: this.tokensForFisherman, - attestation: this.dispute.attestation, + // Event emitted + expectEvent.inTransaction(tx, this.disputeManager.constructor, 'DisputeCreated', { + disputeID: this.dispute.id, + subgraphID: this.dispute.receipt.subgraphID, + indexNode: indexNode, + fisherman: fisherman, + tokens: this.tokensForFisherman, + attestation: this.dispute.attestation, + }) }) }) + }) - it('reject create duplicated dispute', async function() { + context('> when dispute is created', function() { + beforeEach(async function() { // Give some funds to the fisherman await this.graphToken.mint(fisherman, this.tokensForFisherman, { from: governor, @@ -340,46 +343,56 @@ contract('Disputes', ([me, other, governor, arbitrator, indexNode, fisherman]) = this.dispute.attestation, { from: fisherman }, ) + }) - // Give some funds to the fisherman - await this.graphToken.mint(fisherman, this.tokensForFisherman, { - from: governor, - }) + describe('create a dispute', function() { + it('should create dispute if receipt is equal but for different indexer', async function() { + // Give some funds to the fisherman + await this.graphToken.mint(fisherman, this.tokensForFisherman, { + from: governor, + }) - // Create dispute (duplicated) - await expectRevert( - this.graphToken.transferToTokenReceiver( + // Create dispute (same receipt but different indexer) + const newDispute = await attestation.createDispute( + this.dispute.receipt, + this.disputeManager.address, + this.otherIndexNodePrivKey, + ) + const { tx } = await this.graphToken.transferToTokenReceiver( this.disputeManager.address, this.tokensForFisherman, - this.dispute.attestation, + newDispute.attestation, { from: fisherman }, - ), - 'Dispute already created', - ) - }) - }) - - context('when dispute is created', function() { - beforeEach(async function() { - // Get index node signed attestation - this.dispute = await attestation.createDisputePayload( - this.subgraphId, - this.disputeManager.address, - this.indexNodePrivKey, - ) + ) - // Give some funds to the fisherman - await this.graphToken.mint(fisherman, this.tokensForFisherman, { - from: governor, + // Event emitted + expectEvent.inTransaction(tx, this.disputeManager.constructor, 'DisputeCreated', { + disputeID: newDispute.id, + subgraphID: newDispute.receipt.subgraphID, + indexNode: otherIndexNode, + fisherman: fisherman, + tokens: this.tokensForFisherman, + attestation: newDispute.attestation, + }) }) - // Create dispute - await this.graphToken.transferToTokenReceiver( - this.disputeManager.address, - this.tokensForFisherman, - this.dispute.attestation, - { from: fisherman }, - ) + it('reject create duplicated dispute', async function() { + // Give some funds to the fisherman + await this.graphToken.mint(fisherman, this.tokensForFisherman, { + from: governor, + }) + + // Create dispute (duplicated) + await expectRevert( + this.graphToken.transferToTokenReceiver( + this.disputeManager.address, + this.tokensForFisherman, + this.dispute.attestation, + { from: fisherman }, + ), + 'Dispute already created', + ) + }) }) describe('accept a dispute', function() { @@ -394,7 +407,7 @@ contract('Disputes', ([me, other, governor, arbitrator, indexNode, fisherman]) = it('reject to accept a dispute if not the arbitrator', async function() { await expectRevert( - this.disputeManager.acceptDispute(this.dispute.messageHash, { + this.disputeManager.acceptDispute(this.dispute.id, { from: me, }), 'Caller is not the Arbitrator', @@ -409,7 +422,7 @@ contract('Disputes', ([me, other, governor, arbitrator, indexNode, fisherman]) = // Perform transaction (accept) await expectRevert( - this.disputeManager.acceptDispute(this.dispute.messageHash, { + this.disputeManager.acceptDispute(this.dispute.id, { from: arbitrator, }), 'Caller is not a Slasher', @@ -424,7 +437,7 @@ contract('Disputes', ([me, other, governor, arbitrator, indexNode, fisherman]) = const reward = await this.disputeManager.getTokensToReward(indexNode) // Perform transaction (accept) - const { tx } = await this.disputeManager.acceptDispute(this.dispute.messageHash, { + const { tx } = await this.disputeManager.acceptDispute(this.dispute.id, { from: arbitrator, }) @@ -446,8 +459,8 @@ contract('Disputes', ([me, other, governor, arbitrator, indexNode, fisherman]) = // Event emitted expectEvent.inTransaction(tx, this.disputeManager.constructor, 'DisputeAccepted', { - disputeID: this.dispute.messageHash, - subgraphID: this.subgraphId, + disputeID: this.dispute.id, + subgraphID: this.dispute.receipt.subgraphID, indexNode: indexNode, fisherman: fisherman, tokens: deposit.add(reward), @@ -467,7 +480,7 @@ contract('Disputes', ([me, other, governor, arbitrator, indexNode, fisherman]) = it('reject to reject a dispute if not the arbitrator', async function() { await expectRevert( - this.disputeManager.rejectDispute(this.dispute.messageHash, { + this.disputeManager.rejectDispute(this.dispute.id, { from: me, }), 'Caller is not the Arbitrator', @@ -479,7 +492,7 @@ contract('Disputes', ([me, other, governor, arbitrator, indexNode, fisherman]) = const totalSupplyBefore = await this.graphToken.totalSupply() // Perform transaction (reject) - const { tx } = await this.disputeManager.rejectDispute(this.dispute.messageHash, { + const { tx } = await this.disputeManager.rejectDispute(this.dispute.id, { from: arbitrator, }) @@ -494,8 +507,8 @@ contract('Disputes', ([me, other, governor, arbitrator, indexNode, fisherman]) = // Event emitted expectEvent.inTransaction(tx, this.disputeManager.constructor, 'DisputeRejected', { - disputeID: this.dispute.messageHash, - subgraphID: this.subgraphId, + disputeID: this.dispute.id, + subgraphID: this.dispute.receipt.subgraphID, indexNode: indexNode, fisherman: fisherman, tokens: this.tokensForFisherman, @@ -515,7 +528,7 @@ contract('Disputes', ([me, other, governor, arbitrator, indexNode, fisherman]) = it('reject to ignore a dispute if not the arbitrator', async function() { await expectRevert( - this.disputeManager.ignoreDispute(this.dispute.messageHash, { + this.disputeManager.ignoreDispute(this.dispute.id, { from: me, }), 'Caller is not the Arbitrator', @@ -526,7 +539,7 @@ contract('Disputes', ([me, other, governor, arbitrator, indexNode, fisherman]) = const fishermanBalanceBefore = await this.graphToken.balanceOf(fisherman) // Perform transaction (ignore) - const { tx } = await this.disputeManager.ignoreDispute(this.dispute.messageHash, { + const { tx } = await this.disputeManager.ignoreDispute(this.dispute.id, { from: arbitrator, }) @@ -537,8 +550,8 @@ contract('Disputes', ([me, other, governor, arbitrator, indexNode, fisherman]) = // Event emitted expectEvent.inTransaction(tx, this.disputeManager.constructor, 'DisputeIgnored', { - disputeID: this.dispute.messageHash, - subgraphID: this.subgraphId, + disputeID: this.dispute.id, + subgraphID: this.dispute.receipt.subgraphID, indexNode: indexNode, fisherman: fisherman, tokens: this.tokensForFisherman, diff --git a/test/lib/attestation.js b/test/lib/attestation.js index ab917d7ca..fa0c2fffc 100644 --- a/test/lib/attestation.js +++ b/test/lib/attestation.js @@ -1,16 +1,10 @@ const ethers = require('ethers') -function createReceipt(subgraphId) { - const receipt = { - requestCID: web3.utils.randomHex(32), - responseCID: web3.utils.randomHex(32), - subgraphId: subgraphId, - } - +function encodeReceipt(receipt) { // ABI encoded return web3.eth.abi.encodeParameters( ['bytes32', 'bytes32', 'bytes32'], - [receipt.requestCID, receipt.responseCID, receipt.subgraphId], + [receipt.requestCID, receipt.responseCID, receipt.subgraphID], ) } @@ -47,22 +41,29 @@ function createMessage(domainSeparatorHash, receiptHash) { return '0x1901' + domainSeparatorHash.substring(2) + receiptHash.substring(2) } -function createAttestation(receipt, messageSig) { +function createAttestation(encodedReceipt, messageSig) { return ( '0x' + - receipt.substring(2) + // Receipt + encodedReceipt.substring(2) + // Receipt messageSig.substring(2) // Signature ) } -async function createDisputePayload(subgraphId, contractAddress, signer) { +function createDisputeID(receipt, indexer) { + return ethers.utils.solidityKeccak256( + ['bytes32', 'bytes32', 'bytes32', 'address'], + [receipt.requestCID, receipt.responseCID, receipt.subgraphID, indexer], + ) +} + +async function createDispute(receipt, contractAddress, signer) { // Receipt - const receipt = createReceipt(subgraphId) + const encodedReceipt = encodeReceipt(receipt) // Receipt signing to create the attestation const message = createMessage( createDomainSeparatorHash(contractAddress), - createReceiptHash(receipt), + createReceiptHash(encodedReceipt), ) const signingKey = new ethers.utils.SigningKey(signer) @@ -75,18 +76,18 @@ async function createDisputePayload(subgraphId, contractAddress, signer) { signature.s.substring(2) // Attestation bytes: 96 (receipt) + 65 (signature) = 161 - const attestation = createAttestation(receipt, messageSig) + const attestation = createAttestation(encodedReceipt, messageSig) return { + id: createDisputeID(receipt, ethers.utils.computeAddress(signingKey.publicKey)), signer, - subgraphId, attestation, + receipt, message, - messageHash, messageSig, } } module.exports = { - createDisputePayload: createDisputePayload, + createDispute: createDispute, }