laantungir/amethyst
1
0
Fork 0
mirror of https://github.com/vitorpamplona/amethyst.git synced 2026-10-05 19:28:25 +00:00
Code Issues Packages Projects Releases Wiki Activity

fix(nip52): stop p-tagging invitees on RSVPs; the a-tag already routes to them

Browse Source 
Audit of the two prior commits. Copying the appointment's participants onto the
RSVP as extra `p` tags was wrong on every axis, and bought nothing:

- It spams the notification feed. Kind 31925 is in NOTIFICATION_KINDS, and
  NotificationFeedFilter.tagsAnEventByUser falls through to `return true` for it
  (CalendarRSVPEvent is a BaseAddressableEvent, so it matches none of the
  BaseNoteEvent branches). Every tagged pubkey therefore gets a notification row.
  On an N-person appointment each invitee got N-1 rows, plus one more per
  co-invitee answer change. Before this feature only the host was tagged.
- It puts a host-controlled, uncapped list into a signed event, which the
  broadcaster then expands into relay URLs with no limit.
- NIP-52 defines this kind's `p` tag as the calendar event author, so the extra
  entries are ambiguous for any client that does not take the first match.

And the routing justification was simply false. The claim was that the tags kept
the fan-out working when the appointment is not cached. Both paths read
`LocalCache.getAddressableNoteIfExists(targetAddress)?.event` - the composer to
list participants, EventBroadcaster to recurse through the a-tag - so they are
reachable in exactly the same cases. The a-tag recursion into the appointment,
whose own p tags are pubkey hints since the previous commit, already delivers
every participant's inbox relays. The extra tags added zero coverage.

Removes the tags and the now-unused rsvpParticipantTags helper. Replaces
RsvpParticipantTagsTest with RsvpTagAssemblyTest, which asserts the tag shape the
composer emits - notably exactly one `p`, the host - so the fan-out cannot be
reintroduced through the notification path by accident.

