Rooms nft #125
Rooms nft #125
Conversation
tanerdurmaz
commented
Aug 29, 2021
tanerdurmaz
commented
- Room ownership changed, now it is nft based
- I used some gas price and gaslimit but not sure how clever was it.
| const HDWalletProvider = require("@truffle/hdwallet-provider"); | ||
| const web3 = require( 'web3'); |
There was a problem hiding this comment.
should use import instead of require whenever possible in typescript projects for proper type checking
might have to install @types/... to get it working
| @@ -72,8 +98,54 @@ roomRouter.post("/:roomId", async (req, res) => { | |||
|
|
|||
| await collection | |||
| .doc(roomId) | |||
| .set({ name: roomId, isLocked, lockedOwnerAddress: address ?? undefined , contractAddress: contractAddress ?? undefined}); | |||
| .set({ name: roomId, isLocked, lockedOwnerAddress: "dynamic" , contractAddress: contractAddress ?? undefined}); | |||
There was a problem hiding this comment.
if lockedOwnerAddress is dynamic every time from now on, do we need to set it as dynamic? Seems redundant
| .set({ name: roomId, isLocked, lockedOwnerAddress: "dynamic" , contractAddress: contractAddress ?? undefined}); | ||
|
|
||
| // get token count then add new metadata to database using it | ||
| const tokenCount = await nftRooms.doc("token-count").get(); |
There was a problem hiding this comment.
Don't want to store this in the database, the database can get out of sync for whatever reason. We can instead query the blockchain directly for token balance using total supply https://docs.openzeppelin.com/contracts/3.x/api/token/erc721#IERC721Enumerable-totalSupply--
| await nftRooms | ||
| .doc(tokenId.toString()) | ||
| .set({ name: roomId }); | ||
|
|
||
| await nftRooms | ||
| .doc("token-count") | ||
| .set({ count: tokenId}); |
There was a problem hiding this comment.
do we need nftRooms? Why not store the data in the room itself in the chatrooms collection?
| const provider = new HDWalletProvider(MNEMONIC, "https://rpc-mainnet.maticvigil.com/v1/41b386d43bd3cfdf37a0fdef86b801ef9836fa7b"); | ||
| const web3Instance = new web3(provider); |
There was a problem hiding this comment.
Since provider and web3Instance don't change, these should be declared outside the function instead of being redeclared each time the function is called
| const nftContract = new web3Instance.eth.Contract( | ||
| NFT_ABI, | ||
| NFT_CONTRACT_ADDRESS, | ||
| { gasLimit: "1000000", | ||
| gasPrice: 15000000000 | ||
| } | ||
| ); |
There was a problem hiding this comment.
this can be declared outside the function too since its value doesn't change whenever you recall the function
|
|
||
| const reciever = address ? address : OWNER_ADDRESS; |
There was a problem hiding this comment.
Also, you can just do const receiver = address || OWNER_ADDRESS
| } catch (e) { | ||
| await nftRooms | ||
| .doc("token-count") | ||
| .set({ count: parseInt(tokenCount.data()!.count) - 1 }); | ||
| console.log(e); |
There was a problem hiding this comment.
we shouldn't need to manually keep track of count since we can query the blockchain at any time for token supply. If we want to do this, you should increase token count on success instead of always increasing token count and decreasing on failure
| await axios.get('https://api.covalenthq.com/v1/137/address/' + address + '/balances_v2/?nft=true&key=ckey_c35e2c388e1b4efea8490fb8c83').then( async (result) => { | ||
| let permission = false; | ||
| for(let i = 0; i < result.data.data.items.length; i++){ | ||
| if(NFT_CONTRACT_ADDRESS!.toLowerCase() === result.data.data.items[i].contract_address.toLowerCase()){ | ||
| if(result.data.data.items[i].nft_data){ | ||
| for(let j = 0; j < result.data.data.items[i].nft_data.length; j++){ | ||
| const doc = await nftRooms.doc(result.data.data.items[i].nft_data[j].token_id).get(); | ||
| const name = await doc.get("name"); | ||
| if(name === roomId){ | ||
| permission = true; | ||
| } | ||
| } | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
I'd prefer if this were a separate method with a clear name explaining what it's doing. Currently it's very hard to read and understand what's happening, especially with nested for loops and very nested objects like result.data.data.items[i].nft_data
I'm not actually sure what this function is doing, can you explain it?
