Skip to content

fix: follow-up fixes for #6855 - rpc annotation and name of repo #6873

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
PastaPastaPasta merged 2 commits into from
Oct 14, 2025
Merged

fix: follow-up fixes for #6855 - rpc annotation and name of repo #6873

PastaPastaPasta merged 2 commits into from
Oct 14, 2025

Conversation

@knst
Copy link
Collaborator

@knst knst commented Oct 4, 2025

Issue being fixed or feature implemented

Follow-up #6855

What was done?

  • fix RPC annotation for arguments sendrawtransaction
  • fix reference to repo dashcore-detached-sigs dash-detached-sigs

How Has This Been Tested?

N/A

Breaking Changes

N/A

Checklist:

  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have added or updated relevant unit/integration/functional/e2e tests
  • I have made corresponding changes to the documentation
  • I have assigned this pull request to a milestone

@knst knst added this to the 23 milestone Oct 4, 2025
@github-actions
Copy link

github-actions bot commented Oct 4, 2025

✅ No Merge Conflicts Detected

This PR currently has no conflicts with other open PRs.

@coderabbitai
Copy link

coderabbitai bot commented Oct 4, 2025

Walkthrough

  • doc/release-process.md: Updated a directory path reference in the Windows/macOS detached codesign payloads section from dashcore-detached-sigs to dash-detached-sigs.
  • src/rpc/mempool.cpp: Updated sendrawtransaction RPC parameter type checking to include an additional boolean (UniValue::VBOOL) for a fourth argument, aligning with existing use of request.params[3] for a bypasslimits-style flag. No other logic or control flow changes.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

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
Title Check ✅ Passed The title accurately reflects the pull request’s two primary fixes by referencing both the RPC annotation adjustment and the repository rename, and it includes the related issue number for context, making the main changes clear. Although it lists both fixes and is slightly verbose, it remains specific and avoids generic terms, ensuring that a reviewer scanning history understands the key updates. Therefore, it aligns with the guidelines for a concise and relevant PR title.
Description Check ✅ Passed The description clearly outlines the issue being addressed and specifies the two fixes applied to the RPC annotation and repository naming, directly matching the changes in the pull request. It provides relevant context about follow-up to the previous issue, lists what was done, and notes testing and breaking change considerations. Hence, it satisfies the requirement of being related to the changeset without introducing off-topic information.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between d2a6c34 and a60cc1b.

📒 Files selected for processing (2)
  • doc/release-process.md (1 hunks)
  • src/rpc/mempool.cpp (1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
doc/**

📄 CodeRabbit inference engine (CLAUDE.md)

Unless specifically prompted, avoid making changes to the doc directory (documentation)

Files:

  • doc/release-process.md
src/**/*.{cpp,h,cc,cxx,hpp}

📄 CodeRabbit inference engine (CLAUDE.md)

src/**/*.{cpp,h,cc,cxx,hpp}: Dash Core C++ codebase must be written in C++20 and require at least Clang 16 or GCC 11.1
Dash uses unordered_lru_cache for efficient caching with LRU eviction

Files:

  • src/rpc/mempool.cpp
🧠 Learnings (1)
📚 Learning: 2025-08-08T07:01:47.332Z 
Learnt from: knst
PR: dashpay/dash#6805
File: src/wallet/rpc/wallet.cpp:357-357
Timestamp: 2025-08-08T07:01:47.332Z
Learning: In src/wallet/rpc/wallet.cpp, the upgradetohd RPC now returns a UniValue string message (RPCResult::Type::STR) instead of a boolean, including guidance about mnemonic backup and null-character passphrase handling; functional tests have been updated to assert returned strings in several cases.

Applied to files:

  • src/rpc/mempool.cpp
⏰ 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 container / Build container
🔇 Additional comments (2)
doc/release-process.md (1)

151-151: LGTM! Repository name correction aligns with actual repository.

The path reference now correctly uses dash-detached-sigs, consistent with the repository URL referenced at line 166.

src/rpc/mempool.cpp (1)

61-62: LGTM! Type checking now properly validates all parameters.

The addition of two UniValue::VBOOL entries correctly validates the instantsend (param[2]) and bypasslimits (param[3]) boolean parameters, aligning type checking with the parameter definitions (lines 40-41) and actual usage at line 79.

Note: The AI summary states "an additional boolean" (singular) but the change adds type checks for two boolean parameters (instantsend and bypasslimits), not just one.

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.

UdjinM6
UdjinM6 approved these changes Oct 5, 2025
View reviewed changes
Copy link

@UdjinM6 UdjinM6 left a comment

Choose a reason for hiding this comment

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

utACK a60cc1b

@knst knst requested a review from PastaPastaPasta October 5, 2025 20:49
kwvg
kwvg approved these changes Oct 14, 2025
View reviewed changes
Copy link
Collaborator

@kwvg kwvg left a comment

Choose a reason for hiding this comment

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

utACK a60cc1b

@PastaPastaPasta PastaPastaPasta merged commit 2350339 into dashpay:develop Oct 14, 2025
32 of 34 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@UdjinM6 UdjinM6 UdjinM6 approved these changes

@kwvg kwvg kwvg approved these changes

@PastaPastaPasta PastaPastaPasta Awaiting requested review from PastaPastaPasta

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

23

Development

Successfully merging this pull request may close these issues.

4 participants

@knst @UdjinM6 @kwvg @PastaPastaPasta