Merge branch 'mantra' into claude/nostr-event-save-issue-6e9467
This commit is contained in:
@@ -435,17 +435,13 @@ abstract class MarmotOutboundDao(
|
||||
|
||||
// Save commitResult... in case we need to broadcast welcomeEvent after relay acknowledgement...
|
||||
database.marmotCommitResultDao().upsert(
|
||||
MarmotCommitResult(
|
||||
id = commitEvent.id,
|
||||
isOneMemberInitialGroupCreation = isOneMemberInitialGroupCreation,
|
||||
MarmotCommitResult.from(
|
||||
commitEventId = commitEvent.id,
|
||||
commitResult = commitResult,
|
||||
chatRoomId = nostrGroupId,
|
||||
commitBytes = commitResult.commitBytes,
|
||||
preCommitExporterSecret = commitResult.preCommitExporterSecret,
|
||||
welcomeBytes = commitResult.welcomeBytes,
|
||||
framedCommitBytes = commitResult.preCommitExporterSecret,
|
||||
groupInfoBytes = commitResult.groupInfoBytes,
|
||||
userPublicKey = userPublicKey,
|
||||
peerKeyPackageEventId = peerKeyPackage.id,
|
||||
isOneMemberInitialGroupCreation = isOneMemberInitialGroupCreation,
|
||||
createdAt = Instant.fromEpochSeconds(commitEvent.createdAt)
|
||||
)
|
||||
)
|
||||
|
||||
@@ -33,6 +33,7 @@ import press.mantra.compose.managers.ChillDkgRitualManager
|
||||
import press.mantra.compose.nostr.dkg.DkgRitualEvents
|
||||
import press.mantra.compose.nostr.frost.FrostSigningEvents
|
||||
import press.mantra.compose.managers.FrostSigningManager
|
||||
import press.mantra.compose.managers.MlsGroupCache
|
||||
import press.mantra.compose.managers.MarmotInboundManager
|
||||
import co.touchlab.kermit.Logger
|
||||
import kotlinx.coroutines.CancellationException
|
||||
@@ -388,9 +389,21 @@ abstract class NostrDao(
|
||||
val localChatRoom = database.chatRoomDao().findChatRoomById(chatRoomId)
|
||||
|
||||
if (localChatRoom != null) {
|
||||
val mlsGroup = localChatRoom.chatRoom.toMlsGroup()
|
||||
|
||||
if (mlsGroup != null) {
|
||||
// Through the cache rather than rebuilt here, so the secret
|
||||
// tree's skipped-generation keys survive from one message to
|
||||
// the next. Two events published in the same instant arrive in
|
||||
// whatever order the relay feels like, and rebuilding between
|
||||
// them loses the earlier one for good -- see MlsGroupCache.
|
||||
val handled = MlsGroupCache.withGroup(
|
||||
chatRoomId = chatRoomId,
|
||||
storedStateHex = localChatRoom.chatRoom.mlsGroupState,
|
||||
build = { localChatRoom.chatRoom.toMlsGroup() },
|
||||
save = { stateHex ->
|
||||
database.chatRoomDao().upsert(
|
||||
localChatRoom.chatRoom.copy(mlsGroupState = stateHex)
|
||||
)
|
||||
}
|
||||
) { mlsGroup ->
|
||||
val memberPubkeys = mlsGroup.members().mapNotNull { (leafIndex, leafNode) ->
|
||||
|
||||
val pubkey = when (val cred = leafNode.credential) {
|
||||
@@ -437,12 +450,6 @@ abstract class NostrDao(
|
||||
}
|
||||
}
|
||||
|
||||
// Save the mls chatRoom state...
|
||||
database.chatRoomDao().upsert(
|
||||
localChatRoom.chatRoom.copy(
|
||||
mlsGroupState = mlsGroup.saveState().encodeTls().toHex()
|
||||
)
|
||||
)
|
||||
ChatMessage.fromGroupEventResult(
|
||||
database = database,
|
||||
activeKeyPair = activeKeyPair,
|
||||
@@ -477,7 +484,11 @@ abstract class NostrDao(
|
||||
} else {
|
||||
throw MarmotNotMemberOfChatGroupException("We are not a member of the chat room ${localChatRoom.chatRoom.id}")
|
||||
}
|
||||
} else {
|
||||
}
|
||||
|
||||
// Null means the room has no usable group state, which is what
|
||||
// a failed toMlsGroup() meant before the cache existed.
|
||||
if (handled == null) {
|
||||
throw MarmotMissingNostrGroupDataExtension("Couldn't find chatRoom for $nostrEvent")
|
||||
}
|
||||
} else {
|
||||
|
||||
@@ -8,6 +8,7 @@ import press.mantra.compose.database.model.traits.SoftDeletableEntity
|
||||
import press.mantra.compose.database.model.traits.TimestampedEntity
|
||||
import press.mantra.compose.database.model.traits.UserViewableEntity
|
||||
import co.touchlab.kermit.Logger
|
||||
import com.vitorpamplona.quartz.marmot.mls.messages.CommitResult
|
||||
import com.vitorpamplona.quartz.nip01Core.core.HexKey
|
||||
import kotlin.time.Clock
|
||||
import kotlin.time.Instant
|
||||
@@ -72,6 +73,40 @@ data class MarmotCommitResult( // TODO: Rename this to GiftWrapPayload...
|
||||
companion object {
|
||||
const val TAG = "MarmotCommitResult"
|
||||
|
||||
/**
|
||||
* The persisted record of a commit, built from the [CommitResult] that produced it.
|
||||
*
|
||||
* The five payload fields are carried over from quartz verbatim -- same names, same
|
||||
* order, same `ByteArray` type on both sides of the copy -- so a value taken from the
|
||||
* wrong field of the right object typechecks and reaches the database unnoticed.
|
||||
* `framedCommitBytes = commitResult.preCommitExporterSecret` survived exactly that way,
|
||||
* storing the group's pre-commit exporter secret in the column documented to hold a
|
||||
* broadcastable MLS envelope.
|
||||
*
|
||||
* Mapping here rather than at the call site means it is written once, in declaration
|
||||
* order, and pinned by MarmotCommitResultMappingTest.
|
||||
*/
|
||||
fun from(
|
||||
commitEventId: HexKey,
|
||||
commitResult: CommitResult,
|
||||
chatRoomId: HexKey,
|
||||
userPublicKey: HexKey,
|
||||
peerKeyPackageEventId: HexKey,
|
||||
isOneMemberInitialGroupCreation: Boolean,
|
||||
createdAt: Instant,
|
||||
): MarmotCommitResult = MarmotCommitResult(
|
||||
id = commitEventId,
|
||||
userPublicKey = userPublicKey,
|
||||
peerKeyPackageEventId = peerKeyPackageEventId,
|
||||
chatRoomId = chatRoomId,
|
||||
isOneMemberInitialGroupCreation = isOneMemberInitialGroupCreation,
|
||||
commitBytes = commitResult.commitBytes,
|
||||
welcomeBytes = commitResult.welcomeBytes,
|
||||
groupInfoBytes = commitResult.groupInfoBytes,
|
||||
framedCommitBytes = commitResult.framedCommitBytes,
|
||||
preCommitExporterSecret = commitResult.preCommitExporterSecret,
|
||||
createdAt = createdAt,
|
||||
)
|
||||
}
|
||||
|
||||
override fun equals(other: Any?): Boolean {
|
||||
|
||||
@@ -0,0 +1,152 @@
|
||||
package press.mantra.compose.managers
|
||||
|
||||
import co.touchlab.kermit.Logger
|
||||
import com.vitorpamplona.quartz.marmot.mls.group.MlsGroup
|
||||
import press.mantra.compose.extensions.toHex
|
||||
import kotlinx.coroutines.sync.Mutex
|
||||
import kotlinx.coroutines.sync.withLock
|
||||
|
||||
/**
|
||||
* Keeps a room's [MlsGroup] alive between messages instead of rebuilding it
|
||||
* from the stored state every time.
|
||||
*
|
||||
* ### The bug this exists for
|
||||
*
|
||||
* MLS is specified to tolerate out-of-order delivery within an epoch: a
|
||||
* receiver that gets generation N+1 before N derives and caches the key for N
|
||||
* so the older message can still be read when it turns up. Quartz's
|
||||
* `SecretTree` does exactly that, in a private `skippedKeys` map.
|
||||
*
|
||||
* `SecretTree.exportSenderStates()` does not include that map, so
|
||||
* `MlsGroup.saveState()` does not carry it. Rebuilding the group from stored
|
||||
* state therefore throws the skipped keys away, and a message for a generation
|
||||
* the ratchet has already passed fails
|
||||
* `require(generation >= state.applicationGeneration)` and is dropped. There is
|
||||
* no recovering it afterwards: the key is gone and the sender will not resend.
|
||||
*
|
||||
* Nostr relays offer no ordering whatsoever, so this is not an edge case. Two
|
||||
* events published in the same second race, and exactly one survives — which is
|
||||
* how a signing session's proposal was lost while the nonce sent immediately
|
||||
* behind it arrived fine.
|
||||
*
|
||||
* ### What this fixes, and what it does not
|
||||
*
|
||||
* Holding the instance means `skippedKeys` survives for as long as the process
|
||||
* does and nothing else writes the room's state. That covers the case that
|
||||
* actually bites — a burst of messages arriving in one sync — because they are
|
||||
* decrypted one after another against the same tree.
|
||||
*
|
||||
* It does not survive a restart, and it does not survive another writer, so
|
||||
* reordering across app launches still loses messages. The real fix is for
|
||||
* `exportSenderStates` to carry the skipped keys; see
|
||||
* `docs/mls-skipped-keys.md`.
|
||||
*
|
||||
* ### Staleness
|
||||
*
|
||||
* The group is only reused when the stored state is still exactly what this
|
||||
* cache last wrote. Anything else that saves a room's state — sending a message
|
||||
* advances the sender ratchet and saves, so does adding a member — changes the
|
||||
* hex, and the next read rebuilds rather than carrying on from a group that has
|
||||
* been overtaken. Losing the skipped keys there is the same behaviour as
|
||||
* before this existed, so the fallback is never worse than not caching.
|
||||
*/
|
||||
object MlsGroupCache {
|
||||
private val cache = LiveInstanceCache<MlsGroup> { it.saveState().encodeTls().toHex() }
|
||||
|
||||
/**
|
||||
* Runs [block] against the room's live group, then stores whatever state it
|
||||
* left behind.
|
||||
*
|
||||
* [storedStateHex] is the room's state as the database currently has it, and
|
||||
* [build] turns it into a group. [save] is handed the state to persist.
|
||||
*
|
||||
* Returns null without calling [block] when the room has no usable group
|
||||
* state, which is the same thing a failed `toMlsGroup()` meant before.
|
||||
*/
|
||||
suspend fun <T> withGroup(
|
||||
chatRoomId: String,
|
||||
storedStateHex: String?,
|
||||
build: () -> MlsGroup?,
|
||||
save: suspend (String) -> Unit,
|
||||
block: suspend (MlsGroup) -> T,
|
||||
): T? = cache.withInstance(
|
||||
key = chatRoomId,
|
||||
storedState = storedStateHex,
|
||||
build = build,
|
||||
save = save,
|
||||
block = block
|
||||
)
|
||||
}
|
||||
|
||||
/**
|
||||
* One live instance per key, reused only while the stored state is still the one
|
||||
* this cache last wrote.
|
||||
*
|
||||
* Split out from [MlsGroupCache] so the decision it makes can be tested without
|
||||
* standing up an MLS group. That decision is the whole safety argument: reuse
|
||||
* when nothing else has written, rebuild when something has, and never carry on
|
||||
* with an instance whose last use failed part-way through.
|
||||
*/
|
||||
internal class LiveInstanceCache<T : Any>(
|
||||
/** The persisted form of an instance, for spotting another writer. */
|
||||
private val stateOf: (T) -> String,
|
||||
) {
|
||||
private val logger = Logger.withTag("LiveInstanceCache")
|
||||
|
||||
private class Entry<T>(val instance: T, val state: String)
|
||||
|
||||
private val entries = mutableMapOf<String, Entry<T>>()
|
||||
|
||||
/**
|
||||
* Serialises use of one key's instance.
|
||||
*
|
||||
* The instance is mutable and [block] advances it, so two callers running at
|
||||
* once would corrupt it. One lock per key rather than one overall, so a busy
|
||||
* key cannot hold up a quiet one.
|
||||
*
|
||||
* Held across [block], which may touch the database. Safe here because a
|
||||
* caller only ever takes this lock while it is already running -- it never
|
||||
* waits on a resource the holder is itself waiting for.
|
||||
*/
|
||||
private val locks = mutableMapOf<String, Mutex>()
|
||||
private val locksGuard = Mutex()
|
||||
|
||||
private suspend fun lockFor(key: String): Mutex =
|
||||
locksGuard.withLock { locks.getOrPut(key) { Mutex() } }
|
||||
|
||||
suspend fun <R> withInstance(
|
||||
key: String,
|
||||
storedState: String?,
|
||||
build: () -> T?,
|
||||
save: suspend (String) -> Unit,
|
||||
block: suspend (T) -> R,
|
||||
): R? = lockFor(key).withLock {
|
||||
val cached = entries[key]
|
||||
|
||||
val instance = if (cached != null && cached.state == storedState) {
|
||||
cached.instance
|
||||
} else {
|
||||
if (cached != null) {
|
||||
logger.d("$key was written elsewhere; rebuilding")
|
||||
}
|
||||
// Dropped before the block runs, so a build that fails does not leave
|
||||
// the old instance behind to be picked up by the next caller.
|
||||
entries.remove(key)
|
||||
build() ?: return@withLock null
|
||||
}
|
||||
|
||||
// Deliberately not in a finally: an instance whose use threw part-way is
|
||||
// in an unknown state, and the next caller should rebuild from whatever
|
||||
// was last persisted rather than carry on with it.
|
||||
val result = block(instance)
|
||||
|
||||
val state = stateOf(instance)
|
||||
save(state)
|
||||
entries[key] = Entry(instance = instance, state = state)
|
||||
|
||||
result
|
||||
}
|
||||
|
||||
/** How many instances are held. For tests. */
|
||||
internal fun size(): Int = entries.size
|
||||
}
|
||||
Reference in New Issue
Block a user