From 8afd92aba408007a015cff3b34f4762a46866dfb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ricardo=20Iv=C3=A1n=20Vieitez=20Parra?= <3857362+corrideat@users.noreply.github.com> Date: Fri, 9 Aug 2024 19:53:54 +0000 Subject: [PATCH 01/24] Fix #2277 - Fix reference count errors - Implement re-syncing contracts with foreign keys on `OP_KEY_SHARE` on foreign contract --- frontend/controller/actions/group.js | 18 +++- frontend/controller/actions/identity.js | 11 ++- frontend/controller/app/group.js | 6 +- frontend/controller/app/identity.js | 4 +- frontend/model/contracts/chatroom.js | 3 +- frontend/model/contracts/group.js | 86 +++++++++++++++++--- frontend/model/contracts/identity.js | 43 ++++++---- frontend/model/contracts/shared/functions.js | 4 - frontend/model/state.js | 6 +- frontend/views/pages/Join.vue | 4 +- shared/domains/chelonia/chelonia.js | 12 +++ shared/domains/chelonia/internals.js | 53 +++++++++++- shared/domains/chelonia/utils.js | 2 +- 13 files changed, 201 insertions(+), 51 deletions(-) diff --git a/frontend/controller/actions/group.js b/frontend/controller/actions/group.js index fc993a44a..1899b59cc 100644 --- a/frontend/controller/actions/group.js +++ b/frontend/controller/actions/group.js @@ -896,7 +896,23 @@ export default (sbp('sbp/selectors/register', { sbp('okTurtles.events/emit', REPLACE_MODAL, 'IncomeDetails') } }, - ...encryptedAction('gi.actions/group/leaveChatRoom', L('Failed to leave chat channel.')), + ...encryptedAction('gi.actions/group/leaveChatRoom', L('Failed to leave chat channel.'), async (sendMessage, params) => { + const state = await sbp('chelonia/contract/state', params.contractID) + const memberID = params.data.memberID || sbp('chelonia/rootState').loggedIn.identityContractID + const reference = state.chatRooms[params.data.chatRoomID].members[memberID].joinedHeight + + // For more efficient and correct processing, augment the leaveChatRoom + // action with the height of the join action. This helps prevent reduce + // the logic running as side-effects when syncing contracts from scratch + // by only considering the most recent join/leave events. + await sendMessage({ + ...params, + data: { + ...params.data, + reference + } + }) + }), ...encryptedAction('gi.actions/group/deleteChatRoom', L('Failed to delete chat channel.')), ...encryptedAction('gi.actions/group/invite', L('Failed to create invite.')), ...encryptedAction('gi.actions/group/inviteAccept', L('Failed to accept invite.'), async function (sendMessage, params) { diff --git a/frontend/controller/actions/identity.js b/frontend/controller/actions/identity.js index 13d241f94..0c66d8f6f 100644 --- a/frontend/controller/actions/identity.js +++ b/frontend/controller/actions/identity.js @@ -302,8 +302,13 @@ export default (sbp('sbp/selectors/register', { ) // update the 'lastLoggedIn' field in user's group profiles - Object.keys(cheloniaState[identityContractID].groups) - .forEach(cId => { + // For this, we select only those groups for which membership is + // active (meaning current groups), instead of historical groups (groups + // that have been joined in the past) + Object.entries(cheloniaState[identityContractID].groups) + // $FlowFixMe[incompatible-use] + .filter(([, { active }]) => active) + .forEach(([cId]) => { // We send this action only for groups we have fully joined (i.e., // accepted an invite and added our profile) if (cheloniaState[cId]?.profiles?.[identityContractID]?.status === PROFILE_STATUS.ACTIVE) { @@ -420,7 +425,7 @@ export default (sbp('sbp/selectors/register', { const rootState = sbp('state/vuex/state') const state = rootState[contractID] // TODO: Also share PEK with DMs - await Promise.all(Object.keys(state.groups || {}).filter(groupID => !!rootState.contracts[groupID]).map(async groupID => { + await Promise.all(Object.keys(state.groups || {}).filter(groupID => state.groups[groupID].active && !!rootState.contracts[groupID]).map(async groupID => { const CEKid = await sbp('chelonia/contract/currentKeyIdByName', groupID, 'cek') const CSKid = await sbp('chelonia/contract/currentKeyIdByName', groupID, 'csk') diff --git a/frontend/controller/app/group.js b/frontend/controller/app/group.js index 5db77f1b5..e29fd89db 100644 --- a/frontend/controller/app/group.js +++ b/frontend/controller/app/group.js @@ -28,9 +28,11 @@ sbp('okTurtles.events/on', LEFT_GROUP, ({ identityContractID, groupContractID }) // grab the groupID of any group that we're a part of const currentGroupId = rootState.currentGroupId if (!currentGroupId || currentGroupId === groupContractID) { - const groupIdToSwitch = Object.keys(state.groups) + const groupIdToSwitch = Object.entries(state.groups) + // $FlowFixMe[incompatible-use] + .map(([cID, { active }]) => active && cID) .filter(cID => - cID !== groupContractID + cID && cID !== groupContractID ).sort(cID => // prefer successfully joined groups sbp('state/vuex/state')[cID]?.profiles?.[groupContractID] ? -1 : 1 diff --git a/frontend/controller/app/identity.js b/frontend/controller/app/identity.js index 4afd15e24..f20e606cb 100644 --- a/frontend/controller/app/identity.js +++ b/frontend/controller/app/identity.js @@ -7,7 +7,7 @@ import { Secret } from '~/shared/domains/chelonia/Secret.js' import { boxKeyPair, buildRegisterSaltRequest, computeCAndHc, decryptContractSalt, hash, hashPassword, randomNonce } from '~/shared/zkpp.js' // Using relative path to crypto.js instead of ~-path to workaround some esbuild bug import * as Common from '@common/common.js' -import { cloneDeep, has } from '@model/contracts/shared/giLodash.js' +import { cloneDeep } from '@model/contracts/shared/giLodash.js' import { CURVE25519XSALSA20POLY1305, EDWARDS25519SHA512BATCH, deriveKeyFromPassword, serializeKey } from '../../../shared/domains/chelonia/crypto.js' import { handleFetchResult } from '../utils/misc.js' @@ -121,7 +121,7 @@ sbp('okTurtles.events/on', LOGIN, async ({ identityContractID, encryptionParams, const currentState = sbp('state/vuex/state') if (!currentState.currentGroupId) { const gId = Object.keys(currentState.contracts) - .find(cID => has(currentState[identityContractID].groups, cID)) + .find(cID => currentState[identityContractID].groups[cID]?.active) if (gId) { sbp('gi.app/group/switch', gId) diff --git a/frontend/model/contracts/chatroom.js b/frontend/model/contracts/chatroom.js index b7da62a41..ab5769f96 100644 --- a/frontend/model/contracts/chatroom.js +++ b/frontend/model/contracts/chatroom.js @@ -177,7 +177,7 @@ sbp('chelonia/defineContract', { if (!state.renderingContext) { if (!state.members) { - state['members'] = {} + state.members = {} } if (state.members[memberID]) { throw new GIChatroomAlreadyMemberError(`Can not join the chatroom which ${memberID} is already part of`) @@ -287,7 +287,6 @@ sbp('chelonia/defineContract', { } } - state.members[memberID].leftHeight = height delete state.members[memberID] if (!state.attributes) return diff --git a/frontend/model/contracts/group.js b/frontend/model/contracts/group.js index 69db64d91..5668a0d49 100644 --- a/frontend/model/contracts/group.js +++ b/frontend/model/contracts/group.js @@ -949,7 +949,7 @@ sbp('chelonia/defineContract', { } // subscribe to founder's IdentityContract & everyone else's - const profileIds = Object.keys(profiles).filter(cID => cID !== userID) + const profileIds = Object.keys(profiles) if (profileIds.length !== 0) { sbp('chelonia/contract/retain', profileIds).catch((e) => { console.error('Error while syncing other members\' contracts at inviteAccept', e) @@ -1220,14 +1220,19 @@ sbp('chelonia/defineContract', { 'gi.contracts/group/leaveChatRoom': { validate: actionRequireActiveMember(objectOf({ chatRoomID: stringMax(MAX_HASH_LEN, 'chatRoomID'), - memberID: optional(stringMax(MAX_HASH_LEN), 'memberID') + memberID: optional(stringMax(MAX_HASH_LEN), 'memberID'), + // `reference` is the height used in the corresponding join action + reference: numberRange(1, Number.MAX_SAFE_INTEGER) })), process ({ data, innerSigningContractID }, { state }) { if (!state.chatRooms[data.chatRoomID]) { throw new Error('Cannot leave a chatroom which isn\'t part of the group') } const memberID = data.memberID || innerSigningContractID - if (state.chatRooms[data.chatRoomID].members[memberID]?.status !== PROFILE_STATUS.ACTIVE) { + if ( + state.chatRooms[data.chatRoomID].members[memberID]?.status !== PROFILE_STATUS.ACTIVE || + state.chatRooms[data.chatRoomID].members[memberID].joinedHeight !== data.reference + ) { throw new Error('Cannot leave a chatroom that you\'re not part of') } removeGroupChatroomProfile(state, data.chatRoomID, memberID) @@ -1253,8 +1258,16 @@ sbp('chelonia/defineContract', { if (innerSigningContractID === identityContractID) { sbp('chelonia/queueInvocation', contractID, async () => { const state = await sbp('chelonia/contract/state', contractID) - if (state?.profiles?.[innerSigningContractID]?.status === PROFILE_STATUS.ACTIVE) { - return leaveChatRoomAction(contractID, state, data.chatRoomID, memberID, innerSigningContractID) + // In order to send the leaveChatRoom action, we need to be an active + // group member that's a chatroom member. + // In addition, we skip sending an action if there there have been + // other join or leave events by checking the `reference` value. + if ( + state?.profiles?.[innerSigningContractID]?.status === PROFILE_STATUS.ACTIVE && + state.chatRooms?.[data.chatRoomID]?.members[memberID]?.status === PROFILE_STATUS.REMOVED && + state.chatRooms[data.chatRoomID].members[memberID].joinedHeight === data.reference + ) { + await leaveChatRoomAction(contractID, state, data.chatRoomID, memberID, innerSigningContractID) } }).catch((e) => { console.error(`[gi.contracts/group/leaveChatRoom/sideEffect] Error for ${contractID}`, { contractID, data, error: e }) @@ -1267,6 +1280,47 @@ sbp('chelonia/defineContract', { groupContractID: contractID, chatRoomID: data.chatRoomID }) + + // The following queued invocation handles reference counting. It uses + // some transient (non-persistent) state to see whether the + // pairing `retain` to `release` was called. This situation could + // arise when syncing a group from scratch (e.g., from a new device) + // for chatrooms that have been joined and then left. In that case, + // there'll be no call to `retain` and therefore calling `release` + // is incorrect. + sbp('chelonia/queueInvocation', contractID, async () => { + // Has `retain` been skipped? + // Note: we check for 'skipped' instead of 'called' because + // side-effects cannot mutate state. The 'skipped' case can be + // checked without persistent storage, while the 'called' case + // would require some kind of persistent state (since the reference + // count is itself persisted) + const skippedRetain = sbp('okTurtles.data/get', `gi.contracts/group/chatroom-skipped-${contractID}-${data.chatRoomID}-${data.reference}`) + // There's no use of this transient value, so it's deleted to + // preserve memory + sbp('okTurtles.data/delete', `gi.contracts/group/chatroom-skipped-${contractID}-${data.chatRoomID}-${data.reference}`) + + const { identityContractID } = sbp('state/vuex/state').loggedIn + // New user session + if (memberID !== identityContractID) { + return + } + + const state = await sbp('chelonia/contract/state', contractID) + + if ( + state.chatRooms?.[data.chatRoomID]?.members[memberID]?.status === PROFILE_STATUS.ACTIVE || + state.chatRooms?.[data.chatRoomID]?.members[memberID]?.joinedHeight !== data.reference || + skippedRetain + ) { + return + } + + // Release chatroom if we're sure `retain` was called + sbp('chelonia/contract/release', data.chatRoomID).catch(e => { + console.error(`[gi.contracts/group/leaveChatRoom/sideEffect] Error releasing chatroom ${data.chatRoomID}`, e) + }) + }) } } }, @@ -1275,7 +1329,7 @@ sbp('chelonia/defineContract', { memberID: optional(stringMax(MAX_HASH_LEN, 'memberID')), chatRoomID: stringMax(MAX_HASH_LEN, 'chatRoomID') })), - process ({ data, innerSigningContractID }, { state }) { + process ({ data, height, innerSigningContractID }, { state }) { const memberID = data.memberID || innerSigningContractID const { chatRoomID } = data @@ -1300,16 +1354,16 @@ sbp('chelonia/defineContract', { // removed members, we would need to possibly fetch every chatroom // contract to account for chatrooms for which the removed member is // a part of. - state.chatRooms[chatRoomID].members[memberID] = { status: PROFILE_STATUS.ACTIVE } + state.chatRooms[chatRoomID].members[memberID] = { status: PROFILE_STATUS.ACTIVE, joinedHeight: height } }, - sideEffect ({ data, contractID, innerSigningContractID }) { + sideEffect ({ data, contractID, height, innerSigningContractID }) { const memberID = data.memberID || innerSigningContractID const { identityContractID } = sbp('state/vuex/state').loggedIn // If we added someone to the chatroom (including ourselves), we issue // the relevant action to the chatroom contract if (innerSigningContractID === identityContractID) { - sbp('chelonia/queueInvocation', contractID, () => sbp('gi.contracts/group/joinGroupChatrooms', contractID, data.chatRoomID, memberID)).catch((e) => { + sbp('chelonia/queueInvocation', contractID, () => sbp('gi.contracts/group/joinGroupChatrooms', contractID, data.chatRoomID, identityContractID, memberID, height)).catch((e) => { console.warn(`[gi.contracts/group/joinChatRoom/sideEffect] Error adding member to group chatroom for ${contractID}`, { e, data }) }) } else if (memberID === identityContractID) { @@ -1412,8 +1466,7 @@ sbp('chelonia/defineContract', { methods: { 'gi.contracts/group/_cleanup': ({ contractID, state }) => { // unsubscribe from other group members identity contract - const identityContractID = sbp('state/vuex/state').loggedIn?.identityContractID - const possiblyUselessContractIDs = Object.keys(state.profiles || {}).filter(cID => cID !== identityContractID) + const possiblyUselessContractIDs = Object.keys(state.profiles || {}) sbp('chelonia/contract/release', possiblyUselessContractIDs).catch(e => console.error('[gi.contracts/group/leaveGroup] Error calling release on all members', e) ) @@ -1585,14 +1638,21 @@ sbp('chelonia/defineContract', { }) } }, - 'gi.contracts/group/joinGroupChatrooms': async function (contractID, chatRoomID, memberID) { + 'gi.contracts/group/joinGroupChatrooms': async function (contractID, chatRoomID, originalActorID, memberID, height) { const state = await sbp('chelonia/contract/state', contractID) const actorID = sbp('state/vuex/state').loggedIn.identityContractID + // Session has changed + if (actorID !== originalActorID) { + return + } + if (state?.profiles?.[actorID]?.status !== PROFILE_STATUS.ACTIVE || state?.profiles?.[memberID]?.status !== PROFILE_STATUS.ACTIVE || - state.chatRooms?.[chatRoomID]?.members[memberID]?.status !== PROFILE_STATUS.ACTIVE + state?.chatRooms?.[chatRoomID]?.members[memberID]?.status !== PROFILE_STATUS.ACTIVE || + state?.chatRooms?.[chatRoomID]?.members[memberID]?.joinedHeight !== height ) { + sbp('okTurtles.data/set', `gi.contracts/group/chatroom-skipped-${contractID}-${chatRoomID}-${height}`, true) return } diff --git a/frontend/model/contracts/identity.js b/frontend/model/contracts/identity.js index 64696a2c9..a37e678f8 100644 --- a/frontend/model/contracts/identity.js +++ b/frontend/model/contracts/identity.js @@ -202,13 +202,13 @@ sbp('chelonia/defineContract', { }), async process ({ hash, data }, { state }) { const { groupContractID, inviteSecret } = data - if (has(state.groups, groupContractID)) { + if (has(state.groups, groupContractID) && state.groups[groupContractID].active) { throw new Error(`Cannot join already joined group ${groupContractID}`) } const inviteSecretId = await sbp('chelonia/crypto/keyId', new Secret(inviteSecret)) - state.groups[groupContractID] = { hash, inviteSecretId } + state.groups[groupContractID] = { hash, inviteSecretId, active: true } }, async sideEffect ({ hash, data, contractID }, { state }) { const { groupContractID, inviteSecret } = data @@ -226,17 +226,14 @@ sbp('chelonia/defineContract', { } // If we've left the group, return - if (!has(state.groups, groupContractID)) { + // If the hash doesn't match (could happen after re-joining), return + if (!has(state.groups, groupContractID) || !state.groups[groupContractID].active || state.groups[groupContractID].hash !== hash) { + sbp('okTurtles.data/set', `gi.contracts/identity/group-skipped-${groupContractID}-${hash}`, true) return } const inviteSecretId = sbp('chelonia/crypto/keyId', new Secret(inviteSecret)) - // If the hash doesn't match (could happen after re-joining), return - if (state.groups[groupContractID].hash !== hash) { - return - } - return inviteSecretId }).then(async (inviteSecretId) => { // Calling 'gi.actions/group/join' here _after_ queueInvoication @@ -277,17 +274,18 @@ sbp('chelonia/defineContract', { reference: string }), process ({ data }, { state }) { - const { groupContractID } = data + const { groupContractID, reference } = data - if (!has(state.groups, groupContractID)) { + if (!has(state.groups, groupContractID) || !state.groups[groupContractID].active) { throw new Error(`Cannot leave group which hasn't been joined ${groupContractID}`) } - if (state.groups[groupContractID].hash !== data.reference) { + if (state.groups[groupContractID].hash !== reference) { throw new Error(`Cannot leave group ${groupContractID} because the reference hash does not match the latest`) } - delete state.groups[groupContractID] + state.groups[groupContractID].active = false + delete state.groups[groupContractID].inviteSecret }, sideEffect ({ data, contractID }) { sbp('chelonia/queueInvocation', contractID, async () => { @@ -298,10 +296,19 @@ sbp('chelonia/defineContract', { return } - const { groupContractID } = data + const { groupContractID, reference } = data + + // For an explanation of skippedRetain, see the chatroom contract. + // TL;DR: This ensures that the reference count is correct when a + // group has been joined and then left and the state is being + // synced from scratch. + const skippedRetain = sbp('okTurtles.data/get', `gi.contracts/identity/group-skipped-${groupContractID}-${reference}`) + // No use for this value beyond this point. + sbp('okTurtles.data/delete', `gi.contracts/identity/group-skipped-${groupContractID}-${reference}`) // If we've re-joined since, return - if (has(state.groups, groupContractID)) { + // If the hash doesn't match (could happen after re-joining), return + if (!has(state.groups, groupContractID) || state.groups[groupContractID].active || state.groups[groupContractID].hash !== reference) { return } @@ -312,9 +319,11 @@ sbp('chelonia/defineContract', { console.warn(`[gi.contracts/identity/leaveGroup/sideEffect] Error removing ourselves from group contract ${data.groupContractID}`, e) }) - sbp('chelonia/contract/release', data.groupContractID).catch((e) => { - console.error('[gi.contracts/identity/leaveGroup/sideEffect] Error calling release', e) - }) + if (!skippedRetain) { + sbp('chelonia/contract/release', data.groupContractID).catch((e) => { + console.error('[gi.contracts/identity/leaveGroup/sideEffect] Error calling release', e) + }) + } // Remove last logged in information if (sbp('state/vuex/state').lastLoggedIn?.[contractID]) { diff --git a/frontend/model/contracts/shared/functions.js b/frontend/model/contracts/shared/functions.js index 3ec9c1517..86d7d6794 100644 --- a/frontend/model/contracts/shared/functions.js +++ b/frontend/model/contracts/shared/functions.js @@ -170,10 +170,6 @@ export async function leaveChatRoom (contractID: string) { // NOTE: The contract that keeps track of chatrooms should now call `/release` // This would be the group contract (for group chatrooms) or the identity // contract (for DMs). - - sbp('chelonia/contract/release', contractID).catch(e => { - console.error(`[gi.contracts/chatroom/leave/sideEffect] Error releasing chatroom ${contractID}`, e) - }) } export function findMessageIdx (hash: string, messages: Array = []): number { diff --git a/frontend/model/state.js b/frontend/model/state.js index c51977c84..c68335cae 100644 --- a/frontend/model/state.js +++ b/frontend/model/state.js @@ -434,8 +434,10 @@ const getters = { // due to the same flow issue as https://github.com/facebook/flow/issues/5838 // we return event pending groups that we haven't finished joining so that we are not stuck // on the /pending-approval page if we are part of another working group already - return Object.keys(groups) - .map(contractID => ({ groupName: state[contractID]?.settings?.groupName || L('Pending'), contractID })) + return Object.entries(groups) + // $FlowFixMe[incompatible-use] + .filter(([, { active }]) => active) + .map(([contractID]) => ({ groupName: state[contractID]?.settings?.groupName || L('Pending'), contractID })) }, profilesByGroup (state, getters) { return groupID => { diff --git a/frontend/views/pages/Join.vue b/frontend/views/pages/Join.vue index dbf45299d..aa4c94fc5 100644 --- a/frontend/views/pages/Join.vue +++ b/frontend/views/pages/Join.vue @@ -188,8 +188,8 @@ export default ({ }, checkAlreadyJoinedGroup (targetGroupId) { if (this.ourIdentityContractId) { - const myGroupIds = Object.keys(this.$store.state[this.ourIdentityContractId]?.groups || {}) - return myGroupIds.includes(targetGroupId) + const myGroups = this.$store.state[this.ourIdentityContractId]?.groups || {} + return !!myGroups[targetGroupId]?.active } else return false }, goToDashboard (toGroupId) { diff --git a/shared/domains/chelonia/chelonia.js b/shared/domains/chelonia/chelonia.js index 4dd2e2c1b..6855df6b9 100644 --- a/shared/domains/chelonia/chelonia.js +++ b/shared/domains/chelonia/chelonia.js @@ -909,6 +909,10 @@ export default (sbp('sbp/selectors/register', { const current = rootState.contracts[id].references if (current === 0) { console.error('[chelonia/contract/release] Invalid negative reference count for', id) + if (process.env.CI) { + // If running in CI, force tests to fail + Promise.reject(new Error('Invalid negative reference count: ' + id)) + } throw new Error('Invalid negative reference count') } if (current <= 1) { @@ -918,6 +922,10 @@ export default (sbp('sbp/selectors/register', { } } else { console.error('[chelonia/contract/release] Invalid negative reference count for', id) + if (process.env.CI) { + // If running in CI, force tests to fail + Promise.reject(new Error('Invalid negative reference count: ' + id)) + } throw new Error('Invalid negative reference count') } }) @@ -932,6 +940,10 @@ export default (sbp('sbp/selectors/register', { } } else { console.error('[chelonia/contract/release] Invalid negative ephemeral reference count for', id) + if (process.env.CI) { + // If running in CI, force tests to fail + Promise.reject(new Error('Invalid negative ephemeral reference count: ' + id)) + } throw new Error('Invalid negative ephemeral reference count') } }) diff --git a/shared/domains/chelonia/internals.js b/shared/domains/chelonia/internals.js index 75b5ff30d..4b4326dbd 100644 --- a/shared/domains/chelonia/internals.js +++ b/shared/domains/chelonia/internals.js @@ -764,7 +764,6 @@ export default (sbp('sbp/selectors/register', { // current height, then the contract must be resynced. const mustResync = !!(newestEncryptionKeyHeight < cheloniaState.contracts[v.contractID]?.height) - // TODO: Handle foreign keys too if (mustResync) { if (!has(targetState, '_volatile')) config.reactiveSet(targetState, '_volatile', Object.create(null)) config.reactiveSet(targetState._volatile, 'dirty', true) @@ -774,6 +773,45 @@ export default (sbp('sbp/selectors/register', { return } + // Mark contracts that have foreign keys that have been received + // as dirty + // First, we group watched keys by key and contracts + const keyDict = Object.create(null) + targetState._volatile?.watch?.forEach(([keyName, contractID]) => { + if (!keyDict[keyName]) { + keyDict[keyName] = [contractID] + return + } + keyDict[keyName].push(contractID) + }) + // Then, see which of those contracts need to be updated + // $FlowFixMe[incompatible-call] + const contractIdsToUpdate = Array.from(new Set(Object.entries(keyDict).flatMap(([keyName, contractIDs]) => { + const keyId = findKeyIdByName(targetState, keyName) + if ( + // Does the key exist? (i.e., is it a current key) + keyId && + // Is it an encryption key? (signing keys don't build up a + // potentially invalid state because the private key isn't + // required for validation; however, missing encryption keys + // prevent message processing) + targetState._vm.authorizedKeys[keyId].purpose.includes('enc') && + // Is this a newly set key? (avoid re-syncing contracts that + // haven't been affected by the `OP_KEY_SHARE`) + targetState._vm.authorizedKeys[keyId]._notBeforeHeight >= newestEncryptionKeyHeight + ) { + return contractIDs + } + return [] + }))) + // Mark these contracts as dirty + contractIdsToUpdate.forEach((contractID) => { + const targetState = cheloniaState[contractID] + if (!targetState) return + if (!has(targetState, '_volatile')) config.reactiveSet(targetState, '_volatile', Object.create(null)) + config.reactiveSet(targetState._volatile, 'dirty', true) + }) + // Since we have received new keys, the current contract state might be wrong, so we need to remove the contract and resync // Note: The following may be problematic when several tabs are open // sharing the same state. This is more of a general issue in this @@ -781,7 +819,18 @@ export default (sbp('sbp/selectors/register', { if (self.subscriptionSet.has(v.contractID)) { const resync = sbp('chelonia/private/queueEvent', v.contractID, [ 'chelonia/private/in/syncContract', v.contractID - ]).catch((e) => { + ]).then(() => { + // Now, if we're subscribed to any of the contracts that were + // marked as dirty, re-sync them + sbp('chelonia/contract/sync', + contractIdsToUpdate.filter((contractID) => { + return self.subscriptionSet.has(contractID) + }), + { force: true, resync: true } + ).catch(e => { + console.error('[chelonia] Error resyncing contracts with foreign key references after key rotation', e) + }) + }).catch((e) => { console.error(`[chelonia] Error during sync for ${v.contractID}during OP_KEY_SHARE for ${contractID}`) if (v.contractID === contractID) { throw e diff --git a/shared/domains/chelonia/utils.js b/shared/domains/chelonia/utils.js index 68e62f80a..66ca27199 100644 --- a/shared/domains/chelonia/utils.js +++ b/shared/domains/chelonia/utils.js @@ -546,7 +546,7 @@ export const recreateEvent = (entry: GIMessage, state: Object, contractsState: O export const getContractIDfromKeyId = (contractID: string, signingKeyId: ?string, state: Object): ?string => { if (!signingKeyId) return - return signingKeyId && state._vm.authorizedKeys[signingKeyId].foreignKey + return signingKeyId && state._vm.authorizedKeys[signingKeyId]?.foreignKey ? new URL(state._vm.authorizedKeys[signingKeyId].foreignKey).pathname : contractID } From 007978e184f3a26686164aed38a1e4932904d384 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ricardo=20Iv=C3=A1n=20Vieitez=20Parra?= <3857362+corrideat@users.noreply.github.com> Date: Thu, 15 Aug 2024 17:39:51 +0000 Subject: [PATCH 02/24] Incorporate feedback --- frontend/controller/actions/group.js | 4 ++-- frontend/controller/actions/identity.js | 4 ++-- frontend/controller/app/group.js | 2 +- frontend/controller/app/identity.js | 2 +- frontend/model/contracts/group.js | 20 ++++++++++++-------- frontend/model/contracts/identity.js | 12 ++++++------ frontend/model/state.js | 2 +- frontend/views/pages/Join.vue | 2 +- 8 files changed, 26 insertions(+), 22 deletions(-) diff --git a/frontend/controller/actions/group.js b/frontend/controller/actions/group.js index 1899b59cc..e0c4b39f9 100644 --- a/frontend/controller/actions/group.js +++ b/frontend/controller/actions/group.js @@ -899,7 +899,7 @@ export default (sbp('sbp/selectors/register', { ...encryptedAction('gi.actions/group/leaveChatRoom', L('Failed to leave chat channel.'), async (sendMessage, params) => { const state = await sbp('chelonia/contract/state', params.contractID) const memberID = params.data.memberID || sbp('chelonia/rootState').loggedIn.identityContractID - const reference = state.chatRooms[params.data.chatRoomID].members[memberID].joinedHeight + const joinedHeight = state.chatRooms[params.data.chatRoomID].members[memberID].joinedHeight // For more efficient and correct processing, augment the leaveChatRoom // action with the height of the join action. This helps prevent reduce @@ -909,7 +909,7 @@ export default (sbp('sbp/selectors/register', { ...params, data: { ...params.data, - reference + joinedHeight } }) }), diff --git a/frontend/controller/actions/identity.js b/frontend/controller/actions/identity.js index 0c66d8f6f..e83432392 100644 --- a/frontend/controller/actions/identity.js +++ b/frontend/controller/actions/identity.js @@ -307,7 +307,7 @@ export default (sbp('sbp/selectors/register', { // that have been joined in the past) Object.entries(cheloniaState[identityContractID].groups) // $FlowFixMe[incompatible-use] - .filter(([, { active }]) => active) + .filter(([, { hasLeft }]) => !hasLeft) .forEach(([cId]) => { // We send this action only for groups we have fully joined (i.e., // accepted an invite and added our profile) @@ -425,7 +425,7 @@ export default (sbp('sbp/selectors/register', { const rootState = sbp('state/vuex/state') const state = rootState[contractID] // TODO: Also share PEK with DMs - await Promise.all(Object.keys(state.groups || {}).filter(groupID => state.groups[groupID].active && !!rootState.contracts[groupID]).map(async groupID => { + await Promise.all(Object.keys(state.groups || {}).filter(groupID => !state.groups[groupID].hasLeft && !!rootState.contracts[groupID]).map(async groupID => { const CEKid = await sbp('chelonia/contract/currentKeyIdByName', groupID, 'cek') const CSKid = await sbp('chelonia/contract/currentKeyIdByName', groupID, 'csk') diff --git a/frontend/controller/app/group.js b/frontend/controller/app/group.js index e29fd89db..75a861272 100644 --- a/frontend/controller/app/group.js +++ b/frontend/controller/app/group.js @@ -30,7 +30,7 @@ sbp('okTurtles.events/on', LEFT_GROUP, ({ identityContractID, groupContractID }) if (!currentGroupId || currentGroupId === groupContractID) { const groupIdToSwitch = Object.entries(state.groups) // $FlowFixMe[incompatible-use] - .map(([cID, { active }]) => active && cID) + .map(([cID, { hasLeft }]) => !hasLeft && cID) .filter(cID => cID && cID !== groupContractID ).sort(cID => diff --git a/frontend/controller/app/identity.js b/frontend/controller/app/identity.js index f20e606cb..a61465bf5 100644 --- a/frontend/controller/app/identity.js +++ b/frontend/controller/app/identity.js @@ -121,7 +121,7 @@ sbp('okTurtles.events/on', LOGIN, async ({ identityContractID, encryptionParams, const currentState = sbp('state/vuex/state') if (!currentState.currentGroupId) { const gId = Object.keys(currentState.contracts) - .find(cID => currentState[identityContractID].groups[cID]?.active) + .find(cID => currentState[identityContractID].groups[cID] && !currentState[identityContractID].groups[cID].hasLeft) if (gId) { sbp('gi.app/group/switch', gId) diff --git a/frontend/model/contracts/group.js b/frontend/model/contracts/group.js index 5668a0d49..03c60bbde 100644 --- a/frontend/model/contracts/group.js +++ b/frontend/model/contracts/group.js @@ -1221,8 +1221,8 @@ sbp('chelonia/defineContract', { validate: actionRequireActiveMember(objectOf({ chatRoomID: stringMax(MAX_HASH_LEN, 'chatRoomID'), memberID: optional(stringMax(MAX_HASH_LEN), 'memberID'), - // `reference` is the height used in the corresponding join action - reference: numberRange(1, Number.MAX_SAFE_INTEGER) + // `joinedHeight` is the height used in the corresponding join action + joinedHeight: numberRange(1, Number.MAX_SAFE_INTEGER) })), process ({ data, innerSigningContractID }, { state }) { if (!state.chatRooms[data.chatRoomID]) { @@ -1231,7 +1231,7 @@ sbp('chelonia/defineContract', { const memberID = data.memberID || innerSigningContractID if ( state.chatRooms[data.chatRoomID].members[memberID]?.status !== PROFILE_STATUS.ACTIVE || - state.chatRooms[data.chatRoomID].members[memberID].joinedHeight !== data.reference + state.chatRooms[data.chatRoomID].members[memberID].joinedHeight !== data.joinedHeight ) { throw new Error('Cannot leave a chatroom that you\'re not part of') } @@ -1261,11 +1261,11 @@ sbp('chelonia/defineContract', { // In order to send the leaveChatRoom action, we need to be an active // group member that's a chatroom member. // In addition, we skip sending an action if there there have been - // other join or leave events by checking the `reference` value. + // other join or leave events by checking the `joinedHeight` value. if ( state?.profiles?.[innerSigningContractID]?.status === PROFILE_STATUS.ACTIVE && state.chatRooms?.[data.chatRoomID]?.members[memberID]?.status === PROFILE_STATUS.REMOVED && - state.chatRooms[data.chatRoomID].members[memberID].joinedHeight === data.reference + state.chatRooms[data.chatRoomID].members[memberID].joinedHeight === data.joinedHeight ) { await leaveChatRoomAction(contractID, state, data.chatRoomID, memberID, innerSigningContractID) } @@ -1295,10 +1295,10 @@ sbp('chelonia/defineContract', { // checked without persistent storage, while the 'called' case // would require some kind of persistent state (since the reference // count is itself persisted) - const skippedRetain = sbp('okTurtles.data/get', `gi.contracts/group/chatroom-skipped-${contractID}-${data.chatRoomID}-${data.reference}`) + const skippedRetain = sbp('okTurtles.data/get', `gi.contracts/group/chatroom-skipped-${contractID}-${data.chatRoomID}-${data.joinedHeight}`) // There's no use of this transient value, so it's deleted to // preserve memory - sbp('okTurtles.data/delete', `gi.contracts/group/chatroom-skipped-${contractID}-${data.chatRoomID}-${data.reference}`) + sbp('okTurtles.data/delete', `gi.contracts/group/chatroom-skipped-${contractID}-${data.chatRoomID}-${data.joinedHeight}`) const { identityContractID } = sbp('state/vuex/state').loggedIn // New user session @@ -1310,7 +1310,7 @@ sbp('chelonia/defineContract', { if ( state.chatRooms?.[data.chatRoomID]?.members[memberID]?.status === PROFILE_STATUS.ACTIVE || - state.chatRooms?.[data.chatRoomID]?.members[memberID]?.joinedHeight !== data.reference || + state.chatRooms?.[data.chatRoomID]?.members[memberID]?.joinedHeight !== data.joinedHeight || skippedRetain ) { return @@ -1466,6 +1466,10 @@ sbp('chelonia/defineContract', { methods: { 'gi.contracts/group/_cleanup': ({ contractID, state }) => { // unsubscribe from other group members identity contract + // NOTE: it is safe to decrease the reference count on our own identity + // contract because it has an extra retain when joining a group + // (in the side-effect for gi.contracts/group/inviteAccept), which is + // the one being decremented const possiblyUselessContractIDs = Object.keys(state.profiles || {}) sbp('chelonia/contract/release', possiblyUselessContractIDs).catch(e => console.error('[gi.contracts/group/leaveGroup] Error calling release on all members', e) diff --git a/frontend/model/contracts/identity.js b/frontend/model/contracts/identity.js index a37e678f8..631215d82 100644 --- a/frontend/model/contracts/identity.js +++ b/frontend/model/contracts/identity.js @@ -202,13 +202,13 @@ sbp('chelonia/defineContract', { }), async process ({ hash, data }, { state }) { const { groupContractID, inviteSecret } = data - if (has(state.groups, groupContractID) && state.groups[groupContractID].active) { + if (has(state.groups, groupContractID) && !state.groups[groupContractID].hasLeft) { throw new Error(`Cannot join already joined group ${groupContractID}`) } const inviteSecretId = await sbp('chelonia/crypto/keyId', new Secret(inviteSecret)) - state.groups[groupContractID] = { hash, inviteSecretId, active: true } + state.groups[groupContractID] = { hash, inviteSecretId } }, async sideEffect ({ hash, data, contractID }, { state }) { const { groupContractID, inviteSecret } = data @@ -227,7 +227,7 @@ sbp('chelonia/defineContract', { // If we've left the group, return // If the hash doesn't match (could happen after re-joining), return - if (!has(state.groups, groupContractID) || !state.groups[groupContractID].active || state.groups[groupContractID].hash !== hash) { + if (!has(state.groups, groupContractID) || state.groups[groupContractID].hasLeft || state.groups[groupContractID].hash !== hash) { sbp('okTurtles.data/set', `gi.contracts/identity/group-skipped-${groupContractID}-${hash}`, true) return } @@ -276,7 +276,7 @@ sbp('chelonia/defineContract', { process ({ data }, { state }) { const { groupContractID, reference } = data - if (!has(state.groups, groupContractID) || !state.groups[groupContractID].active) { + if (!has(state.groups, groupContractID) || state.groups[groupContractID].hasLeft) { throw new Error(`Cannot leave group which hasn't been joined ${groupContractID}`) } @@ -284,7 +284,7 @@ sbp('chelonia/defineContract', { throw new Error(`Cannot leave group ${groupContractID} because the reference hash does not match the latest`) } - state.groups[groupContractID].active = false + state.groups[groupContractID].hasLeft = true delete state.groups[groupContractID].inviteSecret }, sideEffect ({ data, contractID }) { @@ -308,7 +308,7 @@ sbp('chelonia/defineContract', { // If we've re-joined since, return // If the hash doesn't match (could happen after re-joining), return - if (!has(state.groups, groupContractID) || state.groups[groupContractID].active || state.groups[groupContractID].hash !== reference) { + if (!has(state.groups, groupContractID) || !state.groups[groupContractID].hasLeft || state.groups[groupContractID].hash !== reference) { return } diff --git a/frontend/model/state.js b/frontend/model/state.js index c68335cae..07ba4831d 100644 --- a/frontend/model/state.js +++ b/frontend/model/state.js @@ -436,7 +436,7 @@ const getters = { // on the /pending-approval page if we are part of another working group already return Object.entries(groups) // $FlowFixMe[incompatible-use] - .filter(([, { active }]) => active) + .filter(([, { hasLeft }]) => !hasLeft) .map(([contractID]) => ({ groupName: state[contractID]?.settings?.groupName || L('Pending'), contractID })) }, profilesByGroup (state, getters) { diff --git a/frontend/views/pages/Join.vue b/frontend/views/pages/Join.vue index aa4c94fc5..c06ef9098 100644 --- a/frontend/views/pages/Join.vue +++ b/frontend/views/pages/Join.vue @@ -189,7 +189,7 @@ export default ({ checkAlreadyJoinedGroup (targetGroupId) { if (this.ourIdentityContractId) { const myGroups = this.$store.state[this.ourIdentityContractId]?.groups || {} - return !!myGroups[targetGroupId]?.active + return myGroups[targetGroupId] && !myGroups[targetGroupId]?.hasLeft } else return false }, goToDashboard (toGroupId) { From 76e13b8ecaf44fe223ac80eaa54219f579bacc30 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ricardo=20Iv=C3=A1n=20Vieitez=20Parra?= <3857362+corrideat@users.noreply.github.com> Date: Thu, 15 Aug 2024 18:13:49 +0000 Subject: [PATCH 03/24] empty commit From 83189db15565e9edb596cbf9afd3db4a74014fa4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ricardo=20Iv=C3=A1n=20Vieitez=20Parra?= <3857362+corrideat@users.noreply.github.com> Date: Fri, 23 Aug 2024 18:51:36 +0000 Subject: [PATCH 04/24] Feedback, improvements & backwards-compatibility --- frontend/controller/actions/group.js | 1 + frontend/controller/app/identity.js | 3 +- frontend/model/contracts/chatroom.js | 15 +--- frontend/model/contracts/group.js | 94 +++++++++++++------- frontend/model/contracts/identity.js | 7 +- frontend/model/contracts/shared/constants.js | 3 +- frontend/model/contracts/shared/functions.js | 8 +- frontend/model/state.js | 22 +++++ 8 files changed, 105 insertions(+), 48 deletions(-) diff --git a/frontend/controller/actions/group.js b/frontend/controller/actions/group.js index e0c4b39f9..e13bec8f4 100644 --- a/frontend/controller/actions/group.js +++ b/frontend/controller/actions/group.js @@ -1030,6 +1030,7 @@ export default (sbp('sbp/selectors/register', { ...encryptedAction('gi.actions/group/updateSettings', L('Failed to update group settings.')), ...encryptedAction('gi.actions/group/updateAllVotingRules', (params, e) => L('Failed to update voting rules. {codeError}', { codeError: e.message })), ...encryptedAction('gi.actions/group/updateDistributionDate', L('Failed to update group distribution date.')), + ...encryptedAction('gi.actions/group/upgradeFrom1.0.6', L('Failed to upgrade from version 1.0.6')), ...((process.env.NODE_ENV === 'development' || process.env.CI) && { ...encryptedAction('gi.actions/group/forceDistributionDate', L('Failed to force distribution date.')) }) diff --git a/frontend/controller/app/identity.js b/frontend/controller/app/identity.js index a61465bf5..083eb61d1 100644 --- a/frontend/controller/app/identity.js +++ b/frontend/controller/app/identity.js @@ -109,7 +109,8 @@ sbp('okTurtles.events/on', LOGIN, async ({ identityContractID, encryptionParams, if (cheloniaState.namespaceLookups) { Vue.set(state, 'namespaceLookups', cheloniaState.namespaceLookups) } - // End exclude contracts + // End exclude contracts + sbp('state/vuex/postUpgradeVerification', state) } if (encryptionParams) { diff --git a/frontend/model/contracts/chatroom.js b/frontend/model/contracts/chatroom.js index ab5769f96..75561b49b 100644 --- a/frontend/model/contracts/chatroom.js +++ b/frontend/model/contracts/chatroom.js @@ -309,14 +309,14 @@ sbp('chelonia/defineContract', { innerSigningContractID: !isKicked ? memberID : innerSigningContractID })) }, - async sideEffect ({ data, hash, contractID, meta, innerSigningContractID }) { + async sideEffect ({ data, hash, contractID, meta, innerSigningContractID }, { state }) { const memberID = data.memberID || innerSigningContractID const itsMe = memberID === sbp('state/vuex/state').loggedIn.identityContractID // NOTE: we don't add this 'if' statement in the queuedInvocation // because these should not be running while rejoining if (itsMe) { - await leaveChatRoom(contractID).catch(e => { + await leaveChatRoom(contractID, state).catch(e => { console.error('[gi.contracts/chatroom/leave] Error at leaveChatRoom', e) }) } @@ -350,11 +350,11 @@ sbp('chelonia/defineContract', { delete state.members[memberID] } }, - async sideEffect ({ contractID }) { + async sideEffect ({ contractID }, { state }) { // NOTE: make sure *not* to await on this, since that can cause // a potential deadlock. See same warning in sideEffect for // 'gi.contracts/group/removeMember' - await leaveChatRoom(contractID) + await leaveChatRoom(contractID, state) } }, 'gi.contracts/chatroom/addMessage': { @@ -721,13 +721,6 @@ sbp('chelonia/defineContract', { } }, methods: { - 'gi.contracts/chatroom/_cleanup': ({ contractID, state }) => { - if (state) { - sbp('chelonia/contract/release', Object.keys(state.members)).catch(e => { - console.error(`[gi.contracts/chatroom/_cleanup] Error releasing chatroom members for ${contractID}`, Object.keys(state.members), e) - }) - } - }, 'gi.contracts/chatroom/rotateKeys': (contractID, state) => { if (!state._volatile) state['_volatile'] = Object.create(null) if (!state._volatile.pendingKeyRevocations) state._volatile['pendingKeyRevocations'] = Object.create(null) diff --git a/frontend/model/contracts/group.js b/frontend/model/contracts/group.js index 03c60bbde..87cc10c92 100644 --- a/frontend/model/contracts/group.js +++ b/frontend/model/contracts/group.js @@ -176,7 +176,7 @@ function updateAdjustedDistribution ({ period, getters }) { } } -function memberLeaves ({ memberID, dateLeft, heightLeft }, { contractID, meta, state, getters }) { +function memberLeaves ({ memberID, dateLeft, heightLeft, ourselvesLeaving }, { contractID, meta, state, getters }) { if (!state.profiles[memberID] || state.profiles[memberID].status !== PROFILE_STATUS.ACTIVE) { throw new Error(`[gi.contracts/group memberLeaves] Can't remove non-exisiting member ${memberID}`) } @@ -188,7 +188,7 @@ function memberLeaves ({ memberID, dateLeft, heightLeft }, { contractID, meta, s updateCurrentDistribution({ contractID, meta, state, getters }) Object.keys(state.chatRooms).forEach((chatroomID) => { - removeGroupChatroomProfile(state, chatroomID, memberID) + removeGroupChatroomProfile(state, chatroomID, memberID, ourselvesLeaving) }) // When a member is leaving, we need to mark the CSK and the CEK as needing @@ -301,13 +301,13 @@ function updateGroupStreaks ({ state, getters }) { } } -const removeGroupChatroomProfile = (state, chatRoomID, member) => { - state.chatRooms[chatRoomID]['members'] = +const removeGroupChatroomProfile = (state, chatRoomID, member, ourselvesLeaving) => { + state.chatRooms[chatRoomID].members = Object.fromEntries( Object.entries(state.chatRooms[chatRoomID].members) .map(([memberKey, profile]) => { if (memberKey === member && (profile: any)?.status === PROFILE_STATUS.ACTIVE) { - return [memberKey, { ...profile, status: PROFILE_STATUS.REMOVED }] + return [memberKey, { ...profile, status: ourselvesLeaving ? PROFILE_STATUS.LEFT_GROUP : PROFILE_STATUS.REMOVED }] } return [memberKey, profile] }) @@ -397,6 +397,30 @@ export const actionRequireActiveMember = (next: Function): Function => (data, pr return next(data, props) } +const hasRetainBeenSkippedOnChatRoom = (state, contractID, chatRoomID, memberID, joinedHeight) => { + // Has `retain` been skipped? + // Note: we check for 'skipped' instead of 'called' because + // side-effects cannot mutate state. The 'skipped' case can be + // checked without persistent storage, while the 'called' case + // would require some kind of persistent state (since the reference + // count is itself persisted) + const skippedRetainKey = `gi.contracts/group/chatroom-skipped-${contractID}-${chatRoomID}-${joinedHeight}` + const skippedRetain = sbp('okTurtles.data/get', skippedRetainKey) + // There's no use of this transient value, so it's deleted to + // preserve memory + sbp('okTurtles.data/delete', skippedRetainKey) + + if ( + state.chatRooms?.[chatRoomID]?.members[memberID]?.status === PROFILE_STATUS.ACTIVE || + state.chatRooms?.[chatRoomID]?.members[memberID]?.joinedHeight !== joinedHeight || + skippedRetain + ) { + return true + } + + return false +} + export const GIGroupAlreadyJoinedError: typeof Error = ChelErrorGenerator('GIGroupAlreadyJoinedError') export const GIGroupNotJoinedError: typeof Error = ChelErrorGenerator('GIGroupNotJoinedError') @@ -856,8 +880,10 @@ sbp('chelonia/defineContract', { } }), process ({ data, meta, contractID, height, innerSigningContractID }, { state, getters }) { + const memberID = data.memberID || innerSigningContractID + const identityContractID = sbp('state/vuex/state').loggedIn?.identityContractID memberLeaves( - { memberID: data.memberID || innerSigningContractID, dateLeft: meta.createdDate, heightLeft: height }, + { memberID, dateLeft: meta.createdDate, heightLeft: height, ourselvesLeaving: memberID === identityContractID }, { contractID, meta, state, getters } ) }, @@ -949,7 +975,7 @@ sbp('chelonia/defineContract', { } // subscribe to founder's IdentityContract & everyone else's - const profileIds = Object.keys(profiles) + const profileIds = Object.keys(profiles).filter(cID => cID !== userID) if (profileIds.length !== 0) { sbp('chelonia/contract/retain', profileIds).catch((e) => { console.error('Error while syncing other members\' contracts at inviteAccept', e) @@ -1289,32 +1315,16 @@ sbp('chelonia/defineContract', { // there'll be no call to `retain` and therefore calling `release` // is incorrect. sbp('chelonia/queueInvocation', contractID, async () => { - // Has `retain` been skipped? - // Note: we check for 'skipped' instead of 'called' because - // side-effects cannot mutate state. The 'skipped' case can be - // checked without persistent storage, while the 'called' case - // would require some kind of persistent state (since the reference - // count is itself persisted) - const skippedRetain = sbp('okTurtles.data/get', `gi.contracts/group/chatroom-skipped-${contractID}-${data.chatRoomID}-${data.joinedHeight}`) - // There's no use of this transient value, so it's deleted to - // preserve memory - sbp('okTurtles.data/delete', `gi.contracts/group/chatroom-skipped-${contractID}-${data.chatRoomID}-${data.joinedHeight}`) - + // Re-grab `identityContractID` as the active user session could + // have changed. For example, Cypress does things very quickly. const { identityContractID } = sbp('state/vuex/state').loggedIn - // New user session + // New user session? The rest of the code isn't relevant if (memberID !== identityContractID) { return } const state = await sbp('chelonia/contract/state', contractID) - - if ( - state.chatRooms?.[data.chatRoomID]?.members[memberID]?.status === PROFILE_STATUS.ACTIVE || - state.chatRooms?.[data.chatRoomID]?.members[memberID]?.joinedHeight !== data.joinedHeight || - skippedRetain - ) { - return - } + if (hasRetainBeenSkippedOnChatRoom(state, contractID, data.chatRoomID, memberID, data.joinedHeight)) return // Release chatroom if we're sure `retain` was called sbp('chelonia/contract/release', data.chatRoomID).catch(e => { @@ -1424,6 +1434,23 @@ sbp('chelonia/defineContract', { } } }, + 'gi.contracts/group/upgradeFrom1.0.6': { + validate: actionRequireActiveMember(optional), + process ({ height }, { state }) { + let changed = false + Object.values(state.chatRooms).forEach((chatroom) => { + Object.values(chatroom.members).forEach(member => { + if (member.status === PROFILE_STATUS.ACTIVE && member.joinedHeight == null) { + member.joinedHeight = height + changed = true + } + }) + }) + if (!changed) { + throw new Error('[gi.contracts/group/upgradeFrom1.0.6/process] Invalid or duplicate upgrade action') + } + } + }, ...((process.env.NODE_ENV === 'development' || process.env.CI) && { 'gi.contracts/group/forceDistributionDate': { validate: optional, @@ -1470,10 +1497,6 @@ sbp('chelonia/defineContract', { // contract because it has an extra retain when joining a group // (in the side-effect for gi.contracts/group/inviteAccept), which is // the one being decremented - const possiblyUselessContractIDs = Object.keys(state.profiles || {}) - sbp('chelonia/contract/release', possiblyUselessContractIDs).catch(e => - console.error('[gi.contracts/group/leaveGroup] Error calling release on all members', e) - ) // NOTE: should remove archived data from IndexedStorage regarding the current group (proposals, payments) Promise.all([ @@ -1769,6 +1792,15 @@ sbp('chelonia/defineContract', { }) if (memberID === identityContractID) { + const possiblyUselessContractIDs = [ + ...Object.keys(state.profiles || {}).filter(cID => cID !== identityContractID), + ...Object.keys(state.chatRooms || {}).filter((chatRoomID) => state.chatRooms[chatRoomID].members[identityContractID]?.status === PROFILE_STATUS.LEFT_GROUP && !hasRetainBeenSkippedOnChatRoom(state, contractID, chatRoomID, memberID, state.chatRooms[chatRoomID].members[identityContractID].joinedHeight)) + ] + console.error('@@@@@XXX', memberID, identityContractID, possiblyUselessContractIDs) + sbp('chelonia/contract/release', possiblyUselessContractIDs).catch(e => + console.error('[gi.contracts/group/leaveGroup] Error calling release on all members', e) + ) + sbp('gi.actions/identity/leaveGroup', { contractID: identityContractID, data: { diff --git a/frontend/model/contracts/identity.js b/frontend/model/contracts/identity.js index 631215d82..74bca1263 100644 --- a/frontend/model/contracts/identity.js +++ b/frontend/model/contracts/identity.js @@ -298,13 +298,14 @@ sbp('chelonia/defineContract', { const { groupContractID, reference } = data - // For an explanation of skippedRetain, see the chatroom contract. + // For an explanation of skippedRetain, see the group contract. // TL;DR: This ensures that the reference count is correct when a // group has been joined and then left and the state is being // synced from scratch. - const skippedRetain = sbp('okTurtles.data/get', `gi.contracts/identity/group-skipped-${groupContractID}-${reference}`) + const skippedRetainKey = `gi.contracts/identity/group-skipped-${groupContractID}-${reference}` + const skippedRetain = sbp('okTurtles.data/get', skippedRetainKey) // No use for this value beyond this point. - sbp('okTurtles.data/delete', `gi.contracts/identity/group-skipped-${groupContractID}-${reference}`) + sbp('okTurtles.data/delete', skippedRetainKey) // If we've re-joined since, return // If the hash doesn't match (could happen after re-joining), return diff --git a/frontend/model/contracts/shared/constants.js b/frontend/model/contracts/shared/constants.js index 7af2005a7..3b6deebe4 100644 --- a/frontend/model/contracts/shared/constants.js +++ b/frontend/model/contracts/shared/constants.js @@ -17,7 +17,8 @@ export const INVITE_INITIAL_CREATOR = 'invite-initial-creator' export const PROFILE_STATUS = { ACTIVE: 'active', // confirmed group join PENDING: 'pending', // shortly after being approved to join the group - REMOVED: 'removed' + REMOVED: 'removed', + LEFT_GROUP: 'left-group' } export const GROUP_NAME_MAX_CHAR = 50 // https://github.com/okTurtles/group-income/issues/2196 export const GROUP_DESCRIPTION_MAX_CHAR = 500 diff --git a/frontend/model/contracts/shared/functions.js b/frontend/model/contracts/shared/functions.js index 86d7d6794..6ada788ce 100644 --- a/frontend/model/contracts/shared/functions.js +++ b/frontend/model/contracts/shared/functions.js @@ -158,11 +158,17 @@ export function createMessage ({ meta, data, hash, height, state, pending, inner return newMessage } -export async function leaveChatRoom (contractID: string) { +export async function leaveChatRoom (contractID: string, state: Object) { if (await sbp('chelonia/contract/isSyncing', contractID, { firstSync: true })) { return } + if (state) { + sbp('chelonia/contract/release', Object.keys(state.members)).catch(e => { + console.error(`[gi.contracts/chatroom] leaveChatRoom: Error releasing chatroom members for ${contractID}`, Object.keys(state.members), e) + }) + } + sbp('gi.actions/identity/kv/deleteChatRoomUnreadMessages', { contractID }).catch((e) => { console.error('[leaveChatroom] Error at deleteChatRoomUnreadMessages ', contractID, e) }) diff --git a/frontend/model/state.js b/frontend/model/state.js index 07ba4831d..3a546c9e5 100644 --- a/frontend/model/state.js +++ b/frontend/model/state.js @@ -84,6 +84,28 @@ sbp('sbp/selectors/register', { if (!state.preferences) { state.preferences = {} } + (() => { + // Upgrade from version 1.0.6 to a newer version + // The new group chatroomo contract introduces a breaking change: the + // `state[groupID].chatRooms[chatRoomID].members[memberID].joinedHeight` + // attribute. + // This code checks if the attribute is missing, and if so, issues the + // corresponing upgrade action. + const ourIdentityContractId = state.loggedIn?.identityContractID + if (!ourIdentityContractId || !state[ourIdentityContractId]?.groups) return + const upgradeRequired = Object.entries(state[ourIdentityContractId].groups).forEach(([groupID, { hasLeft }]) => { + if (hasLeft || !state[groupID]?.chatRooms) return + Object.values(state[groupID].chatRooms).flatMap(({ members }) => { + return Object.values(members) + }).reduce((upgradeRequired, member) => { + return upgradeRequired || (member.status === PROFILE_STATUS.ACTIVE && member.joinedHeight == null) + }, false) + }) + if (!upgradeRequired) return + sbp('gi.actions/group/upgradeFrom1.0.6').catch(e => { + console.error('[state/vuex/postUpgradeVerification] Error during gi.actions/group/upgradeFrom1.0.6', e) + }) + })() }, 'state/vuex/save': (encrypted: ?boolean, state: ?Object) => { return sbp('okTurtles.eventQueue/queueEvent', 'state/vuex/save', async function () { From cfa6b25651efb13f5d3fd1f131f83a0f0c62eacc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ricardo=20Iv=C3=A1n=20Vieitez=20Parra?= <3857362+corrideat@users.noreply.github.com> Date: Sun, 1 Sep 2024 17:09:39 +0000 Subject: [PATCH 05/24] Improved reference counting --- frontend/controller/namespace.js | 8 +- frontend/model/contracts/chatroom.js | 35 +- frontend/model/contracts/group.js | 175 +++------- frontend/model/contracts/identity.js | 29 +- frontend/model/contracts/shared/functions.js | 37 +++ frontend/model/state.js | 12 +- .../views/containers/chatroom/ChatMain.vue | 2 +- scripts/refcount-fuzzer.js | 299 ++++++++++++++++++ test/backend.test.js | 1 + 9 files changed, 416 insertions(+), 182 deletions(-) create mode 100644 scripts/refcount-fuzzer.js diff --git a/frontend/controller/namespace.js b/frontend/controller/namespace.js index 41c6ad116..74ee2386c 100644 --- a/frontend/controller/namespace.js +++ b/frontend/controller/namespace.js @@ -9,6 +9,10 @@ sbp('sbp/selectors/register', { const cache = sbp('state/vuex/state').namespaceLookups return cache?.[name] ?? null }, + 'namespace/lookupReverseCached': (id: string) => { + const cache = sbp('state/vuex/state').reverseNamespaceLookups + return cache?.[id] ?? null + }, 'namespace/lookup': (name: string, { skipCache }: { skipCache: boolean } = { skipCache: false }) => { if (!skipCache) { const cached = sbp('namespace/lookupCached', name) @@ -29,9 +33,11 @@ sbp('sbp/selectors/register', { } return r['text']() }).then(value => { - const cache = sbp('state/vuex/state').namespaceLookups if (value !== null) { + const cache = sbp('state/vuex/state').namespaceLookups + const reverseCache = sbp('state/vuex/state').reverseNamespaceLookups Vue.set(cache, name, value) + Vue.set(reverseCache, value, name) } return value }) diff --git a/frontend/model/contracts/chatroom.js b/frontend/model/contracts/chatroom.js index 75561b49b..60bd283fc 100644 --- a/frontend/model/contracts/chatroom.js +++ b/frontend/model/contracts/chatroom.js @@ -8,14 +8,14 @@ import { actionRequireInnerSignature, arrayOf, number, object, objectOf, optiona import { ChelErrorGenerator } from '~/shared/domains/chelonia/errors.js' import { findForeignKeysByContractID, findKeyIdByName } from '~/shared/domains/chelonia/utils.js' import { - MAX_HASH_LEN, CHATROOM_ACTIONS_PER_PAGE, CHATROOM_DESCRIPTION_LIMITS_IN_CHARS, CHATROOM_MAX_MESSAGES, - CHATROOM_NAME_LIMITS_IN_CHARS, CHATROOM_MAX_MESSAGE_LEN, + CHATROOM_NAME_LIMITS_IN_CHARS, CHATROOM_PRIVACY_LEVEL, CHATROOM_TYPES, + MAX_HASH_LEN, MESSAGE_NOTIFICATIONS, MESSAGE_NOTIFY_SETTINGS, MESSAGE_RECEIVE, @@ -27,6 +27,7 @@ import { findMessageIdx, leaveChatRoom, makeMentionFromUserID, + referenceTally, swapMentionIDForDisplayname } from './shared/functions.js' import chatroomGetters from './shared/getters/chatroom.js' @@ -207,9 +208,11 @@ sbp('chelonia/defineContract', { addMessage(state, createMessage({ meta, hash, height, state, data: notificationData, innerSigningContractID })) }, sideEffect ({ data, contractID, hash, meta, innerSigningContractID, height }, { state }) { + const memberID = data.memberID || innerSigningContractID + sbp('gi.contracts/chatroom/referenceTally', contractID, memberID, 'retain') + sbp('chelonia/queueInvocation', contractID, async () => { const state = await sbp('chelonia/contract/state', contractID) - const memberID = data.memberID || innerSigningContractID if (!state?.members?.[memberID]) { return @@ -221,16 +224,6 @@ sbp('chelonia/defineContract', { sbp('gi.actions/identity/kv/initChatRoomUnreadMessages', { contractID, messageHash: hash, createdHeight: height }) - - // subscribe to founder's IdentityContract & everyone else's - const profileIds = Object.keys(state.members) - sbp('chelonia/contract/retain', profileIds).catch((e) => { - console.error('Error while syncing other members\' contracts at chatroom join', e) - }) - } else { - sbp('chelonia/contract/retain', memberID).catch((e) => { - console.error(`Error while syncing new memberID's contract ${memberID}`, e) - }) } }).catch((e) => { console.error('[gi.contracts/chatroom/join/sideEffect] Error at sideEffect', e?.message || e) @@ -321,6 +314,7 @@ sbp('chelonia/defineContract', { }) } + sbp('gi.contracts/chatroom/referenceTally', contractID, memberID, 'release') sbp('chelonia/queueInvocation', contractID, async () => { const state = await sbp('chelonia/contract/state', contractID) if (!state || !!state.members?.[data.memberID] || !state.attributes) { @@ -343,9 +337,12 @@ sbp('chelonia/defineContract', { throw new TypeError(L('Only the channel creator can delete channel.')) } }), - process ({ meta }, { state }) { + process ({ meta, contractID }, { state }) { if (!state.attributes) return state.attributes['deletedDate'] = meta.createdDate + sbp('gi.contracts/chatroom/pushSideEffect', contractID, + ['gi.contracts/chatroom/referenceTally', contractID, Object.keys(state.members), 'release'] + ) for (const memberID in state.members) { delete state.members[memberID] } @@ -721,6 +718,13 @@ sbp('chelonia/defineContract', { } }, methods: { + 'gi.contracts/chatroom/_cleanup': ({ contractID, state }) => { + if (state?.members) { + sbp('chelonia/contract/release', Object.keys(state.members)).catch(e => { + console.error('[gi.contracts/chatroom/_cleanup] Error calling release', contractID, e) + }) + } + }, 'gi.contracts/chatroom/rotateKeys': (contractID, state) => { if (!state._volatile) state['_volatile'] = Object.create(null) if (!state._volatile.pendingKeyRevocations) state._volatile['pendingKeyRevocations'] = Object.create(null) @@ -759,6 +763,7 @@ sbp('chelonia/defineContract', { }).catch(e => { console.warn(`removeForeignKeys: ${e.name} thrown during queueEvent to ${contractID}:`, e) }) - } + }, + ...referenceTally('gi.contracts/chatroom/referenceTally') } }) diff --git a/frontend/model/contracts/group.js b/frontend/model/contracts/group.js index 87cc10c92..0cd74c3b1 100644 --- a/frontend/model/contracts/group.js +++ b/frontend/model/contracts/group.js @@ -38,7 +38,7 @@ import { STATUS_CANCELLED, STATUS_EXPIRED, STATUS_OPEN } from './shared/constants.js' import { adjustedDistribution, unadjustedDistribution } from './shared/distribution/distribution.js' -import { paymentHashesFromPaymentPeriod } from './shared/functions.js' +import { paymentHashesFromPaymentPeriod, referenceTally } from './shared/functions.js' import groupGetters from './shared/getters/group.js' import { cloneDeep, deepEqualJSONType, merge, omit } from './shared/giLodash.js' import { PAYMENT_COMPLETED, paymentStatusType, paymentType } from './shared/payments/index.js' @@ -397,30 +397,6 @@ export const actionRequireActiveMember = (next: Function): Function => (data, pr return next(data, props) } -const hasRetainBeenSkippedOnChatRoom = (state, contractID, chatRoomID, memberID, joinedHeight) => { - // Has `retain` been skipped? - // Note: we check for 'skipped' instead of 'called' because - // side-effects cannot mutate state. The 'skipped' case can be - // checked without persistent storage, while the 'called' case - // would require some kind of persistent state (since the reference - // count is itself persisted) - const skippedRetainKey = `gi.contracts/group/chatroom-skipped-${contractID}-${chatRoomID}-${joinedHeight}` - const skippedRetain = sbp('okTurtles.data/get', skippedRetainKey) - // There's no use of this transient value, so it's deleted to - // preserve memory - sbp('okTurtles.data/delete', skippedRetainKey) - - if ( - state.chatRooms?.[chatRoomID]?.members[memberID]?.status === PROFILE_STATUS.ACTIVE || - state.chatRooms?.[chatRoomID]?.members[memberID]?.joinedHeight !== joinedHeight || - skippedRetain - ) { - return true - } - - return false -} - export const GIGroupAlreadyJoinedError: typeof Error = ChelErrorGenerator('GIGroupAlreadyJoinedError') export const GIGroupNotJoinedError: typeof Error = ChelErrorGenerator('GIGroupNotJoinedError') @@ -888,6 +864,8 @@ sbp('chelonia/defineContract', { ) }, sideEffect ({ data, meta, contractID, height, innerSigningContractID, proposalHash }, { state, getters }) { + const memberID = data.memberID || innerSigningContractID + sbp('gi.contracts/group/referenceTally', contractID, memberID, 'release') // Put this invocation at the end of a sync to ensure that leaving and re-joining works sbp('chelonia/queueInvocation', contractID, () => sbp('gi.contracts/group/leaveGroup', { data, meta, contractID, getters, height, innerSigningContractID, proposalHash @@ -921,6 +899,8 @@ sbp('chelonia/defineContract', { sideEffect ({ meta, contractID, height, innerSigningContractID }) { const { loggedIn } = sbp('state/vuex/state') + // subscribe to the contract of the new member + sbp('gi.contracts/group/referenceTally', contractID, innerSigningContractID, 'retain') sbp('chelonia/queueInvocation', contractID, async () => { const state = await sbp('chelonia/contract/state', contractID) @@ -974,33 +954,12 @@ sbp('chelonia/defineContract', { })() } - // subscribe to founder's IdentityContract & everyone else's - const profileIds = Object.keys(profiles).filter(cID => cID !== userID) - if (profileIds.length !== 0) { - sbp('chelonia/contract/retain', profileIds).catch((e) => { - console.error('Error while syncing other members\' contracts at inviteAccept', e) - }) - } - sbp('okTurtles.events/emit', JOINED_GROUP, { identityContractID: userID, groupContractID: contractID }) - } else { - // we're an existing member of the group getting notified that a - // new member has joined, so subscribe to their identity contract - // TODO: Check if member is active; will be easier once profiles - // are indexed by contract ID - sbp('chelonia/contract/retain', innerSigningContractID).then(() => { - const { profiles = {} } = state - const myProfile = profiles[userID] - - if (isActionNewerThanUserJoinedDate(height, myProfile)) { - sbp('gi.notifications/emit', 'MEMBER_ADDED', { - createdDate: meta.createdDate, - groupID: contractID, - memberID: innerSigningContractID - }) - } - }).catch((e) => { - console.error(`Error subscribing to identity contract ${innerSigningContractID} of group member for group ${contractID}`, e) + } else if (isActionNewerThanUserJoinedDate(height, state?.profiles?.[userID])) { + sbp('gi.notifications/emit', 'MEMBER_ADDED', { + createdDate: meta.createdDate, + groupID: contractID, + memberID: innerSigningContractID }) } }).catch(e => { @@ -1223,9 +1182,12 @@ sbp('chelonia/defineContract', { } }), process ({ contractID, data }, { state }) { - sbp('gi.contracts/group/pushSideEffect', contractID, - ['gi.contracts/group/releaseDeletedChatRoom', data.chatRoomID, state.chatRooms[data.chatRoomID].members] - ) + const identityContractID = sbp('state/vuex/state').loggedIn?.identityContractID + if (identityContractID && state?.chatRooms[data.chatRoomID]?.members[identityContractID]?.status === PROFILE_STATUS.ACTIVE) { + sbp('gi.contracts/group/pushSideEffect', contractID, + ['gi.contracts/group/referenceTally', contractID, data.chatRoomID, 'release'] + ) + } delete state.chatRooms[data.chatRoomID] }, sideEffect ({ data, contractID, innerSigningContractID }) { @@ -1301,36 +1263,12 @@ sbp('chelonia/defineContract', { } if (memberID === identityContractID) { + sbp('gi.contracts/group/referenceTally', contractID, data.chatRoomID, 'release') sbp('okTurtles.events/emit', LEFT_CHATROOM, { identityContractID, groupContractID: contractID, chatRoomID: data.chatRoomID }) - - // The following queued invocation handles reference counting. It uses - // some transient (non-persistent) state to see whether the - // pairing `retain` to `release` was called. This situation could - // arise when syncing a group from scratch (e.g., from a new device) - // for chatrooms that have been joined and then left. In that case, - // there'll be no call to `retain` and therefore calling `release` - // is incorrect. - sbp('chelonia/queueInvocation', contractID, async () => { - // Re-grab `identityContractID` as the active user session could - // have changed. For example, Cypress does things very quickly. - const { identityContractID } = sbp('state/vuex/state').loggedIn - // New user session? The rest of the code isn't relevant - if (memberID !== identityContractID) { - return - } - - const state = await sbp('chelonia/contract/state', contractID) - if (hasRetainBeenSkippedOnChatRoom(state, contractID, data.chatRoomID, memberID, data.joinedHeight)) return - - // Release chatroom if we're sure `retain` was called - sbp('chelonia/contract/release', data.chatRoomID).catch(e => { - console.error(`[gi.contracts/group/leaveChatRoom/sideEffect] Error releasing chatroom ${data.chatRoomID}`, e) - }) - }) } } }, @@ -1370,37 +1308,16 @@ sbp('chelonia/defineContract', { const memberID = data.memberID || innerSigningContractID const { identityContractID } = sbp('state/vuex/state').loggedIn + if (memberID === identityContractID) { + sbp('gi.contracts/group/referenceTally', contractID, data.chatRoomID, 'retain') + } + // If we added someone to the chatroom (including ourselves), we issue // the relevant action to the chatroom contract if (innerSigningContractID === identityContractID) { sbp('chelonia/queueInvocation', contractID, () => sbp('gi.contracts/group/joinGroupChatrooms', contractID, data.chatRoomID, identityContractID, memberID, height)).catch((e) => { console.warn(`[gi.contracts/group/joinChatRoom/sideEffect] Error adding member to group chatroom for ${contractID}`, { e, data }) }) - } else if (memberID === identityContractID) { - // If we were the ones added to the chatroom, we sync the chatroom. - // This is an `else` block because joinGroupChatrooms already calls - // sync - sbp('chelonia/queueInvocation', contractID, async () => { - const state = await sbp('chelonia/contract/state', contractID) - - if (state?.chatRooms[data.chatRoomID]?.members[memberID]?.status === PROFILE_STATUS.ACTIVE) { - // If we were added by someone else, we might sync the chatroom - // contract before the corresponding `/join` action is issued. - // If we were previously a member of the chatroom, we would have - // a `/leave` action for ourselves, causing us to remove the - // chatroom contract. To handle this situation, we use - // `okTurtles.data/set` to define a special key that will be - // checked by the chatroom contract to tell it not to remove the - // contract if we're in the process of joining. - // This is a temporary measure until reference counting is - // implemented in Chelonia. With reference counting, we'd keep - // track of the 'reason' we're subscribing to a contract, and - // we won't need this special key. - sbp('chelonia/contract/retain', data.chatRoomID).catch((e) => { - console.warn(`[gi.contracts/group/joinChatRoom/sideEffect] Error syncing chatroom contract for ${contractID}`, { e, data }) - }) - } - }) } } }, @@ -1438,8 +1355,8 @@ sbp('chelonia/defineContract', { validate: actionRequireActiveMember(optional), process ({ height }, { state }) { let changed = false - Object.values(state.chatRooms).forEach((chatroom) => { - Object.values(chatroom.members).forEach(member => { + Object.values(state.chatRooms).forEach((chatroom: Object) => { + Object.values(chatroom.members).forEach((member: Object) => { if (member.status === PROFILE_STATUS.ACTIVE && member.joinedHeight == null) { member.joinedHeight = height changed = true @@ -1493,10 +1410,16 @@ sbp('chelonia/defineContract', { methods: { 'gi.contracts/group/_cleanup': ({ contractID, state }) => { // unsubscribe from other group members identity contract - // NOTE: it is safe to decrease the reference count on our own identity - // contract because it has an extra retain when joining a group - // (in the side-effect for gi.contracts/group/inviteAccept), which is - // the one being decremented + const { identityContractID } = sbp('state/vuex/state').loggedIn + const dependentContractIDs = [ + ...Object.entries(state?.profiles || {}).filter(([, state]: [string, Object]) => state.status === PROFILE_STATUS.ACTIVE).map(([cID]) => cID), + ...Object.entries(state?.chatRooms || {}).filter(([, state]: [string, Object]) => state.members[identityContractID]?.status === PROFILE_STATUS.ACTIVE).map(([cID]) => cID) + ] + if (dependentContractIDs.length) { + sbp('chelonia/contract/release', dependentContractIDs).catch(e => { + console.error('[gi.contracts/group/_cleanup] Error calling release', contractID, e) + }) + } // NOTE: should remove archived data from IndexedStorage regarding the current group (proposals, payments) Promise.all([ @@ -1506,14 +1429,6 @@ sbp('chelonia/defineContract', { console.error(`[gi.contracts/group/_cleanup] Error removing entries for archive for ${contractID}`, e) }) }, - 'gi.contracts/group/releaseDeletedChatRoom': (contractID, members) => { - const identityContractID = sbp('state/vuex/state').loggedIn?.identityContractID - if (identityContractID && members[identityContractID]?.status === PROFILE_STATUS.ACTIVE) { - sbp('chelonia/contract/release', contractID).catch(e => { - console.error('[gi.contracts/group/releaseDeletedChatRoom] Error', e) - }) - } - }, 'gi.contracts/group/archiveProposal': async function (contractID, proposalHash, proposal) { const { identityContractID } = sbp('state/vuex/state').loggedIn const key = `proposals/${identityContractID}/${contractID}` @@ -1700,7 +1615,7 @@ sbp('chelonia/defineContract', { // complex. // (*) Yes, usually we'd be a member of the chatroom in this case, but // we could have left afterwards. - await sbp('chelonia/contract/retain', chatRoomID, actorID !== memberID ? { ephemeral: true } : {}) + await sbp('chelonia/contract/retain', chatRoomID, { ephemeral: true }) if (!await sbp('chelonia/contract/hasKeysToPerformOperation', chatRoomID, 'gi.contracts/chatroom/join')) { throw new Error(`Missing keys to join chatroom ${chatRoomID}`) @@ -1722,9 +1637,7 @@ sbp('chelonia/defineContract', { console.warn(`Unable to join ${memberID} to chatroom ${chatRoomID} for group ${contractID}`, e) }).finally(() => { - if (actorID !== memberID) { - sbp('chelonia/contract/release', chatRoomID, { ephemeral: true }).catch(e => console.error('[gi.contracts/group/joinGroupChatrooms] Error during release', e)) - } + sbp('chelonia/contract/release', chatRoomID, { ephemeral: true }).catch(e => console.error('[gi.contracts/group/joinGroupChatrooms] Error during release', e)) }) } }, @@ -1792,15 +1705,6 @@ sbp('chelonia/defineContract', { }) if (memberID === identityContractID) { - const possiblyUselessContractIDs = [ - ...Object.keys(state.profiles || {}).filter(cID => cID !== identityContractID), - ...Object.keys(state.chatRooms || {}).filter((chatRoomID) => state.chatRooms[chatRoomID].members[identityContractID]?.status === PROFILE_STATUS.LEFT_GROUP && !hasRetainBeenSkippedOnChatRoom(state, contractID, chatRoomID, memberID, state.chatRooms[chatRoomID].members[identityContractID].joinedHeight)) - ] - console.error('@@@@@XXX', memberID, identityContractID, possiblyUselessContractIDs) - sbp('chelonia/contract/release', possiblyUselessContractIDs).catch(e => - console.error('[gi.contracts/group/leaveGroup] Error calling release on all members', e) - ) - sbp('gi.actions/identity/leaveGroup', { contractID: identityContractID, data: { @@ -1813,12 +1717,6 @@ sbp('chelonia/defineContract', { } else { const myProfile = getters.groupProfile(identityContractID) - // Do _not_ release memberID, because doing so will remove their profile - // from our view - // TODO: Instead, we could save the profile information to be deleted - // somewhere in the state or local storage and remove the contract - // // sbp('chelonia/contract/release', memberID) - if (isActionNewerThanUserJoinedDate(height, myProfile)) { if (!proposalHash) { // NOTE: Do not make notification when the member is removed by proposal @@ -1883,6 +1781,7 @@ sbp('chelonia/defineContract', { }).catch(e => { console.warn(`removeForeignKeys: ${e.name} error thrown:`, e) }) - } + }, + ...referenceTally('gi.contracts/group/referenceTally') } }) diff --git a/frontend/model/contracts/identity.js b/frontend/model/contracts/identity.js index 74bca1263..96a61f220 100644 --- a/frontend/model/contracts/identity.js +++ b/frontend/model/contracts/identity.js @@ -7,12 +7,13 @@ import { LEFT_GROUP } from '~/frontend/utils/events.js' import { Secret } from '~/shared/domains/chelonia/Secret.js' import { findForeignKeysByContractID, findKeyIdByName } from '~/shared/domains/chelonia/utils.js' import { - IDENTITY_USERNAME_MAX_CHARS, - IDENTITY_EMAIL_MAX_CHARS, IDENTITY_BIO_MAX_CHARS, + IDENTITY_EMAIL_MAX_CHARS, + IDENTITY_USERNAME_MAX_CHARS, MAX_HASH_LEN, MAX_URL_LEN } from './shared/constants.js' +import { referenceTally } from './shared/functions.js' import identityGetters from './shared/getters/identity.js' import { has, merge } from './shared/giLodash.js' import { @@ -217,6 +218,7 @@ sbp('chelonia/defineContract', { key: inviteSecret, transient: true }])) + sbp('gi.contracts/identity/referenceTally', contractID, groupContractID, 'retain') sbp('chelonia/queueInvocation', contractID, async () => { const state = await sbp('chelonia/contract/state', contractID) @@ -247,10 +249,6 @@ sbp('chelonia/defineContract', { // a deadlock. if (!inviteSecretId) return - sbp('chelonia/contract/retain', data.groupContractID).catch((e) => { - console.error('[gi.contracts/identity/joinGroup/sideEffect] Error calling retain', e) - }) - sbp('gi.actions/group/join', { originatingContractID: contractID, originatingContractName: 'gi.contracts/identity', @@ -288,6 +286,7 @@ sbp('chelonia/defineContract', { delete state.groups[groupContractID].inviteSecret }, sideEffect ({ data, contractID }) { + sbp('gi.contracts/identity/referenceTally', contractID, data.groupContractID, 'release') sbp('chelonia/queueInvocation', contractID, async () => { const state = await sbp('chelonia/contract/state', contractID) @@ -298,15 +297,6 @@ sbp('chelonia/defineContract', { const { groupContractID, reference } = data - // For an explanation of skippedRetain, see the group contract. - // TL;DR: This ensures that the reference count is correct when a - // group has been joined and then left and the state is being - // synced from scratch. - const skippedRetainKey = `gi.contracts/identity/group-skipped-${groupContractID}-${reference}` - const skippedRetain = sbp('okTurtles.data/get', skippedRetainKey) - // No use for this value beyond this point. - sbp('okTurtles.data/delete', skippedRetainKey) - // If we've re-joined since, return // If the hash doesn't match (could happen after re-joining), return if (!has(state.groups, groupContractID) || !state.groups[groupContractID].hasLeft || state.groups[groupContractID].hash !== reference) { @@ -320,12 +310,6 @@ sbp('chelonia/defineContract', { console.warn(`[gi.contracts/identity/leaveGroup/sideEffect] Error removing ourselves from group contract ${data.groupContractID}`, e) }) - if (!skippedRetain) { - sbp('chelonia/contract/release', data.groupContractID).catch((e) => { - console.error('[gi.contracts/identity/leaveGroup/sideEffect] Error calling release', e) - }) - } - // Remove last logged in information if (sbp('state/vuex/state').lastLoggedIn?.[contractID]) { delete sbp('state/vuex/state').lastLoggedIn[contractID] @@ -412,6 +396,7 @@ sbp('chelonia/defineContract', { sbp('chelonia/queueInvocation', identityContractID, ['gi.actions/out/rotateKeys', identityContractID, 'gi.contracts/identity', 'pending', 'gi.actions/identity/shareNewPEK']).catch(e => { console.warn(`revokeGroupKeyAndRotateOurPEK: ${e.name} thrown during queueEvent to ${identityContractID}:`, e) }) - } + }, + ...referenceTally('gi.contracts/identity/referenceTally') } }) diff --git a/frontend/model/contracts/shared/functions.js b/frontend/model/contracts/shared/functions.js index 6ada788ce..4b0759577 100644 --- a/frontend/model/contracts/shared/functions.js +++ b/frontend/model/contracts/shared/functions.js @@ -267,3 +267,40 @@ export function swapMentionIDForDisplayname ( .map(t => regEx.test(t) ? swap(t) : t) .join('') } + +export const referenceTally = (selector: string): Object => { + const delta = { + 'retain': 1, + 'release': -1 + } + return { + [selector]: (parentContractID: string, childContractIDs: string | string[], op: 'retain' | 'release') => { + if (!Array.isArray(childContractIDs)) childContractIDs = [childContractIDs] + if (op !== 'retain' && op !== 'release') throw new Error('Invalid operation') + for (const childContractID of childContractIDs) { + const key = `${selector}-${parentContractID}-${childContractID}` + const count = (sbp('okTurtles.data/get', key) || 0) + delta[op] + sbp('okTurtles.data/set', key, count) + sbp('chelonia/queueInvocation', childContractID, () => { + const count = sbp('okTurtles.data/get', key) + sbp('okTurtles.data/delete', key) + if (count !== Math.sign(count)) { + console.warn(`[${selector}] Unexpected value`, parentContractID, childContractID, count) + } + switch (Math.sign(count)) { + case -1: + sbp('chelonia/contract/release', childContractID).catch(e => { + console.error(`[${selector}] Error calling release`, parentContractID, childContractID, e) + }) + break + case 1: + sbp('chelonia/contract/retain', childContractID).catch(e => console.error(`[${selector}] Error calling retain`, parentContractID, childContractID, e)) + break + } + }).catch(e => { + console.error(`[${selector}] Error in queued invocation`, parentContractID, childContractID, e) + }) + } + } + } +} diff --git a/frontend/model/state.js b/frontend/model/state.js index 3a546c9e5..c57cef660 100644 --- a/frontend/model/state.js +++ b/frontend/model/state.js @@ -29,6 +29,7 @@ const initialState = { contracts: {}, // contractIDs => { type:string, HEAD:string, height:number } (for contracts we've successfully subscribed to) loggedIn: false, // false | { username: string, identityContractID: string } namespaceLookups: Object.create(null), // { [username]: sbp('namespace/lookup') } + reverseNamespaceLookups: Object.create(null), // { [contractID]: username } periodicNotificationAlreadyFiredMap: {}, // { notificationKey: boolean }, contractSigningKeys: Object.create(null), lastLoggedIn: {}, // Group last logged in information @@ -93,11 +94,12 @@ sbp('sbp/selectors/register', { // corresponing upgrade action. const ourIdentityContractId = state.loggedIn?.identityContractID if (!ourIdentityContractId || !state[ourIdentityContractId]?.groups) return - const upgradeRequired = Object.entries(state[ourIdentityContractId].groups).forEach(([groupID, { hasLeft }]) => { + const upgradeRequired = Object.entries(state[ourIdentityContractId].groups).forEach(([groupID, { hasLeft }]: [string, Object]) => { if (hasLeft || !state[groupID]?.chatRooms) return - Object.values(state[groupID].chatRooms).flatMap(({ members }) => { + // $FlowFixMe[incompatible-use] + Object.values((state[groupID].chatRooms: { [string]: Object })).flatMap(({ members }) => { return Object.values(members) - }).reduce((upgradeRequired, member) => { + }).reduce((upgradeRequired: boolean, member: Object) => { return upgradeRequired || (member.status === PROFILE_STATUS.ACTIVE && member.joinedHeight == null) }, false) }) @@ -290,7 +292,7 @@ const getters = { usernameFromID (state, getters) { return (userID) => { const profile = getters.ourContactProfilesById[userID] - return profile?.username || userID + return profile?.username || state.reverseNamespaceLookups[userID] || userID } }, userDisplayNameFromID (state, getters) { @@ -299,7 +301,7 @@ const getters = { return getters.ourUserDisplayName } const profile = getters.ourContactProfilesById[userID] - return profile?.displayName || profile?.username || userID + return profile?.displayName || profile?.username || state.reverseNamespaceLookups[userID] || userID } }, // this getter gets recomputed automatically according to the setInterval on reactiveDate diff --git a/frontend/views/containers/chatroom/ChatMain.vue b/frontend/views/containers/chatroom/ChatMain.vue index 172130348..825c13558 100644 --- a/frontend/views/containers/chatroom/ChatMain.vue +++ b/frontend/views/containers/chatroom/ChatMain.vue @@ -357,7 +357,7 @@ export default ({ }, who (message) { const user = this.isMsgSender(message.from) ? this.currentUserAttr : this.summary.participants[message.from] - return user?.displayName || user?.username || message.from + return user?.displayName || user?.username || sbp('namespace/lookupReverseCached', message.from) || message.from }, variant (message) { if (message.hasFailed) { diff --git a/scripts/refcount-fuzzer.js b/scripts/refcount-fuzzer.js new file mode 100644 index 000000000..f42cbf055 --- /dev/null +++ b/scripts/refcount-fuzzer.js @@ -0,0 +1,299 @@ +/* @noflow */ +const queueFactory = () => { + const events = [] + const queue = async (fn) => { + let accept + const promise = new Promise((resolve) => { accept = resolve }) + if (!fn) fn = () => Promise.resolve() + const thisEvent = { + fn, + promise + } + events.push(thisEvent) + while (events.length > 0) { + const event = events[0] + if (event === thisEvent) { + try { + return await fn() + } finally { + accept() + events.shift() + } + } else { + await event.promise + } + } + } + Object.defineProperty(queue, 'length', { get: () => events.length }) + + return queue +} + +const referenceCountFactory = () => { + const map = new Map() + + return { + retain (element) { + const current = map.get(element) || 0 + console.debug('retain', element, current) + if (current !== 0) { + throw new Error('Unexpected reference count') + } + map.set(element, current + 1) + }, + release (element) { + const current = map.get(element) + console.debug('release', element, current) + if (!current) throw new Error('Negative reference count') + if (current === 1) { + map.delete(element) + } else { + map.set(element, current - 1) + } + }, + done () { + if (!map.keys().next().done) { + console.info('@@@ Dangling references: ', JSON.stringify(Array.from(map.entries()).sort())) + throw new Error('There are dangling references') + } + } + } +} + +const $join = 'join' +const $leave = 'leave' + +const entityFactory = (ourselves, { queue, retain, release, done }) => { + const members = new Map() + let height = 0 + const _ephemeralRetain = new Map() + const _history = [] + + const applyReferenceOps = (member) => () => { + const count = _ephemeralRetain.get(member) + _ephemeralRetain.delete(member) + console.debug('applyReferenceOps', 'm=', member, 'count=', count) + if (!count) return + try { + switch (count) { + case -1: + release(member) + break + case 1: + retain(member) + break + default: + throw new Error('Unexpected value: ' + count) + } + } catch (e) { + console.debug('applyReferenceOps error', member, count, e) + throw e + } + } + + return { + [$join] (member) { + const id = (0, Math.random)().toFixed(6).slice(2) + const h = height + console.debug(`[${id}] join `, h, member, _history) + + const props = members.get(member) || {} + if (props.active) { + console.error(id, h, member, props) + throw new Error(`${[id]} Can't join twice`) + } + props.active = true + props.joinedHeight = height + members.set(member, props) + height++ + _history.push([$join, member]) + + // queue(() => { + console.debug(`[${id}] join (side-effect)`, h, member, _history) + + if (!_ephemeralRetain.has(member)) { + queue(applyReferenceOps(member)) + } + const count = (_ephemeralRetain.get(member) || 0) + 1 + console.debug(`[${id}] join (side-effect)`, 'h=', h, 'm=', member, 'count=', count) + _ephemeralRetain.set(member, count) + // }) + }, + + [$leave] (member) { + const id = (0, Math.random)().toFixed(6).slice(2) + const h = height + console.debug(`[${id}] leave `, h, member, _history) + + _history.push([$leave, member]) + const props = members.get(member) + if (!props?.active) { + throw new Error(`${[id]} Can't leave without joining`) + } + delete props.active + height++ + + // queue(() => { + console.debug(`[${id}] leave (side-effect)`, 'h=', h, 'm=', member) + + if (!_ephemeralRetain.has(member)) { + queue(applyReferenceOps(member)) + } + const count = (_ephemeralRetain.get(member) || 0) - 1 + console.debug(`[${id}] leave (side-effect)`, 'h=', h, 'm=', member, 'count=', count) + _ephemeralRetain.set(member, count) + // }) + }, + + remove () { + const id = (0, Math.random)().toFixed(6).slice(2) + const h = height + console.debug(`[${id}] remove (queued)`, h, history) + + const members_ = + [...members.entries()].filter(([member, { active }]) => { + return active + }) + + console.debug(`[${id}] remove (queued)`, h, 'members=', members_) + + members_.forEach(([member, props]) => { + /* if (_ephemeralRetain.get(props.leftHeight) === norelease) { + _ephemeralRetain.delete(props.leftHeight) + console.debug(`[${id}] remove (queued), norelease`, h, member) + return + } */ + + try { + release(member) + } catch (e) { + console.debug(`[${id}] remove Error`, h, member, e) + throw e + } + }) + + members.clear() + height = 0 + this.done() + }, + + done () { + if (!_ephemeralRetain.values().next().done) { + console.error('Dangling elements in _ephemeralRetain', [..._ephemeralRetain.entries()]) + throw new Error('Entity has dangling elements in the _ephemeralRetain set') + } + }, + + resync () { + const id = (0, Math.random)().toFixed(6).slice(2) + const h = height + console.debug(`[${id}] resync (queued)`, h) + queue(() => this.remove()) + queue(() => { + console.debug(`[${id}] resync `, h) + _history.splice(0).forEach(([action, ...params]) => { + console.debug(`[${id}] resync`, h, 'op=', action) + this[action](...params) + }) + }) + } + + } +} + +const run = async (queue, group, op, ...arg) => { + if (op === 'resync') console.error('-------------') + console.error('--- run:', op, ...arg) + switch (op) { + case $join: { + await queue(() => group[$join](...arg)) + break + } + case $leave: { + await queue(() => group[$leave](...arg)) + break + } + case 'resync': { + group.resync(...arg) + await queue() + break + } + case 'queue': { + await queue(...arg) + break + } + } +} + +const random = async (history) => { + const ourselves = Symbol('ourselves') + ourselves.toString = () => 'ourselves' + ourselves.toJSON = () => 'ourselves' + const queue = queueFactory() + const referenceCount = referenceCountFactory() + const users = new Array(15).fill(undefined).map((_, i) => i).concat([ourselves]) + const activeUsers = new Set() + const inactiveUsers = new Set(users) + + const group = entityFactory(ourselves, { + queue, + retain: referenceCount.retain, + release: referenceCount.release, + done: referenceCount.done + }) + + for (let i = 0; i < 16; i++) { + const randomAction = [$join, $leave, 'resync', 'queue'][((0, Math.random)() * 4) | 0] + switch (randomAction) { + case $join: { + const users = Array.from(inactiveUsers) + if (!users.length) { + i-- + break + } + const user = users.sort(() => Math.sign((0, Math.random()) - 0.5)).pop() + inactiveUsers.delete(user) + activeUsers.add(user) + history.push([$join, user]) + await run(queue, group, $join, user) + break + } + case $leave: { + const users = Array.from(activeUsers) + if (!users.length) { + i-- + break + } + const user = users.sort(() => Math.sign((0, Math.random()) - 0.5)).pop() + activeUsers.delete(user) + inactiveUsers.add(user) + history.push([$leave, user]) + await run(queue, group, $leave, user) + break + } + case 'resync': { + history.push(['resync']) + await run(queue, group, 'resync') + break + } + case 'queue': { + history.push(['queue']) + await run(queue, group, 'queue') + break + } + } + } + + await queue() + console.info('Done. Calling remove.') + queue(() => group.remove()) + + // Barrier + await queue() + referenceCount.done() +} + +const history = [] +random(history).catch((e) => { + console.error(history) +}) diff --git a/test/backend.test.js b/test/backend.test.js index 65893dbc5..e9390487d 100644 --- a/test/backend.test.js +++ b/test/backend.test.js @@ -52,6 +52,7 @@ const vuexState = { fontSize: 1, increasedContrast: false, namespaceLookups: Object.create(null), + reverseNamespaceLookups: Object.create(null), reducedMotion: false, appLogsFilter: ['error', 'info', 'warn'], contractSigningKeys: Object.create(null) From 10e4bebf3d7067919ede5232e89f3ec5c6d04162 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ricardo=20Iv=C3=A1n=20Vieitez=20Parra?= <3857362+corrideat@users.noreply.github.com> Date: Sun, 1 Sep 2024 17:10:13 +0000 Subject: [PATCH 06/24] Build #2 From c34449ca9c1d6c918acee055fc0c9adf5393a3ba Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ricardo=20Iv=C3=A1n=20Vieitez=20Parra?= <3857362+corrideat@users.noreply.github.com> Date: Sun, 1 Sep 2024 17:10:56 +0000 Subject: [PATCH 07/24] Build #3 From cc90eb03ae5d78797118bef96516a72918ef324b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ricardo=20Iv=C3=A1n=20Vieitez=20Parra?= <3857362+corrideat@users.noreply.github.com> Date: Sun, 1 Sep 2024 17:11:28 +0000 Subject: [PATCH 08/24] Build #4 From 121fdfb3177b36b215a7628a7b0194c0d9845ea2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ricardo=20Iv=C3=A1n=20Vieitez=20Parra?= <3857362+corrideat@users.noreply.github.com> Date: Sun, 1 Sep 2024 17:12:00 +0000 Subject: [PATCH 09/24] Build #5 From 64ae5a37dccc7fc285c35c13ed671dc20c25c960 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ricardo=20Iv=C3=A1n=20Vieitez=20Parra?= <3857362+corrideat@users.noreply.github.com> Date: Sun, 1 Sep 2024 17:12:33 +0000 Subject: [PATCH 10/24] Build #6 From 88da4b9cedbb7823c18eb8ee4fb392f8cfe0b469 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ricardo=20Iv=C3=A1n=20Vieitez=20Parra?= <3857362+corrideat@users.noreply.github.com> Date: Sun, 1 Sep 2024 17:13:05 +0000 Subject: [PATCH 11/24] Build #7 From d24022b7d166b937e2c745a5858fa2873bb091de Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ricardo=20Iv=C3=A1n=20Vieitez=20Parra?= <3857362+corrideat@users.noreply.github.com> Date: Sun, 1 Sep 2024 17:13:37 +0000 Subject: [PATCH 12/24] Build #8 From 287f1d0763246f0ed8711a880a7715727d3abdb4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ricardo=20Iv=C3=A1n=20Vieitez=20Parra?= <3857362+corrideat@users.noreply.github.com> Date: Sun, 1 Sep 2024 17:14:09 +0000 Subject: [PATCH 13/24] Build #9 From 028b8fc8938d52a20d4051bb3fb455a0fc01b474 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ricardo=20Iv=C3=A1n=20Vieitez=20Parra?= <3857362+corrideat@users.noreply.github.com> Date: Sun, 1 Sep 2024 18:07:19 +0000 Subject: [PATCH 14/24] Remove release --- frontend/model/contracts/shared/functions.js | 6 ------ 1 file changed, 6 deletions(-) diff --git a/frontend/model/contracts/shared/functions.js b/frontend/model/contracts/shared/functions.js index 4b0759577..fcf19f472 100644 --- a/frontend/model/contracts/shared/functions.js +++ b/frontend/model/contracts/shared/functions.js @@ -163,12 +163,6 @@ export async function leaveChatRoom (contractID: string, state: Object) { return } - if (state) { - sbp('chelonia/contract/release', Object.keys(state.members)).catch(e => { - console.error(`[gi.contracts/chatroom] leaveChatRoom: Error releasing chatroom members for ${contractID}`, Object.keys(state.members), e) - }) - } - sbp('gi.actions/identity/kv/deleteChatRoomUnreadMessages', { contractID }).catch((e) => { console.error('[leaveChatroom] Error at deleteChatRoomUnreadMessages ', contractID, e) }) From 7a8a33527414553b80f9f8343a6e8f3b109ed93f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ricardo=20Iv=C3=A1n=20Vieitez=20Parra?= <3857362+corrideat@users.noreply.github.com> Date: Sun, 1 Sep 2024 18:33:03 +0000 Subject: [PATCH 15/24] Build no. 2 From f01321ee1d38b180d80331d18cd3d81c78140a7b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ricardo=20Iv=C3=A1n=20Vieitez=20Parra?= <3857362+corrideat@users.noreply.github.com> Date: Sun, 1 Sep 2024 18:33:59 +0000 Subject: [PATCH 16/24] Build no. 3 From 32624785726a4a7fd1565e7547222a55dda4a442 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ricardo=20Iv=C3=A1n=20Vieitez=20Parra?= <3857362+corrideat@users.noreply.github.com> Date: Sun, 1 Sep 2024 18:41:11 +0000 Subject: [PATCH 17/24] CI: Add test for unexpected tally --- frontend/model/contracts/shared/functions.js | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/frontend/model/contracts/shared/functions.js b/frontend/model/contracts/shared/functions.js index fcf19f472..dd28be17e 100644 --- a/frontend/model/contracts/shared/functions.js +++ b/frontend/model/contracts/shared/functions.js @@ -278,8 +278,11 @@ export const referenceTally = (selector: string): Object => { sbp('chelonia/queueInvocation', childContractID, () => { const count = sbp('okTurtles.data/get', key) sbp('okTurtles.data/delete', key) - if (count !== Math.sign(count)) { + if (count && count !== Math.sign(count)) { console.warn(`[${selector}] Unexpected value`, parentContractID, childContractID, count) + if (process.env.CI) { + Promise.reject(new Error(`Unexpected value ${parentContractID} ${childContractID}: ${count}`)) + } } switch (Math.sign(count)) { case -1: From ae05b3229e56f8d1335fe76fff48e9f642b01554 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ricardo=20Iv=C3=A1n=20Vieitez=20Parra?= <3857362+corrideat@users.noreply.github.com> Date: Sun, 1 Sep 2024 19:20:27 +0000 Subject: [PATCH 18/24] Build no. 2 From 1ffa097d8ab935a3cc039878e58b0ce6104ad832 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ricardo=20Iv=C3=A1n=20Vieitez=20Parra?= <3857362+corrideat@users.noreply.github.com> Date: Sun, 1 Sep 2024 23:00:19 +0000 Subject: [PATCH 19/24] Add comment with example to referenceTally --- frontend/model/contracts/shared/functions.js | 69 ++++++++++++++++++++ 1 file changed, 69 insertions(+) diff --git a/frontend/model/contracts/shared/functions.js b/frontend/model/contracts/shared/functions.js index dd28be17e..a1b555fad 100644 --- a/frontend/model/contracts/shared/functions.js +++ b/frontend/model/contracts/shared/functions.js @@ -262,6 +262,75 @@ export function swapMentionIDForDisplayname ( .join('') } +// The `referenceTally` function is meant as an utility function to handle +// reference counting in contracts that import other contracts. +// The selector returned is to be called in side-effects that 'retain' or +// 'release' other contracts, and it works by pushing a single callback into +// the contract queue that maintains a temporary reference count to be applied +// at the end of a chain processing events. +// For example, a chatroom supports the 'join' and 'leave' actions, and those +// call 'retain' or 'release', respectively, on the identity contracts of +// members. +// Now, imagine this sequence of events: `[join, leave, join, leave]` (all +// involving the same member). +// Imagine all actions are processed at once (for example, the chatroom is being +// synced from scratch). By calling the `referenceTally` selector, this would +// happen in the event queue: +// queue slot 0: [sync]: +// (join) event 0: [process] +// event 0: [sideEffect]: this calls `referenceTally`, which +// increases the temp count to 1 and pushes a +// function into the queue. +// (leave) event 1: [process] +// event 1: [sideEffect]: this calls `referenceTally`, which +// decreases the temp count to 0. No function is +// pushed into the queue as one already exists. +// (join) event 2: [process] +// event 2: [sideEffect]: this calls `referenceTally`, which +// increases the temp count to 1. No function is +// pushed into the queue as one already exists. +// (leave) event 3: [process] +// event 3: [sideEffect]: this calls `referenceTally`, which +// decreases the temp count to 0. No function is +// pushed into the queue as one already exists. +// queue slot 1: [referenceTally]: Function pushed onto the queue by event 0. +// Since the temp count is 0, no call to retain +// or release happens. +// +// Now, imagine a different scenario, where the same events happen but they are +// processed differently. Let's say that the grouping is: +// 1. [join, leave] +// 2. [join] +// 3. [leave] +// This situation could happen when syncing the chatroom from scratch (with +// only the first two events having happened at this point in time) with the +// other events being received over the web socket later. +// queue slot 0: [sync]: +// (join) event 0: [process] +// event 0: [sideEffect]: this calls `referenceTally`, which +// increases the temp count to 1 and pushes a +// function into the queue. +// (leave) event 1: [process] +// event 1: [sideEffect]: this calls `referenceTally`, which +// decreases the temp count to 0. No function is +// pushed into the queue as one already exists. +// queue slot 1: [referenceTally]: Function pushed onto the queue by event 0. +// Since the temp count is 0, no call to retain +// or release happens. +// queue slot 2: [sync]: +// (join) event 2: [process] +// event 2: [sideEffect]: this calls `referenceTally`, which +// increases the temp count to and pushes a +// function into the queue. +// queue slot 3: [referenceTally]: Function pushed onto the queue by event 2. +// Since the temp count is 1, retain is called. +// queue slot 4: [sync]: +// (leave) event 3: [process] +// event 3: [sideEffect]: this calls `referenceTally`, which +// decreases the temp count to -1 and pushes a +// function into the queue. +// queue slot 5: [referenceTally]: Function pushed onto the queue by event 3. +// Since the temp count is -1, release is called. export const referenceTally = (selector: string): Object => { const delta = { 'retain': 1, From 5a38fa677947434f5f2c0019b74b5b8d0353ffe1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ricardo=20Iv=C3=A1n=20Vieitez=20Parra?= <3857362+corrideat@users.noreply.github.com> Date: Sun, 1 Sep 2024 23:15:36 +0000 Subject: [PATCH 20/24] Rectify referenceTally implementation errors --- frontend/model/contracts/shared/functions.js | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/frontend/model/contracts/shared/functions.js b/frontend/model/contracts/shared/functions.js index a1b555fad..96da7c772 100644 --- a/frontend/model/contracts/shared/functions.js +++ b/frontend/model/contracts/shared/functions.js @@ -342,9 +342,10 @@ export const referenceTally = (selector: string): Object => { if (op !== 'retain' && op !== 'release') throw new Error('Invalid operation') for (const childContractID of childContractIDs) { const key = `${selector}-${parentContractID}-${childContractID}` - const count = (sbp('okTurtles.data/get', key) || 0) + delta[op] - sbp('okTurtles.data/set', key, count) - sbp('chelonia/queueInvocation', childContractID, () => { + const count = sbp('okTurtles.data/get', key) + sbp('okTurtles.data/set', key, (count || 0) + delta[op]) + if (count != null) return + sbp('chelonia/queueInvocation', parentContractID, () => { const count = sbp('okTurtles.data/get', key) sbp('okTurtles.data/delete', key) if (count && count !== Math.sign(count)) { From b1b9c9c0006a8d99abb46fd4f6dd078ef03b40ef Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ricardo=20Iv=C3=A1n=20Vieitez=20Parra?= <3857362+corrideat@users.noreply.github.com> Date: Mon, 2 Sep 2024 00:00:07 +0000 Subject: [PATCH 21/24] Add release on leaving group --- frontend/model/contracts/group.js | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/frontend/model/contracts/group.js b/frontend/model/contracts/group.js index 0cd74c3b1..e53eb4e54 100644 --- a/frontend/model/contracts/group.js +++ b/frontend/model/contracts/group.js @@ -858,6 +858,13 @@ sbp('chelonia/defineContract', { process ({ data, meta, contractID, height, innerSigningContractID }, { state, getters }) { const memberID = data.memberID || innerSigningContractID const identityContractID = sbp('state/vuex/state').loggedIn?.identityContractID + if (memberID === identityContractID) { + const ourChatrooms = Object.entries(state?.chatRooms || {}).filter(([, state]: [string, Object]) => state.members[identityContractID]?.status === PROFILE_STATUS.ACTIVE).map(([cID]) => cID) + if (ourChatrooms.length) { + sbp('gi.contracts/group/pushSideEffect', contractID, + ['gi.contracts/group/referenceTally', contractID, ourChatrooms, 'release']) + } + } memberLeaves( { memberID, dateLeft: meta.createdDate, heightLeft: height, ourselvesLeaving: memberID === identityContractID }, { contractID, meta, state, getters } From a00ed0dfcf964b3210ff223e3f05555857be3012 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ricardo=20Iv=C3=A1n=20Vieitez=20Parra?= <3857362+corrideat@users.noreply.github.com> Date: Mon, 2 Sep 2024 00:17:05 +0000 Subject: [PATCH 22/24] Build no. 2 From 9d0de1318b73dc6d997c900f971efd7b2bff7cba Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ricardo=20Iv=C3=A1n=20Vieitez=20Parra?= <3857362+corrideat@users.noreply.github.com> Date: Mon, 2 Sep 2024 00:32:58 +0000 Subject: [PATCH 23/24] Build no. 3 From 26fc6b0fb57e43197f29dd58b7ec7ad9f2186bd3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ricardo=20Iv=C3=A1n=20Vieitez=20Parra?= <3857362+corrideat@users.noreply.github.com> Date: Thu, 5 Sep 2024 18:06:30 +0000 Subject: [PATCH 24/24] Feedback --- frontend/controller/actions/group.js | 2 +- frontend/controller/namespace.js | 4 ++-- frontend/model/contracts/group.js | 19 ++++++------------- frontend/model/contracts/shared/constants.js | 3 +-- frontend/model/contracts/shared/functions.js | 13 ++++++++++++- frontend/model/state.js | 10 +++++++--- 6 files changed, 29 insertions(+), 22 deletions(-) diff --git a/frontend/controller/actions/group.js b/frontend/controller/actions/group.js index e13bec8f4..acc147628 100644 --- a/frontend/controller/actions/group.js +++ b/frontend/controller/actions/group.js @@ -1030,7 +1030,7 @@ export default (sbp('sbp/selectors/register', { ...encryptedAction('gi.actions/group/updateSettings', L('Failed to update group settings.')), ...encryptedAction('gi.actions/group/updateAllVotingRules', (params, e) => L('Failed to update voting rules. {codeError}', { codeError: e.message })), ...encryptedAction('gi.actions/group/updateDistributionDate', L('Failed to update group distribution date.')), - ...encryptedAction('gi.actions/group/upgradeFrom1.0.6', L('Failed to upgrade from version 1.0.6')), + ...encryptedAction('gi.actions/group/upgradeFrom1.0.7', L('Failed to upgrade from version 1.0.6')), ...((process.env.NODE_ENV === 'development' || process.env.CI) && { ...encryptedAction('gi.actions/group/forceDistributionDate', L('Failed to force distribution date.')) }) diff --git a/frontend/controller/namespace.js b/frontend/controller/namespace.js index 74ee2386c..3d147b567 100644 --- a/frontend/controller/namespace.js +++ b/frontend/controller/namespace.js @@ -7,11 +7,11 @@ import Vue from 'vue' sbp('sbp/selectors/register', { 'namespace/lookupCached': (name: string) => { const cache = sbp('state/vuex/state').namespaceLookups - return cache?.[name] ?? null + return cache[name] ?? null }, 'namespace/lookupReverseCached': (id: string) => { const cache = sbp('state/vuex/state').reverseNamespaceLookups - return cache?.[id] ?? null + return cache[id] ?? null }, 'namespace/lookup': (name: string, { skipCache }: { skipCache: boolean } = { skipCache: false }) => { if (!skipCache) { diff --git a/frontend/model/contracts/group.js b/frontend/model/contracts/group.js index e53eb4e54..cc8389857 100644 --- a/frontend/model/contracts/group.js +++ b/frontend/model/contracts/group.js @@ -301,17 +301,10 @@ function updateGroupStreaks ({ state, getters }) { } } -const removeGroupChatroomProfile = (state, chatRoomID, member, ourselvesLeaving) => { - state.chatRooms[chatRoomID].members = - Object.fromEntries( - Object.entries(state.chatRooms[chatRoomID].members) - .map(([memberKey, profile]) => { - if (memberKey === member && (profile: any)?.status === PROFILE_STATUS.ACTIVE) { - return [memberKey, { ...profile, status: ourselvesLeaving ? PROFILE_STATUS.LEFT_GROUP : PROFILE_STATUS.REMOVED }] - } - return [memberKey, profile] - }) - ) +const removeGroupChatroomProfile = (state, chatRoomID, memberID, ourselvesLeaving) => { + if (!state.chatRooms[chatRoomID].members[memberID]) return + + state.chatRooms[chatRoomID].members[memberID].status = PROFILE_STATUS.REMOVED } const leaveChatRoomAction = async (groupID, state, chatRoomID, memberID, actorID, leavingGroup) => { @@ -1358,7 +1351,7 @@ sbp('chelonia/defineContract', { } } }, - 'gi.contracts/group/upgradeFrom1.0.6': { + 'gi.contracts/group/upgradeFrom1.0.7': { validate: actionRequireActiveMember(optional), process ({ height }, { state }) { let changed = false @@ -1371,7 +1364,7 @@ sbp('chelonia/defineContract', { }) }) if (!changed) { - throw new Error('[gi.contracts/group/upgradeFrom1.0.6/process] Invalid or duplicate upgrade action') + throw new Error('[gi.contracts/group/upgradeFrom1.0.7/process] Invalid or duplicate upgrade action') } } }, diff --git a/frontend/model/contracts/shared/constants.js b/frontend/model/contracts/shared/constants.js index 3b6deebe4..7af2005a7 100644 --- a/frontend/model/contracts/shared/constants.js +++ b/frontend/model/contracts/shared/constants.js @@ -17,8 +17,7 @@ export const INVITE_INITIAL_CREATOR = 'invite-initial-creator' export const PROFILE_STATUS = { ACTIVE: 'active', // confirmed group join PENDING: 'pending', // shortly after being approved to join the group - REMOVED: 'removed', - LEFT_GROUP: 'left-group' + REMOVED: 'removed' } export const GROUP_NAME_MAX_CHAR = 50 // https://github.com/okTurtles/group-income/issues/2196 export const GROUP_DESCRIPTION_MAX_CHAR = 500 diff --git a/frontend/model/contracts/shared/functions.js b/frontend/model/contracts/shared/functions.js index 96da7c772..7bc2c7202 100644 --- a/frontend/model/contracts/shared/functions.js +++ b/frontend/model/contracts/shared/functions.js @@ -350,8 +350,19 @@ export const referenceTally = (selector: string): Object => { sbp('okTurtles.data/delete', key) if (count && count !== Math.sign(count)) { console.warn(`[${selector}] Unexpected value`, parentContractID, childContractID, count) + // If we're running tests, we enforce checking that the temporary + // count _must_ be either of 0, 1 or -1. This is a correct + // assumption, based on the fact that a single contract should only + // call retain or release at most once after all operations are + // processed, per chunk of operations (e.g., there is no valid + // reason for a group contract to call `retain` twice on the same + // contract ID, without having called `release` first). + // This rule (or assumption) also applies to non-CI environments, + // but we are more lax in this case to allow for more leniency when + // running contracts with real users. However, this type of error + // indicates faulty reference bookkeeping that must be corrected. if (process.env.CI) { - Promise.reject(new Error(`Unexpected value ${parentContractID} ${childContractID}: ${count}`)) + Promise.reject(new Error(`[${selector}] Unexpected value ${parentContractID} ${childContractID}: ${count}`)) } } switch (Math.sign(count)) { diff --git a/frontend/model/state.js b/frontend/model/state.js index c57cef660..e9a48a280 100644 --- a/frontend/model/state.js +++ b/frontend/model/state.js @@ -85,8 +85,12 @@ sbp('sbp/selectors/register', { if (!state.preferences) { state.preferences = {} } + if (!state.reverseNamespaceLookups) { + // $FlowFixMe[incompatible-call] + Vue.set(state, 'reverseNamespaceLookups', Object.fromEntries(Object.entries(state.namespaceLookups).map(([k, v]: [string, string]) => [v, k]))) + } (() => { - // Upgrade from version 1.0.6 to a newer version + // Upgrade from version 1.0.7 to a newer version // The new group chatroomo contract introduces a breaking change: the // `state[groupID].chatRooms[chatRoomID].members[memberID].joinedHeight` // attribute. @@ -104,8 +108,8 @@ sbp('sbp/selectors/register', { }, false) }) if (!upgradeRequired) return - sbp('gi.actions/group/upgradeFrom1.0.6').catch(e => { - console.error('[state/vuex/postUpgradeVerification] Error during gi.actions/group/upgradeFrom1.0.6', e) + sbp('gi.actions/group/upgradeFrom1.0.7').catch(e => { + console.error('[state/vuex/postUpgradeVerification] Error during gi.actions/group/upgradeFrom1.0.7', e) }) })() },