Conversation
✅ Heimdall Review Status
|
reth/reth-entrypoint
Outdated
| # shut down gracefully | ||
| kill "$PID" | ||
|
|
||
| wait "$PID" |
There was a problem hiding this comment.
Has this been tested?
From Devin:
wait "$PID" exits script due to set -e after killing the reth process
After kill "$PID" sends SIGTERM to the background reth process on line 89, wait "$PID" on line 91 returns exit code 143 (128 + 15 for SIGTERM). Because the script runs with set -eu (reth/reth-entrypoint:2), this non-zero exit code causes the script to abort immediately. Lines 92-104 (including the proofs init command and the final exec) are never reached.
There was a problem hiding this comment.
It has been tested. I think Devin is wrong. I don't think it gets aborted because it's forked to a different process.
There was a problem hiding this comment.
Ah I see, I guess wait returns the exit code of the child process. SIGTERM should return exit code 0 for reth though because it gracefully shuts down.
There was a problem hiding this comment.
Only exits with 1 on error, otherwise exits with 0.
There was a problem hiding this comment.
and the exit handler does catch SIGTERM gracefully
There was a problem hiding this comment.
interestingly, it seems like Reth might be technically wrong to do this... https://stackoverflow.com/a/57944209
There was a problem hiding this comment.
ignored exit code here just in case: dc52b6c
reth/reth-entrypoint
Outdated
| P2P_PORT="${P2P_PORT:-30303}" | ||
| ADDITIONAL_ARGS="" | ||
| BINARY="./base-reth-node" | ||
| NODE_TYPE="${NODE_TYPE:-base}" |
There was a problem hiding this comment.
not used, maybe delete?
| "warn") | ||
| LOG_LEVEL="vv" | ||
| ;; | ||
| "info"|*) |
There was a problem hiding this comment.
please be exhaustive here, otherwise, error / trace level maps to info
reth/reth-entrypoint
Outdated
| --datadir="$RETH_DATA_DIR" \ | ||
| --proofs-history.storage-path=$RETH_HISTORICAL_PROOFS_STORAGE_PATH | ||
|
|
||
| sleep 60 |
There was a problem hiding this comment.
do we need the sleep here? is proofs init async?
There was a problem hiding this comment.
I don't think we do. Not sure why this was added.
3 participants
This isn't really supported until 0.5.0, but adds configuration for enabling the proofs ExEx.