Skip to content

Conversation

@FarzeenKist
Copy link

Fixes and Improvement

  1. Refactored codebase by removing code that were unnecessary (for e.g auctionsCount was redundant since it was essentially doing the same thing as _tokenIds) and switched comments for functions into the Nat spec format. Furthermore, I've removed some variables in the struct as they could easily be stored onto ipfs instead of the smart contract.
  2. I've noticed that the modifier isTimeUp would always reassign isActive which would cost unnecessary gas, so I've went ahead and modified the condition of the if statement to prevent that.
  3. I've added the mapping participatedIn to keep tracks of the auctions users have participated in as a bidder or is the owner to simplify the logic of getUserAuctions which could be really slow in retrieving data
  4. Applied best practices when dealing with funds
  5. I've also noticed that getAuctions would not return auctions that are no longer active. This could cause an issue on the frontend where users who had bid on this particular auction could not retrieve back their funds, the owner could not receive the highestbid amount and the highestbidder the reward (NFT). The users would also then need to know the id of the auction to be able to interact with it directly on the smartcontract. So I've went ahead and modified the logic to return all Auctions.

@netlify
Copy link

netlify bot commented Aug 30, 2022

Deploy Preview for celo-auction ready!

Name Link
🔨 Latest commit ebda118
🔍 Latest deploy log https://app.netlify.com/sites/celo-auction/deploys/630dcb654f2ea2000882145d
😎 Deploy Preview https://deploy-preview-1--celo-auction.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant