Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

add poi verification before minting function #169

Merged
merged 11 commits into from
Nov 27, 2024
27 changes: 18 additions & 9 deletions src/IPNFT.sol
Original file line number Diff line number Diff line change
Expand Up @@ -21,9 +21,9 @@ import { IReservable } from "./IReservable.sol";
_| ▓▓_| ▓▓ | ▓▓ \▓▓▓▓ ▓▓ | ▓▓
| ▓▓ \ ▓▓ | ▓▓ \▓▓▓ ▓▓ | ▓▓
\▓▓▓▓▓▓\▓▓ \▓▓ \▓▓\▓▓ \▓▓
*/
*/

/// @title IPNFT V2.5
/// @title IPNFT V2.5.1
/// @author molecule.to
/// @notice IP-NFTs capture intellectual property to be traded and synthesized
contract IPNFT is ERC721URIStorageUpgradeable, ERC721BurnableUpgradeable, IReservable, UUPSUpgradeable, OwnableUpgradeable, PausableUpgradeable {
Expand Down Expand Up @@ -103,15 +103,17 @@ contract IPNFT is ERC721URIStorageUpgradeable, ERC721BurnableUpgradeable, IReser
}

/**
* @notice mints an IPNFT with `tokenURI` as source of metadata. Invalidates the reservation. Redeems `mintpassId` on the authorizer contract
* @notice We are charging a nominal fee to symbolically represent the transfer of ownership rights, for a price of .001 ETH (<$2USD at current prices). This helps the ensure the protocol is affordable to almost all projects, but discourages frivolous IP-NFT minting.
* @notice mints an IPNFT with `tokenURI` as source of metadata.
* Minting the IPNFT can happen either with a reservation id or poi hash (Proof of Idea).
* if the tokenId is a reservationId then it invalidates the reservation.
* @notice We are charging a nominal fee to symbolically represent the transfer of ownership rights, for a price of .001 ETH (<$2USD at current prices). This helps ensure the protocol is affordable to almost all projects, but discourages frivolous IP-NFT minting.
*
* @param to the recipient of the NFT
* @param reservationId the reserved token id that has been reserved with `reserve()`
* @param reservationId the reserved token id that has been reserved with `reserve()` / or the poi hash
* @param _tokenURI a location that resolves to a valid IP-NFT metadata structure
* @param _symbol a symbol that represents the IPNFT's derivatives. Can be changed by the owner
* @param authorization a bytes encoded parameter that's handed to the current authorizer
* @return the `reservationId`
* @return the `tokenId`
*/
function mintReservation(address to, uint256 reservationId, string calldata _tokenURI, string calldata _symbol, bytes calldata authorization)
external
Expand All @@ -120,7 +122,8 @@ contract IPNFT is ERC721URIStorageUpgradeable, ERC721BurnableUpgradeable, IReser
whenNotPaused
returns (uint256)
{
if (reservations[reservationId] != _msgSender()) {
bool _isPoi = isPoi(reservationId);
if (!_isPoi && reservations[reservationId] != _msgSender()) {
revert NotOwningReservation(reservationId);
}

Expand All @@ -131,8 +134,10 @@ contract IPNFT is ERC721URIStorageUpgradeable, ERC721BurnableUpgradeable, IReser
if (!mintAuthorizer.authorizeMint(_msgSender(), to, abi.encode(SignedMintAuthorization(reservationId, _tokenURI, authorization)))) {
revert Unauthorized();
}
if (!_isPoi) {
delete reservations[reservationId];
}

delete reservations[reservationId];
symbol[reservationId] = _symbol;
mintAuthorizer.redeem(authorization);

Expand Down Expand Up @@ -188,7 +193,7 @@ contract IPNFT is ERC721URIStorageUpgradeable, ERC721BurnableUpgradeable, IReser
(bool success,) = _msgSender().call{ value: address(this).balance }("");
require(success, "transfer failed");
}

/// @inheritdoc UUPSUpgradeable
function _authorizeUpgrade(address /*newImplementation*/ )
internal
Expand All @@ -201,6 +206,10 @@ contract IPNFT is ERC721URIStorageUpgradeable, ERC721BurnableUpgradeable, IReser
super._burn(tokenId);
}

function isPoi(uint256 poi) public pure returns (bool) {
return poi > type(uint128).max;
}
elmariachi111 marked this conversation as resolved.
Show resolved Hide resolved

