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

Add a getter for an array of tokens held by an owner #1512

Closed
chebykin opened this issue Nov 23, 2018 · 6 comments

Comments

@chebykin
Copy link

commented Nov 23, 2018

Problem

I want to inherit ERC721Full.sol contract and add an extra getter for an array of tokens held by a given user.

Expected behaviour

In previous versions it was possible by adding an extra function and accessing an internal _ownedTokens mapping:

function tokensOfOwner(address _owner) external view returns (uint256[]) {
    return ownedTokens[_owner];
}

Current behaviour

In version 2.x it is impossible to access a private _ownedTokens mapping.

@frangio

This comment has been minimized.

Copy link
Member

commented Nov 23, 2018

I think we could add an internal function in ERC721Enumerable for this.

function _tokensOfOwner(address owner) internal view returns (uint256[] storage) {
    return _ownedTokens[owner];
}
@chebykin

This comment has been minimized.

Copy link
Author

commented Nov 24, 2018

Thank you! Will wait for this feature.

@nventuro

This comment has been minimized.

Copy link
Member

commented Nov 24, 2018

Isn't this a duplicate of #1102?

@frangio

This comment has been minimized.

Copy link
Member

commented Nov 24, 2018

Hm, that issue is about an external interface, this one is about an internal one for extension purposes.

@Aniket-Engg

This comment has been minimized.

Copy link
Contributor

commented Nov 27, 2018

shouldn't uint256[] storage be uint256[] memory?
@frangio Clear it, i will take this one.

@frangio

This comment has been minimized.

Copy link
Member

commented Nov 27, 2018

@Aniket-Engg I think it should be storage because the function would be returning a pointer to the array that is held in storage. Copying the array to memory would be very expensive unnecessarily IMO.

Aniket-Engg added a commit to Aniket-Engg/zeppelin-solidity that referenced this issue Nov 28, 2018

@frangio frangio closed this in #1522 Dec 5, 2018

frangio added a commit that referenced this issue Dec 5, 2018
getter added for an array of tokens held by an owner (#1522)
* signing prefix added

* Minor improvement

* Tests changed

* Successfully tested

* Minor improvements

* Minor improvements

* Revert "Dangling commas are now required. (#1359)"

This reverts commit a688977.

* updates

* fixes #1404

* approve failing test

* suggested changes done

* ISafeERC20 removed

* conflict fixes

* fixes #1512

* Update test/token/ERC721/ERC721Full.test.js

Co-Authored-By: Aniket-Engg <30843294+Aniket-Engg@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.