Also corrects a coverage claim from the previous commit: the `test` alias maps to
jvmTest, so a KMP module's other targets stay outside it. :quartz:testAndroidHostTest
(~3.9k tests) is run by nothing - not `test`, not pre-push, not CI - and is red on
main, verified on 7236e952 in a clean worktree: NostrServerTest x3 and
LiveNegentropyIndexStoreTest x1 fail there too. Aliasing onto allTests would have
turned `test` red for everyone instead of fixing that, so the limitation is
documented in CLAUDE.md and the build comment rather than papered over.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Nxd2uiYajDGuymQK93txzG
This commit is contained in:
Claude
2026-09-18 21:58:59 +00:00
parent 91b9e29588
commit 1a958ff153
5 changed files with 125 additions and 193 deletions
Download Patch File Download Diff File Expand all files Collapse all files
.claude/CLAUDE.md
+11 -4
View File
@@ -224,14 +224,21 @@ version. `quartz/` is protocol-only — no composables.
# Build Quartz for all targets
./gradlew :quartz:build
# Run tests (every module, including the KMP ones)
# Run the JVM tests of every module, KMP ones included. KMP modules register no
# `test` task of their own; the root build aliases it onto their jvmTest.
./gradlew test
# One module. KMP modules (quartz, commons, commonsUI, quic, nestsClient,
# marmotQuic) have no `test` task of their own - the root build registers an
# alias onto jvmTest, which is the task to name directly:
# A single module. For a KMP module name jvmTest directly:
./gradlew :quartz:jvmTest --tests "com.vitorpamplona.quartz.nip52Calendar.*"
# NOT covered by `test`: each KMP module's OTHER targets, notably
# :quartz:testAndroidHostTest (~3.9k tests, its own androidHostTest source set).
# Nothing runs it today - not `test`, not pre-push, not CI - and it has 4 known
# failures on main (NostrServerTest x3, LiveNegentropyIndexStoreTest x1), which
# is why the alias maps to jvmTest rather than allTests. Run it explicitly when
# touching relay-server or store code:
./gradlew :quartz:testAndroidHostTest
# Format code
./gradlew spotlessApply
```
amethyst/src/main/java/com/vitorpamplona/amethyst/ui/note/types/CalendarRsvpRow.kt
+13 -33
View File
@@ -45,14 +45,10 @@ import com.vitorpamplona.amethyst.commons.resources.calendar_rsvp_not_going
import com.vitorpamplona.amethyst.model.LocalCache
import com.vitorpamplona.amethyst.ui.screen.loggedIn.AccountViewModel
import com.vitorpamplona.quartz.nip01Core.core.Address
import com.vitorpamplona.quartz.nip01Core.core.Event
import com.vitorpamplona.quartz.nip01Core.tags.aTag.ATag
import com.vitorpamplona.quartz.nip01Core.tags.events.ETag
import com.vitorpamplona.quartz.nip01Core.tags.people.PTag
import com.vitorpamplona.quartz.nip01Core.tags.people.pTags
import com.vitorpamplona.quartz.nip52Calendar.appt.day.CalendarDateSlotEvent
import com.vitorpamplona.quartz.nip52Calendar.appt.tags.RSVPStatusTag
import com.vitorpamplona.quartz.nip52Calendar.appt.time.CalendarTimeSlotEvent
import com.vitorpamplona.quartz.nip52Calendar.rsvp.CalendarRSVPEvent
import org.jetbrains.compose.resources.stringResource
@@ -202,9 +198,19 @@ private fun sendRsvp(
val eTag = ETag(eventId, relayHint, targetAddress.pubKeyHex)
val pTag = PTag(targetAddress.pubKeyHex)
val dTag = rsvpDTagFor(targetAddress)
val appointment = LocalCache.getAddressableNoteIfExists(targetAddress)?.event
val others = rsvpParticipantTags(appointment, targetAddress.pubKeyHex, myPubKey)
// Only the host is p-tagged, per NIP-52 ("pubkey of the author of the calendar event being
// responded to"). The other invitees still receive this RSVP: EventBroadcaster follows the
// a-tag into the appointment and reads its participants' inbox relays from the appointment's
// own p tags (CalendarTimeSlotEvent/CalendarDateSlotEvent are PubKeyHintProviders).
//
// Copying those participants onto the RSVP as extra p tags would add no routing - the
// broadcaster's recursion and any local participant lookup read the same
// LocalCache.getAddressableNoteIfExists(targetAddress), so they are reachable in exactly the
// same cases - while giving every invitee a notification row for every other invitee's RSVP
// (kind 31925 is in NOTIFICATION_KINDS and tagsAnEventByUser returns true for it), bloating
// the signed event by a host-controlled number of tags, and muddying the spec's meaning of
// this kind's p tag.
accountViewModel.launchSigner {
accountViewModel.account.signAndComputeBroadcast(
CalendarRSVPEvent.build(
@@ -213,33 +219,7 @@ private fun sendRsvp(
calendarEventId = eTag,
calendarEventAuthor = pTag,
dTag = dTag,
) {
pTags(others)
},
),
)
}
}
/**
* The [appointment]'s own NIP-52 `p` tags, minus [hostPubKey] (already tagged as the event author)
* and minus [myPubKey]. Tagging them on the RSVP is what puts it in every invitee's inbox: the
* broadcaster resolves `p` tags to inbox relays, so without these the RSVP only reaches the host.
*
* Empty when the appointment isn't cached yet; the a-tag still routes the RSVP through the host.
*/
fun rsvpParticipantTags(
appointment: Event?,
hostPubKey: String,
myPubKey: String,
): List<PTag> {
val participants =
when (appointment) {
is CalendarTimeSlotEvent -> appointment.participants()
is CalendarDateSlotEvent -> appointment.participants()
else -> return emptyList()
}
return participants
.distinctBy { it.pubKey }
.filter { it.pubKey != hostPubKey && it.pubKey != myPubKey }
}
amethyst/src/test/java/com/vitorpamplona/amethyst/calendar/RsvpParticipantTagsTest.kt
-156
View File
@@ -1,156 +0,0 @@
/*
* 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.calendar
import com.vitorpamplona.amethyst.ui.note.types.rsvpParticipantTags
import com.vitorpamplona.quartz.nip01Core.core.Address
import com.vitorpamplona.quartz.nip01Core.relay.normalizer.RelayUrlNormalizer
import com.vitorpamplona.quartz.nip01Core.tags.aTag.ATag
import com.vitorpamplona.quartz.nip01Core.tags.events.ETag
import com.vitorpamplona.quartz.nip01Core.tags.people.PTag
import com.vitorpamplona.quartz.nip01Core.tags.people.pTags
import com.vitorpamplona.quartz.nip52Calendar.appt.day.CalendarDateSlotEvent
import com.vitorpamplona.quartz.nip52Calendar.appt.tags.RSVPStatusTag
import com.vitorpamplona.quartz.nip52Calendar.appt.time.CalendarTimeSlotEvent
import com.vitorpamplona.quartz.nip52Calendar.rsvp.CalendarRSVPEvent
import org.junit.Assert.assertEquals
import org.junit.Assert.assertTrue
import org.junit.Test
/**
* The RSVP's `p` tags are what the broadcaster turns into inbox relays, so this list decides who
* actually receives the RSVP beyond the host. See EventBroadcaster.computeRelayListToBroadcast.
*/
class RsvpParticipantTagsTest {
private val host = "460c25e682fda7832b52d1f22d3d22b3176d972f60dcdc3212ed8c92ef85065c"
private val invitee = "99bb5591c9116600f845107d31f9b59e2f7c7e09a1ff802e84f1d43da557ca64"
private val speaker = "3bf0c63fcb93463407af97a5e5ee64fa883d107ef9e558472c4eb9aaaefa459d"
private val me = "82341f882b6eabcd2ba7f1ef90aad961cf074af15b9ef44a09f9d2a8fbfbe6a2"
private fun timeSlot(vararg tags: Array<String>) =
CalendarTimeSlotEvent(
id = "11".repeat(32),
pubKey = host,
createdAt = 1700000000,
tags = arrayOf(*tags),
content = "",
sig = "00".repeat(64),
)
private fun dateSlot(vararg tags: Array<String>) =
CalendarDateSlotEvent(
id = "22".repeat(32),
pubKey = host,
createdAt = 1700000000,
tags = arrayOf(*tags),
content = "",
sig = "00".repeat(64),
)
@Test
fun collectsInviteesFromATimeSlot() {
val appt = timeSlot(arrayOf("d", "party"), arrayOf("p", invitee), arrayOf("p", speaker))
assertEquals(
listOf(invitee, speaker),
rsvpParticipantTags(appt, host, me).map { it.pubKey },
)
}
@Test
fun collectsInviteesFromADateSlot() {
val appt = dateSlot(arrayOf("d", "party"), arrayOf("p", invitee))
assertEquals(listOf(invitee), rsvpParticipantTags(appt, host, me).map { it.pubKey })
}
@Test
fun keepsTheRelayHintOnEachParticipant() {
val appt = timeSlot(arrayOf("d", "party"), arrayOf("p", invitee, "wss://relay.damus.io/", "speaker"))
val tag = rsvpParticipantTags(appt, host, me).single()
assertEquals(invitee, tag.pubKey)
assertEquals("wss://relay.damus.io/", tag.relayHint?.url)
}
@Test
fun dropsTheHostSinceBuildAlreadyTagsThem() {
// CalendarRSVPEvent.build() writes the host as calendarEventAuthor; re-adding it here
// would put two `p` tags for the same key on the event and break calendarEventAuthor()
// for readers that expect the first one to be the host.
val appt = timeSlot(arrayOf("d", "party"), arrayOf("p", host), arrayOf("p", invitee))
assertEquals(listOf(invitee), rsvpParticipantTags(appt, host, me).map { it.pubKey })
}
@Test
fun dropsOurselves() {
val appt = timeSlot(arrayOf("d", "party"), arrayOf("p", me), arrayOf("p", invitee))
assertEquals(listOf(invitee), rsvpParticipantTags(appt, host, me).map { it.pubKey })
}
@Test
fun dedupesRepeatedParticipants() {
val appt =
timeSlot(
arrayOf("d", "party"),
arrayOf("p", invitee, "wss://relay.damus.io/"),
arrayOf("p", invitee, "wss://nos.lol/"),
)
assertEquals(listOf(invitee), rsvpParticipantTags(appt, host, me).map { it.pubKey })
}
@Test
fun emptyWhenTheAppointmentIsNotCachedYet() {
assertTrue(rsvpParticipantTags(null, host, me).isEmpty())
}
@Test
fun buildWritesBothTheCoordinateAndThePinnedRevision() {
// Mirrors what sendRsvp assembles: the `a` tag follows the host's edits, the `e` tag
// pins the revision that was on screen, and the first `p` tag stays the host so
// calendarEventAuthor() still resolves.
val target = Address(31923, host, "party")
val hint = RelayUrlNormalizer.normalizeOrNull("wss://relay.damus.io/")
val apptId = "43575072239da152afe3d7b5c70ed2beb48db2b10e60c60da45229c09c877d2a"
val template =
CalendarRSVPEvent.build(
calendarEventAddress = ATag(target, hint),
status = RSVPStatusTag.STATUS.ACCEPTED,
calendarEventId = ETag(apptId, hint, host),
calendarEventAuthor = PTag(host),
dTag = "rsvp-d",
) {
pTags(listOf(PTag(invitee)))
}
val aTags = template.tags.filter { it[0] == "a" }
val eTags = template.tags.filter { it[0] == "e" }
val pTags = template.tags.filter { it[0] == "p" }
assertEquals("31923:$host:party", aTags.single()[1])
assertEquals(apptId, eTags.single()[1])
assertEquals(listOf(host, invitee), pTags.map { it[1] })
}
}
amethyst/src/test/java/com/vitorpamplona/amethyst/calendar/RsvpTagAssemblyTest.kt
+96
View File
@@ -0,0 +1,96 @@
/*
* 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.calendar
import com.vitorpamplona.quartz.nip01Core.core.Address
import com.vitorpamplona.quartz.nip01Core.relay.normalizer.RelayUrlNormalizer
import com.vitorpamplona.quartz.nip01Core.tags.aTag.ATag
import com.vitorpamplona.quartz.nip01Core.tags.events.ETag
import com.vitorpamplona.quartz.nip01Core.tags.people.PTag
import com.vitorpamplona.quartz.nip52Calendar.appt.tags.RSVPStatusTag
import com.vitorpamplona.quartz.nip52Calendar.rsvp.CalendarRSVPEvent
import org.junit.Assert.assertEquals
import org.junit.Test
/**
* Locks in the tag shape [com.vitorpamplona.amethyst.ui.note.types.CalendarRsvpRow] assembles.
*
* The `p` count is the load-bearing assertion. Kind 31925 is in `NOTIFICATION_KINDS`, and
* `NotificationFeedFilter.tagsAnEventByUser` returns true for it (a `BaseAddressableEvent` falls
* through every `BaseNoteEvent` branch), so every pubkey tagged here gets a notification row for
* this RSVP. Tagging the appointment's other invitees would therefore hand each of them one row
* per co-invitee per answer change, and buys no routing: `EventBroadcaster` already reaches them
* by following the a-tag into the appointment, whose own p tags it reads as pubkey hints.
*/
class RsvpTagAssemblyTest {
private val host = "460c25e682fda7832b52d1f22d3d22b3176d972f60dcdc3212ed8c92ef85065c"
private val apptId = "43575072239da152afe3d7b5c70ed2beb48db2b10e60c60da45229c09c877d2a"
private val relay = "wss://relay.damus.io/"
private fun buildRsvp(): Array<Array<String>> {
val target = Address(31923, host, "party")
val hint = RelayUrlNormalizer.normalizeOrNull(relay)
return CalendarRSVPEvent
.build(
calendarEventAddress = ATag(target, hint),
status = RSVPStatusTag.STATUS.ACCEPTED,
calendarEventId = ETag(apptId, hint, host),
calendarEventAuthor = PTag(host),
dTag = "rsvp-d",
).tags
}
@Test
fun tagsTheAppointmentCoordinateAndThePinnedRevision() {
val tags = buildRsvp()
// The `a` tag follows the host's later edits; the `e` tag pins the revision answered.
assertEquals("31923:$host:party", tags.single { it[0] == "a" }[1])
assertEquals(apptId, tags.single { it[0] == "e" }[1])
assertEquals("rsvp-d", tags.single { it[0] == "d" }[1])
assertEquals("accepted", tags.single { it[0] == "status" }[1])
}
@Test
fun tagsExactlyOnePubkeyAndItIsTheHost() {
val pTags = buildRsvp().filter { it[0] == "p" }
assertEquals(listOf(host), pTags.map { it[1] })
}
@Test
fun theHostRemainsTheFirstPubkeySoCalendarEventAuthorResolves() {
val tags = buildRsvp()
val event =
CalendarRSVPEvent(
id = "00".repeat(32),
pubKey = "11".repeat(32),
createdAt = 1700000000,
tags = tags,
content = "",
sig = "00".repeat(64),
)
assertEquals(host, event.calendarEventAuthor()?.pubKey)
assertEquals(apptId, event.calendarEventId()?.eventId)
}
}
build.gradle.kts
+5
View File
@@ -105,6 +105,11 @@ subprojects {
// Linux) and all of which change what `test` costs. jvmTest is exactly what the
// pre-push hook and CI already run for these modules, so the alias matches the coverage
// they expect rather than inventing a third definition of "the tests".
//
// The flip side: a KMP module's non-JVM targets stay outside `test`. :quartz's
// androidHostTest source set (~3.9k tests) is the big one - nothing runs it today and it
// is red on main, so aliasing onto allTests would have turned `test` red for everyone
// rather than fixing anything. Tracked in CLAUDE.md's Build Commands section.
if (plugins.hasPlugin("org.jetbrains.kotlin.multiplatform") &&
tasks.findByName("test") == null &&
tasks.findByName("jvmTest") != null