Handle restricted self registration #6610
Conversation
📝 Walkthrough📝 Walkthrough📝 Walkthrough📝 WalkthroughWalkthroughThe changes in this pull request modify the Changes
Assessment against linked issues
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (2)
hoprd/rest-api/src/checks.rs (2)
75-75: Fix typo and enhance comment clarity.The comment contains a typo and could be more descriptive about the business logic.
- // when the error is deivision by zero, it's caused by the self-registration is forbidden to the public, and thus returns false. + // A "division by zero" error indicates that self-registration is not enabled in the NetworkRegistry contract, making the node ineligible.
77-81: Improve error message clarity and reduce duplication.The error message could be more specific about the self-registration restriction, and the duplicate message should be extracted to a constant.
+ const INELIGIBLE_MESSAGE: &str = "Node not eligible: self-registration is not enabled"; + match hopr.get_eligibility_status().await { Ok(true) => (StatusCode::OK, "").into_response(), - Ok(false) => (StatusCode::PRECONDITION_FAILED, "Node not eligible").into_response(), + Ok(false) => (StatusCode::PRECONDITION_FAILED, INELIGIBLE_MESSAGE).into_response(), Err(e) => { match e { hopr_lib::errors::HoprLibError::ChainApi(chain_api_err) => { if chain_api_err.to_string().contains("division by zero") { - (StatusCode::PRECONDITION_FAILED, "Node not eligible").into_response() + (StatusCode::PRECONDITION_FAILED, INELIGIBLE_MESSAGE).into_response()
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (1)
hoprd/rest-api/src/checks.rs(1 hunks)
🔇 Additional comments (2)
hoprd/rest-api/src/checks.rs (2)
73-85: LGTM! Error handling approach is well-structured.
The new error handling correctly differentiates between chain API errors and other errors, providing appropriate status codes and maintaining backward compatibility.
76-82: Verify error handling in the NetworkRegistry contract.
The code assumes that a "division by zero" error specifically indicates disabled self-registration. Let's verify this assumption in the contract code.
|
Yes, and also throw error when it's actually an error |
Co-authored-by: Tibor <9529609+Teebor-Choka@users.noreply.github.com>
Co-authored-by: Lukas <lukas.pohanka@inina.net>
00236cb to
38ce66e
Compare
Close #6560
The root cause it that self-registration on the network registry is not enabled. The "div 0" error is expected.
The error can be better handled on the smart contract level, which will be tackled in #5501