alo9507 / blockchain-developer-bootcamp-final-project

0 stars 0 forks source link

Final Project Feedback #1

Open ConsenSys-Academy opened 2 years ago

ConsenSys-Academy commented 2 years ago

Hey @alo9507 – Congratulations, your Final Project passed! We'll be approving it on the course itself in a week or two, but here's your feedback in the meanwhile:

Frontend

Contract Design and Security

Testing

Overall

FlacoJones commented 2 years ago

Thanks for your feedback! All great stuff. Some of my responses below:

I wasn't able to see this in the presentation or on the frontend, but did you need to do the approve then transfer for the ERC-20? So INITIALLY, for the platform you used, yes, we just had a few approved contract addresses and people simply sent directly to the issue.

We realized this, while good UX, made it hard to impossible to facilitate refunds without listening to Transfer events for all possible ERC20/ERC721. So now we use approve/transfer in later versions.

Submitted some MATIC to my test bounty but got an Out of Gas error: Argh! Wouldn't that be on your end not having enough MATIC on Mumbai to send money to the address?

Have you all considered doing a selfdestruct function to also remove the contract from state?: Is this just kind of a public good to reduce the hardware costs of running a node? Like picking up after yourself?

Didn't realize that about Hardhat deploying with predictable addresses, how do you determine the address, say for bountyAddress? I think that it's order based and maybe contract name based. On my machine and my partners machines, its always the same addresses somehow. Same contract name, same order, only two constant variables.

If you're thinking about user flow, would you consider a faucet for $FAKE for people to try out? Or you could manually send, just an idea. HELL YEAH! I've never made a faucet - great new project.

Good to test for existing Bounties, the crossover with Github and public blockchains is very interesting and does bring up different tests than pure blockchain cases.: Indeed....integration testing with ALL systems involved is a must

Thanks again for a great course!

FlacoJones commented 2 years ago

Oh actually, if the Out of Gas error came from the withdraw functionality then that must have been because I forgot to add some test MATIC onto mumbai Oracles wallet!