Skip to content

Comments

IPN-54 tokenizer adds function to reserve and tokenize at once#164

Open
elmariachi111 wants to merge 2 commits intomainfrom
feature/ipn-54-tokenizer-page-action-to-clone-mint-ipts-from-a-reservation
Open

IPN-54 tokenizer adds function to reserve and tokenize at once#164
elmariachi111 wants to merge 2 commits intomainfrom
feature/ipn-54-tokenizer-page-action-to-clone-mint-ipts-from-a-reservation

Conversation

@elmariachi111
Copy link
Contributor

No description provided.

Signed-off-by: stadolf <stefan@molecule.to>
full test case for reservation issuance

Signed-off-by: stadolf <stefan@molecule.to>
@elmariachi111 elmariachi111 marked this pull request as ready for review August 7, 2024 16:21
function controllerOf(uint256 ipnftId) public view override returns (address) {
//todo: check whether this is safe (or if I can trick myself to be the controller somehow)
//reservations are deleted upon mints, so this imo should be good
if (ipnft.reservations(ipnftId) != address(0)) {
Copy link
Contributor

Choose a reason for hiding this comment

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

is this correct:
return ipnft.reservations(ipnftId) || ipnft.ownerOf(ipnftId)

if the ipnft is minted it will return ipnft.ownerOf(ipnftId) as ipnft.reservations(ipnftId) would be address(0) abd vice versa.

Copy link
Contributor Author

@elmariachi111 elmariachi111 Aug 12, 2024

Choose a reason for hiding this comment

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

I would never assume by default whether a language besides Javascript would automatically cast an (address(0)) to a boolean (true). That code is pretty concise imo. If there's a reservation it returns its initiator, otherwise the holder.

Copy link
Contributor

Choose a reason for hiding this comment

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

yes yes the code is clear, I just wanted to know whether the other optiion is also correct or not

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants