TwentyOne

Summary

Solidity compiler version is not explicity set to be unchangeable, which in extremy rare scenarios could lead to some unexpected behaviour or might lead to missing out on improvement possiblities. I will use this section to suggest other best practices that can be employed. While some of them might be out of scope of the security review they provide value by teaching the protocol creators about security and general best practices thus making web3 more secure.

Vulnerability Details

Caret (^) Operator: The caret symbol in ^0.8.13 allows the compiler to use any version from 0.8.13 up to (but not including) 0.9.0. This means the contract code could be compiled with newer compiler versions that you haven't tested with.

Risk of Breaking Changes: While minor versions (e.g., 0.8.x) are supposed to be backward compatible, there can still be subtle changes or deprecations that affect the contract's behavior.

Impact

Minimal/Extremely unlikely to cause any issues

Tools Used

1. Manual review

2. Foundry toolkit (anvil, forge test, etc)

3. ChatGPT

Recommendations

1. Follow best practice of Using Exact Solidity Compiler Version: Replace the caret pragma with a specific version. For example:
   pragma solidity 0.8.13;

2. Follow best practice of using custom errors instead of require. This can save Gas, while being as verbose, for example

could be re-written to something like:

3. Test suite is not passing. Just donwloading the code and running forge test has one test (test_Call_PlayerWins) failling, due to inssuficient funds in the TwentyOne contract. More info on this can be found in my H-1 report. Having good test suite that includes unit & integration tests as well as fuzz tests and some sort of formal verification is crucial for ensuring project security. This can be fixed by:

4. Deploy scripts not leveraged for testing. Deploy script can be very helpful for having a streamlined DevOps process for the protocol.

5. Consider using private function visibility if the contract is not meant to be inherited

6. Consider using indexed params (player addresses might be a good idea) in events for better trackability of events

7. Consider using constants for repeated numbers and magic numbers throughout the code

8. Adding Explicit visibility for variables like playersDeck and dealersDeck is considered best practice

9. Consider using natspec