From 596ce4de48754fb5c8a6625f4824862aacdf63ed Mon Sep 17 00:00:00 2001
From: Andres Aiello <aaiello@gmail.com>
Date: Wed, 21 Aug 2024 14:30:41 -0300
Subject: [PATCH] feat: security improvements on nft

---
 .../contracts/xp-nft/xpNFT.sol                | 10 ++++++---
 .../scripts/xp-nft/deploy.ts                  |  3 ++-
 .../zevm-app-contracts/test/xp-nft/xp-nft.ts  | 21 ++++++++++++++++++-
 3 files changed, 29 insertions(+), 5 deletions(-)

diff --git a/packages/zevm-app-contracts/contracts/xp-nft/xpNFT.sol b/packages/zevm-app-contracts/contracts/xp-nft/xpNFT.sol
index a195535..836fa3d 100644
--- a/packages/zevm-app-contracts/contracts/xp-nft/xpNFT.sol
+++ b/packages/zevm-app-contracts/contracts/xp-nft/xpNFT.sol
@@ -3,6 +3,7 @@ pragma solidity 0.8.7;
 
 import "@openzeppelin/contracts-upgradeable/token/ERC721/ERC721Upgradeable.sol";
 import "@openzeppelin/contracts-upgradeable/access/OwnableUpgradeable.sol";
+import "@openzeppelin/contracts/utils/cryptography/ECDSA.sol";
 
 contract ZetaXP is ERC721Upgradeable, OwnableUpgradeable {
     /* An ECDSA signature. */
@@ -47,10 +48,12 @@ contract ZetaXP is ERC721Upgradeable, OwnableUpgradeable {
         string memory name,
         string memory symbol,
         string memory baseTokenURI_,
-        address signerAddress_
+        address signerAddress_,
+        address owner
     ) public initializer {
         __ERC721_init(name, symbol);
         __Ownable_init();
+        transferOwnership(owner);
         baseTokenURI = baseTokenURI_;
         signerAddress = signerAddress_;
         _currentTokenId = 1; // Start token IDs from 1
@@ -95,9 +98,10 @@ contract ZetaXP is ERC721Upgradeable, OwnableUpgradeable {
 
     function _verify(uint256 tokenId, UpdateData memory updateData) private view {
         bytes32 payloadHash = _calculateHash(updateData);
-        bytes32 messageHash = keccak256(abi.encodePacked("\x19Ethereum Signed Message:\n32", payloadHash));
 
-        address messageSigner = ecrecover(
+        bytes32 messageHash = ECDSA.toEthSignedMessageHash(payloadHash);
+
+        address messageSigner = ECDSA.recover(
             messageHash,
             updateData.signature.v,
             updateData.signature.r,
diff --git a/packages/zevm-app-contracts/scripts/xp-nft/deploy.ts b/packages/zevm-app-contracts/scripts/xp-nft/deploy.ts
index b06e6c5..9ee6e11 100644
--- a/packages/zevm-app-contracts/scripts/xp-nft/deploy.ts
+++ b/packages/zevm-app-contracts/scripts/xp-nft/deploy.ts
@@ -8,6 +8,7 @@ const networkName = network.name;
 
 const ZETA_BASE_URL = "https://api.zetachain.io/nft/";
 const signer = "0x1d24d94520B94B26351f6573de5ef9731c48531A";
+const owner = "0x1d24d94520B94B26351f6573de5ef9731c48531A";
 
 const verifyContract = async (contractAddress: string, constructorArguments: any[]) => {
   // Verification process
@@ -27,7 +28,7 @@ const deployZetaXP = async () => {
   if (!isProtocolNetworkName(networkName)) throw new Error("Invalid network name");
 
   const ZetaXPFactory = (await ethers.getContractFactory("ZetaXP")) as ZetaXP__factory;
-  const zetaXP = await upgrades.deployProxy(ZetaXPFactory, ["ZETA NFT", "ZNFT", ZETA_BASE_URL, signer]);
+  const zetaXP = await upgrades.deployProxy(ZetaXPFactory, ["ZETA NFT", "ZNFT", ZETA_BASE_URL, signer, owner]);
 
   await zetaXP.deployed();
 
diff --git a/packages/zevm-app-contracts/test/xp-nft/xp-nft.ts b/packages/zevm-app-contracts/test/xp-nft/xp-nft.ts
index 8cc3b77..86e4f22 100644
--- a/packages/zevm-app-contracts/test/xp-nft/xp-nft.ts
+++ b/packages/zevm-app-contracts/test/xp-nft/xp-nft.ts
@@ -2,6 +2,7 @@ import { expect, use } from "chai";
 import { solidity } from "ethereum-waffle";
 use(solidity);
 import { SignerWithAddress } from "@nomiclabs/hardhat-ethers/signers";
+import exp from "constants";
 import { ethers, upgrades } from "hardhat";
 
 import { ZetaXP } from "../../typechain-types";
@@ -17,7 +18,13 @@ describe("XP NFT Contract test", () => {
     [signer, user, ...addrs] = await ethers.getSigners();
     const zetaXPFactory = await ethers.getContractFactory("ZetaXP");
 
-    zetaXP = await upgrades.deployProxy(zetaXPFactory, ["ZETA NFT", "ZNFT", ZETA_BASE_URL, signer.address]);
+    zetaXP = await upgrades.deployProxy(zetaXPFactory, [
+      "ZETA NFT",
+      "ZNFT",
+      ZETA_BASE_URL,
+      signer.address,
+      signer.address,
+    ]);
 
     await zetaXP.deployed();
     const tag = ethers.utils.keccak256(ethers.utils.defaultAbiCoder.encode(["string"], ["XP_NFT"]));
@@ -314,4 +321,16 @@ describe("XP NFT Contract test", () => {
     const queriedTag = await zetaXP.tagByTokenId(tokenId);
     await expect(queriedTag).to.be.eq(sampleNFT.tag);
   });
+
+  it("Should transfer ownership", async () => {
+    {
+      const ownerAddr = await zetaXP.owner();
+      expect(ownerAddr).to.be.eq(signer.address);
+    }
+    await zetaXP.transferOwnership(user.address);
+    {
+      const ownerAddr = await zetaXP.owner();
+      expect(ownerAddr).to.be.eq(user.address);
+    }
+  });
 });