feat: use ERC721EnumerableUpgradeable in SBT

Author: Yash Agrawal

.vscode/settings.json

@@ -10,6 +10,6 @@
 		}
 	],
 	"editor.codeActionsOnSave": {
-		"source.fixAll.eslint": true
+		"source.fixAll.eslint": "explicit"
 	}
 }

contracts/SBTToken.sol

@@ -1,141 +1,37 @@
 // SPDX-License-Identifier: MPL-2.0
 pragma solidity =0.8.9;
 
-import {IERC721EnumerableUpgradeable} from "@openzeppelin/contracts-upgradeable/token/ERC721/extensions/ERC721EnumerableUpgradeable.sol";
-import {IERC165Upgradeable} from "@openzeppelin/contracts-upgradeable/utils/introspection/IERC165Upgradeable.sol";
-import {ERC721Upgradeable} from "@openzeppelin/contracts-upgradeable/token/ERC721/ERC721Upgradeable.sol";
+import {Counters} from "@openzeppelin/contracts/utils/Counters.sol";
 import {ProxyAdmin} from "@openzeppelin/contracts/proxy/transparent/ProxyAdmin.sol";
 import {EnumerableSet} from "@openzeppelin/contracts/utils/structs/EnumerableSet.sol";
-import {Counters} from "@openzeppelin/contracts/utils/Counters.sol";
+import {ERC721EnumerableUpgradeable} from "@openzeppelin/contracts-upgradeable/token/ERC721/extensions/ERC721EnumerableUpgradeable.sol";
 
 import {ISBTToken} from "./interfaces/ISBTToken.sol";
 
