Skip to content

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

Open
elmariachi111 wants to merge 2 commits into
mainfrom
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 into
mainfrom
feature/ipn-54-tokenizer-page-action-to-clone-mint-ipts-from-a-reservation

Conversation

@elmariachi111

Copy link
Copy Markdown
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
Comment thread src/Tokenizer.sol
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
Copy Markdown
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.

@elmariachi111 elmariachi111 Aug 12, 2024

Copy link
Copy Markdown
Contributor Author

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
Copy Markdown
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