feat(apps): namespace migration flag [DNM]

[julienrbrt]

Overview

Example on how to implement namespace migration for sync nodes.
Basically use --ns-migrations flag.

[github-actions]

The latest Buf updates on your PR. Results from workflow CI and Release / buf-check (pull_request).

Build	Format	Lint	Breaking	Updated (UTC)
✅ passed	⏩ skipped	✅ passed	✅ passed	Oct 6, 2025, 8:24 AM

[claude]

Code Review for PR #2708: Namespace Migration Flag

Overview

This PR introduces a namespace migration feature for the EVM single sequencer application, allowing namespaces to change at specific block heights. The implementation wraps the existing DA API with migration logic.

✅ Strengths

1. Clean abstraction: The namespaceMigrationDAAPI wrapper pattern cleanly extends the existing DA API without modifying the core interface
2. Non-breaking change: The implementation is backward compatible - when no migrations are defined, it falls back to current behavior
3. Good separation of concerns: Migration logic is isolated from the core DA implementation

🔍 Issues and Recommendations

1. Missing Critical DA Methods 🚨

The namespaceMigrationDAAPI only overrides GetIDs but not other DA interface methods that accept namespaces:

• Get(ctx, ids, namespace)
• GetProofs(ctx, ids, namespace)
• Commit(ctx, blobs, namespace)
• Submit(ctx, blobs, gasPrice, namespace)
• SubmitWithOptions(ctx, blobs, gasPrice, namespace, options)
• Validate(ctx, ids, proofs, namespace)

Impact: This could lead to inconsistent behavior where GetIDs uses the migrated namespace but other operations use the original namespace, potentially causing data retrieval failures or incorrect submissions.

Recommendation: Override all DA interface methods that accept a namespace parameter:

func (api *namespaceMigrationDAAPI) Get(ctx context.Context, ids []da.ID, ns []byte) ([]da.Blob, error) {
    ns = api.findNamespaceForHeight(/* need height here */, bytes.Equal(ns, api.currentDataNamespace))
    return api.API.Get(ctx, ids, ns)
}

// Similar for other methods...

2. Height Information Not Available 🔴

The current implementation has a fundamental issue: methods like Get, Submit, etc. don't receive height information, making it impossible to determine which namespace to use for operations other than GetIDs.

Recommendation: Consider one of these approaches:

• Store the current operating height in the wrapper and update it as blocks progress
• Refactor to pass height information through context
• Use a different migration strategy that doesn't require height-based lookups for all operations

3. No Migration Configuration ⚠️

The migrations map is initialized empty and never populated. There's no mechanism to configure migrations via CLI flags or configuration files.

Recommendation: Add CLI flags to configure migrations:

cmd.Flags().StringSlice("namespace-migrations", nil, "Namespace migrations in format height:namespace:dataNamespace")

4. Missing Tests ⚠️

No unit tests are provided for the new namespace migration functionality.

Recommendation: Add comprehensive tests covering:

• Migration lookup logic with various height scenarios
• Edge cases (empty migrations, boundary conditions)
• Integration with the actual DA layer

5. Potential Race Condition ⚠️

The migrations map is read without synchronization. If migrations can be updated at runtime, this could cause race conditions.

Recommendation: If migrations are static (configured at startup), document this clearly. Otherwise, add proper synchronization.

6. Documentation Missing 📝

No documentation explains:

• How to configure namespace migrations
• Use cases for this feature
• Migration format and behavior

🎯 Performance Considerations

• The migration lookup is O(n) where n is the number of migrations. For a small number of migrations this is fine, but consider using a sorted slice for O(log n) lookups if many migrations are expected
• The findNamespaceForHeight method is called on every DA operation - ensure it's optimized

🔒 Security Considerations

• Ensure namespace migrations are validated to prevent arbitrary namespace access
• Consider adding access controls or validation for who can configure migrations
• Log all namespace migrations for audit purposes

Summary

While the namespace migration concept is well-structured, the current implementation is incomplete and will likely not work as intended. The main blocking issues are:

1. Only GetIDs respects migrations - other DA operations will use wrong namespaces
2. No way to determine correct namespace for operations that don't receive height
3. No configuration mechanism for migrations

This PR should be updated to address these issues before merging. Consider breaking this into smaller PRs: first implementing the complete wrapper with all methods, then adding configuration and migration logic.

🤖 Generated with Claude Code

Co-Authored-By: Claude noreply@anthropic.com

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 62.28%. Comparing base (298f62e) to head (7ac1637).

Additional details and impacted files

@@           Coverage Diff           @@
##             main    #2708   +/-   ##
=======================================
  Coverage   62.28%   62.28%           
=======================================
  Files          79       79           
  Lines        8490     8490           
=======================================
  Hits         5288     5288           
  Misses       2711     2711           
  Partials      491      491           

Flag	Coverage Δ
combined	62.28% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[claude]

Code Review for PR #2708: Namespace Migration Flag

Summary

This PR introduces a namespace migration feature that allows dynamic namespace switching at specific DA heights. This is useful for handling namespace changes without node restarts.

✅ Strengths

• Clean interface implementation: The wrapper pattern around jsonrpc.API is well-designed
• Backward compatibility: Existing configurations work without modifications
• Flexible configuration: Supports both namespace and dataNamespace migrations

🔍 Areas for Improvement

1. Potential Bug: Race Condition in Height Selection

The findNamespaceForHeight logic has an issue when multiple migrations exist:

for migrationHeight := range api.migrations {
    if migrationHeight <= height && migrationHeight > selectedHeight {
        selectedHeight = migrationHeight
        found = true
    }
}

Issue: Map iteration order is non-deterministic in Go. This could lead to inconsistent namespace selection.
Fix: Sort migration heights before processing or use a sorted data structure.

2. Missing Validation

• No validation for duplicate heights in migration configuration
• No validation that namespace strings are valid before use
• No ordering validation (migrations should be in ascending height order)

3. Error Handling

• Silent fallback to empty string when dataNamespace is not set could mask configuration errors
• Consider explicit validation during initialization

4. Performance Consideration

The findNamespaceForHeight method iterates through all migrations on every GetIDs call. For frequently called methods:

• Consider caching the namespace for ranges of heights
• Use a sorted slice or tree structure for O(log n) lookups

5. Test Coverage

No unit tests are included for:

• parseMigrations function with various input formats
• findNamespaceForHeight edge cases
• namespaceMigrationDAAPI.GetIDs with different migration scenarios

🛡️ Security Considerations

• Input validation needed for user-supplied migration configuration
• Consider max limits on number of migrations to prevent DoS

📋 Recommendations

1. Add unit tests covering:

   • Migration parsing with valid/invalid formats
   • Namespace selection at various heights
   • Edge cases (empty migrations, boundary conditions)

2. Fix the non-deterministic iteration:

// Sort heights for deterministic selection
var heights []uint64
for h := range api.migrations {
    heights = append(heights, h)
}
sort.Slice(heights, func(i, j int) bool { return heights[i] < heights[j] })

3. Add validation in newNamespaceMigrationDAAPI:

func validateMigrations(migrations map[uint64]namespaces) error {
    // Check for valid namespace formats
    // Ensure no duplicate heights
    // Validate namespace strings
}

4. Consider adding logging for namespace switches to aid debugging

🎯 Overall Assessment

The implementation provides a clean solution for namespace migrations but needs refinement in error handling, validation, and test coverage before production use. The core logic is sound but the non-deterministic map iteration is a critical bug that must be fixed.

Recommendation: Address the iteration bug and add comprehensive tests before merging.

[claude]

Code Review for Namespace Migration Feature

I've completed a thorough review of this PR implementing namespace migration functionality. Here's my detailed feedback:

✅ Strengths

1. Well-designed migration logic: The implementation correctly handles namespace transitions at specified height boundaries, with proper inclusive semantics (namespace used until untilHeight inclusive).

2. Comprehensive test coverage: Excellent test suite covering edge cases including:

   • Empty migrations
   • Single and multiple migrations
   • Height boundaries (at, before, after)
   • Data namespace fallback behavior
   • Input validation and error handling

3. Clean separation of concerns: The namespaceMigrationDAAPI wrapper pattern keeps migration logic isolated without modifying core DA interfaces.

4. Good documentation: Clear comments explaining the migration semantics, especially the inclusive nature of untilHeight boundaries.

🔍 Issues & Suggestions

1. Incomplete DA Method Coverage 🚨

Critical: Only GetIDs is overridden in namespaceMigrationDAAPI. Other DA methods that accept namespace parameters (Get, GetProofs, Submit, SubmitWithOptions, Validate) will bypass migration logic and use incorrect namespaces.

Recommendation: Override all namespace-accepting methods:

func (api *namespaceMigrationDAAPI) Get(ctx context.Context, ids []ID, namespace []byte) ([]Blob, error) {
    // Need to determine height context for namespace resolution
    // This is challenging - may need to track height context
}

func (api *namespaceMigrationDAAPI) Submit(ctx context.Context, blobs []Blob, gasPrice float64, namespace []byte) ([]ID, error) {
    // For writes, likely use current namespace or track block height
}
// ... similar for other methods

2. Migration Ordering Validation ⚠️

The current implementation doesn't validate that migration heights are properly ordered. This could lead to confusing configurations.

Suggestion: Add validation in parseMigrations:

// Validate migrations are in ascending order
var prevHeight uint64
for height := range migrations {
    if prevHeight > 0 && height <= prevHeight {
        return nil, fmt.Errorf("migrations must be in ascending order")
    }
    prevHeight = height
}

3. Performance Consideration 📊

The findNamespaceForHeight method iterates through all migrations on every call. For frequent operations, this could be optimized.

