Skip to content
This repository was archived by the owner on Aug 21, 2025. It is now read-only.

Feature/holo 1480 ownership s1#304

Open
Yashiru wants to merge 22 commits intodevelopfrom
feature/HOLO-1480-ownership-s1
Open

Feature/holo 1480 ownership s1#304
Yashiru wants to merge 22 commits intodevelopfrom
feature/HOLO-1480-ownership-s1

Conversation

@Yashiru
Copy link
Copy Markdown
Contributor

@Yashiru Yashiru commented May 10, 2024

Describe Changes

I changed the ownership management in the HolographERC721 and the CountdownERC721.

  • Give ownership to the source contract in the init of the HolographERC721 enforcer
  • Call setOwner in the init of the source contract to set the owner to the one defined in the init payload
  • Override the function (and modifiers) that get the owner in the source contract to make them call the owner() or getOwner function of the enforcer contract

Checklist before requesting a review

  • I have performed a self-review of my code
  • Code styles have been enforced
  • All Hardhat tests are passing
  • All Foundry tests are passing

@Yashiru Yashiru changed the base branch from develop to feature/HOLO-1480-countdown-erc721 May 10, 2024 15:18
@Yashiru Yashiru self-assigned this May 14, 2024
Base automatically changed from feature/HOLO-1480-countdown-erc721 to develop May 15, 2024 03:18
@Yashiru Yashiru marked this pull request as ready for review May 15, 2024 13:10
Copy link
Copy Markdown
Contributor

@alexanderattar alexanderattar left a comment

Choose a reason for hiding this comment

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

I think we will not merge this until we also have the script to fix previously deployed contracts. At the moment though we are also blocked on merging this until the tests are passing successfully @Yashiru so please try to focus on resolving those

@Yashiru
Copy link
Copy Markdown
Contributor Author

Yashiru commented May 16, 2024

All tests are passing now 🎉

Capture d’écran 2024-05-16 à 14 18 35

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants