Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

SYS-379: Add verifyUnseen test #245

Draft
wants to merge 9 commits into
base: dev
Choose a base branch
from
Draft

SYS-379: Add verifyUnseen test #245

wants to merge 9 commits into from

Conversation

muni-corn
Copy link
Contributor

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

Copy link

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant