mirror of
https://github.com/vitorpamplona/amethyst.git
synced 2026-10-06 03:38:23 +00:00
fix(marmot): mint a rotated KeyPackage the way the first one was minted
MIP-00 replaces a KeyPackage as soon as a Welcome consumes it, so rotation
is not a rare path — it runs right after the first group we are ever
invited to. It went through the legacy generator, so from that moment on
the only KeyPackage on relays for us was a MIP-era one with no account
identity proof. A current-profile peer refuses that outright ("member
KeyPackage identity or profile is invalid") and keeps inviting from
whatever stale copy it still has cached, so an account went silently
uninvitable one join after it was set up.
Rotation and first publication now share one mint path, so a replacement
cannot land on a different profile than the KeyPackage it replaces.
Harness, two tests that were reporting our bugs as theirs and one that
was reporting the reverse:
- Test 16 asked `wn keys publish` to rotate. That verb is the
idempotent retry of the durable stable-slot replacement — with
nothing pending it republishes the same event id, so there is no
rotation to observe. `wn keys rotate` is the one that mints.
- Test 13 swallowed `wn keys check`'s output, so "no prior KP for A"
read as a missing fixture when it was MDK refusing what we had
published. The raw answer goes to the log now and the message says
what actually happened.
- Test 09 polled `reactions.by_emoji`, which belongs to the
materialized timeline; `wn messages list` reads the raw app-event
log, where a reaction is its own kind:7 entry with an "e" tag naming
the anchor. The reaction had been arriving and being stored
correctly the whole time.
The MDK 0.9.20 interop harness is now green, 17 of 17, twice in a row
from a clean state.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016kCuA6tc4JQzHPCDd39GHq
This commit is contained in:
Claude
@@ -60,16 +60,26 @@ test_09_reply_react_unreact() {
|
||||
record_result "$id" fail "amy marmot message react failed"; return
|
||||
fi
|
||||
|
||||
# Round-trip: B should surface amy's kind:7 reaction. wn aggregates
|
||||
# reactions onto the anchor message (`.reactions.by_emoji[<emoji>]`),
|
||||
# not as a standalone entry whose `.content` is the emoji — so polling
|
||||
# `messages list` for an entry whose content equals "🍕" would never
|
||||
# match, even when the reaction was successfully decrypted. Look for
|
||||
# the emoji under any message's `reactions.by_emoji` keys instead.
|
||||
# Round-trip: B should surface amy's kind:7 reaction. `wn messages list`
|
||||
# reads the raw app-event log, where a reaction is its own kind:7 entry
|
||||
# carrying the emoji and an "e" tag naming the anchor — the aggregated
|
||||
# `reactions.by_emoji` summary belongs to the materialized timeline, which
|
||||
# this command does not project. Match the raw shape, and accept an
|
||||
# aggregated one too so a wn that starts summarising here still passes.
|
||||
local deadline=$(( $(date +%s) + 90 )) saw=0
|
||||
while [[ $(date +%s) -lt $deadline ]]; do
|
||||
local payload
|
||||
payload=$(wn_b_json messages list "$mls_gid" --limit 50 2>/dev/null || true)
|
||||
if [[ -n "$payload" ]] && \
|
||||
printf '%s' "$payload" \
|
||||
| jq_list messages \
|
||||
| jq -e --arg anchor "$msg_id" --arg emoji "🍕" \
|
||||
'select(.kind == 7)
|
||||
| select((.plaintext // .content // "") == $emoji)
|
||||
| select([(.tags // [])[] | select(.[0] == "e") | .[1]] | index($anchor))' \
|
||||
>/dev/null 2>&1; then
|
||||
saw=1; break
|
||||
fi
|
||||
if [[ -n "$payload" ]] && \
|
||||
printf '%s' "$payload" \
|
||||
| jq_list messages | jq -e '(.reactions.by_emoji // {}) | keys[]?' \
|
||||
@@ -200,11 +210,15 @@ test_13_keypackage_rotation() {
|
||||
banner "Test 13 — KeyPackage rotation"
|
||||
local id="13 keypackage rotation"
|
||||
|
||||
local before
|
||||
before=$(wn_b --json keys check "$A_NPUB" 2>/dev/null \
|
||||
| jq -r '.result.key_package.key_package_event_id // .result.event_id // empty')
|
||||
# `keys check` resolves A's KeyPackage the way an invite would, so an empty
|
||||
# answer here is a real finding, not a missing fixture: it means MDK looked
|
||||
# at what A published and refused it. Keep the raw JSON in the log.
|
||||
local before raw
|
||||
raw=$(wn_b --json keys check "$A_NPUB" 2>&1)
|
||||
printf 'wn keys check %s -> %s\n' "$A_NPUB" "$raw" >>"$LOG_FILE"
|
||||
before=$(printf '%s' "$raw" | jq -r '.result.key_package.key_package_event_id // .result.event_id // empty')
|
||||
if [[ -z "$before" ]]; then
|
||||
record_result "$id" fail "no prior KP for A"; return
|
||||
record_result "$id" fail "wn cannot resolve a KeyPackage for A"; return
|
||||
fi
|
||||
|
||||
amy_json marmot key-package publish >/dev/null || {
|
||||
@@ -373,11 +387,12 @@ test_16_wn_keypackage_rotation() {
|
||||
record_result "$id" fail "no prior KP visible to amy for B"; return
|
||||
fi
|
||||
|
||||
# Ask B to rotate. `wn keys publish` writes a new kind:443 with a fresh
|
||||
# created_at; the old event may or may not be evicted depending on the
|
||||
# relay's retention policy, so both may coexist for a while.
|
||||
wn_b keys publish >/dev/null 2>&1 || {
|
||||
record_result "$id" fail "wn_b keys publish failed"; return
|
||||
# Ask B to rotate. It has to be `keys rotate` ("force mint and publish a
|
||||
# fresh replacement"), not `keys publish` — the latter is the idempotent
|
||||
# retry of the durable stable-slot replacement, so with nothing pending it
|
||||
# republishes the same event id and there is no rotation to observe.
|
||||
wn_b keys rotate >/dev/null 2>&1 || {
|
||||
record_result "$id" fail "wn_b keys rotate failed"; return
|
||||
}
|
||||
|
||||
local deadline=$(( $(date +%s) + 60 )) after=""
|
||||
|
||||
+27
-17
@@ -1126,8 +1126,22 @@ class MarmotManager(
|
||||
* — which is what kept us uninvitable.
|
||||
*/
|
||||
currentProfile: Boolean = true,
|
||||
): KeyPackageEvent = mintKeyPackageEventForSlot(keyPackageRotationManager.getOrCreateSlotDTag(slotName), relays, currentProfile)
|
||||
|
||||
/**
|
||||
* Mint, sign and index a KeyPackage for an already-resolved d-tag slot.
|
||||
*
|
||||
* Both the first publication and every rotation go through here, and that
|
||||
* is the point: a replacement minted any other way can end up on a
|
||||
* different protocol profile than the KeyPackage it replaces, which makes
|
||||
* the account uninvitable to peers that require the current one.
|
||||
*/
|
||||
@OptIn(ExperimentalEncodingApi::class)
|
||||
private suspend fun mintKeyPackageEventForSlot(
|
||||
dTag: String,
|
||||
relays: List<NormalizedRelayUrl>,
|
||||
currentProfile: Boolean,
|
||||
): KeyPackageEvent {
|
||||
val dTag = keyPackageRotationManager.getOrCreateSlotDTag(slotName)
|
||||
val identity = signer.pubKey.hexToByteArray()
|
||||
val bundle =
|
||||
if (currentProfile) {
|
||||
@@ -1184,26 +1198,22 @@ class MarmotManager(
|
||||
* Rotate consumed KeyPackage slots.
|
||||
* Returns list of KeyPackageEvents to publish.
|
||||
*/
|
||||
@OptIn(ExperimentalEncodingApi::class)
|
||||
suspend fun rotateConsumedKeyPackages(relays: List<NormalizedRelayUrl>): List<KeyPackageEvent> {
|
||||
suspend fun rotateConsumedKeyPackages(
|
||||
relays: List<NormalizedRelayUrl>,
|
||||
currentProfile: Boolean = true,
|
||||
): List<KeyPackageEvent> {
|
||||
val pendingSlots = keyPackageRotationManager.pendingRotationSlots()
|
||||
if (pendingSlots.isEmpty()) return emptyList()
|
||||
|
||||
val identity = signer.pubKey.hexToByteArray()
|
||||
// Same mint path as the first publication. MIP-00 makes us replace a
|
||||
// KeyPackage the moment a Welcome consumes it, so this runs right
|
||||
// after the first group we are ever invited to — minting the
|
||||
// replacement any other way would silently downgrade the only
|
||||
// KeyPackage on relays for us, and a peer that requires the current
|
||||
// profile refuses it and can never add us again.
|
||||
return pendingSlots.map { slot ->
|
||||
val bundle = keyPackageRotationManager.rotateSlot(identity, slot)
|
||||
val keyPackageBase64 = Base64.encode(KeyPackageUtils.frameKeyPackage(bundle.keyPackage))
|
||||
val keyPackageRef = bundle.keyPackage.reference().toHexKey()
|
||||
|
||||
val template =
|
||||
KeyPackageEvent.build(
|
||||
keyPackageBase64 = keyPackageBase64,
|
||||
dTagSlot = slot,
|
||||
keyPackageRef = keyPackageRef,
|
||||
relays = relays,
|
||||
)
|
||||
val signed = signer.sign<KeyPackageEvent>(template)
|
||||
keyPackageRotationManager.recordPublishedEventId(slot, signed.id)
|
||||
val signed = mintKeyPackageEventForSlot(slot, relays, currentProfile)
|
||||
keyPackageRotationManager.clearPendingRotation(slot)
|
||||
signed
|
||||
}
|
||||
}
|
||||
|
||||
+152
@@ -0,0 +1,152 @@
|
||||
/*
|
||||
* Copyright (c) 2025 Vitor Pamplona
|
||||
*
|
||||
* Permission is hereby granted, free of charge, to any person obtaining a copy of
|
||||
* this software and associated documentation files (the "Software"), to deal in
|
||||
* the Software without restriction, including without limitation the rights to use,
|
||||
* copy, modify, merge, publish, distribute, sublicense, and/or sell copies of the
|
||||
* Software, and to permit persons to whom the Software is furnished to do so,
|
||||
* subject to the following conditions:
|
||||
*
|
||||
* The above copyright notice and this permission notice shall be included in all
|
||||
* copies or substantial portions of the Software.
|
||||
*
|
||||
* THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
|
||||
* IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, FITNESS
|
||||
* FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR
|
||||
* COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN
|
||||
* AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION
|
||||
* WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.
|
||||
*/
|
||||
package com.vitorpamplona.amethyst.commons.marmot
|
||||
|
||||
import com.vitorpamplona.quartz.marmot.mip00KeyPackages.KeyPackageBundleStore
|
||||
import com.vitorpamplona.quartz.marmot.mip00KeyPackages.KeyPackageUtils
|
||||
import com.vitorpamplona.quartz.marmot.mls.group.MarmotMessageStore
|
||||
import com.vitorpamplona.quartz.marmot.mls.group.MlsGroupStateStore
|
||||
import com.vitorpamplona.quartz.nip01Core.crypto.KeyPair
|
||||
import com.vitorpamplona.quartz.nip01Core.relay.normalizer.RelayUrlNormalizer
|
||||
import com.vitorpamplona.quartz.nip01Core.signers.NostrSignerInternal
|
||||
import kotlinx.coroutines.runBlocking
|
||||
import kotlin.test.Test
|
||||
import kotlin.test.assertEquals
|
||||
import kotlin.test.assertTrue
|
||||
|
||||
/**
|
||||
* A rotation must publish the same kind of KeyPackage the first publication
|
||||
* did.
|
||||
*
|
||||
* MIP-00 makes us replace a KeyPackage as soon as a Welcome consumes it, so
|
||||
* rotation is not a rare path — it runs right after the first group we are
|
||||
* ever invited to. Minting the replacement through the legacy generator meant
|
||||
* that from that moment on the only KeyPackage on relays for us was a
|
||||
* MIP-era one, without the account identity proof (`0x8009`) that a
|
||||
* current-profile peer requires. MDK then refuses it outright
|
||||
* (`member KeyPackage identity or profile is invalid`) and keeps inviting from
|
||||
* whatever stale copy it still has cached, so the account silently becomes
|
||||
* uninvitable one join after it was set up.
|
||||
*/
|
||||
class MarmotKeyPackageRotationProfileTest {
|
||||
private val relay =
|
||||
RelayUrlNormalizer.normalizeOrNull("wss://example.invalid/")
|
||||
?: error("test relay must normalize")
|
||||
|
||||
@Test
|
||||
fun aRotatedKeyPackageStaysOnTheCurrentProfile() =
|
||||
runBlocking {
|
||||
val signer = NostrSignerInternal(KeyPair())
|
||||
val manager = MarmotManager(signer, RotationStateStore(), RotationMessageStore(), RotationBundleStore())
|
||||
|
||||
val first = manager.generateKeyPackageEvent(listOf(relay))
|
||||
assertTrue(first.isCurrentProfile(), "the first publication is current-profile")
|
||||
|
||||
// A Welcome consumed it, which is what schedules the replacement.
|
||||
manager.keyPackageRotationManager.markConsumedByEventId(first.id)
|
||||
assertTrue(manager.needsKeyPackageRotation())
|
||||
|
||||
val rotated = manager.rotateConsumedKeyPackages(listOf(relay))
|
||||
assertEquals(1, rotated.size, "one consumed slot means one replacement")
|
||||
val replacement = rotated.single()
|
||||
|
||||
assertTrue(
|
||||
replacement.isCurrentProfile(),
|
||||
"the replacement must carry the account identity proof too, or no current-profile " +
|
||||
"peer can add us after our first join",
|
||||
)
|
||||
assertEquals(
|
||||
first.dTag(),
|
||||
replacement.dTag(),
|
||||
"the replacement lands in the same addressable slot",
|
||||
)
|
||||
assertTrue(
|
||||
replacement.id != first.id,
|
||||
"the replacement must actually be a different KeyPackage",
|
||||
)
|
||||
assertTrue(
|
||||
KeyPackageUtils.isValid(replacement),
|
||||
"and it must validate under the same MIP-00 rules a peer applies",
|
||||
)
|
||||
assertTrue(
|
||||
!manager.needsKeyPackageRotation(),
|
||||
"the slot is no longer pending once its replacement is minted",
|
||||
)
|
||||
}
|
||||
}
|
||||
|
||||
// Minimal in-memory stores, matching the ones the other MarmotManager tests
|
||||
// use; the file-backed implementations live in the platform modules.
|
||||
|
||||
private class RotationStateStore : MlsGroupStateStore {
|
||||
private val states = mutableMapOf<String, ByteArray>()
|
||||
private val retained = mutableMapOf<String, List<ByteArray>>()
|
||||
|
||||
override suspend fun save(
|
||||
nostrGroupId: String,
|
||||
state: ByteArray,
|
||||
) {
|
||||
states[nostrGroupId] = state
|
||||
}
|
||||
|
||||
override suspend fun load(nostrGroupId: String): ByteArray? = states[nostrGroupId]
|
||||
|
||||
override suspend fun delete(nostrGroupId: String) {
|
||||
states.remove(nostrGroupId)
|
||||
retained.remove(nostrGroupId)
|
||||
}
|
||||
|
||||
override suspend fun listGroups(): List<String> = states.keys.toList()
|
||||
|
||||
override suspend fun saveRetainedEpochs(
|
||||
nostrGroupId: String,
|
||||
retainedSecrets: List<ByteArray>,
|
||||
) {
|
||||
retained[nostrGroupId] = retainedSecrets
|
||||
}
|
||||
|
||||
override suspend fun loadRetainedEpochs(nostrGroupId: String): List<ByteArray> = retained[nostrGroupId] ?: emptyList()
|
||||
}
|
||||
|
||||
private class RotationMessageStore : MarmotMessageStore {
|
||||
override suspend fun appendMessage(
|
||||
nostrGroupId: String,
|
||||
innerEventJson: String,
|
||||
) = Unit
|
||||
|
||||
override suspend fun loadMessages(nostrGroupId: String): List<String> = emptyList()
|
||||
|
||||
override suspend fun delete(nostrGroupId: String) = Unit
|
||||
}
|
||||
|
||||
private class RotationBundleStore : KeyPackageBundleStore {
|
||||
private var snapshot: ByteArray? = null
|
||||
|
||||
override suspend fun save(snapshot: ByteArray) {
|
||||
this.snapshot = snapshot
|
||||
}
|
||||
|
||||
override suspend fun load(): ByteArray? = snapshot
|
||||
|
||||
override suspend fun delete() {
|
||||
snapshot = null
|
||||
}
|
||||
}
|
||||
@@ -586,16 +586,15 @@ Writing the producer side immediately found two bugs the reader-side tests could
|
||||
|
||||
## Interop status (2026-09-09)
|
||||
|
||||
The MDK 0.9.20 harness runs end to end. It went **1 passed / 12 failed** to
|
||||
**9 passed** over this pass, and the failures that remain are named below rather
|
||||
than lumped together.
|
||||
The MDK 0.9.20 harness runs end to end and is **green: 17 of 17**, twice in a
|
||||
row from a clean state. It started this pass at **1 passed / 12 failed**. The
|
||||
defects it found are below — every one of them a place where two
|
||||
implementations have to agree on something one implementation alone never
|
||||
disagrees with, which is why none of them was visible to any same-implementation
|
||||
test we have.
|
||||
|
||||
### Defects the harness found in our own code
|
||||
|
||||
Each of these was invisible to every same-implementation test we have, because
|
||||
each is a place where two implementations have to agree on something one
|
||||
implementation alone never disagrees with.
|
||||
|
||||
1. **A Commit rebuilt our own leaf from defaults.** The UpdatePath leaf replaces
|
||||
OUR leaf — same member, new key material — so its capabilities and extensions
|
||||
must carry over. `buildLeafNode` was called with neither, so the very first
|
||||
@@ -628,24 +627,59 @@ implementation alone never disagrees with.
|
||||
exists to prevent. `PublishOutcome.UNKNOWN` keeps the record and holds the
|
||||
group.
|
||||
|
||||
5. **The harness was parsing a wire format `wn` no longer speaks.** MDK 0.9.x
|
||||
5. **We treated a last-resort KeyPackage as single-use.** We publish every
|
||||
KeyPackage marked last resort and then dropped its private keys the moment
|
||||
one Welcome consumed it. OpenMLS deletes a consumed bundle only
|
||||
`if !key_package.last_resort()`; MDK marks all of its own last resort, caches
|
||||
the peer KeyPackage it resolved, and invites from that cached copy every time
|
||||
after. So the first invite addressed to us worked and every one after it died
|
||||
on "No matching KeyPackageBundle". Two things had to be fixed together: the
|
||||
last-resort check had to read the current profile's carrier (a
|
||||
`last_resort_key_package` component inside the KeyPackage-level
|
||||
`app_data_dictionary`, not the MIP-era `0x000A` extension type), and the
|
||||
Welcome lookup had to trust the MLS KeyPackageRefs over the Nostr `e` tag,
|
||||
which MDK stamps from its stale cached copy.
|
||||
|
||||
6. **Every rotation downgraded us off the current profile.** MIP-00 replaces a
|
||||
KeyPackage as soon as a Welcome consumes it, so rotation runs right after the
|
||||
first group we are ever invited to — and it minted the replacement through
|
||||
the legacy generator. From that moment the only KeyPackage on relays for us
|
||||
had no account identity proof, MDK refused it outright, and the account was
|
||||
silently uninvitable one join after setup. Rotation and first publication now
|
||||
share one mint path.
|
||||
|
||||
7. **The harness was parsing a wire format `wn` no longer speaks.** MDK 0.9.x
|
||||
returns `{"ok":true,"result":{"invites":[…]}}`; iterating `.result` walked
|
||||
that object's three VALUES, so every poll matched nothing and reported "never
|
||||
received invite" for welcomes that had arrived and been accepted.
|
||||
|
||||
### Harness defects (not ours)
|
||||
|
||||
- **Runs inherited each other's state.** wnd wipes B's and C's data dirs on
|
||||
start, but A's amy home and the relay's SQLite file survived, and the
|
||||
leftovers are not inert — a KeyPackage from an earlier run is still on the
|
||||
relay to be invited with, and old kind:445 events still arrive undecryptable.
|
||||
Tests 03 and 08 failed on a dirty tree and passed on a clean one. Every run
|
||||
now starts from empty stores; `--reuse-state` opts out and `--tests "…"` runs
|
||||
a subset.
|
||||
- **Test 16 asked `wn keys publish` to rotate.** That verb is the idempotent
|
||||
retry of the durable stable-slot replacement, so with nothing pending it
|
||||
republishes the same event id and there is no rotation to observe.
|
||||
`wn keys rotate` is the one that mints.
|
||||
- **Test 09 polled the wrong surface.** `wn messages list` reads the raw
|
||||
app-event log, where a reaction is its own kind:7 entry with an `e` tag naming
|
||||
the anchor; `reactions.by_emoji` is the materialized timeline's aggregate,
|
||||
which that command does not project. The reaction had been arriving and being
|
||||
stored correctly the whole time.
|
||||
|
||||
### What is NOT done
|
||||
|
||||
- **Test 03 gets further but does not pass.** We join MDK's group and see its
|
||||
name; the first kind-445 after the join is not delivered. Tests 05/12/14/15
|
||||
fail behind it with "A never received invite".
|
||||
- **Tests 13 and 16 (KeyPackage rotation) fail.** `wn keys check` finds no prior
|
||||
KeyPackage for A at that point in the run, and amy keeps seeing B's
|
||||
pre-rotation KeyPackage.
|
||||
- **Test 09 fails on the relay, not on us**: `disconnected before OK` from the
|
||||
local nostr-rs-relay under the load of a full run. The durable ingest markers
|
||||
added in this pass cut a large part of that load (a backdated gift wrap used
|
||||
to be re-unwrapped on every sync, forever) but the test has not been re-run
|
||||
since.
|
||||
- **The local relay drops a publish occasionally.** One run in several,
|
||||
`amy` gets `disconnected before OK` from nostr-rs-relay and reports the send
|
||||
as unconfirmed even though the event is on the relay a moment later. It is a
|
||||
harness-relay flake, not a protocol failure, and it costs whichever test is
|
||||
running at the time. Worth making the CLI's publish confirmation tolerate a
|
||||
reconnect rather than papering over it in the tests.
|
||||
- **Agent-text-stream is receive-only.** We decode the `0x8006` policy, derive
|
||||
per-stream record keys, open records and fold the transcript, and we advertise
|
||||
the `0xF2D1` receive capability. We do NOT advertise `send` (`0xF2D2`) or
|
||||
|
||||
Reference in New Issue
Block a user