Skip to content

Show every OP_RETURN output, what it burns, and oversized payloads - #1043

Draft
Chaitanya-Keyal wants to merge 6 commits into
SeedSigner:devfrom
Chaitanya-Keyal:op-return-accounting
Draft

Show every OP_RETURN output, what it burns, and oversized payloads#1043
Chaitanya-Keyal wants to merge 6 commits into
SeedSigner:devfrom
Chaitanya-Keyal:op-return-accounting

Conversation

@Chaitanya-Keyal

@Chaitanya-Keyal Chaitanya-Keyal commented Sep 19, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Note

This builds on #1042, which should be merged first. This PR is in draft until then. To view changes only in this PR, see: 38db4a4..54605d0 (this PR)

Description

Problem or Issue being addressed

#1042 fixed reading an OP_RETURN payload. This fixes what we do with it afterwards.

  • Only the last OP_RETURN is kept. op_return_data is a single bytes value and the output loop overwrites it, so a transaction with more than one OP_RETURN only ever shows the last payload. Multiple OP_RETURNs have always been valid by consensus, and Bitcoin Core v30 dropped the relay rule that allowed only one per transaction.

  • Sats burned on an OP_RETURN are never shown. They are added to neither the spend total nor the change total. embit's fee() is inputs minus all outputs, so the fee is still correct, but inputs = spend + change + fee no longer adds up and the burned amount appears nowhere on screen.

  • The payload is drawn with no size limit. TextArea works out the line breaks for the whole payload before drawing any of it, and only logs a warning if the result doesn't fit, so a large payload runs off the bottom of the screen with nothing to say so. Finding those line breaks is also slow on long text, since reflow_text_for_width binary-searches every break and re-measures a growing run of words on each probe:

    payload time to lay out
    1 KB 83 ms
    4 KB 1.2 s
    16 KB 15.8 s

    That's on a laptop; the raspi is slower. Bitcoin Core v30 raised the default -datacarriersize to 100,000 bytes, and consensus never capped OP_RETURN data at all, so payloads this large can now arrive.

  • A fourth bug turned up while testing: a bare OP_RETURN gets a row in the overview's flow diagram, but the routing then skips its screen, because it checks whether the payload is non-empty rather than whether the output exists. A bare OP_RETURN can still burn sats.

Solution

  • op_return_data becomes a list with one entry per output, in output order. op_return_amounts runs alongside it and op_return_amount is their total — the same shape destination_addresses, destination_amounts and spend_amount already have.
  • PSBTOpReturnView steps through the outputs the way PSBTAddressDetailsView steps through recipients, and pages through each payload.
  • The math screen gets a burned line so the numbers on screen add up to the inputs again. It only appears when sats were actually burned.
  • Burned sats are marked the way an excessive fee is in [Feature] Warning Screen for High Tx Fees #722: the dire warning colour and a (!), on the flow diagram and the math screen. The mark is per output rather than for the whole set, so it points at the right one. Where the diagram has collapsed rows, the ellipsis carries the mark if a burn is hidden behind it.
  • The payload screen labels what you can't tell from the page itself: that it's hex, the full size, which page you're on, and how many bytes were cut. Readable text that fits on one screen has none of that to say, so it gets no label and looks exactly as it did before. The last page of a cut-short payload says how many bytes weren't shown, because otherwise it looks just like the last page of a complete one.
  • Two smaller fixes on the same screen. Decoding as UTF-8 doesn't make a payload readable — control characters decode fine and then draw as a row of empty boxes — so those go to hex too (line breaks excepted). And an output carrying no data says (no data) instead of leaving the screen blank, which reads as a rendering bug.

New Screenshots

PSBTOpReturnView_paged PSBTOpReturnView_truncated PSBTOpReturnView_second_output_burns_sats PSBTOpReturnView_empty PSBTMathView_op_return_burns_sats PSBTOverviewView_one_op_return_burns_sats

Note

This PR also fixes the existing PSBTOverviewView_op_return screenshot, which picked up a stray fee (!) mark after #722 because its fixture was paying a ~9.3 BTC fee.

Also, I'm not sure all six new screenshots need to stay in the generator. If some aren't worth keeping, I'm happy to drop them.

This pull request is categorized as a:

  • Bug fix

Checklist

I ran pytest locally

  • All tests passed before submitting the PR

I included screenshots of any new or modified screens

  • Yes

I added or updated tests

  • Yes

I tested this PR hands-on on the following platform(s):

Tested with my fork of Keith's testqrs.com (kdmukAI-bot/btc-datagen#2)

I have reviewed these notes:

  • Keep your changes limited in scope.
  • If you uncover other issues or improvements along the way, ideally submit those as a separate PR.
  • The more complicated the PR, the harder it is to review, test, and merge.
  • We appreciate your efforts, but we're a small team of volunteers so PR review can be a very slow process.
  • Please only "@" mention a contributor if their input is truly needed to enable further progress.
  • I understand

@Chaitanya-Keyal Chaitanya-Keyal mentioned this pull request Sep 19, 2026
Open
@Chaitanya-Keyal Chaitanya-Keyal moved this to 0.8.8 Needs Review in @SeedSigner Development Board Sep 19, 2026
@Chaitanya-Keyal
Chaitanya-Keyal force-pushed the op-return-accounting branch 2 times, most recently from 2b37b23 to 5ad487f Compare September 20, 2026 10:21
@Chaitanya-Keyal Chaitanya-Keyal mentioned this pull request Sep 20, 2026
Open
7 tasks
Chaitanya-Keyal and others added 6 commits September 23, 2026 11:45
@Chaitanya-Keyal @ruipereira1 @kwsantiago
Parse the OP_RETURN push opcode instead of assuming OP_PUSHDATA1
c8ead6d 
The payload was read at a fixed 3-byte offset, which only holds when the script
uses OP_PUSHDATA1. Opcodes 0x01-0x4b are themselves the byte count, and that is
the minimal encoding Bitcoin Core emits, so every payload of 75 bytes or fewer
lost its first byte:

    scriptPubKey  6a 28 "Chancellor on the brink of third bailout"
    parsed        b'hancellor on the brink of third bailout'

PSBTOpReturnView is a pre-signing review screen, so the user approved a
transaction whose OP_RETURN content was displayed wrong. When the payload is not
human-readable it renders as hex, where a one byte shift is invisible.

Read the push opcode and slice to the declared length, in a static helper so the
loop stays readable. OP_PUSHDATA2 and OP_PUSHDATA4 are handled too: Bitcoin Core
v30 raised the default -datacarriersize to 100,000 bytes, and consensus never
limited OP_RETURN data at all, so a payload past what OP_PUSHDATA1 can describe
is no longer exotic.

Several pushes after OP_RETURN are legal and are concatenated; the user is being
shown the data the transaction commits to, and the push boundaries carry no
consensus meaning. Anything that is not a data push, or a push that overruns the
script, ends the walk and the remaining bytes are surfaced verbatim rather than
dropped.

Also reset op_return_data in _parse_outputs, which only ever set it in
__init__, and compare the first byte by slice so an empty scriptPubKey does not
raise IndexError.

Fixes SeedSigner#963

Co-authored-by: ruipereira1 <159703278+ruipereira1@users.noreply.github.com>
Co-authored-by: kwsantiago <44934418+kwsantiago@users.noreply.github.com>
@Chaitanya-Keyal @ruipereira1 @kwsantiago
Cover every OP_RETURN push encoding in the parser tests
d76681e 
Every existing OP_RETURN vector hand-built OP_PUSHDATA1, which is exactly the one
encoding the parser assumed, so the mis-slice never showed up. The regtest psbt
behind test_parse_op_return_content carries 2b 6a 4c 28 43 68 61 6e ... and the
screenshot generator concatenated OP_PUSHDATA1 by hand.

Add fixture helpers that build an OP_RETURN scriptPubKey with the minimal push by
default, and that can force a specific OP_PUSHDATA* so the non-minimal encodings
a coordinator is free to produce are testable too. Note in the helper that the
scriptPubKey and value have to be set on the OutputScope: PSBT.tx rebuilds the
transaction from the scopes on every access, so assigning through psbt.tx.vout[i]
is silently discarded and leaves a test asserting against the fixture it meant to
replace.

The new tests cover the direct-push/OP_PUSHDATA1 boundary at 75/76, OP_PUSHDATA2
and OP_PUSHDATA4, a non-minimal encoding, several pushes in one script, a bare
OP_RETURN, an empty scriptPubKey, a truncated length field, an over-long declared
length, and a leading opcode that is not a data push.

Co-authored-by: ruipereira1 <159703278+ruipereira1@users.noreply.github.com>
Co-authored-by: kwsantiago <44934418+kwsantiago@users.noreply.github.com>
@Chaitanya-Keyal @kwsantiago
Build the screenshot generator's OP_RETURN with a minimal push
38db4a4 
The generator concatenated OP_PUSHDATA1 unconditionally, so its 49-byte payload
was encoded a way Bitcoin Core would never produce. Push directly at 75 bytes or
fewer and via OP_PUSHDATA1 above that, matching the parser. The rendered
screenshots are unchanged; the vector behind them is no longer wrong.

Co-authored-by: kwsantiago <44934418+kwsantiago@users.noreply.github.com>
@Chaitanya-Keyal
Show every OP_RETURN output, what it burns, and oversized payloads
72e2a4e 
Three separate defects in how the review flow treats a data carrier, all of them
on adjacent lines:

Only the last OP_RETURN survived the parse. op_return_data was a single `bytes`
that the output loop overwrote, so a transaction carrying more than one showed
only the final payload. Several have always been consensus-valid; the
one-per-transaction limit was Bitcoin Core relay policy and v30 dropped it. It
is now a list, one entry per output in output order.

Sats sent to an OP_RETURN were counted nowhere. They belong to neither the spend
nor the change, and embit's fee() is inputs minus ALL outputs, so the fee stayed
correct while `inputs = spend + change + fee` quietly stopped holding and the
burn never appeared on screen at all. op_return_amounts now runs parallel to
op_return_data the way destination_amounts runs parallel to destination_addresses,
with op_return_amount as their total, and the math screen gains a "burned" line
so the arithmetic on screen reconciles against the inputs again. That line shows
only when something was actually burned: nearly every OP_RETURN carries no value,
and a row reading "0 burned" is noise on the one screen whose job is to make the
arithmetic legible.

The payload was drawn into one unbounded TextArea. TextArea reflows the whole
string before drawing any of it and only logs a warning when the result does not
fit, so a large payload ran off the bottom of the screen with nothing to say so,
and the reflow cost is quadratic: 1 KB takes 83ms, 4 KB 1.2s and 16 KB 15.8s on a
development machine, with the device slower again. Since Bitcoin Core v30 raised
the default -datacarriersize to 100,000 bytes, and consensus never limited
OP_RETURN data at all, that is a size a signer can now be handed. The payload is
paged instead, at a fixed number of characters or bytes per page, and the screen
always reports the payload's real size so a truncated one cannot be mistaken for
the whole of it.

The page size is fixed rather than measured from the rendered screen because the
View decides how many pages there are and has to reach the same answer under the
FlowTests, where the renderer is mocked out.

Whether the payload is human-readable is decided once for the whole payload
rather than per page, so that slicing cannot cut a multi-byte character in half
and flip a page in the middle of readable text over to hex. Decoding cleanly is not on
its own enough to call a payload readable: control bytes decode perfectly well
and then draw as a row of empty boxes, so those go to hex too, and an output that
pushes nothing at all says so rather than leaving a blank panel that reads as the
screen having failed.

Value destroyed by an OP_RETURN is marked the way an excessive fee is marked in
the high-fee work: the dire warning colour and a "(!)" on the row, in the overview
flow diagram, on the math screen's burned line, and above the payload itself on
every page of that output. A screen carrying that warning has one line less for
the payload, so it is paged against a smaller budget.

"truncated" on the earlier pages warns that something is missing; the last page
says how much, because otherwise the end of a truncated payload looks exactly like
the end of a complete one.
@Chaitanya-Keyal
Cover OP_RETURN accounting and paging in tests and screenshots
4f70ba7 
Parser tests for the three shapes the previous commit fixes: a nonzero-value
OP_RETURN whose total reconciles against the inputs, three OP_RETURN outputs in
one transaction that all survive in output order, and a transaction with none at
all, which must report zero of them rather than one empty one.

FlowTests for the routing, which unit tests cannot reach: one screen per
OP_RETURN output, one screen per page of a payload too large for one, and a bare
OP_RETURN, which the old routing skipped entirely because it tested the payload's
truthiness rather than whether the output existed. That last one matters more
than it sounds, since a bare OP_RETURN can still carry value.

Screenshots for a paged payload, one too large to page through in full, one that
burns sats, the second of two OP_RETURN outputs, the math screen with its burned
line, and the overview with two OP_RETURN rows in the flow diagram.

The screenshot generator could previously only add a zero-value OP_RETURN, and
its push encoding stopped at OP_PUSHDATA1. It now takes a value, funding it out
of an existing output so that the outputs cannot exceed the inputs and leave the
fixture quoting a negative fee, and encodes with OP_PUSHDATA2 and 4 as the length
demands.
@Chaitanya-Keyal
Keep the trimmed OP_RETURN screenshot psbts at their base fee
54605d0 
Two OP_RETURN fixtures simplify the multisig psbt down to its last output.
Dropping the other outputs turned their ~9.3 BTC into fee, which now trips
the high fee warning, so those overview and math screenshots showed a
"fee (!)" mark that has nothing to do with OP_RETURN. Fold the dropped value
into the kept output instead, so the fee stays what the base psbt paid.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

@SeedSigner Development Board
Status: 0.8.8 Needs Review

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@Chaitanya-Keyal