fix: store the framed commit on MarmotCommitResult, not the exporter secret
`MarmotOutboundDao.inviteMember` persisted the commit row with
framedCommitBytes = commitResult.preCommitExporterSecret,
two lines below the argument that value belongs to, which was already assigning it
correctly. It now reads `commitResult.framedCommitBytes`.
The row is written on the deferred branch, after the kind:445 commit has gone out
and while the welcome waits on a relay acknowledgement, so what it holds is meant
to be the record of what was published.
## Why the compiler had nothing to say
`MarmotCommitResult` carries quartz's `CommitResult` payload fields verbatim --
`commitBytes`, `welcomeBytes`, `groupInfoBytes`, `framedCommitBytes`,
`preCommitExporterSecret`, same names, same order, same defaults. Both of the
fields in question are `ByteArray`, so the wrong field of the right object is
indistinguishable from the correct one at the type level.
The call site lists its named arguments in a different order than the declaration,
which is what put `preCommitExporterSecret` and `framedCommitBytes` two lines
apart. The entity also repeats quartz's `framedCommitBytes: ByteArray = commitBytes`
default, so the explicit argument was overriding a fallback that -- while still the
raw commit rather than the framed envelope -- was at least a commit.
## What it cost, and what it would have cost
Nothing so far. `framedCommitBytes` has exactly two references in the tree: this
assignment, and `encryptedCommitEvent` at the top of the same branch, which takes
`commitResult.framedCommitBytes` from the in-memory `CommitResult` rather than from
the row. The bytes that reached the relay were always the right ones; the wrong
ones only ever sat in the column.
They would stop merely sitting there as soon as anything reads the row back.
`DatabaseNostrRepository` already reloads these rows on acknowledgement, at
`getMarmotCommitRequestById`, to pick up `welcomeBytes` and fire `deliveryWelcome`.
An ack-triggered rebroadcast or a replay reaching one field further along would
publish 32 bytes of exporter secret where a
`MlsMessage(PublicMessage(FramedContent(commit)))` envelope was expected: not a
message recipients drop, but a group key on a relay.
The smaller half holds whether or not anything ever reads it. The group's
pre-commit `MLS-Exporter("marmot", "group-event", 32)` output was being written to
a second column that is not intended to hold key material, doubling its footprint
at rest alongside the `preCommitExporterSecret` field that exists for it. Only at
rest -- the ack path logs the row, but the data class has no `toString` override,
so `ByteArray` prints as an identity hash rather than contents.
## Scope
`MarmotCommitResult` has a single construction site in the codebase, the one
changed here, so there is no second copy of this to fix. Worth checking rather than
assuming: the shape that produced it -- adjacent `ByteArray` fields with identical
names on both sides of the copy -- reproduces anywhere the entity is built again.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
@@ -442,7 +442,7 @@ abstract class MarmotOutboundDao(
|
||||
commitBytes = commitResult.commitBytes,
|
||||
preCommitExporterSecret = commitResult.preCommitExporterSecret,
|
||||
welcomeBytes = commitResult.welcomeBytes,
|
||||
framedCommitBytes = commitResult.preCommitExporterSecret,
|
||||
framedCommitBytes = commitResult.framedCommitBytes,
|
||||
groupInfoBytes = commitResult.groupInfoBytes,
|
||||
userPublicKey = userPublicKey,
|
||||
peerKeyPackageEventId = peerKeyPackage.id,
|
||||
|
||||
Reference in New Issue
Block a user