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
23 changes: 16 additions & 7 deletions src/IPNFT.sol
Original file line number Diff line number Diff line change
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 = _verifyPoi(reservationId);
elmariachi111 marked this conversation as resolved.
Show resolved Hide resolved
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 _verifyPoi(uint256 poi) internal pure returns (bool) {
elmariachi111 marked this conversation as resolved.
Show resolved Hide resolved
return poi > type(uint128).max;
}

/// @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()
}

14 changes: 7 additions & 7 deletions subgraph/subgraph.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ dataSources:
file: ./src/swapMapping.ts
- kind: ethereum/contract
name: Tokenizer
network: foundry
network: mainnet
elmariachi111 marked this conversation as resolved.
Show resolved Hide resolved
source:
abi: Tokenizer
address: "0xB7f8BC63BbcaD18155201308C8f3540b07f84F5e"
Expand All @@ -84,7 +84,7 @@ dataSources:
file: ./src/tokenizerMapping.ts
- kind: ethereum/contract
name: CrowdSale
network: foundry
network: mainnet
source:
abi: CrowdSale
address: "0x7a2088a1bFc9d81c55368AE168C2C02570cB814F"
Expand Down Expand Up @@ -121,7 +121,7 @@ dataSources:
file: ./src/crowdSaleMapping.ts
- kind: ethereum/contract
name: StakedLockingCrowdSale
network: foundry
network: mainnet
source:
abi: StakedLockingCrowdSale
address: "0x0B306BF915C4d645ff596e518fAf3F9669b97016"
Expand Down Expand Up @@ -167,7 +167,7 @@ dataSources:
file: ./src/stakedLockingCrowdSaleMapping.ts
- kind: ethereum/contract
name: TermsAcceptedPermissioner
network: foundry
network: mainnet
source:
abi: TermsAcceptedPermissioner
address: "0x8A791620dd6260079BF849Dc5567aDC3F2FdC318"
Expand All @@ -188,7 +188,7 @@ dataSources:
templates:
- name: IPToken
kind: ethereum/contract
network: foundry
network: mainnet
source:
abi: IPToken
mapping:
Expand All @@ -208,7 +208,7 @@ templates:
handler: handleCapped
- name: TimelockedToken
kind: ethereum/contract
network: foundry
network: mainnet
source:
abi: TimelockedToken
mapping:
Expand Down Expand Up @@ -240,4 +240,4 @@ templates:
abis:
- name: IPNFT
file: ./abis/IPNFT.json
network: foundry
network: mainnet
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 = verifyPoi(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();
}


}
4 changes: 4 additions & 0 deletions test/IPNFTMintHelper.sol
Original file line number Diff line number Diff line change
Expand Up @@ -36,4 +36,8 @@ abstract contract IPNFTMintHelper is Test {
vm.stopPrank();
return reservationId;
}

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