diff --git a/contracts/contracts/recipientRegistry/BaseRecipientRegistry.sol b/contracts/contracts/recipientRegistry/BaseRecipientRegistry.sol index dcf66048e..6cd022472 100644 --- a/contracts/contracts/recipientRegistry/BaseRecipientRegistry.sol +++ b/contracts/contracts/recipientRegistry/BaseRecipientRegistry.sol @@ -146,4 +146,19 @@ abstract contract BaseRecipientRegistry is IRecipientRegistry { function getRecipientCount() public view returns(uint256) { return slots.length - removed.length; } + + /** + * @dev Make a unique recipient id for different registries + * @param _registry Recipient registry address + * @param _recipient Recipient address + * @param _metadata Recipient metadata + * @return recipient id + */ + function makeRecipientId(address _registry, address _recipient, string calldata _metadata) + internal + pure + returns(bytes32) + { + return keccak256(abi.encodePacked(_registry, _recipient, _metadata)); + } } diff --git a/contracts/contracts/recipientRegistry/OptimisticRecipientRegistry.sol b/contracts/contracts/recipientRegistry/OptimisticRecipientRegistry.sol index 9b479ebf3..d10afe677 100644 --- a/contracts/contracts/recipientRegistry/OptimisticRecipientRegistry.sol +++ b/contracts/contracts/recipientRegistry/OptimisticRecipientRegistry.sol @@ -97,7 +97,7 @@ contract OptimisticRecipientRegistry is Ownable, BaseRecipientRegistry { { require(_recipient != address(0), 'RecipientRegistry: Recipient address is zero'); require(bytes(_metadata).length != 0, 'RecipientRegistry: Metadata info is empty string'); - bytes32 recipientId = keccak256(abi.encodePacked(_recipient, _metadata)); + bytes32 recipientId = makeRecipientId(address(this), _recipient, _metadata); require(recipients[recipientId].index == 0, 'RecipientRegistry: Recipient already registered'); require(requests[recipientId].submissionTime == 0, 'RecipientRegistry: Request already submitted'); require(msg.value == baseDeposit, 'RecipientRegistry: Incorrect deposit amount'); diff --git a/contracts/contracts/recipientRegistry/PermissionedRecipientRegistry.sol b/contracts/contracts/recipientRegistry/PermissionedRecipientRegistry.sol index 02265f2d5..744c31b6f 100644 --- a/contracts/contracts/recipientRegistry/PermissionedRecipientRegistry.sol +++ b/contracts/contracts/recipientRegistry/PermissionedRecipientRegistry.sol @@ -97,7 +97,7 @@ contract PermissionedRecipientRegistry is Ownable, BaseRecipientRegistry { { require(_recipient != address(0), 'RecipientRegistry: Recipient address is zero'); require(bytes(_metadata).length != 0, 'RecipientRegistry: Metadata info is empty string'); - bytes32 recipientId = keccak256(abi.encodePacked(_recipient, _metadata)); + bytes32 recipientId = makeRecipientId(address(this), _recipient, _metadata); require(recipients[recipientId].index == 0, 'RecipientRegistry: Recipient already registered'); require(requests[recipientId].submissionTime == 0, 'RecipientRegistry: Request already submitted'); require(msg.value == baseDeposit, 'RecipientRegistry: Incorrect deposit amount'); diff --git a/contracts/contracts/recipientRegistry/SimpleRecipientRegistry.sol b/contracts/contracts/recipientRegistry/SimpleRecipientRegistry.sol index 9f37215e8..efa40a093 100644 --- a/contracts/contracts/recipientRegistry/SimpleRecipientRegistry.sol +++ b/contracts/contracts/recipientRegistry/SimpleRecipientRegistry.sol @@ -47,7 +47,7 @@ contract SimpleRecipientRegistry is Ownable, BaseRecipientRegistry { { require(_recipient != address(0), 'RecipientRegistry: Recipient address is zero'); require(bytes(_metadata).length != 0, 'RecipientRegistry: Metadata info is empty string'); - bytes32 recipientId = keccak256(abi.encodePacked(_recipient, _metadata)); + bytes32 recipientId = makeRecipientId(address(this), _recipient, _metadata); uint256 recipientIndex = _addRecipient(recipientId, _recipient); emit RecipientAdded(recipientId, _recipient, _metadata, recipientIndex, block.timestamp); } diff --git a/contracts/tests/recipientRegistry.ts b/contracts/tests/recipientRegistry.ts index 2b1b76c0b..180a8bca6 100644 --- a/contracts/tests/recipientRegistry.ts +++ b/contracts/tests/recipientRegistry.ts @@ -6,7 +6,7 @@ import { keccak256 } from '@ethersproject/solidity' import { gtcrEncode } from '@kleros/gtcr-encoder' import { UNIT, ZERO_ADDRESS } from '../utils/constants' -import { getTxFee } from '../utils/contracts' +import { getTxFee, getEventArg } from '../utils/contracts' import { deployContract } from '../utils/deployment' use(solidity) @@ -18,6 +18,17 @@ async function getCurrentTime(): Promise { return (await provider.getBlock('latest')).timestamp } +function getRecipientId( + registryAddress: string, + address: string, + metadata: string +): string { + return keccak256( + ['address', 'address', 'string'], + [registryAddress, address, metadata] + ) +} + describe('Simple Recipient Registry', () => { const [, deployer, controller, recipient] = provider.getWallets() @@ -69,10 +80,6 @@ describe('Simple Recipient Registry', () => { let metadata: string let recipientId: string - function getRecipientId(address: string, metadata: string): string { - return keccak256(['address', 'string'], [address, metadata]) - } - beforeEach(async () => { await registry.connect(controller).setMaxRecipients(MAX_RECIPIENTS) recipientAddress = recipient.address @@ -81,7 +88,7 @@ describe('Simple Recipient Registry', () => { description: 'Description', imageHash: 'Ipfs imageHash', }) - recipientId = getRecipientId(recipientAddress, metadata) + recipientId = getRecipientId(registry.address, recipientAddress, metadata) }) it('allows owner to add recipient', async () => { @@ -110,6 +117,7 @@ describe('Simple Recipient Registry', () => { const anotherRecipientAddress = '0xd8dA6BF26964aF9D7eEd9e03E53415D37aA96045' const anotherRecipientId = getRecipientId( + registry.address, anotherRecipientAddress, metadata ) @@ -256,9 +264,17 @@ describe('Simple Recipient Registry', () => { // Replace recipients const removedRecipient1 = '0x0000000000000000000000000000000000000001' - const removedRecipient1Id = getRecipientId(removedRecipient1, metadata) + const removedRecipient1Id = getRecipientId( + registry.address, + removedRecipient1, + metadata + ) const removedRecipient2 = '0x0000000000000000000000000000000000000002' - const removedRecipient2Id = getRecipientId(removedRecipient2, metadata) + const removedRecipient2Id = getRecipientId( + registry.address, + removedRecipient2, + metadata + ) await registry.removeRecipient(removedRecipient1Id) await registry.removeRecipient(removedRecipient2Id) const addedRecipient1 = '0xd8dA6BF26964aF9D7eEd9e03E53415D37aA96045' @@ -571,10 +587,6 @@ describe('Optimistic recipient registry', () => { let metadata: string let recipientId: string - function getRecipientId(address: string, metadata: string): string { - return keccak256(['address', 'string'], [address, metadata]) - } - beforeEach(async () => { await registry.connect(controller).setMaxRecipients(MAX_RECIPIENTS) recipientAddress = recipient.address @@ -583,7 +595,7 @@ describe('Optimistic recipient registry', () => { description: 'Description', imageHash: 'Ipfs imageHash', }) - recipientId = getRecipientId(recipientAddress, metadata) + recipientId = getRecipientId(registry.address, recipientAddress, metadata) }) it('allows anyone to submit registration request', async () => { @@ -808,7 +820,11 @@ describe('Optimistic recipient registry', () => { imageHash: 'Ipfs imageHash', }) recipientAddress = `0x000000000000000000000000000000000000${recipientName}` - recipientId = getRecipientId(recipientAddress, metadata) + recipientId = getRecipientId( + registry.address, + recipientAddress, + metadata + ) if (i < MAX_RECIPIENTS) { await registry.addRecipient(recipientAddress, metadata, { value: baseDeposit, @@ -953,5 +969,40 @@ describe('Optimistic recipient registry', () => { ) ).to.equal(ZERO_ADDRESS) }) + + it('creates different recipient id for different recipient registries', async () => { + const txOne = await registry.addRecipient(recipientAddress, metadata, { + value: baseDeposit, + }) + const idOne = await getEventArg( + txOne, + registry, + 'RequestSubmitted', + '_recipientId' + ) + + const anotherRegistry = await deployContract( + deployer, + 'OptimisticRecipientRegistry', + [baseDeposit, challengePeriodDuration, controller.address] + ) + const txTwo = await anotherRegistry.addRecipient( + recipientAddress, + metadata, + { + value: baseDeposit, + } + ) + const idTwo = await getEventArg( + txTwo, + anotherRegistry, + 'RequestSubmitted', + '_recipientId' + ) + + expect(idOne.length).to.be.gt(0) + expect(idTwo.length).to.be.gt(0) + expect(idOne).to.not.eq(idTwo) + }) }) })