Skip to content

Conversation

@dmytro-samoylenko
Copy link
Contributor

In addition to %s (TxID) -walletnotify can also set %t which is replaced by transaction type: 'spark' or 'regular'

In addition to %s (TxID)  -walletnotify can also set %t which is replaced by transaction type: 'spark' or 'regular'
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Nov 17, 2025

Walkthrough

Enhanced wallet notifications with a new replacement token %t that expands to transaction type ("spark" or "regular"). The implementation adds the token substitution in AddToWallet and documents the feature in wallet help text.

Changes

Cohort / File(s) Summary
Wallet Notification Token Enhancement
src/wallet/wallet.cpp
Added %t replacement token to wallet notification commands indicating transaction type; updated help text documentation for the new token

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

  • Single file modified with straightforward, homogeneous changes (token replacement additions)
  • Two related modifications: AddToWallet implementation and GetWalletHelpString documentation update
  • No complex logic or control flow changes

Poem

🐰 A token hops along so bright,
%t shows the transaction type!
Spark or regular, now you'll see,
In notifications, clear as can be!

Pre-merge checks and finishing touches

❌ Failed checks (2 warnings)
Check name Status Explanation Resolution
Description check ⚠️ Warning The PR description is incomplete. It lacks a 'PR intention' section explaining what the change does and what issue it solves, as required by the template. Add a 'PR intention' section documenting the purpose of adding the %t format code, the use case it enables, and any related issues or requirements.
Docstring Coverage ⚠️ Warning Docstring coverage is 20.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (1 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and specifically summarizes the main change: adding a transaction type format code (%t) to the -walletnotify feature.
✨ 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 fecd68a and 1ae720f.

📒 Files selected for processing (1)
  • src/wallet/wallet.cpp (6 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
src/wallet/wallet.cpp (1)
src/util.cpp (2)
  • HelpMessageOpt (472-477)
  • HelpMessageOpt (472-472)
🔇 Additional comments (6)
src/wallet/wallet.cpp (6)

1304-1306: %t replacement for -walletnotify looks correct

Token substitution mirrors existing %s logic; using IsSparkTransaction() to emit "spark"/"regular" matches the PR spec. LGTM.


6058-6058: Help text updated appropriately

Docs clearly state %s (TxID) and new %t (transaction type: spark/regular). Matches implementation.


814-814: No functional change detected in this hunk.


7207-7207: No functional change detected in this hunk.


7261-7261: No functional change detected in this hunk.


7273-7273: No functional change detected in this hunk.

Tip

📝 Customizable high-level summaries are now available in beta!

You can now customize how CodeRabbit generates the high-level summary in your pull requests — including its content, structure, tone, and formatting.

  • Provide your own instructions using the high_level_summary_instructions setting.
  • Format the summary however you like (bullet lists, tables, contributor stats, etc.).
  • Use high_level_summary_in_walkthrough to move the summary from the description to the walkthrough section.

Example:

"Create a concise high-level summary as a bullet-point list. Then include a Markdown table showing lines added and removed by each contributing author."

Note: This feature is currently in beta for Pro-tier users, and pricing will be announced later.


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.

Copy link
Contributor

@levonpetrosyan93 levonpetrosyan93 left a comment

Choose a reason for hiding this comment

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

LGTM

@reubenyap reubenyap merged commit 0fb0139 into firoorg:master Nov 18, 2025
5 of 11 checks passed
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.

5 participants