docs: clarify that shared registration consent is per share, not per funding input - #7771
Conversation
…funding input Consensus requires a consent signature from every share in a shared ProRegTx's table, but funding inputs are not tied to any share. A wallet can therefore sign its inputs into a registration whose table has no share for it, and the transaction is valid. The release notes said "every participant consents", which suggested otherwise, and the shared_register_prepare help went straight from shared_combine to signrawtransactionwithwallet without a step to check the table. Say "share owner" where consent is meant, state that inputs are not tied to shares, and tell each participant to find its own share in the decoded table before signing its funding inputs. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
✅ Final review complete — Phase 1 only — no blockers (commit 055ccd7) · triage: trivial |
|
Navigate logical layers of code changes, visualize relationships, and explore their blast radius. No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (2)
Included review availability: This review used your included allowance. Your plan provides up to 4 included reviews per hour; 3 remain after this review. WalkthroughThe Priority: ⬇️ Low Estimated code review effort: 1 (Trivial) | ~3 minutes Merge Risk: ⚪ Minimal · up to The updated guidance matches how share consent and decoded registration details work. No behavior changes or remaining merge risks were identified. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Full details: Docstring CoverageExplanation Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 1 functions across 1 files. (1 skipped: 1 unsupported.)
✨ 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 |
thepastaclaw
left a comment
thepastaclaw
left a comment
There was a problem hiding this comment.
Final review — Phase 1 only (trivial change)
Verified the documentation-only diff at head 055ccd7 against registration validation, consent hashing, and RPC JSON output. The clarification correctly distinguishes share-owner consent from funding-input signatures and directs funders to inspect the actual share table before signing. No defects were found; diff checking and whitespace lint passed.
Review provenance
Source: reviewer 1: glm-5.3-flash (agent: phase1-reviewer, role: general); reviewer 2: glm-5.3-flash (agent: phase1-reviewer, role: dash-core-commit-history); final verifier: gpt-6.1-sol (agent: sol-gate-verifier, role: final-verifier)
- Triage:
trivialbygpt-6.1-sol(effort low) — The diff only clarifies release notes and RPC help text about per-share consent and checking ownership before signing, with no changes to transaction validation, signatures, or wallet behavior. - Phase 1 reviewers:
glm-5.3-flash— general (completed, effort high); agentphase1-reviewer,glm-5.3-flash— dash-core-commit-history (completed, effort high); agentphase1-reviewer - Phase 1 model:
glm-5.3-flash— zai quota: 5h 99% left, weekly 80% left; passed overgemini-3.8-flash-high(antigravity below 15% reserve: weekly 13% left, 5h 100% left) - Fresh verifier:
gpt-6.1-sol— final-verifier; agentsol-gate-verifier - Phase 2 reviewers: not run (triage rated this change trivial); this review comments and never approves
thepastaclaw
added
the
pastaclaw:commented
thepastaclaw
removed
the
pastaclaw:commented
2 participants
Issue being fixed or feature implemented
Addresses #7761.
In a shared ProRegTx, consensus requires a consent signature from every share in the table. Funding inputs are not tied to any share. A wallet can sign its inputs into a registration whose table has no share for it, and the transaction is valid. #7761 shows this on v24.0.0-rc.1: the coordinator lists its own addresses on the share that participant A funded, and A's
signrawtransactionwithwalletsigns without complaint.Working as designed: the table decides who owns what, and it is fully visible in
decoderawtransaction(proRegTx.shares) and in thetermsreturned byprotx shared_sign. But the docs suggested a stronger guarantee:protx shared_register_preparehelp went straight fromshared_combinetosignrawtransactionwithwalletand never said to check the table first.The Qt wizard from #7701 already checks each participant's terms and funding before signing. This PR covers only the direct RPC workflow.
What was done?
Documentation only:
doc/release-notes-7437.md: "every participant consents" becomes "every share owner consents", and a new sentence says consent is tied to the table's shares, not to funding inputs. Theshared_register_prepareentry now says to confirm your own share inproRegTx.sharesbefore signing your funding inputs.protx shared_register_preparehelp: a new paragraph says consent is per share, not per input. Before signing its inputs, each participant should find its own share, with its own amount and addresses, in the decoded table.No behavior changes. An RPC that checks a funder's agreed terms and signs its inputs in one step (the #7761 proposal) could be a follow-up if third-party tooling wants it. It adds nothing that
decoderawtransactionplussignrawtransactionwithwalletcan't already do, so it is left out here.How Has This Been Tested?
Only string-literal and Markdown changes.
git diff --checkandtest/lint/lint-whitespace.pypass. I did not rebuilddashdto view the new help text.Breaking Changes
None.
Checklist:
🤖 Generated with Claude Code