-
Notifications
You must be signed in to change notification settings - Fork 23
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
Move questNFT to a 1155 #141
Conversation
function mint(address to_, string memory questId_) | ||
public | ||
onlyMinter questChecks(questId_) whenNotPaused nonReentrant | ||
{ | ||
QuestData storage quest = quests[questId_]; | ||
|
||
_mint(to_, quest.tokenId, 1, ""); | ||
|
||
(bool success, ) = protocolFeeRecipient.call{value: quest.questFee}(""); | ||
require(success, 'protocol fee transfer failed'); | ||
} |
Check warning
Code scanning / Slither
Low-level calls
contracts/QuestNFT.sol
Outdated
function addQuest( | ||
uint256 questFee_, | ||
uint256 startTime_, | ||
uint256 endTime_, | ||
uint256 totalParticipants_, | ||
string memory questId_, | ||
string memory description_, | ||
string memory imageIPFSHash_ | ||
) public onlyMinter returns (uint256) { | ||
require (endTime_ > block.timestamp, 'endTime_ in the past'); | ||
require (startTime_ > block.timestamp, 'startTime_ in the past'); | ||
require (endTime_ > startTime_, 'startTime_ before endTime_'); | ||
|
||
_tokenIdCounter.increment(); | ||
QuestData storage quest = quests[questId_]; | ||
quest.startTime = startTime_; | ||
quest.endTime = endTime_; | ||
quest.totalParticipants = totalParticipants_; | ||
quest.questFee = questFee_; | ||
quest.imageIPFSHash = imageIPFSHash_; | ||
quest.description = description_; | ||
quest.tokenId = _tokenIdCounter.current(); | ||
tokenIdToQuestId[_tokenIdCounter.current()] = questId_; | ||
|
||
return _tokenIdCounter.current(); | ||
} |
Check notice
Code scanning / Slither
Block timestamp
contracts/QuestNFT.sol
Outdated
@@ -15,73 +16,53 @@ import {ReentrancyGuardUpgradeable} from "@openzeppelin/contracts-upgradeable/se | |||
/// @title QuestNFT | |||
/// @author RabbitHole.gg | |||
/// @notice This contract is the Erc721 Quest Completion contract. It is the NFT that can be minted after a quest is completed. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should update this comment to use 1155
contracts/QuestNFT.sol
Outdated
string memory imageIPFSHash_ | ||
) public onlyMinter returns (uint256) { | ||
require (endTime_ > block.timestamp, 'endTime_ in the past'); | ||
require (startTime_ > block.timestamp, 'startTime_ in the past'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we remove this check? I believe we removed it in Quest.sol
so that we can start quests immediately
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh yes, we removed the start time checks when creating a quest here: 9a3556a
Good point, I'll remove it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Left a couple of comments, but lgtm!
function mint(address to_, string memory questId_) | ||
public | ||
onlyMinter questChecks(questId_) whenNotPaused nonReentrant | ||
{ | ||
QuestData storage quest = quests[questId_]; | ||
|
||
_mint(to_, quest.tokenId, 1, ""); | ||
|
||
(bool success, ) = protocolFeeRecipient.call{value: quest.questFee}(""); | ||
require(success, 'protocol fee transfer failed'); | ||
} |
Check warning
Code scanning / Slither
Unchecked low-level calls
This changes the NFTQuest type from an ERC721 to an ERC1155 type. When an erc1155 is created we call this a collection. A collection can have many quests which map to token ids in the 1155. The flow is this:
Quest creator:
QuestFactory.createCollection
QuestFactory.addQuestToCollection
Quest completer:
QuestFactory.mintQuestNFT
And we have some handy functions for the frontend to use:
QuestFactory.ownerCollectionsByOwner
returns all the collections given an user address. This will be useful because a user can choose to either use an existing collection or create a new one when creating a NFT type quest.