feat: add use_cj feature to RPCs send, sendall and fundrawtransaction#7261
feat: add use_cj feature to RPCs send, sendall and fundrawtransaction#7261knst wants to merge 1 commit intodashpay:developfrom
Conversation
✅ No Merge Conflicts DetectedThis PR currently has no conflicts with other open PRs. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1160ac9529
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| if (options.exists("use_cj")) { | ||
| coin_control.UseCoinJoin(options["use_cj"].get_bool()); | ||
| } |
There was a problem hiding this comment.
use_cj for manually specified sendall inputs
When sendall is called with both options.use_cj=true and options.inputs, this new flag does not actually constrain the provided inputs to fully mixed coins: the inputs branch later accepts any wallet UTXO and skips the AvailableCoins(..., &coin_control, ...) filtering path. In this scenario, callers can unintentionally spend non-CoinJoin funds despite opting into CoinJoin-only behavior, so this should either validate each specified input as fully mixed or reject the combination as incompatible.
Useful? React with 👍 / 👎.
WalkthroughThis change introduces a new Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes 🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/wallet/rpc/spend.cpp`:
- Line 525: The shared options schema for FundTransaction() currently includes
the undocumented "use_cj" flag which unintentionally exposes it on
walletcreatefundedpsbt; either remove "use_cj" from the shared schema and add it
only to the RPCs that should accept it (e.g. include the {"use_cj", ...} entry
in the specific RPC handler's options instead of the common list used by
FundTransaction()), or keep it in the schema but add the corresponding
documentation for walletcreatefundedpsbt; locate the {"use_cj",
UniValueType(UniValue::VBOOL)} entry, the FundTransaction() call sites, and the
walletcreatefundedpsbt RPC handler and apply one of these changes so the option
is either documented or only accepted by the intended RPCs.
- Around line 1149-1151: In sendall, options.inputs bypass CoinJoin
restrictions: when coin_control.UseCoinJoin is true and options.inputs is
provided, validate and enforce CoinJoin eligibility on those explicit inputs
instead of accepting them unconditionally; specifically, in the branch handling
options.inputs check coin_control.UseCoinJoin() (or the internal flag set by
options["use_cj"]) and filter the provided inputs to only include UTXOs that
meet the wallet's CoinJoin/mixing eligibility (or return an error if any
provided input is not eligible), updating the inputs used for constructing the
transaction so CoinJoin rules are enforced.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 6c7262d7-30a0-4177-87ea-ca0b6144b1f2
📒 Files selected for processing (1)
src/wallet/rpc/spend.cpp
| {"conf_target", UniValueType(UniValue::VNUM)}, | ||
| {"estimate_mode", UniValueType(UniValue::VSTR)}, | ||
| {"input_sizes", UniValueType(UniValue::VARR)}, | ||
| {"use_cj", UniValueType(UniValue::VBOOL)}, |
There was a problem hiding this comment.
use_cj becomes accepted in walletcreatefundedpsbt without being documented there.
Because FundTransaction() is shared, adding "use_cj" to this strict options schema also enables it for walletcreatefundedpsbt (which still doesn’t advertise it in its options docs). Please either document it there too or explicitly gate it per-RPC to avoid undocumented API behavior.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/wallet/rpc/spend.cpp` at line 525, The shared options schema for
FundTransaction() currently includes the undocumented "use_cj" flag which
unintentionally exposes it on walletcreatefundedpsbt; either remove "use_cj"
from the shared schema and add it only to the RPCs that should accept it (e.g.
include the {"use_cj", ...} entry in the specific RPC handler's options instead
of the common list used by FundTransaction()), or keep it in the schema but add
the corresponding documentation for walletcreatefundedpsbt; locate the
{"use_cj", UniValueType(UniValue::VBOOL)} entry, the FundTransaction() call
sites, and the walletcreatefundedpsbt RPC handler and apply one of these changes
so the option is either documented or only accepted by the intended RPCs.
| if (options.exists("use_cj")) { | ||
| coin_control.UseCoinJoin(options["use_cj"].get_bool()); | ||
| } |
There was a problem hiding this comment.
sendall does not enforce use_cj when options.inputs is provided.
use_cj is set on coin_control, but explicit inputs bypass CoinJoin filtering in the options.inputs branch. With use_cj=true, non-mixed UTXOs can still be spent there.
Proposed fix
@@
} else if (options.exists("inputs")) {
for (const CTxIn& input : rawTx.vin) {
+ if (coin_control.IsUsingCoinJoin() && !pwallet->IsFullyMixed(input.prevout)) {
+ throw JSONRPCError(RPC_INVALID_PARAMETER,
+ strprintf("Input not available. UTXO (%s:%d) is not fully mixed while use_cj=true.",
+ input.prevout.hash.ToString(), input.prevout.n));
+ }
if (pwallet->IsSpent(input.prevout)) {
throw JSONRPCError(RPC_INVALID_PARAMETER, strprintf("Input not available. UTXO (%s:%d) was already spent.", input.prevout.hash.ToString(), input.prevout.n));
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/wallet/rpc/spend.cpp` around lines 1149 - 1151, In sendall,
options.inputs bypass CoinJoin restrictions: when coin_control.UseCoinJoin is
true and options.inputs is provided, validate and enforce CoinJoin eligibility
on those explicit inputs instead of accepting them unconditionally;
specifically, in the branch handling options.inputs check
coin_control.UseCoinJoin() (or the internal flag set by options["use_cj"]) and
filter the provided inputs to only include UTXOs that meet the wallet's
CoinJoin/mixing eligibility (or return an error if any provided input is not
eligible), updating the inputs used for constructing the transaction so CoinJoin
rules are enforced.
1 participant
Issue being fixed or feature implemented
The RPCs
send,sendall,fundrawtransactiondoesn't have a functionality to use cj to fund transaction.What was done?
Added flag use_cj to the options and use it coin-select.
How Has This Been Tested?
Call RPC
send '[{"yRgziYdxyApbFsvph89JAVMzjsv8en16bA":0.01}]' null "unset" null '{"use_cj":true}'It produced tx
getrawtransaction 8df1998514388b9c19705077e743c5bc2fd6dd378a6287cd865f0a1b91e8afaf 1which looks as expected.Breaking Changes
N/A
Checklist: