feat(subgroups): the refusals that are about capability, not preference

Phase 8 of docs/subgroups.md. Four of the seven refusals landed with the code
they guard; these are the three that did not, and two of them are about what this
device *can* do rather than what the user should pick.

**No share of the parent's key.** `FrostSigningManager.proposeSigningBatch` throws
outright for a device with no share, so without this the button crashes rather
than declines. It is checked first because it is the one refusal a user cannot fix
by picking differently -- every other message says "pick differently" and this one
cannot.

**Not an admin of the parent.** A non-admin proposing the group's signature is a
proposal the admins have to decline by hand, which is worse than not offering it.
Read off `Participant.adminAt`, which is the epoch-0 admin list MIP-01 carries and
the same reading every side of the group makes.

Both are checked in `refuseCeremonyRoom` as well as at the button that hides
itself on `canAddSubgroup`, because a screen not drawing something is not a guard
-- and a resumed flow reaches the manager without passing that screen at all.

**A child already certified.** One certificate per child: a second is a `d`-tag
replacement of the first rather than a second subgroup, and spending a quorum's
attention to restate something they have already signed is worse than doing
nothing. This is a `check` at propose time rather than a picker refusal, since the
child's id does not exist until the ceremony finishes.

It deliberately does not try to stop the race. Two coordinators can each propose a
certificate for the same child, neither able to see the other's session before it
completes; `certificateFor` folds those together because both say the same true
thing. This only stops the case somebody can actually see.

The blank-name `require` gets a test of its own for the same reason the name
exists at all: without it the parent's admins would be approving a hash.

Six tests in `SubgroupManagerJvmTest`, and the fixture had to grow to carry them.
`parentWith` now gives the parent a completed ceremony and marks the coordinator
an admin, with both switchable, because a fixture missing either tests only the
first refusal -- which is how the two new checks were found to short-circuit the
existing cases the moment they were added.

The plan called for a pure `SubgroupGuardsTest` in commonTest. It is not one:
every refusal here reads the database -- a key-holding session, an admin flag, a
ceremony in the derived room -- so the tests live in jvmTest beside the manager's
others rather than being reshaped into something pure that would test less.

397 common tests, 705 jvm tests, `m3Audit` meets every budget.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
Kgothatso Ngako
2026-09-08 23:50:05 +02:00
parent 6bd59e05fe
commit b10c79f9ed
2 changed files with 165 additions and 4 deletions

View File

