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

fix: rename bugs #5

Open
wants to merge 1 commit into
base: zkthunder
Choose a base branch
from
Open

fix: rename bugs #5

wants to merge 1 commit into from

Conversation

tmors
Copy link
Collaborator

@tmors tmors commented Nov 28, 2024

What ❔

  1. change AKSK with 4EVERLAND to add a bucket named zkthunder
  2. add docker ignore directory
  3. add execute privilege to shell

Why ❔

fix bugs on local-node to

  1. build the image correctly
  2. run the container correctly

fix IPFS bucket
add x to shell
Copy link
Member

@erubboli erubboli left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I noticed that there's a hardcoded secret in this implementation. This is a significant security risk, even for testing purposes, and shouldn't be included in the codebase. Additionally, since this repository is publicly readable, the exposed API key must be disabled immediately to prevent any misuse.

Another broader concern I have is regarding our dependency on the 4EVERLAND API. Since this is an external service, how do we handle potential failures on their end? For instance, what happens to the zkRollup if the API becomes temporarily unavailable, permanently shut down, or their interface changes unexpectedly? We should consider adding mechanisms to mitigate these risks to ensure the reliability of the zkRollup.

@@ -116,8 +116,8 @@ services:
# - IPFS_API_URL=http://ipfs:5001
- ML_RPC_URL=http://host.docker.internal:13034 # change to mainnet if needed
- ML_BATCH_SIZE=10 # change if necessary
- 4EVERLAND_API_KEY=5F2R8SK2EQNSNCHSRWIK # only for test
- 4EVERLAND_SECRET_KEY=sCGfIdQZfis8YVCXnQP53SL8cPdRxyzjPLh1KYmF # only for test
- 4EVERLAND_API_KEY=PKN5TUJTPYNSSOGC3JW9 # only for test
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We must move all secrets out of the configuration file, an environment variable is good option.

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.

2 participants