Skip to content
This repository was archived by the owner on Feb 1, 2025. It is now read-only.

fix: rename bugs#5

Open
tmors wants to merge 1 commit intomintlayer:zkthunderfrom
tmors:fix-rename
Open

fix: rename bugs#5
tmors wants to merge 1 commit intomintlayer:zkthunderfrom
tmors:fix-rename

Conversation

@tmors
Copy link
Copy Markdown

@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
Copy Markdown
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.

- 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
Copy Markdown
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 subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants