Add ERC721EnumerableComponent extension #983
Conversation
…eat/add-hooks-#965
There was a problem hiding this comment.
Sorry for the delay @andrew-fleming. I reviewed the component and it looks great. I think we can go ahead with this version (we may use Vec in the future if we find it worth it). I didn't review the documentation and tests yet, but that's on my priorities.
| let zero_address = Zero::zero(); | ||
|
|
||
| if previous_owner == zero_address { | ||
| PrivateImpl::_add_token_to_all_tokens_enumeration(ref self, token_id); |
There was a problem hiding this comment.
Can't we use self instead of PrivateImpl here?
packages/token/src/erc721/extensions/erc721_enumerable/erc721_enumerable.cairo
Outdated
Show resolved
Hide resolved
packages/token/src/erc721/extensions/erc721_enumerable/erc721_enumerable.cairo
Show resolved
Hide resolved
packages/token/src/erc721/extensions/erc721_enumerable/erc721_enumerable.cairo
Show resolved
Hide resolved
Co-authored-by: Eric Nordelo <eric.nordelo39@gmail.com>
There was a problem hiding this comment.
We are very close @andrew-fleming. I left a suggestion for consistency.
packages/token/src/erc721/extensions/erc721_enumerable/erc721_enumerable.cairo
Show resolved
Hide resolved
Co-authored-by: Eric Nordelo <eric.nordelo39@gmail.com>
| auth: ContractAddress | ||
| ) { | ||
| let mut contract_state = ERC721Component::HasComponent::get_contract_mut(ref self); | ||
| contract_state.erc721_enumerable.before_update(to, token_id); |
There was a problem hiding this comment.
Could you check if self.get_contract_mut().erc721_enumerable.before_update(to, token_id); will do the job?
There was a problem hiding this comment.
The compiler yells that the ref arg must be a variable. Keeping it in one line would be much better though
error: ref argument must be a variable.
self.get_contract_mut().erc721_enumerable.before_update(to, token_id);
^***************************************^
Co-authored-by: immrsd <103599616+immrsd@users.noreply.github.com>
Fixes #722.
Deps:
PR Checklist