Require port for Monero node URLs#9387
Merged
Merged
Conversation
Add validation to ensure that manually entered Monero node URLs include a port number. If a port is missing, an error message is displayed to the user. - Update `AddMoneroNodeViewModel` to validate the presence of a port in the RPC URL. - Update `AddMoneroNodeScreen` input hint to show the expected format (e.g., `https://node.com:port`). - Add a fallback to port 443 in `MoneroNodeManager` for nodes saved before this validation was implemented. - Add `AddMoneroNode_Error_PortRequired` string resource.
Passing base64-encoded contents of large `strings.xml` files as command-line arguments to `jq` can exceed the kernel's 128KB per-argument limit (`MAX_ARG_STRLEN`), causing the workflow to fail. Update the script to write encoded file contents to temporary files and use `jq`'s `--rawfile` and `--slurpfile` flags to process the data, avoiding the argument length restriction.
[skip ci]
Contributor
📝 WalkthroughWalkthroughMonero node URLs now require explicit ports, normalize default HTTPS ports, and use canonical duplicate detection. The translation workflow now builds GraphQL commit additions through JSONL and adds a skip-CI message body. New localization entries and URL guidance were added. ChangesMonero node validation
Translation commit workflow
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant AddMoneroNodeScreen
participant AddMoneroNodeViewModel
participant MoneroNodeManager
AddMoneroNodeScreen->>AddMoneroNodeViewModel: submit node URL
AddMoneroNodeViewModel->>MoneroNodeManager: check canonical endpoint
MoneroNodeManager-->>AddMoneroNodeViewModel: return duplicate status
AddMoneroNodeViewModel-->>AddMoneroNodeScreen: show validation result
sequenceDiagram
participant TranslationWorkflow
participant additions.jsonl
participant GitHubGraphQL
TranslationWorkflow->>additions.jsonl: write encoded translation files
additions.jsonl-->>TranslationWorkflow: provide JSONL additions
TranslationWorkflow->>GitHubGraphQL: call createCommitOnBranch
GitHubGraphQL-->>TranslationWorkflow: return commit result
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Comment |
abdrasulov
approved these changes
Jul 13, 2026
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
.github/workflows/translate.yml (1)
50-60: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winSimplify: use jq's
@base64filter instead of externalbase64command.The current approach base64-encodes each file with the external
base64command, writes to an intermediate temp file, then reads it back with--rawfile. This works but introduces an unnecessary temp file and a trailing newline from GNUbase64 -w0that ends up in thecontentsstring.jq's built-in
@base64filter can encode the raw file content directly, eliminating the intermediate file and the newline issue in one step:♻️ Proposed refactor
# Pass file contents to jq via --rawfile/--slurpfile, never as # command-line arguments: base64 of a large strings.xml exceeds # the kernel's 128KB per-argument limit (MAX_ARG_STRLEN). additions="$RUNNER_TEMP/additions.jsonl" : > "$additions" while IFS= read -r f; do - base64 -w0 "$f" > "$RUNNER_TEMP/file.b64" - jq -n --arg path "$f" --rawfile contents "$RUNNER_TEMP/file.b64" \ - '{path: $path, contents: $contents}' >> "$additions" + jq -n --arg path "$f" --rawfile contents "$f" \ + '{path: $path, contents: ($contents | `@base64`)}' >> "$additions" done <<< "$files"This reads the original file once, base64-encodes inside jq (no trailing newline), and removes the
file.b64temp file entirely.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/translate.yml around lines 50 - 60, Update the loop around the additions JSONL generation to remove the external base64 command and RUNNER_TEMP/file.b64 intermediate file, and use jq’s `@base64` filter on each file’s raw contents while preserving the existing path and contents fields and record-count validation.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In @.github/workflows/translate.yml:
- Around line 50-60: Update the loop around the additions JSONL generation to
remove the external base64 command and RUNNER_TEMP/file.b64 intermediate file,
and use jq’s `@base64` filter on each file’s raw contents while preserving the
existing path and contents fields and record-count validation.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: cb5a633a-6a59-40d6-9419-12735cede63c
⛔ Files ignored due to path filters (9)
walletkit/src/main/res/values-de/strings.xmlis excluded by!**/res/values-*/strings.xmlwalletkit/src/main/res/values-es/strings.xmlis excluded by!**/res/values-*/strings.xmlwalletkit/src/main/res/values-fa/strings.xmlis excluded by!**/res/values-*/strings.xmlwalletkit/src/main/res/values-fr/strings.xmlis excluded by!**/res/values-*/strings.xmlwalletkit/src/main/res/values-ko/strings.xmlis excluded by!**/res/values-*/strings.xmlwalletkit/src/main/res/values-pt-rBR/strings.xmlis excluded by!**/res/values-*/strings.xmlwalletkit/src/main/res/values-ru/strings.xmlis excluded by!**/res/values-*/strings.xmlwalletkit/src/main/res/values-tr/strings.xmlis excluded by!**/res/values-*/strings.xmlwalletkit/src/main/res/values-zh/strings.xmlis excluded by!**/res/values-*/strings.xml
📒 Files selected for processing (6)
.github/workflows/translate.ymltranslation_snapshot.jsonwalletkit/src/main/java/io/horizontalsystems/walletkit/core/managers/MoneroNodeManager.ktwalletkit/src/main/java/io/horizontalsystems/walletkit/modules/moneronetwork/addnode/AddMoneroNodeScreen.ktwalletkit/src/main/java/io/horizontalsystems/walletkit/modules/moneronetwork/addnode/AddMoneroNodeViewModel.ktwalletkit/src/main/res/values/strings.xml
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
#8358
Summary by CodeRabbit
New Features
Bug Fixes
Chores