6529-Collections / nftdelegation

Delegation contract
MIT License
16 stars 7 forks source link

Lacking Test Suite #52

Open mpeyfuss opened 10 months ago

mpeyfuss commented 10 months ago

The current unit tests are lacking compared to what I would expect from something going into production.

This project would significantly benefit from more rigorous unit testing, as a lot of the issues I've created could have been caught early on with proper requirements being set and tests being executed against those requirements (test driven development).

If you are looking into improving the test suite, hardhat can be OK, but I would recommend Foundry and the fuzzing capabilities are unmatched. I am happy to provide guidance/insight on this in my freetime! However, it is very scary to interact with something that hasn't been fully tested at the unit test level, especially as I am looking into integrating this into my own product.

a2rocket commented 10 months ago

Dear @mpeyfuss I would like to thank you for spending time to look the NFTD smart contract. I do understand that there may be lack of tests on our github repo (we can expand the tests) but I would like to inform you that the smart contract was fully tested not just internally from our team but also from external solidity developers and was fully audited by an external firm. The smart contract is currently being used by the 6529 ecosystem (with over 2k interactions on the contract) and soon with the release of NFTDelegation.com we will provide the opportunity for other projects to integrate it.

mpeyfuss commented 10 months ago

Is the audit published anywhere? Ultimately, an audit is not a replacement for a fuzzed test suite. They are both needed. Was the testing by the team/external devs done in a unit testing environment or on testnet? Again, the purpose of unit testing is that you are able to easily and quickly test for all edge cases, plus verify your logic matches the requirements.