Merge pull request #14 from dev-protocol/feat-update-sbt

feat: add minor improvements to the SBT.

Author: yashdesu

contracts/SBT.sol

@@ -3,7 +3,6 @@ pragma solidity =0.8.9;
 
 import {Strings} from "@openzeppelin/contracts/utils/Strings.sol";
 import {Base64} from "@devprotocol/util-contracts/contracts/utils/Base64.sol";
-import {ProxyAdmin} from "@openzeppelin/contracts/proxy/transparent/ProxyAdmin.sol";
 import {ERC721EnumerableUpgradeable} from "@openzeppelin/contracts-upgradeable/token/ERC721/extensions/ERC721EnumerableUpgradeable.sol";
 
 import {ISBT} from "./interfaces/ISBT.sol";
@@ -12,8 +11,6 @@ contract SBT is ISBT, ERC721EnumerableUpgradeable {
 	using Base64 for bytes;
 	using Strings for uint256;
 
-	/// @dev Account with proxy adming rights.
-	address private _proxyAdmin;
 	/// @dev EOA with rights to allow(add)/disallow(remove) minter.
 	address private _minterUpdater;
 
@@ -70,12 +67,6 @@ contract SBT is ISBT, ERC721EnumerableUpgradeable {
 		}
 	}
 
-	function setProxyAdmin(address proxyAdmin) external {
-		require(_proxyAdmin == address(0), "Already set");
-		_proxyAdmin = proxyAdmin;
-		emit SetProxyAdmin(proxyAdmin);
-	}
-
 	function addMinter(address minter) external override onlyMinterUpdater {
 		_minters[minter] = true;
 		emit MinterAdded(minter);
@@ -106,14 +97,16 @@ contract SBT is ISBT, ERC721EnumerableUpgradeable {
 	}
 
 	function _tokenURI(uint256 tokenId) private view returns (string memory) {
+		require(tokenId < currentIndex(), "Token not found");
+
 		(
 			string memory name,
 			string memory description,
 			string memory tokenUriImage,
 			StringAttribute[] memory stringAttributes,
 			NumberAttribute[] memory numberAttributes
 		) = abi.decode(
-				_sbtdata[tokenId],
+				metadataOf(tokenId),
 				(string, string, string, StringAttribute[], NumberAttribute[])
 			);
 
@@ -230,8 +223,11 @@ contract SBT is ISBT, ERC721EnumerableUpgradeable {
 		return super.totalSupply();
 	}
 
-	function owner() external view returns (address) {
-		return ProxyAdmin(_proxyAdmin).owner();
+	function metadataOf(
+		uint256 tokenId
+	) public view override returns (bytes memory) {
+		require(tokenId < currentIndex(), "Token not found");
+		return _sbtdata[tokenId];
 	}
 
 	function tokensOfOwner(

contracts/SBTProxy.sol

@@ -0,0 +1,12 @@
+// SPDX-License-Identifier: MPL-2.0
+pragma solidity =0.8.9;
+
+import {TransparentUpgradeableProxy} from "@openzeppelin/contracts/proxy/transparent/TransparentUpgradeableProxy.sol";
+
+contract SBTProxy is TransparentUpgradeableProxy {
+	constructor(
+		address _logic,
+		address admin_,
+		bytes memory _data
+	) TransparentUpgradeableProxy(_logic, admin_, _data) {}
+}

contracts/interfaces/ISBT.sol

@@ -111,4 +111,11 @@ interface ISBT {
 	 * @return uint256 current token id
 	 */
 	function currentIndex() external view returns (uint256);
+
+	/*
+	 * @dev get mapped metadata bytes of token id
+	 * @param tokenId the token id of the NFT
+	 * @return bytes The mapped metadata bytes
+	 */
+	function metadataOf(uint256 tokenId) external view returns (bytes memory);
 }

test/SBT.test.ts

@@ -30,25 +30,6 @@ describe('SBT', () => {
 		})
 	})
 
-	describe('setProxyAdmin', () => {
-		it('The setProxyAdmin function should execute once', async () => {
-			const sbt = await init()
-			const signers = await getSigners()
-			await expect(sbt.setProxyAdmin(signers.proxyAdmin.address))
-				.to.emit(sbt, 'SetProxyAdmin')
-				.withArgs(signers.proxyAdmin.address)
-		})
-
-		it('The setProxyAdmin function can only be executed once', async () => {
-			const sbt = await init()
-			const signers = await getSigners()
-			await sbt.setProxyAdmin(signers.proxyAdmin.address)
-			await expect(
-				sbt.setProxyAdmin(signers.proxyAdmin.address)
-			).to.be.revertedWith('Already set')
-		})
-	})
-
 	describe('addMinter', () => {
 		it('The addMinter function can be executed by minterUpdater', async () => {
 			const sbt = await init()
@@ -68,7 +49,11 @@ describe('SBT', () => {
 			)
 
 			await expect(
-				sbt.connect(signers.proxyAdmin).addMinter(signers.minterC.address)
+				sbt.connect(signers.deployer).addMinter(signers.minterC.address)
+			).to.revertedWith('Not minter updater')
+
+			await expect(
+				sbt.connect(signers.userA).addMinter(signers.minterC.address)
 			).to.revertedWith('Not minter updater')
 		})
 	})
@@ -92,7 +77,11 @@ describe('SBT', () => {
 			)
 
 			await expect(
-				sbt.connect(signers.proxyAdmin).removeMinter(signers.minterA.address)
+				sbt.connect(signers.deployer).removeMinter(signers.minterA.address)
+			).to.revertedWith('Not minter updater')
+
+			await expect(
+				sbt.connect(signers.userA).removeMinter(signers.minterA.address)
 			).to.revertedWith('Not minter updater')
 		})
 	})
@@ -134,7 +123,7 @@ describe('SBT', () => {
 			).to.be.revertedWith('Illegal access')
 
 			await expect(
-				sbt.connect(signers.proxyAdmin).mint(signers.userA.address, metadata)
+				sbt.connect(signers.userB).mint(signers.userA.address, metadata)
 			).to.be.revertedWith('Illegal access')
 
 			await expect(
@@ -493,7 +482,7 @@ describe('SBT', () => {
 
 			metadata = await getDummyEncodedMetadata(sbt, 'USERC')
 			await expect(
-				sbt.connect(signers.proxyAdmin).setTokenURI(0, metadata)
+				sbt.connect(signers.userB).setTokenURI(0, metadata)
 			).to.be.revertedWith('Illegal access')
 
 			metadata = await getDummyEncodedMetadata(sbt, 'USERD')