Skip to content

Observability, error handling, and API improvements#101

Open
moshemalawach wants to merge 14 commits into
EYBlockchain:masterfrom
moshemalawach:improvements/phase-2
Open

Observability, error handling, and API improvements#101
moshemalawach wants to merge 14 commits into
EYBlockchain:masterfrom
moshemalawach:improvements/phase-2

Conversation

@moshemalawach
Copy link
Copy Markdown

Summary

Structured improvements to observability, code quality, security, and developer experience across the Nightfall 4 CE codebase.

Observability

  • Migrate from log crate to tracing for structured logging with span support (57 source files updated)
  • Add Prometheus-compatible GET /metrics endpoint to both client and proposer services, with counters for requests, histograms for latency, and gauges for queue/mempool depth
  • Add GET /v1/ready readiness probe that verifies MongoDB connectivity, returning 200 or 503 with component-level status JSON
  • Restore panic logging via std::panic::set_hook (replaces log-panics removed during tracing migration)

Code quality

  • Adopt thiserror derive macros for error types in lib/src/error.rs, nightfall_client, and nightfall_proposer, reducing ~264 lines of manual Display/Error/From boilerplate while preserving all error messages and behavior
  • Split the monolithic client_nf_3.rs (1190 lines) into focused deposit.rs, transfer.rs, and withdraw.rs modules with a shared mod.rs, preserving the public API

Security

  • Cap the maximum number of registered proposers at 100 in RoundRobin.sol to prevent gas DoS from unbounded iteration in add_proposer() and rotate_proposer(). Includes a new Foundry test test_addProposer_revertsWhenMaxReached. V2/V3 contracts inherit the cap automatically.

Documentation

  • Add OpenAPI 3.0.3 specification (openapi.yaml) covering all 29 REST endpoints across client and proposer services, with request/response schemas derived from the actual Rust types

Test plan

  • cargo build succeeds
  • cargo clippy --all-targets -- -D warnings passes
  • cargo test unit tests pass
  • forge test Solidity tests pass (including new test_addProposer_revertsWhenMaxReached)
  • GET /metrics returns Prometheus text exposition format on client (:3000) and proposer (:3001)
  • GET /v1/ready returns {"status": "ready", "checks": {"database": "ok"}} when DB is up, 503 otherwise
  • Existing CI pipeline passes without regressions

The workspace Cargo.toml declared edition = "2024" which does not exist.
All individual crates already override to 2021, so this was silently
ignored but would break if any crate removed its local edition field.
…lers

Convert panicking unwrap/expect calls in production request handlers to
proper error propagation. Fixes hash().unwrap(), try_into().unwrap(),
and .first().unwrap() patterns. Also replaces a dbg!() macro with
debug!() logging and resolves two stale TODO comments.
Add limit and offset query parameters (default: limit=100, offset=0)
to prevent unbounded response bodies. Backwards-compatible — existing
clients without query params receive up to 100 results.
Wraps docker-compose, cargo, and forge commands into simple make
targets. Run 'make help' to see all available targets.
Replace the minimal 9-line README with comprehensive documentation
including architecture overview, prerequisites, quick start guide,
development commands, API endpoint tables, and documentation links.
- Log orphaned commitments in rollback paths instead of silently dropping
- Clamp pagination limit to i64::MAX before casting to prevent overflow
- Scope docker-clean to project images only (--rmi local)
- Restore panic logging via set_hook (replaces removed log-panics crate)
- Add /v1/ready and /metrics to OpenAPI spec
- Improve RoundRobin test storage slot comment for maintainability
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.

1 participant