@@ -101,6 +101,28 @@ object SubgroupManager {
coordinatorPublicKey: HexKey
): String? {
val admins = adminPublicKeys + coordinatorPublicKey
val parentChatRoomId = parentRoom.chatRoom.id
// A subgroup is certified by the parent's key, so a device holding no
// share of it cannot open the session at all --
// `FrostSigningManager.proposeSigningBatch` throws outright, and throwing
// at a button is not a UI. Checked before the size rules because it is the
// one refusal the user cannot fix by picking differently.
if (!FrostSigningManager.canSign(database, parentChatRoomId)) {
return "Only a member who holds a share of this group's key can make a subgroup."
}
// A non-admin proposing the group's signature is a proposal the admins
// have to decline by hand, which is a worse outcome than not offering it.
// Read off the epoch-0 admin list MIP-01 carries, the same reading every
// side of the group makes.
val isAdmin = parentRoom.localParticipants.any {
it.participant.participantPublicKey == coordinatorPublicKey &&
it.participant.adminAt != null
}
if (!isAdmin) {
return "Only an admin of this group can make a subgroup."
}
if (admins.size < MINIMUM_ADMINS) {
return "A subgroup needs at least $MINIMUM_ADMINS admins. Pick at least " +
@@ -168,6 +190,16 @@ object SubgroupManager {
require(name.isNotBlank()) { "A subgroup has to be called something" }
// One certificate per child. A second is a `d`-tag replacement of the
// first rather than a second subgroup, and spending a quorum's attention
// to restate something they have already signed is worse than doing
// nothing. Two coordinators racing still produce two -- neither can see
// the other's session before it completes -- and `certificateFor` folds
// those together; this only stops the case somebody can actually see.
check(certificateFor(database, subgroupChatRoomId, parentChatRoomId) == null) {
"$parentChatRoomId has already certified $subgroupChatRoomId"
}
logger.i("Asking $parentChatRoomId to certify $subgroupChatRoomId as its subgroup")
return FrostSigningManager.proposeSigning(

View File

@@ -18,6 +18,7 @@ import kotlinx.coroutines.runBlocking
import kotlin.test.AfterTest
import kotlin.test.Test
import kotlin.test.assertEquals
import kotlin.test.assertFailsWith
import kotlin.test.assertFalse
import kotlin.test.assertNotNull
import kotlin.test.assertNull
@@ -27,6 +28,7 @@ import press.mantra.compose.database.MantraDatabase
import press.mantra.compose.database.builder.getRoomDatabase
import press.mantra.compose.database.model.ChatRoom
import press.mantra.compose.database.model.DkgSession
import press.mantra.compose.database.model.types.DkgRitualStage
import press.mantra.compose.database.model.GroupSignedEvent
import press.mantra.compose.database.model.NostrEvent
import press.mantra.compose.database.model.Profile
@@ -251,6 +253,24 @@ class SubgroupManagerJvmTest {
// ---- the refusals ------------------------------------------------------
@Test
fun `a member holding no share of the parent's key is refused`(): Unit = runBlocking {
// Not a permission rule: `proposeSigningBatch` throws outright for a
// device with no share, so without this the button would crash rather
// than decline. It is also the one refusal the user cannot fix by picking
// differently, which is why it is checked first.
val parent = parentWith(listOf(alice, bob, carol), canSign = false)
assertNotNull(refusal(parent, setOf(alice, bob)))
}
@Test
fun `a member who is not an admin of the parent is refused`(): Unit = runBlocking {
val parent = parentWith(listOf(alice, bob, carol), isAdmin = false)
assertNotNull(refusal(parent, setOf(alice, bob)))
}
@Test
fun `a subgroup of fewer than three admins is refused`(): Unit = runBlocking {
val parent = parentWith(listOf(alice, bob, carol))
@@ -288,8 +308,8 @@ class SubgroupManagerJvmTest {
// of the members. A second subgroup with the same admins would come back
// with the first one's key and therefore the first one's room id.
//
// Stood up the way `openCeremony` does, room first: the ceremony room is
// a real NIP-17 room and `DkgSession.chatRoomId` is a foreign key onto it.
// Room first, the way the picker stands one up: the ceremony room is a
// real NIP-17 room and `DkgSession.chatRoomId` is a foreign key onto it.
val ceremonyRoom = assertNotNull(
db.nostrNip17Dao().createNip17ChatRoom(
userPublicKey = user,
@@ -336,8 +356,70 @@ class SubgroupManagerJvmTest {
)
}
@Test
fun `certifying a child the parent has already certified is refused`(): Unit = runBlocking {
seedRoom(parentRoomId)
record(certificate())
val parentRoom = assertNotNull(db.chatRoomDao().findChatRoomById(parentRoomId))
// One certificate per child: a second is a `d`-tag replacement rather
// than a second subgroup, and spending a quorum's attention to restate
// something they have already signed is worse than doing nothing.
//
// Two coordinators racing still produce two -- neither can see the
// other's session before it completes -- which is why `certificateFor`
// folds them. This only stops the case somebody can actually see.
assertFailsWith<IllegalStateException> {
SubgroupManager.proposeBirthCertificate(
database = db,
parentRoom = parentRoom,
userPublicKey = user,
key = completedChildCeremony(),
adminPublicKeys = listOf(user, alice, bob),
name = "Translation team",
)
}
}
@Test
fun `an unnamed subgroup cannot be certified`(): Unit = runBlocking {
seedRoom(parentRoomId)
val parentRoom = assertNotNull(db.chatRoomDao().findChatRoomById(parentRoomId))
// The parent's admins would be approving a hash. The name is what makes
// the proposal legible, so it is required rather than defaulted.
assertFailsWith<IllegalArgumentException> {
SubgroupManager.proposeBirthCertificate(
database = db,
parentRoom = parentRoom,
userPublicKey = user,
key = completedChildCeremony(),
adminPublicKeys = listOf(user, alice, bob),
name = " ",
)
}
}
// ---- fixtures ----------------------------------------------------------
/** A finished ceremony for the child, as far as these guards can tell. */
private fun completedChildCeremony() = DkgSession(
id = "child-ceremony",
chatRoomId = parentRoomId,
coordinatorPublicKey = user,
userPublicKey = user,
threshold = 2,
participantCount = 3,
stage = DkgRitualStage.COMPLETE,
hostPublicKey = user,
round1Random = "aa".repeat(32),
round2AuxRandom = "bb".repeat(32),
thresholdPublicKey = childMaterial.thresholdPublicKey.value.toHex(),
secretShare = "dd".repeat(32),
)
private fun keyMaterial(secret: String): KeyMaterial = Frost.trustedDealerKeygen(
thresholdSecretKey = PrivateKey(ByteVector32(secret)),
nParticipants = participants,
@@ -355,16 +437,63 @@ class SubgroupManagerJvmTest {
coordinatorPublicKey = user
)
private suspend fun parentWith(members: List<String>): LocalChatRoom {
/**
* A parent this device could actually make a subgroup of: an admin of it, and
* holding a share of its key.
*
* Both are conditions of `refuseCeremonyRoom` before it looks at the picking
* at all, so a fixture without them tests only the first refusal. [canSign]
* and [isAdmin] are here so the tests that are *about* those two can turn
* each off on its own.
*/
private suspend fun parentWith(
members: List<String>,
canSign: Boolean = true,
isAdmin: Boolean = true,
): LocalChatRoom {
(members + user).forEach { seedProfile(it) }
return assertNotNull(
val room = assertNotNull(
db.nostrNip17Dao().createNip17ChatRoom(
userPublicKey = user,
participantPublicKeys = members,
subject = "Ekklesia",
)
)
if (canSign) {
// `completedKey` falls back to the newest completed ceremony held in
// the room, which is what a NIP-17 room's key looks like from here.
db.dkgSessionDao().upsert(
DkgSession(
id = "parent-ceremony",
chatRoomId = room.chatRoom.id,
coordinatorPublicKey = user,
userPublicKey = user,
threshold = 2,
participantCount = members.size + 1,
stage = DkgRitualStage.COMPLETE,
hostPublicKey = user,
round1Random = "aa".repeat(32),
round2AuxRandom = "bb".repeat(32),
thresholdPublicKey = parentMaterial.thresholdPublicKey.value.toHex(),
secretShare = "cc".repeat(32),
)
)
}
if (isAdmin) {
db.participantDao().upsert(
assertNotNull(
db.chatRoomDao().findChatRoomById(room.chatRoom.id)
).localParticipants
.map { it.participant }
.filter { it.participantPublicKey == user }
.map { it.copy(adminAt = Instant.fromEpochSeconds(1_700_000_000)) }
)
}
return assertNotNull(db.chatRoomDao().findChatRoomById(room.chatRoom.id))
}
private suspend fun seedProfile(publicKey: String) {