Skip to content

qt: address CodeRabbit review feedback on data transactions#3

Open
thepastaclaw wants to merge 3 commits intokwvg:data_txfrom
thepastaclaw:fix/coderabbit-7144-payload-scope
Open

qt: address CodeRabbit review feedback on data transactions#3
thepastaclaw wants to merge 3 commits intokwvg:data_txfrom
thepastaclaw:fix/coderabbit-7144-payload-scope

Conversation

@thepastaclaw
Copy link

Addresses two review items from CodeRabbit on dashpay#7144, both confirmed by @UdjinM6:

  1. Scope payload display to rec->idx (transactiondesc.cpp) — The detail view is for a specific TransactionRecord pointing to output rec->idx, but the loop was displaying payloads from every OP_RETURN output. Now scoped to the specific output, which correctly handles mined transactions with multiple OP_RETURN outputs.

  2. Replace CScript forward declaration with direct include (transactionrecord.h) — The inline IsDataScript method uses CScript methods and OP_RETURN, requiring the full definition. The forward declaration only worked due to a fragile transitive include chain.


🤖 This was generated by an automated review bot.
Don't want automated PRs or comments on your code? You can opt out by replying here or messaging @PastaPastaPasta on Slack — we'll make sure the bot skips your PRs/repos going forward.

kwvg and others added 3 commits February 18, 2026 23:17
Proposals are made in the form of data transactions but look like oddly
shaped transactions in the UI, this should help better explain where the
proposal fee went.
Replace the `class CScript;` forward declaration with a direct
`#include <script/script.h>` to provide the full type definition
needed by `IsDataScript`.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Replace the for-loop over all vouts with a bounds-checked single
access to `wtx.tx->vout[rec->idx]`, so the transaction detail view
only shows the payload for the output that corresponds to this
particular transaction record.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
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.

2 participants

Comments