Build images for mainnet full nodes #2032
Conversation
WalkthroughThe recent changes introduce several key automation features for the DYDX protocol, including a new GitHub Actions workflow for building and pushing Docker images to AWS ECR. Additionally, scripts and Dockerfiles have been added to facilitate the setup and management of full nodes on the mainnet, ensuring robust functionalities like snapshot creation and environment variable configuration. Changes
Sequence Diagram(s)sequenceDiagram
participant PR as Pull Request
participant GitHub as GitHub Actions
participant ECR as AWS ECR
participant Node as Full Node
PR->>GitHub: Trigger on push or PR
GitHub->>ECR: Build and push Docker image
Node->>Node: Initialize full node
Node->>Node: Create snapshots periodically
Node->>ECR: Upload snapshots to S3
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (7)
- .github/workflows/protocol-build-and-push-mainnet.yml (1 hunks)
- protocol/.gitignore (1 hunks)
- protocol/testing/mainnet/Dockerfile (1 hunks)
- protocol/testing/mainnet/mainnet.sh (1 hunks)
- protocol/testing/mainnet/snapshot.sh (1 hunks)
- protocol/testing/mainnet/start.sh (1 hunks)
- protocol/testing/mainnet/vars.sh (1 hunks)
Files skipped from review due to trivial changes (1)
- protocol/.gitignore
Additional context used
Shellcheck
protocol/testing/mainnet/vars.sh
[warning] 4-4: CURRENT_VERSION_DIR appears unused. Verify use (or export if used externally).
(SC2034)
[warning] 7-7: LAST_FULL_NODE_INDEX appears unused. Verify use (or export if used externally).
(SC2034)
protocol/testing/mainnet/snapshot.sh
[warning] 54-54: RPC_ADDRESS appears unused. Verify use (or export if used externally).
(SC2034)
[warning] 85-85: p2p_seeds is referenced but not assigned.
(SC2154)
[warning] 85-85: dd_agent_host is referenced but not assigned.
(SC2154)
[warning] 87-87: upload_period is referenced but not assigned.
(SC2154)
[warning] 88-88: Quote this to prevent word splitting.
(SC2046)
[warning] 92-92: s3_snapshot_bucket is referenced but not assigned.
(SC2154)
[warning] 93-93: Use "${var:?}" to ensure this never expands to / .
(SC2115)
Additional comments not posted (28)
protocol/testing/mainnet/vars.sh (4)
1-1: Shebang line is correct.The shebang line is correctly set for a bash script.
2-2: Error handling is correctly set.The
set -eo pipefailcommand ensures the script exits on error and handles pipeline errors correctly.
4-4: Verify the usage ofCURRENT_VERSION_DIR.The
CURRENT_VERSION_DIRvariable is declared but not used within the script. Verify if it is intended for external use or future use within the script.Tools
Shellcheck
[warning] 4-4: CURRENT_VERSION_DIR appears unused. Verify use (or export if used externally).
(SC2034)
7-7: Verify the usage ofLAST_FULL_NODE_INDEX.The
LAST_FULL_NODE_INDEXvariable is declared but not used within the script. Verify if it is intended for external use or future use within the script.Tools
Shellcheck
[warning] 7-7: LAST_FULL_NODE_INDEX appears unused. Verify use (or export if used externally).
(SC2034)
protocol/testing/mainnet/Dockerfile (7)
1-1: Verify the base imagedydxprotocol-base.Ensure that the base image
dydxprotocol-baseis appropriate and available.
3-3: Dependencies are correctly installed.The
apk addcommand installsbash,jq, andaws-cli, which are necessary dependencies.
4-4:cosmovisoris correctly installed.The
go installcommand installs[email protected], which is necessary for the Docker image.
6-6: Verify the source directory.Ensure that the
./testing/mainnet/directory exists and contains the necessary files to be copied to/dydxprotocol/.
8-9: Environment variable and working directory are correctly set.The
ENVandWORKDIRcommands set the environment variableHOMEand the working directory to/dydxprotocol/.
11-11: Verify themainnet.shscript.Ensure that the
/dydxprotocol/mainnet.shscript exists and is executable.
13-13: Verify thestart.shscript.Ensure that the
/dydxprotocol/start.shscript exists and is executable.protocol/testing/mainnet/start.sh (6)
1-1: Shebang line is correct.The shebang line is correctly set for a bash script.
2-2: Error handling is correctly set.The
set -eo pipefailcommand ensures the script exits on error and handles pipeline errors correctly.
4-7: Comments are clear and accurate.The comments explain the purpose and usage of the script clearly and accurately.
9-9: Verify thevars.shscript.Ensure that the
vars.shscript exists and is correctly sourced.
11-17: Setup process forcosmovisoris correct.The setup process for
cosmovisoris correct, copying binaries to the home directories of full nodes.
19-19: Command execution is correct.The
cosmovisorcommand is executed with any passed arguments, ensuring the node starts correctly..github/workflows/protocol-build-and-push-mainnet.yml (3)
3-11: LGTM!The workflow triggers are correctly specified for the relevant branches.
13-18: LGTM!The job configuration is standard and correctly sets the working directory.
20-48: LGTM! But verify the presence of required secrets.The steps are well-defined and follow best practices.
However, ensure that the secrets
AWS_ACCESS_KEY_ID_VALIDATOR_MAINNETandAWS_SECRET_ACCESS_KEY_VALIDATOR_MAINNETare correctly set up in the repository.protocol/testing/mainnet/mainnet.sh (4)
1-8: LGTM!The initial setup is standard and follows best practices.
20-45: LGTM!The function is correctly defined and follows best practices.
47-78: LGTM!The function is well-defined and follows best practices.
16-18: LGTM! But verify the environment.The function is correctly defined to install
daselandjqusingapk.However, ensure that the environment uses Alpine Linux.
protocol/testing/mainnet/snapshot.sh (4)
1-28: LGTM!The initial setup is standard and follows best practices.
30-39: LGTM!The environment setup is well-defined and follows best practices.
48-80: LGTM!The snapshot configuration is well-defined and follows best practices.
Tools
Shellcheck
[warning] 54-54: RPC_ADDRESS appears unused. Verify use (or export if used externally).
(SC2034)
41-43: LGTM! But verify the environment.The function is correctly defined to install
dasel,jq, andcurlusingapk.However, ensure that the environment uses Alpine Linux.
| # Do not index tx_index.db | ||
| sed -i 's/indexer = "kv"/indexer = "null"/' /dydxprotocol/chain/.full-node-2/config/config.toml | ||
|
|
||
| # TODO: add metrics around snapshot upload latency/frequency/success rate |
There was a problem hiding this comment.
Add metrics around snapshot upload latency/frequency/success rate.
The TODO comment indicates that metrics are missing.
Do you want me to generate the code for adding these metrics or open a GitHub issue to track this task?
| SNAP_NAME=$(echo "${CHAIN_ID}_$(date '+%Y-%m-%d-%H-%M').tar.gz") | ||
| tar cvzf ${SNAP_PATH}/${SNAP_NAME} ${DATA_PATH} | ||
| aws s3 cp ${SNAP_PATH}/${SNAP_NAME} s3://${s3_snapshot_bucket}/ --region ap-northeast-1 --debug || true | ||
| rm -rf ${SNAP_PATH}/${SNAP_NAME} |
There was a problem hiding this comment.
Ensure the variable never expands to /.
Use ${var:?} to ensure this never expands to /.
- rm -rf ${SNAP_PATH}/${SNAP_NAME}
+ rm -rf ${SNAP_PATH:?}/${SNAP_NAME}Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| rm -rf ${SNAP_PATH}/${SNAP_NAME} | |
| rm -rf ${SNAP_PATH:?}/${SNAP_NAME} |
Tools
Shellcheck
[warning] 93-93: Use "${var:?}" to ensure this never expands to / .
(SC2115)
| cosmovisor run start --log_level info --home /dydxprotocol/chain/.full-node-2 --p2p.seeds "${p2p_seeds}" --non-validating-full-node=true --dd-agent-host=${dd_agent_host} & | ||
|
|
||
| sleep ${upload_period} | ||
| kill -TERM $(pidof cosmovisor) |
There was a problem hiding this comment.
Quote the variable to prevent word splitting.
The variable $(pidof cosmovisor) should be quoted to prevent word splitting.
- kill -TERM $(pidof cosmovisor)
+ kill -TERM "$(pidof cosmovisor)"Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| kill -TERM $(pidof cosmovisor) | |
| kill -TERM "$(pidof cosmovisor)" |
Tools
Shellcheck
[warning] 88-88: Quote this to prevent word splitting.
(SC2046)
|
|
||
| SNAP_NAME=$(echo "${CHAIN_ID}_$(date '+%Y-%m-%d-%H-%M').tar.gz") | ||
| tar cvzf ${SNAP_PATH}/${SNAP_NAME} ${DATA_PATH} | ||
| aws s3 cp ${SNAP_PATH}/${SNAP_NAME} s3://${s3_snapshot_bucket}/ --region ap-northeast-1 --debug || true |
There was a problem hiding this comment.
Assign value to s3_snapshot_bucket.
The variable s3_snapshot_bucket is referenced but not assigned.
- aws s3 cp ${SNAP_PATH}/${SNAP_NAME} s3://${s3_snapshot_bucket}/ --region ap-northeast-1 --debug || true
+ aws s3 cp ${SNAP_PATH}/${SNAP_NAME} s3://${s3_snapshot_bucket:-default_bucket}/ --region ap-northeast-1 --debug || trueCommittable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| aws s3 cp ${SNAP_PATH}/${SNAP_NAME} s3://${s3_snapshot_bucket}/ --region ap-northeast-1 --debug || true | |
| aws s3 cp ${SNAP_PATH}/${SNAP_NAME} s3://${s3_snapshot_bucket:-default_bucket}/ --region ap-northeast-1 --debug || true |
Tools
Shellcheck
[warning] 92-92: s3_snapshot_bucket is referenced but not assigned.
(SC2154)
| # p2p.seeds taken from --p2p.persistent_peers flag of full node | ||
| cosmovisor run start --log_level info --home /dydxprotocol/chain/.full-node-2 --p2p.seeds "${p2p_seeds}" --non-validating-full-node=true --dd-agent-host=${dd_agent_host} & | ||
|
|
||
| sleep ${upload_period} |
There was a problem hiding this comment.
Quote the variable to prevent word splitting.
The variable upload_period should be quoted to prevent word splitting.
- sleep ${upload_period}
+ sleep "${upload_period}"Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| sleep ${upload_period} | |
| sleep "${upload_period}" |
Tools
Shellcheck
[warning] 87-87: upload_period is referenced but not assigned.
(SC2154)
| # TODO: add metrics around snapshot upload latency/frequency/success rate | ||
| while true; do | ||
| # p2p.seeds taken from --p2p.persistent_peers flag of full node | ||
| cosmovisor run start --log_level info --home /dydxprotocol/chain/.full-node-2 --p2p.seeds "${p2p_seeds}" --non-validating-full-node=true --dd-agent-host=${dd_agent_host} & |
There was a problem hiding this comment.
Assign values to p2p_seeds and dd_agent_host.
The variables p2p_seeds and dd_agent_host are referenced but not assigned.
- cosmovisor run start --log_level info --home /dydxprotocol/chain/.full-node-2 --p2p.seeds "${p2p_seeds}" --non-validating-full-node=true --dd-agent-host=${dd_agent_host} &
+ cosmovisor run start --log_level info --home /dydxprotocol/chain/.full-node-2 --p2p.seeds "${p2p_seeds:-default_seed}" --non-validating-full-node=true --dd-agent-host=${dd_agent_host:-default_host} &Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| cosmovisor run start --log_level info --home /dydxprotocol/chain/.full-node-2 --p2p.seeds "${p2p_seeds}" --non-validating-full-node=true --dd-agent-host=${dd_agent_host} & | |
| cosmovisor run start --log_level info --home /dydxprotocol/chain/.full-node-2 --p2p.seeds "${p2p_seeds:-default_seed}" --non-validating-full-node=true --dd-agent-host=${dd_agent_host:-default_host} & |
Tools
Shellcheck
[warning] 85-85: p2p_seeds is referenced but not assigned.
(SC2154)
[warning] 85-85: dd_agent_host is referenced but not assigned.
(SC2154)
|
|
||
| # Define the mapping from version to URL | ||
| declare -A version_to_url | ||
| # version_to_url["v5.1.0"]="https://github.com/dydxprotocol/v4-chain/releases/download/protocol%2Fv5.1.0-dev4/dydxprotocold-v5.1.0-dev4-linux-amd64.tar.gz" |
There was a problem hiding this comment.
Doesn't this still need to be defined?
There was a problem hiding this comment.
No, since mainnet is on v5.1.x we are using the current commit for v5.1.0 in this PR
|
@Mergifyio backport release/protocol/v5.1.x |
✅ Backports have been created
|
(cherry picked from commit 340ea7c)
|
https://github.com/Mergifyio backport release/protocol/v5.2.x |
Co-authored-by: roy-dydx <[email protected]>
✅ Backports have been created
|
(cherry picked from commit 340ea7c)
Co-authored-by: roy-dydx <[email protected]>
|
https://github.com/Mergifyio backport release/protocol/v6.x |
✅ Backports have been created
|
(cherry picked from commit 340ea7c)
Co-authored-by: roy-dydx <[email protected]>
Changelist
Build these images here instead of separate repo.
Test Plan
Cherry-picked this PR onto release branch and ran workflows to test images.
Author/Reviewer Checklist
state-breakinglabel.indexer-postgres-breakinglabel.PrepareProposalorProcessProposal, manually add the labelproposal-breaking.feature:[feature-name].backport/[branch-name].refactor,chore,bug.Summary by CodeRabbit
mainnet.sh,snapshot.sh,start.sh, andvars.sh, enhancing operational efficiency for the dYdX mainnet..gitignoreto now track thebindirectory, allowing for better management of compiled binaries.