Suggestion: Consider caching or pre-sorting migrations for binary search if performance becomes an issue.

4. Missing GetIDs Test 🧪

While findNamespaceForHeight and parseMigrations are well-tested, there's no test for the GetIDs method itself.

Recommendation: Add integration test:

func TestNamespaceMigrationDAAPI_GetIDs(t *testing.T) {
    // Mock the underlying API
    // Test that correct namespace is passed based on height
}

5. Error Message Enhancement 💬

Migration entry parsing errors could be more helpful by including the problematic entry.

Suggestion:

return nil, fmt.Errorf("invalid migration entry format: '%s' (expected untilHeight:namespace[:dataNamespace])", entry)

🔒 Security Considerations

• ✅ No security vulnerabilities identified
• ✅ Input validation properly handles edge cases
• ✅ No risk of integer overflow in height comparisons

📈 Performance Analysis

• Current O(n) lookup for each namespace resolution is acceptable for small migration sets
• Consider optimization if migrations exceed ~100 entries
• No memory leaks or goroutine concerns

📝 Additional Recommendations

1. Add migration example to documentation:

   --migrations "100:old-ns:old-data-ns,500:new-ns:new-data-ns"

2. Consider logging migrations on startup for operational visibility

3. Add metrics/monitoring for namespace switches to track migration events

Verdict

The implementation is solid with good test coverage, but the incomplete DA method coverage is a critical issue that must be addressed before merging. The feature appears to be marked [DNM] (Do Not Merge) which is appropriate given the current state.

Once the DA method coverage is complete, this will be a well-implemented feature that follows the codebase patterns effectively.

[claude]

Code Review for PR #2708: Namespace Migration Flag

I've reviewed the namespace migration implementation and have the following feedback:

✅ Strengths

1. Well-structured implementation: The decorator pattern using namespaceMigrationDAAPI wrapper cleanly extends the existing DA API without modifying core interfaces
2. Comprehensive test coverage: Excellent test suite covering edge cases, boundary conditions, and various migration scenarios
3. Clear migration semantics: The "untilHeight" approach (inclusive) provides intuitive migration boundaries

🎯 Code Quality & Best Practices

Good:

• Clean separation of concerns with dedicated parsing and namespace resolution functions
• Proper error handling in the parsing logic
• Follows Go conventions and project patterns

Areas for improvement:

1. Missing DA interface methods: The wrapper only implements GetIDs but the DA interface has many other methods that also use namespaces (Get, GetProofs, Commit, Submit, SubmitWithOptions, Validate). This could lead to incorrect behavior when these methods are called with migrated namespaces.

2. Documentation: While the code has some comments, the flag description could be clearer about the migration behavior:

   // Current: "namespace is used from height 0 up to and including untilHeight"
   // Suggestion: Add example like "100:ns1 means use ns1 for heights 0-100, then fall back to default"

🐛 Potential Issues

1. Incomplete wrapper implementation: Critical issue - other DA methods won't respect namespace migrations
2. Race condition potential: The namespaceMigrationDAAPI struct fields are not protected against concurrent access if the DA API is used from multiple goroutines
3. Migration ordering ambiguity: If migrations overlap (e.g., both 100 and 150 specified), the current logic picks the lowest applicable height, but this behavior isn't documented

⚡ Performance Considerations

1. Map lookups: The findNamespaceForHeight function iterates over all migrations for each call. For frequently called methods, consider caching or using a sorted slice for binary search
2. Memory efficiency: Each call to findNamespaceForHeight creates new byte slices from namespace strings. Consider pre-computing these during initialization

🔒 Security Considerations

1. Input validation: Good validation in parseMigrations, but consider adding namespace format validation (e.g., hex format, length constraints)
2. No injection risks: The parsing properly handles user input without risk of injection

📊 Test Coverage

Excellent test coverage with comprehensive scenarios including:

• Edge cases (height 0, max uint64)
• Error conditions
• Multiple migration scenarios
• Whitespace handling

Consider adding integration tests to verify the full DA API behavior with migrations.

📝 Recommendations

1. Priority 1 (Critical): Implement all DA interface methods in the wrapper to ensure consistent namespace migration behavior
2. Priority 2 (Important): Add mutex protection if the DA API might be accessed concurrently
3. Priority 3 (Nice to have): Optimize namespace lookup for better performance with many migrations
4. Priority 4 (Documentation): Add more detailed documentation and examples for the migration flag

Summary

This is a good foundation for namespace migration support, but it needs completion of the wrapper implementation for all DA methods before it's ready for production. The test coverage is excellent, and the overall approach is sound. With the critical issue addressed, this will be a valuable feature for handling namespace transitions in sync nodes.

[julienrbrt]

We have decided to make this app based. No need to bake this in our default app to keep things KISS.
An example implementation can be found in https://github.com/01builders/eden-testnet.