From 2584d5c09d68d4d484d17e0a909deb6c3f6d1923 Mon Sep 17 00:00:00 2001 From: Satyajeet Kolhapure Date: Sat, 9 Nov 2024 11:45:59 +0000 Subject: [PATCH 1/3] added chain prefix in attestation id generation where it was missing --- contracts/src/AttestationRegistry.sol | 8 +++++++ contracts/src/examples/portals/EASPortal.sol | 4 +--- contracts/src/stdlib/IndexerModuleV2.sol | 3 +-- contracts/test/AttestationRegistry.t.sol | 23 +++++++++++++++++++ .../test/examples/portals/EASPortal.t.sol | 2 +- .../test/mocks/AttestationRegistryMock.sol | 9 ++++++++ contracts/test/stdlib/IndexerModuleV2.t.sol | 5 ++-- 7 files changed, 46 insertions(+), 8 deletions(-) diff --git a/contracts/src/AttestationRegistry.sol b/contracts/src/AttestationRegistry.sol index 5d1e20a1..344da776 100644 --- a/contracts/src/AttestationRegistry.sol +++ b/contracts/src/AttestationRegistry.sol @@ -332,4 +332,12 @@ contract AttestationRegistry is OwnableUpgradeable { // Combine the chain prefix and the ID return bytes32(abi.encode(chainPrefix + id)); } + + /** + * @notice Get the next attestation ID including chain identifier + * @return The next attestation ID + */ + function getNextAttestationId() public view returns (bytes32) { + return generateAttestationId(attestationIdCounter + 1); + } } diff --git a/contracts/src/examples/portals/EASPortal.sol b/contracts/src/examples/portals/EASPortal.sol index f3a2b46e..a7b24139 100644 --- a/contracts/src/examples/portals/EASPortal.sol +++ b/contracts/src/examples/portals/EASPortal.sol @@ -71,9 +71,7 @@ contract EASPortal is AbstractPortal { if (!attestationRegistry.isRegistered(attestationRequest.data.refUID)) { revert ReferenceAttestationNotRegistered(); } - - uint32 attestationIdCounter = attestationRegistry.getAttestationIdCounter(); - bytes32 attestationId = bytes32(abi.encode(attestationIdCounter)); + bytes32 attestationId = attestationRegistry.getNextAttestationId(); AttestationPayload memory relationshipAttestationPayload = AttestationPayload( _relationshipSchemaId, diff --git a/contracts/src/stdlib/IndexerModuleV2.sol b/contracts/src/stdlib/IndexerModuleV2.sol index f5a4c02c..360d65cd 100644 --- a/contracts/src/stdlib/IndexerModuleV2.sol +++ b/contracts/src/stdlib/IndexerModuleV2.sol @@ -192,8 +192,7 @@ contract IndexerModuleV2 is AbstractModuleV2 { AttestationRegistry attestationRegistry = AttestationRegistry(router.getAttestationRegistry()); return Attestation( - // TODO: Consider the chain prefix to generate the attestation ID - bytes32(abi.encode(attestationRegistry.getAttestationIdCounter() + 1)), + attestationRegistry.getNextAttestationId(), attestationPayload.schemaId, bytes32(0), attester, diff --git a/contracts/test/AttestationRegistry.t.sol b/contracts/test/AttestationRegistry.t.sol index 33e177fd..b3b632f0 100644 --- a/contracts/test/AttestationRegistry.t.sol +++ b/contracts/test/AttestationRegistry.t.sol @@ -661,6 +661,29 @@ contract AttestationRegistryTest is Test { assertEq(attestationId, 0x000300000000000000000000000000000000000000000000000000000000000a); } + function test_getNextAttestationId(AttestationPayload memory attestationPayload) public { + vm.assume(attestationPayload.subject.length != 0); + vm.assume(attestationPayload.attestationData.length != 0); + SchemaRegistryMock schemaRegistryMock = SchemaRegistryMock(router.getSchemaRegistry()); + attestationPayload.schemaId = schemaRegistryMock.getIdFromSchemaString("schemaString"); + schemaRegistryMock.createSchema("name", "description", "context", "schemaString"); + bytes32 nextAttestationId = attestationRegistry.getNextAttestationId(); + + bytes32 expectedNextAttestationId = bytes32(abi.encode(initialChainPrefix + 1)); + assertEq(nextAttestationId, expectedNextAttestationId); + + vm.startPrank(portal); + + attestationRegistry.attest(attestationPayload, attester); + + expectedNextAttestationId = bytes32(abi.encode(initialChainPrefix + 2)); + nextAttestationId = attestationRegistry.getNextAttestationId(); + + assertEq(nextAttestationId, expectedNextAttestationId); + + vm.stopPrank(); + } + function _createAttestation( AttestationPayload memory attestationPayload, uint256 id diff --git a/contracts/test/examples/portals/EASPortal.t.sol b/contracts/test/examples/portals/EASPortal.t.sol index 571418d1..081d260b 100644 --- a/contracts/test/examples/portals/EASPortal.t.sol +++ b/contracts/test/examples/portals/EASPortal.t.sol @@ -68,7 +68,7 @@ contract EASPortalTest is Test { emit AttestationRegistered(); easPortal.attest(attestationRequestWithoutRefUID); - bytes32 attestationIdWithoutRefUID = bytes32(abi.encode(1)); + bytes32 attestationIdWithoutRefUID = attestationRegistryMock.getNextAttestationId(); // Create second EAS attestation request with RefUID EASPortal.AttestationRequestData memory attestationRequestDataWithRefUID = EASPortal.AttestationRequestData( diff --git a/contracts/test/mocks/AttestationRegistryMock.sol b/contracts/test/mocks/AttestationRegistryMock.sol index 17439381..eb31afaf 100644 --- a/contracts/test/mocks/AttestationRegistryMock.sol +++ b/contracts/test/mocks/AttestationRegistryMock.sol @@ -84,4 +84,13 @@ contract AttestationRegistryMock { function getAttestation(bytes32 attestationId) public view returns (Attestation memory) { return attestations[attestationId]; } + + function generateAttestationId(uint256 id) internal view returns (bytes32) { + // This is a mock implementation, 1000 is considered as chain prefix + return bytes32(abi.encode(1000 + id)); + } + + function getNextAttestationId() public view returns (bytes32) { + return generateAttestationId(attestationIdCounter + 1); + } } diff --git a/contracts/test/stdlib/IndexerModuleV2.t.sol b/contracts/test/stdlib/IndexerModuleV2.t.sol index 6c40a7ba..8d250fe2 100644 --- a/contracts/test/stdlib/IndexerModuleV2.t.sol +++ b/contracts/test/stdlib/IndexerModuleV2.t.sol @@ -63,10 +63,11 @@ contract IndexerModuleV2Test is Test { function test_run() public { address[] memory modules = new address[](0); + bytes32 expectedAtttestationId = attestationRegistry.getNextAttestationId(); ValidPortalMock validPortal = new ValidPortalMock(modules, address(router)); vm.prank(address(validPortal)); vm.expectEmit({ emitter: address(indexerModule) }); - emit AttestationIndexed(bytes32(abi.encode(3))); + emit AttestationIndexed(expectedAtttestationId); indexerModule.run( payload3, bytes(""), @@ -76,7 +77,7 @@ contract IndexerModuleV2Test is Test { address(validPortal), OperationType.Attest ); - assertEq(indexerModule.getIndexedAttestationStatus(bytes32(abi.encode(3))), true); + assertEq(indexerModule.getIndexedAttestationStatus(expectedAtttestationId), true); } function test_indexAttestation() public { From 4a74554165b0c3ee0508d90c403fe98b7b7623c4 Mon Sep 17 00:00:00 2001 From: Satyajeet Kolhapure Date: Sat, 9 Nov 2024 11:46:45 +0000 Subject: [PATCH 2/3] fixed lint issues --- contracts/src/AttestationRegistry.sol | 2 +- contracts/test/AttestationRegistry.t.sol | 2 +- contracts/test/mocks/AttestationRegistryMock.sol | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/contracts/src/AttestationRegistry.sol b/contracts/src/AttestationRegistry.sol index 344da776..8d49f6b4 100644 --- a/contracts/src/AttestationRegistry.sol +++ b/contracts/src/AttestationRegistry.sol @@ -339,5 +339,5 @@ contract AttestationRegistry is OwnableUpgradeable { */ function getNextAttestationId() public view returns (bytes32) { return generateAttestationId(attestationIdCounter + 1); - } + } } diff --git a/contracts/test/AttestationRegistry.t.sol b/contracts/test/AttestationRegistry.t.sol index b3b632f0..f61f3709 100644 --- a/contracts/test/AttestationRegistry.t.sol +++ b/contracts/test/AttestationRegistry.t.sol @@ -661,7 +661,7 @@ contract AttestationRegistryTest is Test { assertEq(attestationId, 0x000300000000000000000000000000000000000000000000000000000000000a); } - function test_getNextAttestationId(AttestationPayload memory attestationPayload) public { + function test_getNextAttestationId(AttestationPayload memory attestationPayload) public { vm.assume(attestationPayload.subject.length != 0); vm.assume(attestationPayload.attestationData.length != 0); SchemaRegistryMock schemaRegistryMock = SchemaRegistryMock(router.getSchemaRegistry()); diff --git a/contracts/test/mocks/AttestationRegistryMock.sol b/contracts/test/mocks/AttestationRegistryMock.sol index eb31afaf..fbc28757 100644 --- a/contracts/test/mocks/AttestationRegistryMock.sol +++ b/contracts/test/mocks/AttestationRegistryMock.sol @@ -92,5 +92,5 @@ contract AttestationRegistryMock { function getNextAttestationId() public view returns (bytes32) { return generateAttestationId(attestationIdCounter + 1); - } + } } From 13a9f5a36f72fda9d516a56e87919b984169fa1d Mon Sep 17 00:00:00 2001 From: Satyajeet Kolhapure Date: Mon, 11 Nov 2024 22:17:36 +0000 Subject: [PATCH 3/3] corrected typo --- contracts/test/stdlib/IndexerModuleV2.t.sol | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/contracts/test/stdlib/IndexerModuleV2.t.sol b/contracts/test/stdlib/IndexerModuleV2.t.sol index 8d250fe2..d5774bf9 100644 --- a/contracts/test/stdlib/IndexerModuleV2.t.sol +++ b/contracts/test/stdlib/IndexerModuleV2.t.sol @@ -63,11 +63,11 @@ contract IndexerModuleV2Test is Test { function test_run() public { address[] memory modules = new address[](0); - bytes32 expectedAtttestationId = attestationRegistry.getNextAttestationId(); + bytes32 expectedAttestationId = attestationRegistry.getNextAttestationId(); ValidPortalMock validPortal = new ValidPortalMock(modules, address(router)); vm.prank(address(validPortal)); vm.expectEmit({ emitter: address(indexerModule) }); - emit AttestationIndexed(expectedAtttestationId); + emit AttestationIndexed(expectedAttestationId); indexerModule.run( payload3, bytes(""), @@ -77,7 +77,7 @@ contract IndexerModuleV2Test is Test { address(validPortal), OperationType.Attest ); - assertEq(indexerModule.getIndexedAttestationStatus(expectedAtttestationId), true); + assertEq(indexerModule.getIndexedAttestationStatus(expectedAttestationId), true); } function test_indexAttestation() public {