fix(server): keep the protocol fee note out of dashboard delta listings - #449
Draft
WiktorStarczewski wants to merge 1 commit into
Draft
Conversation
WiktorStarczewski
requested review from
haseebrabbani and
zeljkoX
as code owners
September 2, 2026 22:27
|
Important
This repository does not receive automatic reviews because it has fewer than 10 stars. ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Team Run ID: Comment |
zeljkoX
reviewed
Sep 3, 2026
zeljkoX
left a comment
Collaborator
There was a problem hiding this comment.
LGTM - just a few nits.
The operator client currently declares fee, but never parses it. parseDeltaEntry handles note_counts, assets, and counterparty, but not record.fee, so TypeScript consumers will always see fee as undefined. This needs a decode branch and a test.
Should we handle FEE_SPONSORSHIP note the same way?
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.
Once a chain charges a fee, every guarded transaction emits a protocol
TX_FEEnote — and the dashboard was reading it as a transfer, so a plain note consumption showed up as an outgoing asset movement andnoteCounts.outputgrew by one.assets,noteCounts.output,category) now iterateuser_output_notes, which drops the note the protocol appended.feeon the delta entry, not dropped — a transaction commits its own conversion rate, so the amount is bounded only by the vault and is a signal worth seeing.noteCounts.outputis deliberately one lower thanoutputNotes.lengththere.guardian-operator-clienttypes + README and the three OpenAPI documents move with the schema, per AGENTS.md §8.2.How the fee note is identified, and where it declines to guess
Provenance is not recoverable from a
TransactionSummary: it carries no mark for where the auth procedure's outputs begin, andTxFeeNote::builderis public, so a caller-builtTX_FEEnote is indistinguishable from the protocol's by script root. Position is the only remaining signal —pay_feeruns at the end of authentication, after the transaction script has created every note it is going to, so the injected note is last.Erring toward exclusion is the dangerous direction: a
TX_FEEnote restricts nobody from consuming it, so a caller who could persuade the filter to skip a note of their own would be concealing a bearer instrument.protocol_fee_note_indextherefore resolves only the unambiguous case — exactly oneTX_FEEnote, in last position. Two means a caller built at least one, and nothing is excluded.Residual, accepted and documented in place: on a chain that charges no fee there is no injected note, so a caller's own
TX_FEEnote placed last would be hidden. Closing that needs a provenance signal the summary does not carry.Also worth knowing:
num_output_notes_before_authis not this boundary.guardian.masmconsults it only on the rotation path, where it must be zero, and the fee note is inside the signed summary regardless.Why this is its own PR
Split out of #440 (guarded-account fee payment), which is client-side. There is no coupling in either direction:
crates/serverdoes not depend onmiden-multisig-client, and this change is a pure consumer ofTransactionSummary. It needs nothing #440 produces, and it builds and tests green against published protocol crates, which #440 currently does not.10 new tests in
delta_summary::projectioncover the single-note, no-note, two-note and not-last cases plus the reportedfee;guardian-serveris 906 passing (896 before) andgen-openapi --check docsis clean.Reviewers:
protocol_fee_note_indexis the part worth your time — specifically whether "exactly one, in last position" is the right refusal boundary.