Skip to content

feat: add DA_SIGNING_ADDRESSES support in configuration and scripts#24

Merged
auricom merged 1 commit intoclaude/celestia_node_0.28.4-mochafrom
feat/align-with-multi-da-address
Nov 26, 2025
Merged

feat: add DA_SIGNING_ADDRESSES support in configuration and scripts#24
auricom merged 1 commit intoclaude/celestia_node_0.28.4-mochafrom
feat/align-with-multi-da-address

Conversation

@auricom
Copy link
Collaborator

@auricom auricom commented Nov 26, 2025

Overview

@auricom auricom merged commit 03bfc1b into claude/celestia_node_0.28.4-mocha Nov 26, 2025
@gemini-code-assist
Copy link
Contributor

Summary of Changes

Hello @auricom, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request enhances the system's data availability (DA) layer by introducing support for DA_SIGNING_ADDRESSES, allowing for the specification of signing addresses crucial for DA operations. This new configuration is seamlessly integrated by updating environment variable definitions, Docker Compose setups for both Celestia and local DA, and the sequencer's entrypoint script. Additionally, a minor but important fix was applied to a DA entrypoint script to ensure a specific configuration value is updated reliably across the configuration file.

Highlights

  • DA_SIGNING_ADDRESSES Support: Introduced a new configuration option, DA_SIGNING_ADDRESSES, to specify signing addresses for data availability operations.
  • Environment Variable Integration: The new DA_SIGNING_ADDRESSES variable is now supported in the .env file and passed to sequencer services via Docker Compose configurations for both Celestia and local DA setups.
  • Sequencer Script Update: The entrypoint.sequencer.sh script is updated to conditionally include the --rollkit.da.signing_addresses flag based on the DA_SIGNING_ADDRESSES environment variable.
  • Configuration Script Improvement: The entrypoint.da.sh script now more robustly updates the TxWorkerAccounts setting in the light node configuration, ensuring it applies correctly regardless of its specific section.
Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature Command Description
Code Review /gemini review Performs a code review for the current pull request in its current state.
Pull Request Summary /gemini summary Provides a summary of the current pull request in its current state.
Comment @gemini-code-assist Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help /gemini help Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Footnotes

  1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution.

Copy link
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request adds support for DA_SIGNING_ADDRESSES in the configuration and scripts, which is reflected in the .env file, docker-compose files, and the sequencer entrypoint script. While the changes to add the new variable are correct, I've found two issues in the shell scripts. One is a critical command injection vulnerability in entrypoint.sequencer.sh due to improper handling of environment variables. The other is a high-severity issue in entrypoint.da.sh where a recent change broke the logic for updating a configuration value within a specific section. Please see my detailed comments for suggestions on how to fix these issues.

Comment on lines +189 to +192
if [ -n "${DA_SIGNING_ADDRESSES:-}" ]; then
default_flags="$default_flags --rollkit.da.signing_addresses $DA_SIGNING_ADDRESSES"
log "DEBUG" "Added DA signing addresses flag: $DA_SIGNING_ADDRESSES"
fi
Copy link
Contributor

Choose a reason for hiding this comment

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

critical

This block of code, and similar blocks in this script, are vulnerable to command injection. The DA_SIGNING_ADDRESSES variable is concatenated into a command string without proper quoting or escaping, and later executed via eval on line 209. If DA_SIGNING_ADDRESSES contains shell metacharacters (e.g., ;, |, &, $(...)), it can lead to arbitrary command execution. This is a critical security risk.

The entire flag-building mechanism should be refactored to avoid using eval with concatenated strings. A safer approach is to properly escape each variable's value before adding it to the default_flags string.

Here is an example of how to safely add a variable:

if [ -n "${DA_SIGNING_ADDRESSES:-}" ]; then
	val_escaped=$(printf '%s' "$DA_SIGNING_ADDRESSES" | sed "s/'/'\\''/g")
	default_flags="$default_flags --rollkit.da.signing_addresses '$val_escaped'"
	log "DEBUG" "Added DA signing addresses flag: $DA_SIGNING_ADDRESSES"
fi

This pattern should be applied to all environment variables used to build flags in this script to mitigate the vulnerability.

