0xdcota / blockchain-developer-bootcamp-final-project Goto Github PK

Pregunta:
En el contrato HouseOfCoin.sol tienes dos functiones para checar si el usuario tiene el MintPower suficiente y en los comentarios dice que checkMintingPower es externa y _checkMintingPower es interna, pero la segunda es "publica". No deberia decir internal allí?
=)

Hi @daigarocota – 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:
When we tried to compile and test your contract locally, we got some errors which were easy to fix, so we preferred to fix them as a sign of appreciation for your hard work and here's some fixes that we got and blocked us from compileing and testing your work locally :

• replace secrets.json withsample.secrets.json
• in node_modules/redstone-flash-storage/lib/contracts/message-based/PriceAware.sol , there was a ParserError: Source "hardhat/console.sol" not found --> redstone-flash-storage/lib/contracts/message-based/PriceAware.sol:7:1: | 7 | import "hardhat/console.sol"; you need to check the version or develop post-install script to fix this issue or maybe manually fix it
• /contracts/interfaces/IAssetsAccountantState.sol" not found , you need to fix its extension as the current one is IAssetsAccountantState.Sol" it's case senstive
  

Front end & UX :
Thank you for your hard work. Below are some recommendations to make your work better:

• Don't force your user to inject meta mask once they open your website, let them connect whenever they want/need. Don't block your user from accessing your site
• Guide your users to connect to the correct network when they are on the wrong network
• Handle errors and display user-friendly messages rather than throwing the error to the user

Smart contract:
awesome code organization and documentation, few tips and tricks if you would love to make your work incredibly amazing

• store the value of keccak256("MINTER_ROLE") in constant vlaue to reduce gas cost when calling mintCoin in HouseOfCoin contract

• _checkMaxWithdrawal visibility should be private since no child contract calls it to reduce cost

• It's recommended to define any function that is not called inside your contract as external rather than public to reduce gas cost e.g setURI -

• Storage Layout & Ordering: Ordering of storage variables and struct members affect how they can be packed tightly. For example, declaring your storage variables in the order of uint128, uint128, uint256 instead of uint128, uint256, uint128, as the former will only take up two slots of storage whereas the latter will take up three.
  https://secureum.substack.com/p/solidity-201

• The best-practices for layout within a contract is the following order: state variables, events, modifiers, constructor and functions to improve contract readability
  https://secureum.substack.com/p/solidity-101

Unit tests
You did a great job with testing. My advice to you is to :

-Check the amazing testing tools like OZ testing environment https://docs.openzeppelin.com/test-environment/0.1/ and waffle https://ethereum-waffle.readthedocs.io/en/latest/index.html as they would give you lots of amazing tools that help you develop more efficient unit tests. also if you would love to check the test coverage, tools like firefly https://fireflyblockchain.com/ would help you

General feedback:
What you have done is amazing and I do encourage you to continue your work on this amazing idea