/// @inheritdoc ERC721Upgradeable
function tokenURI(uint256 tokenId) public view virtual override(ERC721URIStorageUpgradeable, ERC721Upgradeable) returns (string memory) {
return super.tokenURI(tokenId);
Expand Down
38 changes: 21 additions & 17 deletions subgraph/src/ipnftMapping.ts
Original file line number Diff line number Diff line change
Expand Up @@ -75,17 +75,21 @@ export function handleReservation(event: ReservedEvent): void {
reservation.save()
}

function updateIpnftMetadata(ipnft: Ipnft, uri: string, timestamp: BigInt): void {
let ipfsLocation = uri.replace('ipfs://', '');
if (!ipfsLocation || ipfsLocation == uri) {
log.error("Invalid URI format for tokenId {}: {}", [ipnft.id, uri])
return
}
function updateIpnftMetadata(
ipnft: Ipnft,
uri: string,
timestamp: BigInt
): void {
let ipfsLocation = uri.replace('ipfs://', '')
if (!ipfsLocation || ipfsLocation == uri) {
log.error('Invalid URI format for tokenId {}: {}', [ipnft.id, uri])
return
}

ipnft.tokenURI = uri
ipnft.metadata = ipfsLocation
ipnft.updatedAtTimestamp = timestamp
IpnftMetadataTemplate.create(ipfsLocation)
ipnft.tokenURI = uri
ipnft.metadata = ipfsLocation
ipnft.updatedAtTimestamp = timestamp
IpnftMetadataTemplate.create(ipfsLocation)
}

//the underlying parameter arrays are misaligned, hence we cannot cast or unify both events
Expand All @@ -97,7 +101,6 @@ export function handleMint(event: IPNFTMintedEvent): void {
updateIpnftMetadata(ipnft, event.params.tokenURI, event.block.timestamp)
store.remove('Reservation', event.params.tokenId.toString())
ipnft.save()

}

export function handleMetadataUpdated(event: MetadataUpdateEvent): void {
Expand All @@ -108,13 +111,14 @@ export function handleMetadataUpdated(event: MetadataUpdateEvent): void {
}

//erc4906 is not emitting the new url, we must query it ourselves
let _ipnftContract = IPNFTContract.bind(event.params._event.address);
let _ipnftContract = IPNFTContract.bind(event.params._event.address)
let newUri = _ipnftContract.tokenURI(event.params._tokenId)
if (!newUri || newUri == "") {
log.debug("no new uri found for token, likely just minted {}", [event.params._tokenId.toString()])
return
if (!newUri || newUri == '') {
log.debug('no new uri found for token, likely just minted {}', [
event.params._tokenId.toString()
])
return
}
updateIpnftMetadata(ipnft, newUri, event.block.timestamp)
updateIpnftMetadata(ipnft, newUri, event.block.timestamp)
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Consider adding URI validation before metadata update.

While updateIpnftMetadata handles invalid URIs internally, adding validation before the update would be more efficient and provide clearer error handling.

Consider this improvement:

+  if (!newUri.startsWith('ipfs://')) {
+    log.error('Invalid URI format received in MetadataUpdate for token {}: {}', [
+      event.params._tokenId.toString(),
+      newUri
+    ])
+    return
+  }
   updateIpnftMetadata(ipnft, newUri, event.block.timestamp)
   ipnft.save()

Committable suggestion was skipped due to low confidence.

ipnft.save()
}

38 changes: 31 additions & 7 deletions test/IPNFT.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,35 @@ contract IPNFTTest is IPNFTMintHelper {
assertEq(ipnft.reservations(2), bob);
}

function testVerifyPoi() public {
uint256 tokenId = uint256(0x073cb54264ef688e56531a2d09ab47b14086b5c7813e3a23a2bd7b1bb6458a52);
bool isPoi = ipnft.isPoi(tokenId);
assertEq(isPoi, true);
}

function testMintWithPoi() public {
bytes32 poiHash = 0x073cb54264ef688e56531a2d09ab47b14086b5c7813e3a23a2bd7b1bb6458a52;
uint256 tokenId = uint256(poiHash);
bytes32 authMessageHash = ECDSA.toEthSignedMessageHash(keccak256(abi.encodePacked(alice, alice, tokenId, ipfsUri)));

vm.startPrank(deployer);
ipnft.setAuthorizer(new SignedMintAuthorizer(deployer));
vm.stopPrank();

vm.startPrank(alice);

(uint8 v, bytes32 r, bytes32 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.mintReservation{ value: MINTING_FEE }(alice, tokenId, ipfsUri, DEFAULT_SYMBOL, authorization);
assertEq(ipnft.ownerOf(tokenId), alice);
assertEq(ipnft.tokenURI(tokenId), ipfsUri);
assertEq(ipnft.symbol(tokenId), DEFAULT_SYMBOL);

vm.stopPrank();
}

function testMintFromReservation() public {
vm.startPrank(deployer);
ipnft.setAuthorizer(new SignedMintAuthorizer(deployer));
Expand Down Expand Up @@ -101,8 +130,6 @@ contract IPNFTTest is IPNFTMintHelper {
assertEq(ipnft.tokenURI(1), ipfsUri);
assertEq(ipnft.symbol(reservationId), DEFAULT_SYMBOL);

assertEq(ipnft.reservations(1), address(0));

vm.stopPrank();
}

Expand Down Expand Up @@ -145,7 +172,6 @@ contract IPNFTTest is IPNFTMintHelper {
/**
* ... but when set as heir of a self destruct operation the contract accepts the money.
*/

function testOwnerCanWithdrawEthFunds() public {
vm.deal(address(bob), 10 ether);
vm.startPrank(bob);
Expand Down Expand Up @@ -232,15 +258,13 @@ contract IPNFTTest is IPNFTMintHelper {
//the signoff only allows alice to call this
vm.startPrank(charlie);
vm.expectRevert(IPNFT.Unauthorized.selector);
ipnft.amendMetadata(1, "ipfs://QmNewUri", authorization);
ipnft.amendMetadata(1, "ipfs://QmNewUri", authorization);

vm.startPrank(alice);
vm.expectEmit(true, true, false, false);
emit MetadataUpdate(1);
ipnft.amendMetadata(1, "ipfs://QmNewUri", authorization);
ipnft.amendMetadata(1, "ipfs://QmNewUri", authorization);
assertEq(ipnft.tokenURI(1), "ipfs://QmNewUri");
vm.stopPrank();
}


}