Comment on lines +79 to +86
# Check if TxWorkerAccounts exists anywhere in the config
if grep -q "^[[:space:]]*TxWorkerAccounts[[:space:]]*=" "$LIGHT_NODE_CONFIG_PATH"; then
# TxWorkerAccounts exists, check if it's set to 8
CURRENT_VALUE=$(grep -A 20 "^\[State\]" "$LIGHT_NODE_CONFIG_PATH" | grep "^[[:space:]]*TxWorkerAccounts" | head -1 | sed 's/.*=[[:space:]]*//')
CURRENT_VALUE=$(grep "^[[:space:]]*TxWorkerAccounts[[:space:]]*=" "$LIGHT_NODE_CONFIG_PATH" | head -1 | sed 's/.*=[[:space:]]*//')
if [ "$CURRENT_VALUE" != "8" ]; then
log "CONFIG" "Updating TxWorkerAccounts from $CURRENT_VALUE to 8"
# Update the value to 8 (only under [State] section)
if ! sed -i '/^\[State\]/,/^\[/ s/^[[:space:]]*TxWorkerAccounts[[:space:]]*=.*/ TxWorkerAccounts = 8/' "$LIGHT_NODE_CONFIG_PATH"; then
# Update the value to 8
if ! sed -i 's/^[[:space:]]*TxWorkerAccounts[[:space:]]*=.*/ TxWorkerAccounts = 8/' "$LIGHT_NODE_CONFIG_PATH"; then
Copy link
Contributor

Choose a reason for hiding this comment

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

high

The logic to update TxWorkerAccounts has been changed to operate on the entire configuration file, instead of being scoped to the [State] section. This is inconsistent with the logic that adds the key if it's missing (lines 95-102), which correctly places it under [State]. If TxWorkerAccounts were to exist in another section, this script could incorrectly modify it. The update logic should be scoped to the [State] section to maintain correctness and consistency. The previous implementation handled this correctly.

Suggested change
# Check if TxWorkerAccounts exists anywhere in the config
if grep -q "^[[:space:]]*TxWorkerAccounts[[:space:]]*=" "$LIGHT_NODE_CONFIG_PATH"; then
# TxWorkerAccounts exists, check if it's set to 8
CURRENT_VALUE=$(grep -A 20 "^\[State\]" "$LIGHT_NODE_CONFIG_PATH" | grep "^[[:space:]]*TxWorkerAccounts" | head -1 | sed 's/.*=[[:space:]]*//')
CURRENT_VALUE=$(grep "^[[:space:]]*TxWorkerAccounts[[:space:]]*=" "$LIGHT_NODE_CONFIG_PATH" | head -1 | sed 's/.*=[[:space:]]*//')
if [ "$CURRENT_VALUE" != "8" ]; then
log "CONFIG" "Updating TxWorkerAccounts from $CURRENT_VALUE to 8"
# Update the value to 8 (only under [State] section)
if ! sed -i '/^\[State\]/,/^\[/ s/^[[:space:]]*TxWorkerAccounts[[:space:]]*=.*/ TxWorkerAccounts = 8/' "$LIGHT_NODE_CONFIG_PATH"; then
# Update the value to 8
if ! sed -i 's/^[[:space:]]*TxWorkerAccounts[[:space:]]*=.*/ TxWorkerAccounts = 8/' "$LIGHT_NODE_CONFIG_PATH"; then
# Check if TxWorkerAccounts exists under [State]
if grep -A 20 "^\[State\]" "$LIGHT_NODE_CONFIG_PATH" | grep -q "^[[:space:]]*TxWorkerAccounts"; then
# TxWorkerAccounts exists, check if it's set to 8
CURRENT_VALUE=$(grep -A 20 "^\[State\]" "$LIGHT_NODE_CONFIG_PATH" | grep "^[[:space:]]*TxWorkerAccounts" | head -1 | sed 's/.*=[[:space:]]*//')
if [ "$CURRENT_VALUE" != "8" ]; then
log "CONFIG" "Updating TxWorkerAccounts from $CURRENT_VALUE to 8"
# Update the value to 8 (only under [State] section)
if ! sed -i '/^\[State\]/,/^\[/ s/^[[:space:]]*TxWorkerAccounts[[:space:]]*=.*/ TxWorkerAccounts = 8/' "$LIGHT_NODE_CONFIG_PATH"; then

@auricom auricom deleted the feat/align-with-multi-da-address branch January 6, 2026 14:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants