Show the fee the transaction pays, not the one the response claims - #224
Merged
Conversation
checkTransactionFee already recomputes the miner fee from the transaction's own inputs and outputs, resolving input values from the chain rather than from the response. That number was used to accept or reject the transaction and then discarded: the review screen rendered result.btc_fee, the composer's own assertion about itself. The bound is deliberately loose -- max(10_000, rate * vsize * 10) -- because a legitimate composer needs room, so a response can pass it while reporting a smaller fee than the transaction actually pays, and the user signs against the number on the screen. The composer now stores the computed fee in place of the reported one, which corrects every review screen at once since they all render that field. A response that stated a different fee also produces a warning, since contradicting a stated value is worth surfacing. A response that stated no fee at all is corrected silently: filling in a value nobody claimed is not a discrepancy. The existing compose test turned out to be a live example -- its fixture asserts btc_fee 5000 while its inputs minus its single output leave 4840 -- and nothing had ever checked, which is the defect in miniature. Claude-Session: https://claude.ai/code/session_01CcjnCrgosSeshymXLBxdGj
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
PRIORITIES.md item 1.
The problem
checkTransactionFeealready recomputes the miner fee from the transaction's own inputs and outputs, resolving input values from the chain rather than from the response (ADR-019 layer 3). That number was used to accept or reject the transaction — and then discarded. The review screen renderedresult.btc_fee: the composer's own assertion about itself.The bound that is enforced is deliberately loose, because a legitimate composer needs room:
max(10_000, rate × vsize × 10)So a response could pass verification while reporting a smaller fee than the transaction actually pays, and the user would sign against the number on screen. Value goes to miners rather than an attacker, so this is griefing rather than theft — but the displayed number was simply not the truth.
The change
The composer stores the computed fee in place of the reported one. Because all 28 review screens render
result.btc_fee, that corrects every one of them at a single point rather than touching each screen.btc_feeentirely.)The test that was already broken
The existing
should compose transactiontest turned out to be a live example of the bug. Its fixture assertsbtc_fee: 5000, while its stubbed input value (100,000) minus its single 95,160-sat output leaves 4,840. The fixture had been internally inconsistent all along and nothing checked — the defect in miniature. The test now asserts the corrected value and the warning.Verification
tsc --noEmitclean.compose/send/index.spec.ts20/20, which includes "review page shows correct transaction details".https://claude.ai/code/session_01CcjnCrgosSeshymXLBxdGj