Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix #2277 #2294

Merged
merged 26 commits into from
Sep 5, 2024
Merged
Show file tree
Hide file tree
Changes from 18 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 18 additions & 1 deletion frontend/controller/actions/group.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 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
// 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,
joinedHeight
}
})
}),
...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) {
Expand Down Expand Up @@ -1014,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.'))
})
Expand Down
11 changes: 8 additions & 3 deletions frontend/controller/actions/identity.js
Original file line number Diff line number Diff line change
Expand Up @@ -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(([, { hasLeft }]) => !hasLeft)
.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) {
Expand Down Expand Up @@ -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].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')

Expand Down
6 changes: 4 additions & 2 deletions frontend/controller/app/group.js
Original file line number Diff line number Diff line change
Expand Up @@ -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, { hasLeft }]) => !hasLeft && cID)
.filter(cID =>
cID !== groupContractID
cID && cID !== groupContractID
).sort(cID =>
// prefer successfully joined groups
sbp('state/vuex/state')[cID]?.profiles?.[groupContractID] ? -1 : 1
Expand Down
7 changes: 4 additions & 3 deletions frontend/controller/app/identity.js
Original file line number Diff line number Diff line change
Expand Up @@ -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'

Expand Down Expand Up @@ -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)
Comment on lines +112 to +113
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yikes. Scary this was missed by both of us.

Yet another reason all of this must be cleaned up (#2271).

}

if (encryptionParams) {
Expand All @@ -121,7 +122,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] && !currentState[identityContractID].groups[cID].hasLeft)

if (gId) {
sbp('gi.app/group/switch', gId)
Expand Down
8 changes: 7 additions & 1 deletion frontend/controller/namespace.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

After implementing postUpgradeVerification should be able to rewrite this as a 1-liner:

return sbp('state/vuex/state').reverseNamespaceLookups[id]

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This function is essentially a copy-paste of the function above, handling the possibility of reverseNamespaceLookups (or namespaceLookups) not being defined. That was because the side-effects in the identity contract call the lookupCached selector, and I thought that an incorrect state should not cause the side-effect to fail. The ??null part also coerces undefined into null, so that the function returns only two types instead of three (string, undefined, null).

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK, I can get with ?? null but not cache?.[id]. That should just be cache[id].

},
'namespace/lookup': (name: string, { skipCache }: { skipCache: boolean } = { skipCache: false }) => {
if (!skipCache) {
const cached = sbp('namespace/lookupCached', name)
Expand All @@ -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
})
Expand Down
43 changes: 20 additions & 23 deletions frontend/model/contracts/chatroom.js
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -27,6 +27,7 @@ import {
findMessageIdx,
leaveChatRoom,
makeMentionFromUserID,
referenceTally,
swapMentionIDForDisplayname
} from './shared/functions.js'
import chatroomGetters from './shared/getters/chatroom.js'
Expand Down Expand Up @@ -177,7 +178,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`)
Expand Down Expand Up @@ -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
Expand All @@ -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)
Expand Down Expand Up @@ -287,7 +280,6 @@ sbp('chelonia/defineContract', {
}
}

state.members[memberID].leftHeight = height
delete state.members[memberID]

if (!state.attributes) return
Expand All @@ -310,18 +302,19 @@ 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)
})
}

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) {
Expand All @@ -344,18 +337,21 @@ 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]
}
},
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': {
Expand Down Expand Up @@ -723,9 +719,9 @@ sbp('chelonia/defineContract', {
},
methods: {
'gi.contracts/chatroom/_cleanup': ({ contractID, state }) => {
if (state) {
if (state?.members) {
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)
console.error('[gi.contracts/chatroom/_cleanup] Error calling release', contractID, e)
})
}
},
Expand Down Expand Up @@ -767,6 +763,7 @@ sbp('chelonia/defineContract', {
}).catch(e => {
console.warn(`removeForeignKeys: ${e.name} thrown during queueEvent to ${contractID}:`, e)
})
}
},
...referenceTally('gi.contracts/chatroom/referenceTally')
}
})
Loading