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

curator.sol updateRenderer calling initializeWithData rather than setBaseURI? #13

Open
0xTranqui opened this issue Oct 2, 2022 · 3 comments

Comments

@0xTranqui
Copy link
Member

@iainnash have ran into some issues with gas limit errors trying to call the updateRenderer function from a UI. I believe the issue is that updateRenderer in curator.sol calls _updateRenderer, which itself calls intitalizeWithData from DefaultMetadataRenderer.sol RATHER than setBaseURI.

My understanding is that the intialize should get called by the constructor, but updates to it should call setBaseURI instead of initialize. Could be wrong but just throwing out there

@0xTranqui 0xTranqui changed the title curator.sol updateRenderer function not doing what it should? curator.sol updateRenderer calling initializeWithData rather than setBaseURI? Oct 2, 2022
@iainnash
Copy link
Contributor

iainnash commented Oct 2, 2022

This follows the pattern of @ourzora/nft-drops-contracts where the setBaseURI function is called directly on the renderer contract and a new renderer can be optionally initialized when setting the new renderer using an initializing string.

This function matches the same pattern we use. We could change that pattern to use permissioned forwarding to call any method on the renderer but i think it works decently well for drops and having a similar method to update / manage renderers may be ideal.

@0xTranqui
Copy link
Member Author

ok got it. wasn't sure if it was different on this. will defer to u hear since clearly its working great on the drops stuff

@iainnash
Copy link
Contributor

iainnash commented Oct 3, 2022

Either way works, but i feel like consistency may be good here.

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

No branches or pull requests

2 participants