-contract SBTToken is
-	ISBTToken,
-	IERC721EnumerableUpgradeable,
-	ERC721Upgradeable
-{
-	Counters.Counter private tokenIdCounter;
-
-	mapping(address => EnumerableSet.UintSet) private tokenIdsMapOfOwner;
-	mapping(uint256 => string) private tokenUriImage;
-	address private proxyAdmin;
+contract SBTToken is ISBTToken, ERC721EnumerableUpgradeable {
 	/// @dev EOA with minting rights.
 	address private minter;
-
-	using Counters for Counters.Counter;
-	using EnumerableSet for EnumerableSet.UintSet;
+	/// @dev Account with proxy adming rights.
+	address private proxyAdmin;
+	/// @dev Holds the URI information of a SBT token.
+	mapping(uint256 => string) private tokenUriImage;
 
 	modifier onlyMinter() {
-		require(minter == _msgSender(), "illegal access");
+		require(minter == _msgSender(), "Illegal access");
 		_;
 	}
 
+	event SetProxyAdmin(address _proxyAdmin);
+
 	function initialize(address _minter) external initializer {
 		__ERC721_init("Dev Protocol SBT V1", "DEV-SBT-V1");
 		minter = _minter;
 	}
 
-	/**
-	 * @dev See {IERC165-supportsInterface}.
-	 */
-	function supportsInterface(
-		bytes4 interfaceId
-	)
-		public
-		view
-		virtual
-		override(IERC165Upgradeable, ERC721Upgradeable)
-		returns (bool)
-	{
-		return
-			interfaceId == type(IERC721EnumerableUpgradeable).interfaceId ||
-			super.supportsInterface(interfaceId);
-	}
-
-	/**
-	 * @dev See {IERC721Enumerable-totalSupply}.
-	 */
-	function totalSupply()
-		public
-		view
-		virtual
-		override(ISBTToken, IERC721EnumerableUpgradeable)
-		returns (uint256)
-	{
-		return tokenIdCounter.current();
-	}
-
-	/**
-	 * @dev See {IERC721Enumerable-tokenOfOwnerByIndex}.
-	 */
-	function tokenOfOwnerByIndex(
-		address _owner,
-		uint256 index
-	)
-		public
-		view
-		virtual
-		override(ISBTToken, IERC721EnumerableUpgradeable)
-		returns (uint256)
-	{
-		// solhint-disable-next-line reason-string
-		require(
-			index < tokenIdsMapOfOwner[_owner].length(),
-			"ERC721Enumerable: owner index out of bounds"
-		);
-		return tokenIdsMapOfOwner[_owner].at(index);
-	}
-
-	/**
-	 * @dev See {IERC721Enumerable-tokenByIndex}.
-	 */
-	function tokenByIndex(
-		uint256 index
-	)
-		public
-		view
-		virtual
-		override(ISBTToken, IERC721EnumerableUpgradeable)
-		returns (uint256)
-	{
-		// solhint-disable-next-line reason-string
-		require(
-			index < tokenIdCounter.current(),
-			"ERC721Enumerable: global index out of bounds"
-		);
-		return index + 1;
-	}
-
-	function owner() external view returns (address) {
-		return ProxyAdmin(proxyAdmin).owner();
-	}
-
 	function setProxyAdmin(address _proxyAdmin) external {
-		require(proxyAdmin == address(0), "already set");
+		require(proxyAdmin == address(0), "Already set");
 		proxyAdmin = _proxyAdmin;
-	}
-
-	function tokenURI(
-		uint256 _tokenId
-	) public view override returns (string memory) {
-		uint256 curretnTokenId = tokenIdCounter.current();
-		require(_tokenId <= curretnTokenId, "not found");
-		return _tokenURI(_tokenId);
-	}
-
-	function currentIndex() external view override returns (uint256) {
-		return tokenIdCounter.current();
-	}
-
-	function mint(
-		address _owner
-	) external override onlyMinter returns (uint256 tokenId_) {
-		tokenIdCounter.increment();
-		uint256 currentId = tokenIdCounter.current();
-		_mint(_owner, currentId);
-		emit Minted(currentId, _owner);
-		return currentId;
+		emit SetProxyAdmin(_proxyAdmin);
 	}
 
 	function setTokenURIImage(
@@ -145,14 +41,13 @@ contract SBTToken is
 		tokenUriImage[_tokenId] = _data;
 	}
 
-	function tokensOfOwner(
+	function mint(
 		address _owner
-	) external view override returns (uint256[] memory) {
-		return tokenIdsMapOfOwner[_owner].values();
-	}
-
-	function _tokenURI(uint256 _tokenId) private view returns (string memory) {
-		return tokenUriImage[_tokenId];
+	) external override onlyMinter returns (uint256 tokenId_) {
+		uint256 currentId = this.currentIndex();
+		_mint(_owner, currentId);
+		emit Minted(currentId, _owner);
+		return currentId;
 	}
 
 	function _beforeTokenTransfer(
@@ -161,18 +56,47 @@ contract SBTToken is
 		uint256 tokenId,
 		uint256 batchSize
 	) internal virtual override {
-		super._beforeTokenTransfer(from, to, tokenId, batchSize);
-
 		if (from == address(0)) {
-			// mint
-			tokenIdsMapOfOwner[to].add(tokenId);
+			// allow mint
+			super._beforeTokenTransfer(from, to, tokenId, batchSize);
 		} else if (to == address(0)) {
-			// burn
-			revert("SBT is not burned");
+			// disallow burn
+			revert("SBT can not burn");
 		} else if (to != from) {
-			revert("SBT is non transferrable");
+			// disallow transfer
+			revert("SBT can not transfer");
 		} else {
+			// disallow other
 			revert("Illegal operation");
 		}
 	}
+
+	function _tokenURI(uint256 _tokenId) private view returns (string memory) {
+		return tokenUriImage[_tokenId];
+	}
+
+	function owner() external view returns (address) {
+		return ProxyAdmin(proxyAdmin).owner();
+	}
+
+	function tokenURI(
+		uint256 _tokenId
+	) public view override returns (string memory) {
+		return _tokenURI(_tokenId);
+	}
+
+	function currentIndex() external view override returns (uint256) {
+		return super.totalSupply();
+	}
+
+	function tokensOfOwner(
+		address _owner
+	) external view override returns (uint256[] memory) {
+		uint256 length = super.balanceOf(_owner);
+		uint256[] memory tokens = new uint256[](length);
+		for (uint256 i = 0; i < length; i++) {
+			tokens[i] = super.tokenOfOwnerByIndex(_owner, i);
+		}
+		return tokens;
+	}
 }

contracts/interfaces/ISBTToken.sol

@@ -38,28 +38,4 @@ interface ISBTToken {
 	 * @return uint256 current token id
 	 */
 	function currentIndex() external view returns (uint256);
-
-	/// @notice Count NFTs tracked by this contract
-	/// @return A count of valid NFTs tracked by this contract, where each one of
-	///  them has an assigned and queryable owner not equal to the zero address
-	function totalSupply() external view returns (uint256);
-
-	/// @notice Enumerate valid NFTs
-	/// @dev Throws if `_index` >= `totalSupply()`.
-	/// @param _index A counter less than `totalSupply()`
-	/// @return The token identifier for the `_index`th NFT,
-	///  (sort order not specified)
-	function tokenByIndex(uint256 _index) external view returns (uint256);
-
-	/// @notice Enumerate NFTs assigned to an owner
-	/// @dev Throws if `_index` >= `balanceOf(_owner)` or if
-	///  `_owner` is the zero address, representing invalid NFTs.
-	/// @param _owner An address where we are interested in NFTs owned by them
-	/// @param _index A counter less than `balanceOf(_owner)`
-	/// @return The token identifier for the `_index`th NFT assigned to `_owner`,
-	///   (sort order not specified)
-	function tokenOfOwnerByIndex(
-		address _owner,
-		uint256 _index
-	) external view returns (uint256);
 }