Skip to content

feat: add nats authentication#1117

Open
bmuddha wants to merge 2 commits intobmuddha/feat/streaming-blockhashfrom
bmuddha/replicator/auth
Open

feat: add nats authentication#1117
bmuddha wants to merge 2 commits intobmuddha/feat/streaming-blockhashfrom
bmuddha/replicator/auth

Conversation

@bmuddha
Copy link
Copy Markdown
Collaborator

@bmuddha bmuddha commented Mar 30, 2026

Summary

Added nkey based nats authentication to the client.

Summary by CodeRabbit

  • New Features

    • New replication modes in config with default "standalone"; examples for standby and replica-only, including optional authority override.
    • Replication connections now support authentication secrets.
  • Improvements

    • Added startup timing and clearer replication broker logging.
    • Improved snapshot fetch logs and automatic reset when a fetched snapshot is applied.
    • Secrets are redacted in debug/serialized outputs.
  • Documentation

    • Example config updated with new replication fields and defaults.
  • Tests

    • Tests added for config defaults and secret redaction.

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai bot commented Mar 30, 2026

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: 234f02ee-2257-438b-bde2-aaa5b7f70c51

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review

Walkthrough

Replication configuration was expanded from a URL-only form to a ReplicationConfig struct containing url, secret, and optional authority_override. ReplicationMode variants now embed ReplicationConfig and expose config() and updated authority_override() behavior. Broker connection API now accepts a secret and uses it for NATS authentication. MagicValidator::try_from_config takes mut config, refactors broker init (with timing), logs snapshot fetch attempts/results, sets accountsdb.reset = true when inserting a snapshot, and uses the new replication config flow. Example TOML docs for replication modes were added. Debug/Serialize redact secrets.

Suggested reviewers

  • GabrielePicco
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch bmuddha/replicator/auth

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@bmuddha bmuddha mentioned this pull request Mar 30, 2026
1 task
Copy link
Copy Markdown
Collaborator Author

bmuddha commented Mar 30, 2026

Warning

This pull request is not mergeable via GitHub because a downstack PR is open. Once all requirements are satisfied, merge this PR as a stack on Graphite.
Learn more

