Skip to content

Conversation

@thlorenz
Copy link
Contributor

@thlorenz thlorenz commented Aug 8, 2025

Summary

Adds the ability to setup validator(s) for a specific test without running the tests.
This is useful when trying to create the environment while running a specific integration test
manually or even for other projects (like the cloning pipeline I'm currently working on).

Details

The env var SETUP_ONLY triggers this behavior when either set to:

export SETUP_ONLY=devnet
export SETUP_ONLY=ephem
export SETUP_ONLY=both

The following convenience tasks were added to the Makefile of the test-integration workspace:

  • setup-schedulecommit-devnet
  • setup-schedulecommit-both
  • setup-issues-frequent-commits-devnet
  • setup-issues-frequent-commits-both
  • setup-cloning-devnet
  • setup-cloning-both
  • setup-restore-ledger-devnet
  • setup-magicblock-api-devnet
  • setup-magicblock-api-both
  • setup-table-mania-devnet
  • setup-committor-devnet
  • setup-pubsub-ephem
  • setup-config-devnet

Adjacent

I fixed a few clippy warnings while working on this, mainly inside ledger-restore tests

Greptile Summary

This PR introduces a new "setup-only" mode for the MagicBlock validator's integration test suite. The feature allows developers to start validator environments (devnet, ephemeral, or both) without executing the actual tests, controlled by the SETUP_ONLY environment variable with values devnet, ephem, or both.

The implementation adds a comprehensive configuration system through the new TestConfigViaEnvVars struct in env_config.rs that parses environment variables to control test execution and validator setup behavior. A new signal handling module (signal.rs) provides graceful Ctrl-C interrupt handling, allowing users to cleanly shut down validators when done with manual testing.

The changes integrate with the existing test infrastructure by modifying the main test runner (run_tests.rs) to branch between traditional test execution and setup-only mode. When in setup mode, validators are started and kept running until the user presses Ctrl-C, at which point the signal handler ensures proper cleanup of validator processes.

Thirteen new convenience Make targets were added to the integration test Makefile (e.g., setup-schedulecommit-devnet, setup-cloning-both) that set the appropriate SETUP_ONLY environment variable and execute the corresponding test binary. This provides an ergonomic interface for developers to quickly spin up specific validator environments.

Additionally, the PR includes minor code quality improvements, such as replacing assert_eq!(executable, true, ...) with the more idiomatic assert!(executable, ...) in ledger restoration tests, and commenting out an unnecessary Memo program configuration.

Confidence score: 4/5

  • This PR is safe to merge with low risk of production issues
  • Score reflects well-structured code with proper error handling, though the setup-only feature adds new execution paths that may need thorough testing
  • Pay close attention to test-integration/test-runner/src/env_config.rs for the configuration logic and signal handling implementation

Dodecahedr0x and others added 11 commits July 28, 2025 10:07
Closes #471 

Create a `Mergeable` derive trait that automatically merges the fields
with default values with the non-default fields of another instance. It
also handles nested Mergeable structs.

Note: in order to handle nested Mergeable structs, it uses the heuristic
that nested mergeable struct have a type name containing `Config`. This
is not a very robust method, but it works as we only use it for internal
config structs.
Closes #459 

Renames the `test-bins` crate into `magicblock-validator` and the binary
from `rpc` to `magicblock-validator`
Solves #418 

This PR:
- Adds a config parameter to skip ledger replay when starting a
validator, discarding the old ledger but preserving accountsDB and the
validator keypair.
- Adds a test that checks that the state is still there but not the
ledger

## Snippets

TOML:

```toml
[ledger]
resume-strategy = "replay-and-resume"
path = "ledger.example.com"
size = 1000000000
```

CLI:

```bash
cargo run -- --ledger-resume-strategy replay-and-resume
LEDGER_RESUME_STRATEGY=replay-and-resume cargo run
```

## Migration

Old config:

```toml
[ledger]
reset = true
```

Becomes:

```toml
[ledger]
resume-strategy = "reset"
```

--- 

Old config:

```toml
[ledger]
reset = false
```

Becomes: 

```toml
[ledger]
resume-strategy = "replay"
```
Closes #468 and closes #469

Explorer was showing the transaction timestamp because it uses
`getBlockTime`, which was returning an estimate of the slot time based
on the slot period and the number of slots between the current slot and
the requested slot. This is fixed by returning the timestamp of the
block from the ledger.

The timestamp written in the ledger was a different system time from the
one used for the clock. This is fixed by writing in the ledger the
latest value of the clock, which should also fix replay.

<!-- greptile_comment -->

## Greptile Summary

This PR fixes a critical timestamp synchronization issue across the
validator system. The core problem was that different parts of the
system were using different sources for timestamps:

1. The explorer was using `getBlockTime` which estimated timestamps
based on slot periods
2. The ledger was using raw system time
3. The clock sysvar had its own timestamp

The fix ensures all components use the bank's clock timestamp as the
single source of truth, specifically:

- Modified `magicblock-api/src/slot.rs` to use
`bank.clock().unix_timestamp` instead of system time
- Updated `magicblock-rpc/src/json_rpc_request_processor.rs` to
prioritize actual block timestamps over estimates
- Added comprehensive tests in `test_clocks_match.rs` to verify
timestamp consistency

This change is particularly important for:
- Accurate transaction replay
- Consistent timestamp reporting between explorer and ledger
- Proper clock sysvar behavior

## Confidence score: 4.5/5

1. This PR is safe to merge as it fixes a critical timestamp consistency
issue without introducing new risks
2. High confidence due to comprehensive test coverage and the
straightforward nature of the fix - using a single source of truth
3. Files needing attention:
- `magicblock-api/src/slot.rs`: Verify no edge cases in timestamp
handling
- `test_clocks_match.rs`: Ensure all critical timestamp scenarios are
covered

<sub>3 files reviewed, 2 comments</sub>
<sub>[Edit PR Review Bot
Settings](https://app.greptile.com/review/github) |
[Greptile](https://greptile.com?utm_source=greptile_expert&utm_medium=github&utm_campaign=code_reviews&utm_content=magicblock-validator_454)</sub>

<!-- /greptile_comment -->
#458)

Closes #299 

Add a parameter in the configuration to disable verification of the
validator's pubkey during replay, enabling anyone to replay the ledger

## Snippets

TOML

```toml
[ledger]
skip-keypair-match-check = false # Defaults to true
```

CLI

```bash
cargo run -- --ledger-skip-keypair-match-check
LEDGER_SKIP_KEYPAIR_MATCH_CHECK=FALSE cargo run
```
Implements fee claiming on validator startup, closes #303. 

- Adds `claim_fees method` to build/send delegation program's
ValidatorClaimFees instruction
- Added integration test `test_validator_claim_fees`

---------

Co-authored-by: Dodecahedr0x <hexadecifish@gmail.com>
* master:
  merge: dev (#497)
  Special case for "fat" ledger. Use compaction + CompactionFilter (#413)
Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

10 files reviewed, 3 comments

Edit Code Review Bot Settings | Greptile

Copy link
Contributor

@Dodecahedr0x Dodecahedr0x left a comment

Choose a reason for hiding this comment

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

LGTM

@thlorenz thlorenz merged commit 60e4b7f into master Aug 12, 2025
4 checks passed
@thlorenz thlorenz deleted the thlorenz/test-setup-only branch August 12, 2025 14:57
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.

4 participants