shardeum / shardus-core

Other
10 stars 2 forks source link

SYS-379: Add `verifyUnseen` test #245

Open muni-corn opened 3 weeks ago

muni-corn commented 3 weeks ago

Depends on #244 . Will keep as a draft until #244 is merged.

github-actions[bot] commented 3 weeks ago

PR Reviewer Guide ๐Ÿ”

โฑ๏ธ Estimated effort to review: 4 ๐Ÿ”ต๐Ÿ”ต๐Ÿ”ต๐Ÿ”ตโšช
๐Ÿงช PR contains tests
๐Ÿ”’ No security concerns identified
โšก Key issues to review

Code Duplication
The `validate.ts` file contains multiple instances of similar error objects being created, such as `BAD_STRUCTURE_ERROR`, `BAD_NODE_INFO_STRUCTURE_ERROR`, and `BAD_SIGNATURE_STRUCTURE_ERROR`. Consider refactoring these into a more generic function or a factory pattern to reduce code duplication and improve maintainability. Complex Logic
The function `validateJoinRequest` and other validation functions in `validate.ts` are quite complex and perform multiple checks. Consider breaking these down into smaller, more manageable functions to improve readability and maintainability. Global State
The logging setup in `logging.ts` uses a global logger instance `p2pLogger`. Using global state can lead to issues with maintainability and testing. Consider passing the logger as a dependency where needed.