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
2 changes: 1 addition & 1 deletion docker-compose.yml
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ services:
postgres_pass: let-me-in
postgres_db: graph-node
ipfs: 'ipfs:5001'
ethereum: 'mainnet:http://anvil:8545'
ethereum: 'foundry:http://anvil:8545'
elmariachi111 marked this conversation as resolved.
Show resolved Hide resolved
GRAPH_LOG: debug
GRAPH_ALLOW_NON_DETERMINISTIC_IPFS: 1
ipfs:
Expand Down
40 changes: 38 additions & 2 deletions src/IPNFT.sol
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ 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);
elmariachi111 marked this conversation as resolved.
Show resolved Hide resolved
event ReadAccessGranted(uint256 indexed tokenId, address indexed reader, uint256 until);
event AuthorizerUpdated(address authorizer);

Expand Down Expand Up @@ -102,9 +103,44 @@ contract IPNFT is ERC721URIStorageUpgradeable, ERC721BurnableUpgradeable, IReser
emit Reserved(_msgSender(), reservationId);
}

/**
* @notice mints an IPNFT with `tokenURI` as source of metadata. This IPNFT is linked a proof of idea (POI) which is a hash of any collection of files that represents an idea, anchored on any chain.
* @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 poi the hash of the poi that will be computed to the tokenId
* @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 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)
elmariachi111 marked this conversation as resolved.
Show resolved Hide resolved
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);
return computedTokenId;
}

/**
* @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 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()`
Expand Down Expand Up @@ -188,7 +224,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 Down
58 changes: 58 additions & 0 deletions subgraph/abis/IPNFT.json
Original file line number Diff line number Diff line change
Expand Up @@ -226,6 +226,45 @@
],
"stateMutability": "payable"
},
{
"type": "function",
"name": "mintWithPOI",
"inputs": [
{
"name": "to",
"type": "address",
"internalType": "address"
},
{
"name": "poi",
"type": "bytes",
"internalType": "bytes"
},
{
"name": "_tokenURI",
"type": "string",
"internalType": "string"
},
{
"name": "_symbol",
"type": "string",
"internalType": "string"
},
{
"name": "authorization",
"type": "bytes",
"internalType": "bytes"
}
],
"outputs": [
{
"name": "",
"type": "uint256",
"internalType": "uint256"
}
],
"stateMutability": "payable"
},
{
"type": "function",
"name": "name",
Expand Down Expand Up @@ -721,6 +760,25 @@
],
"anonymous": false
},
{
"type": "event",
"name": "IPNFTPOI",
"inputs": [
{
"name": "tokenId",
"type": "uint256",
"indexed": true,
"internalType": "uint256"
},
{
"name": "poi",
"type": "bytes",
"indexed": false,
"internalType": "bytes"
}
],
"anonymous": false
},
{
"type": "event",
"name": "Initialized",
Expand Down
1 change: 1 addition & 0 deletions subgraph/schema.graphql
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ type Ipnft @entity {
ipts: [IPT!] @derivedFrom(field: "ipnft")
metadata: IpnftMetadata
updatedAtTimestamp: BigInt
poi: Bytes
}

type IpnftMetadata @entity {
Expand Down
50 changes: 33 additions & 17 deletions subgraph/src/ipnftMapping.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,8 @@ import {
MetadataUpdate as MetadataUpdateEvent,
ReadAccessGranted as ReadAccessGrantedEvent,
Reserved as ReservedEvent,
Transfer as TransferEvent
Transfer as TransferEvent,
IPNFTPOI as IPNFTPOIEvent
} from '../generated/IPNFT/IPNFT'
import { IpnftMetadata as IpnftMetadataTemplate } from '../generated/templates'
import { CanRead, Ipnft, Reservation } from '../generated/schema'
Expand Down Expand Up @@ -75,17 +76,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 +102,17 @@ 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 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 {
Expand All @@ -108,13 +123,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()
}

2 changes: 2 additions & 0 deletions subgraph/subgraph.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,8 @@ 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)
Expand Down
28 changes: 23 additions & 5 deletions test/IPNFT.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,27 @@ contract IPNFTTest is IPNFTMintHelper {
assertEq(ipnft.reservations(2), bob);
}

function testMintWithPoi() public {
bytes memory poiHash = "0x5d7ca63d6e6065ef928d3f7cf770062ddd621b84de99732662a71b966db97c3b";
uint256 tokenId = uint256(keccak256(poiHash));
elmariachi111 marked this conversation as resolved.
Show resolved Hide resolved
bytes memory authorization = abi.encode("0xsignature");
vm.startPrank(deployer);
ipnft.setAuthorizer(new AcceptAllAuthorizer());
vm.stopPrank();

vm.startPrank(alice);
vm.expectRevert(IPNFT.MintingFeeTooLow.selector);
ipnft.mintWithPOI(alice, poiHash, ipfsUri, DEFAULT_SYMBOL, authorization);

vm.expectEmit(true, true, false, true);
emit IPNFTMinted(alice, tokenId, ipfsUri, DEFAULT_SYMBOL);
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);
vm.stopPrank();
}

function testMintFromReservation() public {
vm.startPrank(deployer);
ipnft.setAuthorizer(new SignedMintAuthorizer(deployer));
Expand Down Expand Up @@ -145,7 +166,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 +252,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();
}


}