Skip to content

Conversation

dannywillems
Copy link
Member

  • Add run-block-producer target with devnet/mainnet support
  • Add generate-block-producer-key target for key generation
  • Include CI workflow to test documentation commands
  • Update docs and CLAUDE.md with new target instructions

Resolves #1198

@dannywillems dannywillems force-pushed the block-producer-makefile-target branch from 17edf94 to 38f58f2 Compare July 21, 2025 12:49
@dannywillems dannywillems force-pushed the add-claude-formatting-instructions branch 3 times, most recently from 429c10e to 1023f56 Compare July 21, 2025 18:03
@dannywillems dannywillems changed the base branch from add-claude-formatting-instructions to develop July 22, 2025 16:35
@dannywillems dannywillems force-pushed the block-producer-makefile-target branch from 38f58f2 to 21d3247 Compare July 22, 2025 16:37
@dannywillems dannywillems requested a review from Copilot July 22, 2025 16:37
Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR adds Makefile targets for block producer operations, providing an alternative to Docker Compose for running block producers. The implementation includes key generation, network-specific targets, and comprehensive documentation.

  • Adds generate-block-producer-key and run-block-producer targets with devnet/mainnet support
  • Includes CI workflow to validate documentation commands functionality
  • Updates documentation with detailed setup instructions and usage examples

Reviewed Changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.

File Description
Makefile Adds block producer configuration variables and new targets for key generation and running producers
docs/block-producer-guide.md Documents the new Make-based alternative with prerequisites, setup steps, and usage examples
CLAUDE.md Adds block producer operations section with target descriptions and usage patterns
.github/workflows/doc-commands.yml New CI workflow to test key generation and validate make targets
Comments suppressed due to low confidence (1)

.github/workflows/doc-commands.yml:76

  • The test uses a placeholder value "test" for COINBASE_RECEIVER but doesn't validate that it's a proper wallet address format. Consider using a valid test wallet address format to ensure proper validation.
          timeout 30s make run-block-producer-devnet COINBASE_RECEIVER="test" || {

- name: Test key file can be read by run-block-producer
run: |
echo "Testing that generated key can be used by run-block-producer target..."
Copy link

Copilot AI Jul 22, 2025

Choose a reason for hiding this comment

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

The test doesn't set MINA_PRIVKEY_PASS which is required for the block producer to decrypt the private key. This could lead to misleading test results or expose error handling issues.

Suggested change
echo "Testing that generated key can be used by run-block-producer target..."
echo "Testing that generated key can be used by run-block-producer target..."
# Set MINA_PRIVKEY_PASS environment variable
export MINA_PRIVKEY_PASS="testpass"

Copilot uses AI. Check for mistakes.

@dannywillems dannywillems force-pushed the block-producer-makefile-target branch 2 times, most recently from dee9780 to 5cc9adb Compare July 22, 2025 16:48
@dannywillems dannywillems enabled auto-merge July 22, 2025 16:49
@dannywillems dannywillems force-pushed the block-producer-makefile-target branch 3 times, most recently from d8d4b4e to b5b7fa6 Compare July 23, 2025 15:07
Or explicitly specify devnet:

```bash
make run-block-producer NETWORK=devnet COINBASE_RECEIVER="YourWalletAddress" \
Copy link
Member Author

Choose a reason for hiding this comment

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

For this kind of command, I will add later a script and add in the CI with a timeout 30 ./scripts/foo_bar.sh.

@dannywillems dannywillems force-pushed the block-producer-makefile-target branch from b5b7fa6 to 076df9c Compare July 23, 2025 15:09
@dannywillems
Copy link
Member Author

I removed the section from CLAUDE.md, it is now in the doc, and CLAUDE should read it to put in its context.

@o1-labs o1-labs deleted a comment from Copilot AI Jul 23, 2025
Copy link
Contributor

@volhovm volhovm left a comment

Choose a reason for hiding this comment

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

Approving, but please check my comment about 30 seconds timeout (& let's make sure CI passes).

I checked generate-block-producer-key, didn't run the other targets but they look reasonable from reading the code. I only have some reservations regarding CI role.

run: |
echo "Testing that generated key can be used by run-block-producer target..."
# Run with --help to avoid actually starting the producer but verify key validation passes
timeout 30s make run-block-producer-devnet COINBASE_RECEIVER="test" || {
Copy link
Contributor

Choose a reason for hiding this comment

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

Will this make call not trigger compilation process? If it does then 30 seconds will not be enough. Or do I misunderstand what this job is doing?

On my machine the time to do the following is about 3 minutes:

її> make run-block-producer-devnet COINBASE_RECEIVER="test"
make run-block-producer NETWORK=devnet
make[1]: Entering directory '/home/volhovm/code/openmina'
cargo build --release --package=cli --bin openmina
warning: profiles for the non root package will be ignored, specify profiles at the workspace root:
package:   /home/volhovm/code/openmina/ledger/Cargo.toml
workspace: /home/volhovm/code/openmina/Cargo.toml
warning: profiles for the non root package will be ignored, specify profiles at the workspace root:
package:   /home/volhovm/code/openmina/tools/fuzzing/Cargo.toml
workspace: /home/volhovm/code/openmina/Cargo.toml
   Compiling mina-tree v0.16.0 (/home/volhovm/code/openmina/ledger)
   Compiling snark v0.16.0 (/home/volhovm/code/openmina/snark)
   Compiling vrf v0.16.0 (/home/volhovm/code/openmina/vrf)
   Compiling node v0.16.0 (/home/volhovm/code/openmina/node)
   Compiling openmina-node-common v0.16.0 (/home/volhovm/code/openmina/node/common)
   Compiling openmina-node-native v0.16.0 (/home/volhovm/code/openmina/node/native)
   Compiling cli v0.16.0 (/home/volhovm/code/openmina/cli)
    Finished `release` profile [optimized] target(s) in 2m 52s
MINA_PRIVKEY_PASS="" \
        cargo run --bin openmina \
        --release -- \
        node \
        --producer-key ./openmina-workdir/producer-key \
        --coinbase-receiver test \
         \
        --libp2p-port 8302 \
        --network devnet

Copy link
Member Author

Choose a reason for hiding this comment

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

It's because the code is recompiled because of action_kind.rs being changed. Gotta fix that first. Good point.

Copy link
Member Author

Choose a reason for hiding this comment

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

It should be better now.
See https://github.com/o1-labs/openmina/actions/runs/16652525657/job/47128853688?pr=1221.

I changed the target definition by relying on build-release, instead of calling `$(MAKE) build-release in the definition.
Also, thanks to #1276, it should not always recompile the binary.

Copy link
Member Author

Choose a reason for hiding this comment

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

FWIW: you can always execute a CI job locally by using act. For instance, you can test locally the doc job using act -j test-doc-commands.

@dannywillems dannywillems force-pushed the block-producer-makefile-target branch 2 times, most recently from 7651eb3 to 1f1f1d9 Compare July 31, 2025 14:44
- Add run-block-producer target with devnet/mainnet support
- Add generate-block-producer-key target for key generation
- Include CI workflow to test documentation commands
- Update docs and CLAUDE.md with new target instructions

Resolves #1198
@dannywillems dannywillems force-pushed the block-producer-makefile-target branch from 1f1f1d9 to 30ab752 Compare July 31, 2025 14:57
@dannywillems dannywillems merged commit b31459d into develop Jul 31, 2025
54 of 55 checks passed
@dannywillems dannywillems deleted the block-producer-makefile-target branch July 31, 2025 19:19
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.

Makefile: add a target to run a block producer

2 participants