bump v - Githubissues

ponderingdemocritus commented 3 months ago

PR Type

Enhancement, Dependencies

Description

Implemented CRUD operations for user management in bot/src/index.ts.
Updated @dojoengine dependencies to stable versions in client/package.json and sdk/packages/eternum/package.json.
Updated network fee token in client/.env.sample.
Updated block number in contracts/manifests/dev/manifest.toml and contracts/manifests/dev/manifest.json.
Updated Dojo installation version in readme.md.
Updated package version in contracts/Scarb.toml.

Changes walkthrough 📝

Relevant files

Enhancement

1 files

index.ts

Implemented CRUD operations for user management.

bot/src/index.ts

Added in-memory storage for users.

Implemented CRUD operations for users.

Added routes for creating, reading, updating, and deleting users.

+43/-0

Dependencies

2 files

package.json Updated `@dojoengine` dependencies to stable versions. client/package.json Updated `@dojoengine` dependencies to stable versions. Moved some dependencies from `dependencies` to `devDependencies`.	+10/-10
package.json Updated `@dojoengine/core` dependency to stable version. sdk/packages/eternum/package.json - Updated `@dojoengine/core` dependency to stable version.	+1/-1

Configuration changes

4 files

.env.sample `Updated network fee token in environment sample.` client/.env.sample - Updated `VITE_NETWORK_FEE_TOKEN` value.	+1/-1
manifest.toml `Updated block number in manifest.` contracts/manifests/dev/manifest.toml - Updated `block_number` from 19 to 18.	+1/-1
manifest.json `Updated block number in manifest.` contracts/manifests/dev/manifest.json - Updated `block_number` from 19 to 18.	+1/-1
Scarb.toml `Updated package version in Scarb.toml.` contracts/Scarb.toml - Updated package version from 0.5.0 to 0.6.8.	+1/-1

Documentation

1 files

readme.md

Updated Dojo installation version in README.

readme.md - Updated Dojo installation version from 0.7.2 to 0.7.3.

+1/-1

Additional files (token-limit)

1 files

pnpm-lock.yaml

...

pnpm-lock.yaml ...

+631/-648

💡 PR-Agent usage: Comment /help on the PR to get a list of all available PR-Agent tools and their descriptions

vercel[bot] commented 3 months ago

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Comments	Updated (UTC)
eternum	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	Jul 5, 2024 1:08pm

github-actions[bot] commented 3 months ago

PR Reviewer Guide 🔍

⏱️ Estimated effort to review: 3 🔵🔵🔵⚪⚪

🧪 No relevant tests

🔒 No security concerns identified

⚡ Key issues to review

**Possible Bug:** The CRUD operations implemented in `bot/src/index.ts` use an in-memory storage for users. This approach is not persistent and will lose data whenever the server restarts. Consider using a persistent storage solution. **Code Quality:** The CRUD operations do not sanitize or validate the input data before processing. This can lead to potential security vulnerabilities or application errors. **Error Handling:** The error messages returned from the CRUD operations are generic and might not provide enough information for debugging or for the end-users to understand the issue.

github-actions[bot] commented 3 months ago

PR Code Suggestions ✨

Category	Suggestion	Score
Possible bug	Add error handling for JSON parsing in user-related endpoints ___ Implement error handling for JSON parsing and other asynchronous operations to prevent unhandled promise rejections and improve the robustness of the API. [bot/src/index.ts [11-34]](https://github.com/BibliothecaDAO/eternum/pull/1084/files#diff-4353c39a9b859d910eb80fd4dd0189625cafe6039c76d2ce75a2d2396e18bed1R11-R34) ```diff -const { discordName, walletAddress } = await c.req.json(); +let discordName, walletName; +try { + const data = await c.req.json(); + discordName = data.discordName; + walletAddress = data.walletAddress; +} catch (error) { + return c.json({ error: "Invalid JSON format" }, 400); +} ``` Suggestion importance[1-10]: 10 Why: Implementing error handling for JSON parsing is crucial for preventing unhandled promise rejections and improving the robustness of the API, making this a highly valuable suggestion.	10
Security	Add input validation for user data in POST and PUT endpoints ___ Consider implementing input validation for the `discordName` and `walletAddress` fields in the POST and PUT endpoints to ensure data integrity and prevent potential security issues. [bot/src/index.ts [11-34]](https://github.com/BibliothecaDAO/eternum/pull/1084/files#diff-4353c39a9b859d910eb80fd4dd0189625cafe6039c76d2ce75a2d2396e18bed1R11-R34) ```diff const { discordName, walletAddress } = await c.req.json(); +if (!discordName \|\| !walletAddress) { + return c.json({ error: "Invalid input" }, 400); +} ``` Suggestion importance[1-10]: 9 Why: This suggestion addresses a significant security concern by ensuring that the input data is valid, which helps prevent potential issues such as data corruption or security vulnerabilities.	9
Enhancement	Replace simple incrementing ID with UUID for user creation ___ Use a more robust method for generating user IDs to avoid potential collisions and ensure uniqueness, especially in a multi-user or concurrent environment. [bot/src/index.ts [12]](https://github.com/BibliothecaDAO/eternum/pull/1084/files#diff-4353c39a9b859d910eb80fd4dd0189625cafe6039c76d2ce75a2d2396e18bed1R12-R12) ```diff -const id = String(nextId++); +const id = crypto.randomUUID(); ``` Suggestion importance[1-10]: 8 Why: Using UUIDs for user IDs is a more robust solution that avoids potential collisions and ensures uniqueness, especially in a multi-user or concurrent environment.	8
Documentation	Update the README to reflect the correct version to install ___ The installation instruction in the README should specify the exact version to match the `Scarb.toml` file. This ensures users install the correct version corresponding to the project's dependencies. [readme.md [77]](https://github.com/BibliothecaDAO/eternum/pull/1084/files#diff-5a831ea67cf5cf8703b0de46901ab25bd191f56b320053be9332d9a3b0d01d15R77-R77) ```diff -Make sure install the same version within the `Scarb.toml` file. Currently this is `0.7.3` +Make sure to install the exact version specified in the `Scarb.toml` file to ensure compatibility. Currently, this is `0.7.9`. ``` Suggestion importance[1-10]: 8 Why: Ensuring the README specifies the correct version to install is important for user guidance and project consistency. This change improves documentation accuracy.	8
Maintainability	Introduce middleware to handle user existence checks ___ Refactor the repeated error response for "User not found" into a middleware function to reduce code duplication and improve maintainability. [bot/src/index.ts [26-42]](https://github.com/BibliothecaDAO/eternum/pull/1084/files#diff-4353c39a9b859d910eb80fd4dd0189625cafe6039c76d2ce75a2d2396e18bed1R26-R42) ```diff -if (!users[id]) return c.json({ error: "User not found" }, 404); +// Middleware to check if user exists +const userExists = (c, next) => { + const id = c.req.param("id"); + if (!users[id]) return c.json({ error: "User not found" }, 404); + return next(); +}; +// Use middleware in routes +app.get("/users/:id", userExists, (c) => {...}); +app.put("/users/:id", userExists, async (c) => {...}); +app.delete("/users/:id", userExists, (c) => {...}); ``` Suggestion importance[1-10]: 7 Why: Refactoring the repeated error response into a middleware function reduces code duplication and improves maintainability, although it is not as critical as the other suggestions.	7
Maintainability	Align `@dojoengine/core` dependency version with project standards ___ Update the dependency version for `@dojoengine/core` to match the latest stable release or the intended target version used in the project to avoid potential issues with alpha versions. [sdk/packages/eternum/package.json [20]](https://github.com/BibliothecaDAO/eternum/pull/1084/files#diff-5266f48f0bafefad1ee13ede26d33070e7e75a9a57b853c4d63654f5a77c18cbR20-R20) ```diff -"@dojoengine/core": "^0.7.9", +"@dojoengine/core": "^0.7.9", # Ensure this matches the stable version used in the project ``` Suggestion importance[1-10]: 5 Why: The suggestion to add a comment for clarity is helpful for maintainability but does not address a critical issue. It is a minor improvement for code readability.	5
Best practice	Ensure consistent versioning for dependencies ___ It's recommended to use consistent versioning for dependencies. The version `0.7.9` for `@dojoengine/core` should be consistent with the specifier, which currently includes a version range. Consider using a fixed version or adjusting the specifier to match the versioning strategy. [pnpm-lock.yaml [92]](https://github.com/BibliothecaDAO/eternum/pull/1084/files#diff-32824c984905bb02bc7ffcef96a77addd1f1602cff71a11fbbfdd7f53ee026bbR92-R92) ```diff -version: 0.7.9(starknet@6.7.0(encoding@0.1.13))(typescript@5.4.4) +version: ^0.7.9(starknet@6.7.0(encoding@0.1.13))(typescript@5.4.4) ``` Suggestion importance[1-10]: 7 Why: The suggestion to use consistent versioning is valid and can help avoid potential compatibility issues. However, it is not a critical change and more of a best practice.	7
Best practice	Use version ranges for better package compatibility ___ The versioning for `@react-three/drei` includes a specific version `9.108.1` which might lead to compatibility issues if not properly managed with its dependencies. Consider using a version range if applicable to ensure compatibility with other packages. [pnpm-lock.yaml [131]](https://github.com/BibliothecaDAO/eternum/pull/1084/files#diff-32824c984905bb02bc7ffcef96a77addd1f1602cff71a11fbbfdd7f53ee026bbR131-R131) ```diff -version: 9.108.1(@react-three/fiber@8.16.8(react-dom@18.2.0(react@18.2.0))(react@18.2.0)(three@0.163.0))(@types/react@18.2.74)(@types/three@0.163.0)(react-dom@18.2.0(react@18.2.0))(react@18.2.0)(three@0.163.0) +version: ^9.108.1(@react-three/fiber@8.16.8(react-dom@18.2.0(react@18.2.0))(react@18.2.0)(three@0.163.0))(@types/react@18.2.74)(@types/three@0.163.0)(react-dom@18.2.0(react@18.2.0))(react@18.2.0)(three@0.163.0) ``` Suggestion importance[1-10]: 7 Why: Using version ranges can improve compatibility with other packages, which is a good practice. However, it is not a critical issue and more of a recommendation.	7

BibliothecaDAO / eternum

bump v #1084

PR Type

Description

Changes walkthrough 📝

PR Reviewer Guide 🔍

PR Code Suggestions ✨