remove unnecessary event emitting / improve test cases

Author: nour-karoui

src/IPNFT.sol

@@ -46,7 +46,6 @@ contract IPNFT is ERC721URIStorageUpgradeable, ERC721BurnableUpgradeable, IReser
 
     event Reserved(address indexed reserver, uint256 indexed reservationId);
     event IPNFTMinted(address indexed owner, uint256 indexed tokenId, string tokenURI, string symbol);
-    event IPNFTPOI(uint256 indexed tokenId, bytes poi);
     event ReadAccessGranted(uint256 indexed tokenId, address indexed reader, uint256 until);
     event AuthorizerUpdated(address authorizer);
 
@@ -114,27 +113,14 @@ contract IPNFT is ERC721URIStorageUpgradeable, ERC721BurnableUpgradeable, IReser
      * @param authorization a bytes encoded parameter that ensures that the poi is owned by the owner (to param)
      * @return computedTokenId
      */
-    function mintWithPOI(address to, bytes calldata poi, string calldata _tokenURI, string calldata _symbol, bytes calldata authorization)
+    function mintWithPOI(address to, bytes32 poi, string calldata _tokenURI, string calldata _symbol, bytes calldata authorization)
         external
         payable
         whenNotPaused
         returns (uint256)
     {
-        uint256 computedTokenId = uint256(keccak256(poi));
-        if (msg.value < SYMBOLIC_MINT_FEE) {
-            revert MintingFeeTooLow();
-        }
-
-        if (!mintAuthorizer.authorizeMint(_msgSender(), to, abi.encode(SignedMintAuthorization(computedTokenId, _tokenURI, authorization)))) {
-            revert Unauthorized();
-        }
-
-        mintAuthorizer.redeem(authorization);
-        symbol[computedTokenId] = _symbol;
-        _mint(to, computedTokenId);
-        _setTokenURI(computedTokenId, _tokenURI);
-        emit IPNFTMinted(to, computedTokenId, _tokenURI, _symbol);
-        emit IPNFTPOI(computedTokenId, poi);
+        uint256 computedTokenId = uint256(poi);
+        _handleMint(to, computedTokenId, _tokenURI, _symbol, authorization);
         return computedTokenId;
     }
 
@@ -160,22 +146,27 @@ contract IPNFT is ERC721URIStorageUpgradeable, ERC721BurnableUpgradeable, IReser
             revert NotOwningReservation(reservationId);
         }
 
+     
+        _handleMint(to, reservationId, _tokenURI, _symbol, authorization);
+        delete reservations[reservationId];
+        return reservationId;
+    }
+
+    function _handleMint(address to, uint256 tokenId, string calldata _tokenURI, string calldata _symbol, bytes calldata authorization) internal {
         if (msg.value < SYMBOLIC_MINT_FEE) {
             revert MintingFeeTooLow();
         }
 
-        if (!mintAuthorizer.authorizeMint(_msgSender(), to, abi.encode(SignedMintAuthorization(reservationId, _tokenURI, authorization)))) {
+       if (!mintAuthorizer.authorizeMint(_msgSender(), to, abi.encode(SignedMintAuthorization(tokenId, _tokenURI, authorization)))) {
             revert Unauthorized();
         }
 
-        delete reservations[reservationId];
-        symbol[reservationId] = _symbol;
+        symbol[tokenId] = _symbol;
         mintAuthorizer.redeem(authorization);
 
-        _mint(to, reservationId);
-        _setTokenURI(reservationId, _tokenURI);
-        emit IPNFTMinted(to, reservationId, _tokenURI, _symbol);
-        return reservationId;
+        _mint(to, tokenId);
+        _setTokenURI(tokenId, _tokenURI);
+        emit IPNFTMinted(to, tokenId, _tokenURI, _symbol);
     }
 
     /**

subgraph/schema.graphql

@@ -9,7 +9,6 @@ type Ipnft @entity {
   ipts: [IPT!] @derivedFrom(field: "ipnft")
   metadata: IpnftMetadata
   updatedAtTimestamp: BigInt
-  poi: Bytes
 }
 
 type IpnftMetadata @entity {

subgraph/src/ipnftMapping.ts

@@ -13,8 +13,7 @@ import {
   MetadataUpdate as MetadataUpdateEvent,
   ReadAccessGranted as ReadAccessGrantedEvent,
   Reserved as ReservedEvent,
-  Transfer as TransferEvent,
-  IPNFTPOI as IPNFTPOIEvent
+  Transfer as TransferEvent
 } from '../generated/IPNFT/IPNFT'
 import { IpnftMetadata as IpnftMetadataTemplate } from '../generated/templates'
 import { CanRead, Ipnft, Reservation } from '../generated/schema'
@@ -104,17 +103,6 @@ export function handleMint(event: IPNFTMintedEvent): void {
   ipnft.save()
 }
 
-export function handlePOI(event: IPNFTPOIEvent): void {
-  let ipnft = Ipnft.load(event.params.tokenId.toString())
-  if (!ipnft) {
-    log.error('ipnft {} not found', [event.params.tokenId.toString()])
-    return
-  }
-
-  ipnft.poi = event.params.poi
-  ipnft.save()
-}
-
 export function handleMetadataUpdated(event: MetadataUpdateEvent): void {
   let ipnft = Ipnft.load(event.params._tokenId.toString())
   if (!ipnft) {

subgraph/subgraph.yaml

@@ -23,8 +23,6 @@ dataSources:
           handler: handleReservation
         - event: IPNFTMinted(indexed address,indexed uint256,string,string)
           handler: handleMint
-        - event: IPNFTPOI(indexed uint256,bytes)
-          handler: handlePOI
         - event: Transfer(indexed address,indexed address,indexed uint256)
           handler: handleTransfer
         - event: ReadAccessGranted(indexed uint256,indexed address,uint256)

test/IPNFT.t.sol

@@ -73,23 +73,35 @@ contract IPNFTTest is IPNFTMintHelper {
     }
 
     function testMintWithPoi() public {
-        bytes memory poiHash = "0x5d7ca63d6e6065ef928d3f7cf770062ddd621b84de99732662a71b966db97c3b";
-        uint256 tokenId = uint256(keccak256(poiHash));
-        bytes memory authorization = abi.encode("0xsignature");
+        bytes32 poiHash = 0x073cb54264ef688e56531a2d09ab47b14086b5c7813e3a23a2bd7b1bb6458a52;
+        uint256 tokenId = uint256(poiHash);
+        (, uint256 maliciousSignerPk) = makeAddrAndKey("malicious");
+        bytes32 authMessageHash = ECDSA.toEthSignedMessageHash(keccak256(abi.encodePacked(alice, alice, tokenId, ipfsUri)));
+        (uint8 v, bytes32 r, bytes32 s) = vm.sign(maliciousSignerPk, authMessageHash);
+        bytes memory maliciousAuthorization = abi.encodePacked(r, s, v);
+
         vm.startPrank(deployer);
-        ipnft.setAuthorizer(new AcceptAllAuthorizer());
+        ipnft.setAuthorizer(new SignedMintAuthorizer(deployer));
         vm.stopPrank();
 
         vm.startPrank(alice);
         vm.expectRevert(IPNFT.MintingFeeTooLow.selector);
-        ipnft.mintWithPOI(alice, poiHash, ipfsUri, DEFAULT_SYMBOL, authorization);
+        ipnft.mintWithPOI(alice, poiHash, ipfsUri, DEFAULT_SYMBOL, maliciousAuthorization);
 
+        vm.expectRevert(IPNFT.Unauthorized.selector);
+        ipnft.mintWithPOI{ value: MINTING_FEE }(alice, poiHash, ipfsUri, DEFAULT_SYMBOL, maliciousAuthorization);
+
+        (v, r, s) = vm.sign(deployerPk, authMessageHash);
+        bytes memory authorization = abi.encodePacked(r, s, v);
         vm.expectEmit(true, true, false, true);
         emit IPNFTMinted(alice, tokenId, ipfsUri, DEFAULT_SYMBOL);
-        ipnft.mintWithPOI{ value: MINTING_FEE }(alice, poiHash, ipfsUri, DEFAULT_SYMBOL, authorization);
+        ipnft.mintWithPOI{ value: MINTING_FEE }(
+            alice, poiHash, ipfsUri, DEFAULT_SYMBOL, authorization
+        );
         assertEq(ipnft.ownerOf(tokenId), alice);
         assertEq(ipnft.tokenURI(tokenId), ipfsUri);
         assertEq(ipnft.symbol(tokenId), DEFAULT_SYMBOL);
+        assertEq(tokenId, 3273451770044532981553402679345217193568252544895634663440128735015952812626);
         vm.stopPrank();
     }