Skip to content

Add trusted dealer security warnings#61

Merged
kwsantiago merged 1 commit intomainfrom
TrustedDealer
Jan 5, 2026
Merged

Add trusted dealer security warnings#61
kwsantiago merged 1 commit intomainfrom
TrustedDealer

Conversation

@wksantiago
Copy link
Contributor

@wksantiago wksantiago commented Jan 3, 2026

Summary by CodeRabbit

  • Documentation
    • Added security comparison guide for key generation modes (trusted dealer vs distributed key generation)
    • Included warnings about testing vs production approaches for key generation
    • Enhanced security documentation for threshold signature operations with production usage guidance

✏️ Tip: You can customize this high-level summary in your review settings.

@coderabbitai
Copy link

coderabbitai bot commented Jan 3, 2026

Walkthrough

This PR adds educational and warning content about FROST key generation, specifically emphasizing the difference between Trusted Dealer (single-machine key generation) and distributed DKG approaches. Changes include documentation updates in the README, CLI warnings during key generation, and code documentation comments.

Changes

Cohort / File(s) Summary
FROST Trusted Dealer Warnings & Documentation
README.md, keep-cli/src/main.rs, keep-core/src/frost/dealer.rs
Added comparison table and explanation of DKG vs. Trusted Dealer in README; added multi-line CLI warning banner output in FROST generation flow alerting users to testing-only nature and private key exposure risks; added documentation comment to TrustedDealer struct with production guidance.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~3 minutes

Possibly related issues

Suggested reviewers

  • kwsantiago

Poem

🐰 A dealer of keys, both trusted and true,
Now warns all the users of what not to do—
"For testing alone!" the bunny did cry,
"Use DKG for production, oh my, oh my!"
With docs and with banners, we hop toward care. 🌟

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'Add trusted dealer security warnings' accurately reflects the main changes across all modified files: adding documentation warnings in dealer.rs, a warning banner in main.rs, and a security comparison section in README.md.
✨ Finishing touches
  • 📝 Generate docstrings

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.

@wksantiago wksantiago requested a review from kwsantiago January 3, 2026 17:31
@wksantiago wksantiago self-assigned this Jan 3, 2026
@wksantiago wksantiago linked an issue Jan 3, 2026 that may be closed by this pull request
3 tasks
Copy link

@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: 0

🧹 Nitpick comments (1)
README.md (1)

175-175: Use proper markdown heading instead of bold emphasis.

Line 175 uses bold text for a section title, but it should use a proper markdown heading to maintain document structure and comply with markdown best practices.

🔎 Proposed fix
-**Security: DKG vs Trusted Dealer**
+#### Security: DKG vs Trusted Dealer
📜 Review details

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 547ce57 and cfe179d.

📒 Files selected for processing (3)
  • README.md
  • keep-cli/src/main.rs
  • keep-core/src/frost/dealer.rs
🧰 Additional context used
🪛 markdownlint-cli2 (0.18.1)
README.md

175-175: Emphasis used instead of a heading

(MD036, no-emphasis-as-heading)

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
  • GitHub Check: build
🔇 Additional comments (3)
keep-core/src/frost/dealer.rs (1)

50-58: LGTM! Clear and actionable security documentation.

The documentation comment effectively communicates the security implications of the trusted dealer approach and provides clear guidance for production use.

README.md (1)

177-184: LGTM! Excellent security comparison.

The comparison table and explanatory text clearly communicate the security differences between trusted dealer and distributed DKG approaches. This will help users make informed decisions about which approach to use.

keep-cli/src/main.rs (1)

1651-1656: LGTM! Well-placed security warning.

The warning banner is properly positioned to alert users before key generation begins, and the messaging is consistent with the documentation added in other files. This ensures users are informed about the security implications of using the trusted dealer approach.

@kwsantiago kwsantiago merged commit 7ce56f9 into main Jan 5, 2026
2 checks passed
@kwsantiago kwsantiago deleted the TrustedDealer branch January 5, 2026 13:56
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.

Add TrustedDealer production warning and DKG security docs

2 participants