This stack of pull requests is managed by Graphite. Learn more about stacking.

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 4

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@config.example.toml`:
- Around line 124-135: The example config currently enables a live standby
replication block (validator.replication-mode.stand-by) which contradicts the
documented default "standalone" and causes a failing startup for users copying
the file; change the file so the replication sample is commented out or replaced
with a commented example snippet (e.g., show the stand-by/replica-only
structures as commented examples) and restore the documented default by ensuring
no active [validator.replication-mode.*] table is present — reference the
validator.replication-mode.stand-by block and remove or comment its url/secret
table entries so the example remains a safe, non-operational baseline.

In `@magicblock-api/src/magic_validator.rs`:
- Around line 211-214: Do not repurpose config.accountsdb.reset as runtime
state; create a separate boolean flag (e.g., loaded_replication_snapshot or
skip_prune_after_snapshot) on the validator/state struct and set that instead of
mutating config.accountsdb.reset after AccountsDb::new. Locate the code that
currently sets config.accountsdb.reset = true (and any runtime checks that read
self.config.accountsdb.reset) and change those to use the new flag, ensure
AccountsDb::new still receives the original config value unchanged, and update
any pruning logic to check the new flag (e.g., loaded_replication_snapshot or
skip_prune_after_snapshot) rather than self.config.accountsdb.reset.

In `@magicblock-config/src/config/validator.rs`:
- Around line 34-38: ReplicationConfig currently stores the raw NKEY seed in the
public field secret (pub secret: String) and derives Debug/Serialize/Clone,
risking accidental leakage; change secret to a redacted newtype (e.g., NKeySeed
or RedactedSecret) or implement custom Debug and Serialize for ReplicationConfig
that omits or masks the secret value (and keep Clone as needed). Locate the
ReplicationConfig struct and replace the String secret type with the redaction
type or implement fmt::Debug and serde::Serialize manually for ReplicationConfig
to ensure the secret is not printed or serialized (but still available for
runtime use), and update any constructors/consumers to use the new type or to
call an accessor that returns the raw seed only when explicitly needed.
- Around line 29-31: The StandBy and ReplicaOnly enum variants (using
ReplicationConfig) now require a new `secret` field and will fail
deserialization for old URL-only configs; update the deserialization to be
backward-compatible by accepting the legacy URL-only shape and mapping it to the
new ReplicationConfig (e.g., implement custom Deserialize for ReplicationConfig
or use a serde untagged/enum helper) so when a string URL is provided you
populate the URL and set the secret to None or a safe default, or alternatively
detect the legacy shape and emit a clear, targeted migration error; locate the
ReplicationConfig type and the StandBy/ReplicaOnly enum in validator.rs and add
the custom Deserialize logic or untagged wrapper to preserve compatibility for
one release.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: d180e652-a132-4f71-8386-ecf5bf4349e6

📥 Commits

Reviewing files that changed from the base of the PR and between 9ce31db and 8f539a7.

⛔ Files ignored due to path filters (1)
  • Cargo.lock is excluded by !**/*.lock
📒 Files selected for processing (4)
  • config.example.toml
  • magicblock-api/src/magic_validator.rs
  • magicblock-config/src/config/validator.rs
  • magicblock-replicator/src/nats/broker.rs

@bmuddha bmuddha force-pushed the bmuddha/replicator/auth branch from 8f539a7 to b961ff9 Compare March 30, 2026 12:14
@bmuddha bmuddha force-pushed the bmuddha/feat/streaming-blockhash branch from 9ce31db to 9b8ab0b Compare March 30, 2026 12:14
@bmuddha bmuddha changed the base branch from bmuddha/feat/streaming-blockhash to graphite-base/1117 April 1, 2026 10:11
@bmuddha bmuddha force-pushed the bmuddha/replicator/auth branch from b961ff9 to 709b6d0 Compare April 1, 2026 15:31
@bmuddha bmuddha force-pushed the graphite-base/1117 branch from 9b8ab0b to 342a8cd Compare April 1, 2026 15:31
@bmuddha bmuddha changed the base branch from graphite-base/1117 to bmuddha/feat/streaming-blockhash April 1, 2026 15:31
@bmuddha bmuddha marked this pull request as ready for review April 1, 2026 15:37
@bmuddha bmuddha changed the base branch from bmuddha/feat/streaming-blockhash to graphite-base/1117 April 2, 2026 08:56
@bmuddha bmuddha force-pushed the bmuddha/replicator/auth branch from 709b6d0 to b0e4aef Compare April 2, 2026 09:26
@bmuddha bmuddha changed the base branch from graphite-base/1117 to bmuddha/feat/streaming-blockhash April 2, 2026 09:26
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 3

♻️ Duplicate comments (1)
config.example.toml (1)

131-140: ⚠️ Potential issue | 🟡 Minor

Use a dialable host in the NATS examples.

0.0.0.0 is a bind address, not a client destination, so uncommenting either sample gives users a broken connection string.

Suggested fix
-# url = "nats://0.0.0.0:4222"
+# url = "nats://127.0.0.1:4222"
 ...
-# url = "nats://0.0.0.0:4222"
+# url = "nats://127.0.0.1:4222"
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@config.example.toml` around lines 131 - 140, Update the example NATS URLs
under the commented blocks for validator.replication-mode.stand-by and
validator.replication-mode.replica-only so they use a dialable host instead of
the bind address 0.0.0.0 (e.g., replace url = "nats://0.0.0.0:4222" with a
reachable host like localhost or a real hostname), ensuring the sample
connection strings work when uncommented.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@magicblock-config/src/tests.rs`:
- Around line 720-729: The test test_replication_config_debug_redacts_secret
currently only checks Debug redaction; extend it to also assert the Serialize
redaction path by serializing the same ReplicationConfig (the cfg variable)
using serde_json::to_string (or serde_json::to_value) and then asserting the
serialized string contains "<redacted>" and does not contain the literal
"SUASECRET". Keep the existing Debug assertions and add these two Serialize
assertions referencing ReplicationConfig and the same cfg instance.
- Around line 407-410: The assertion currently moves
config.validator.replication_mode out of config which prevents further use of
config; change the match to borrow the field instead, e.g. use
&config.validator.replication_mode in the matches! call so the enum is matched
by reference (keep the same variant path
crate::config::validator::ReplicationMode::Standalone and the assert! wrapper)
to avoid moving the value.

---

Duplicate comments:
In `@config.example.toml`:
- Around line 131-140: Update the example NATS URLs under the commented blocks
for validator.replication-mode.stand-by and
validator.replication-mode.replica-only so they use a dialable host instead of
the bind address 0.0.0.0 (e.g., replace url = "nats://0.0.0.0:4222" with a
reachable host like localhost or a real hostname), ensuring the sample
connection strings work when uncommented.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: b5eade75-b4d5-4335-b37d-d86b8986bd7e

📥 Commits

Reviewing files that changed from the base of the PR and between 709b6d0 and b0e4aef.

📒 Files selected for processing (5)
  • config.example.toml
  • magicblock-api/src/magic_validator.rs
  • magicblock-config/src/config/validator.rs
  • magicblock-config/src/tests.rs
  • magicblock-replicator/src/nats/broker.rs

@bmuddha bmuddha force-pushed the bmuddha/replicator/auth branch from b0e4aef to 24e341c Compare April 2, 2026 09:38
@bmuddha bmuddha changed the base branch from bmuddha/feat/streaming-blockhash to graphite-base/1117 April 2, 2026 11:01
@bmuddha bmuddha force-pushed the bmuddha/replicator/auth branch from 24e341c to 6553ce1 Compare April 2, 2026 12:00
@bmuddha bmuddha changed the base branch from graphite-base/1117 to bmuddha/feat/streaming-blockhash April 2, 2026 12:00
@bmuddha bmuddha changed the base branch from bmuddha/feat/streaming-blockhash to graphite-base/1117 April 2, 2026 12:48
@bmuddha bmuddha force-pushed the bmuddha/replicator/auth branch from 6553ce1 to ed5d218 Compare April 3, 2026 06:57
@bmuddha bmuddha changed the base branch from graphite-base/1117 to bmuddha/feat/streaming-blockhash April 3, 2026 06:58
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