-
Notifications
You must be signed in to change notification settings - Fork 160
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
proofs api refactor #4177
proofs api refactor #4177
Conversation
caa81b3
to
5bba381
Compare
const DIR_ENV: &str = "FIL_PROOFS_PARAMETER_CACHE"; | ||
const GATEWAY_ENV: &str = "IPFS_GATEWAY"; | ||
const TRUST_PARAMS_ENV: &str = "TRUST_PARAMS"; | ||
const IPFS_GATEWAY_ENV: &str = "IPFS_GATEWAY"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe worth mentioning in doc somewhere
https://lotus.filecoin.io/kb/nodes-in-china/
Running Lotus requires the download of chain’s proof parameters which are large files which by default are hosted outside of China and very slow to download there. To get around that, users should set the following environment variable when running either of lotus, lotus-miner and lotus-worker:
export IPFS_GATEWAY=https://proof-parameters.s3.cn-south-1.jdcloud-oss.com/ipfs/
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
At first glance I feel we should prefix the environment variable with FOREST_
but in this case maybe it's more convenient to re-use the lotus one. They don't prefix it with LOTUS_
for some reason.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was a bit hesitant to change the variable names to avoid breaking changes, but better do it now than later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
At first glance I feel we should prefix the environment variable with
FOREST_
but in this case maybe it's more convenient to re-use the lotus one. They don't prefix it withLOTUS_
for some reason.
Agreed on the prefix. As stated above - it would be nice to have some sort of tool that would allow us to specify the prefix for everything we load. Then we can load variables with and without the prefix.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@hanabi1224 Great point there, I was not aware of that. On top of that, I was able to find a bug (not sure how long it was there, I guess pretty long) where the IPFS_GATEWAY
was not overridable - the environment variable only changed what was logged. 🤡 See a4c53c5
a4c53c5
to
59e6923
Compare
59e6923
to
122c4a1
Compare
Summary of changes
Changes introduced in this pull request:
Reference issue to close (if applicable)
Closes
Other information and links
Change checklist