Move General chatroom creation to side-effect, fix bug with sometimes…

… not joining it

frontend/controller/actions/group.js

@@ -2,9 +2,7 @@
 
 import { GIErrorUIRuntimeError, L, LError } from '@common/common.js'
 import {
-  CHATROOM_GENERAL_NAME,
   CHATROOM_PRIVACY_LEVEL,
-  CHATROOM_TYPES,
   INVITE_EXPIRES_IN_DAYS,
   INVITE_INITIAL_CREATOR,
   MAX_GROUP_MEMBER_COUNT,
@@ -174,6 +172,12 @@ export default (sbp('sbp/selectors/register', {
           }
         ],
         data: {
+          invites: {
+            [inviteKeyId]: {
+              creator: INVITE_INITIAL_CREATOR,
+              inviteKeyId
+            }
+          },
           settings: {
             // authorizations: [contracts.CanModifyAuths.dummyAuth()], // TODO: this
             groupName: name,
@@ -216,44 +220,14 @@ export default (sbp('sbp/selectors/register', {
         () => [CEK, CSK, inviteKey].map(key => ({ key }))
       )
 
-      // Save the initial invite
-      await sbp('gi.actions/group/invite', {
-        contractID,
-        data: {
-          inviteKeyId,
-          creator: INVITE_INITIAL_CREATOR
-        },
-        // The initial invite does not have an inner signature as it's part
-        // of the group creation process
-        innerSigningContractID: null
-      })
-
-      // create a 'General' chatroom contract
-      await sbp('gi.actions/group/addChatRoom', {
-        contractID,
-        data: {
-          attributes: {
-            name: CHATROOM_GENERAL_NAME,
-            type: CHATROOM_TYPES.GROUP,
-            description: '',
-            privacyLevel: CHATROOM_PRIVACY_LEVEL.GROUP
-          }
-        },
-        signingKeyId: CSKid,
-        encryptionKeyId: CEKid,
-        // The #General chatroom does not have an inner signature as it's part
-        // of the group creation process
-        innerSigningContractID: null
-      })
-
-      await sbp('gi.actions/identity/joinGroup', {
+      await sbp('chelonia/queueInvocation', contractID, ['gi.actions/identity/joinGroup', {
         contractID: userID,
         data: {
           groupContractID: contractID,
           inviteSecret: serializeKey(CSK, true),
           creator: true
         }
-      })
+      }])
 
       return message
     } catch (e) {
@@ -283,7 +257,7 @@ export default (sbp('sbp/selectors/register', {
   // secret keys to be shared with us, (b) ready to call the inviteAccept
   // action if we haven't done so yet (because we were previously waiting for
   // the keys), or (c) already a member and ready to interact with the group.
-  'gi.actions/group/join': async function (params: $Exact<ChelKeyRequestParams> & { blockOriginatingContract?: boolean }) {
+  'gi.actions/group/join': async function (params: $Exact<ChelKeyRequestParams>) {
     sbp('okTurtles.data/set', 'JOINING_GROUP-' + params.contractID, true)
     try {
       const rootState = sbp('state/vuex/state')
@@ -349,15 +323,26 @@ export default (sbp('sbp/selectors/register', {
       // After syncing the group contract, we send a key request
       if (sendKeyRequest) {
         // Send the key request
-        // We cannot await because this may be called from a section that is
-        // already waiting on the identity contract. Calls to keyRequest require
-        // simultaneously waiting on the group and the identity contract.
-        const keyRequestPromise = sbp('chelonia/out/keyRequest', {
+        // **IMPORTANT**: DO NOT AWAIT ON /join from a function that is
+        // already waiting on the identity contract. Details:
+        // The way that chelonia/out/keyRequest works is by sending two
+        // messages to connect both contracts together.
+        // The first step is adding a new key to the identity contract.
+        // This new key has OP_KEY_SHARE permissions, as well as the
+        // permissions specified in the parameters.
+        // The second stap is sending an OP_KEY_REQUEST message to the
+        // group contract.
+        // Note that this is a two-step process that involves writing to
+        // two contracts: the current group contract and the originating
+        // (identity) contract. Calls to keyRequest require
+        // simultaneously waiting on the group and the identity
+        // (originating) contract.
+        await sbp('chelonia/out/keyRequest', {
           ...omit(params, ['options']),
           innerEncryptionKeyId: sbp('chelonia/contract/currentKeyIdByName', params.contractID, 'cek'),
           permissions: [GIMessage.OP_ACTION_ENCRYPTED],
           allowedActions: ['gi.contracts/identity/joinDirectMessage'],
-          fullEncryption: true,
+          encryptKeyRequestMetadata: true,
           hooks: {
             prepublish: params.hooks?.prepublish,
             postpublish: null
@@ -367,10 +352,6 @@ export default (sbp('sbp/selectors/register', {
           throw e
         })
 
-        if (params.blockOriginatingContract !== false) {
-          await keyRequestPromise
-        }
-
         // Nothing left to do until the keys are received
 
       // Called after logging in or during an existing session from the event
@@ -424,6 +405,7 @@ export default (sbp('sbp/selectors/register', {
 
             // Send inviteAccept action to the group to add ourselves to the
             // members list
+            await sbp('chelonia/contract/wait', params.contractID)
             await sbp('gi.actions/group/inviteAccept', {
               ...omit(params, ['options', 'action', 'hooks', 'encryptionKeyId', 'signingKeyId']),
               hooks: {

frontend/main.js

@@ -120,7 +120,7 @@ async function startApp () {
           'chelonia/contract/waitingForKeyShareTo',
           'gi.actions/chatroom/leave',
           'gi.actions/group/removeOurselves',
-          'gi.actions/group/groupProfileUpdate', 'gi.actions/group/displayMincomeChangedPrompt',
+          'gi.actions/group/groupProfileUpdate', 'gi.actions/group/displayMincomeChangedPrompt', 'gi.actions/group/addChatRoom',
           'gi.actions/group/join', 'gi.actions/group/joinChatRoom',
           'gi.actions/identity/addJoinDirectMessageKey', 'gi.actions/identity/leaveGroup',
           'gi.notifications/emit',

frontend/model/contracts/group.js

@@ -11,7 +11,7 @@ import {
   PROPOSAL_INVITE_MEMBER, PROPOSAL_REMOVE_MEMBER, PROPOSAL_GROUP_SETTING_CHANGE, PROPOSAL_PROPOSAL_SETTING_CHANGE, PROPOSAL_GENERIC,
   STATUS_OPEN, STATUS_CANCELLED, STATUS_EXPIRED, MAX_ARCHIVED_PROPOSALS, MAX_ARCHIVED_PERIODS, PROPOSAL_ARCHIVED, PAYMENTS_ARCHIVED, MAX_SAVED_PERIODS,
   INVITE_INITIAL_CREATOR, PROFILE_STATUS, INVITE_EXPIRES_IN_DAYS,
-  CHATROOM_GENERAL_NAME
+  CHATROOM_GENERAL_NAME, CHATROOM_PRIVACY_LEVEL, CHATROOM_TYPES
 } from './shared/constants.js'
 import { paymentStatusType, paymentType, PAYMENT_COMPLETED } from './shared/payments/index.js'
 import { createPaymentInfo, paymentHashesFromPaymentPeriod } from './shared/functions.js'
@@ -152,6 +152,10 @@ function updateAdjustedDistribution ({ period, getters }) {
 }
 
 function memberLeaves ({ username, dateLeft }, { contractID, meta, state, getters }) {
+  if (!state.profiles[meta.username] || state.profiles[meta.username].status !== PROFILE_STATUS.ACTIVE) {
+    throw new Error(`[gi.contracts/group memberLeaves] Can't remove non-exisiting member ${meta.username}`)
+  }
+
   state.profiles[username].status = PROFILE_STATUS.REMOVED
   state.profiles[username].departedDate = dateLeft
   // remove any todos for this member from the adjusted distribution
@@ -677,6 +681,38 @@ sbp('chelonia/defineContract', {
           Vue.set(state, key, initialState[key])
         }
         initFetchPeriodPayments({ contractID, meta, state, getters })
+      },
+      sideEffect ({ contractID }, { state }) {
+        if (!state.generalChatRoomId) {
+        // create a 'General' chatroom contract
+          sbp('chelonia/queueInvocation', contractID, () => {
+            const rootState = sbp('state/vuex/state')
+            if (!rootState[contractID] || rootState[contractID].generalChatRoomId) return
+
+            const CSKid = findKeyIdByName(state, 'csk')
+            const CEKid = findKeyIdByName(state, 'cek')
+
+            // create a 'General' chatroom contract
+            return sbp('gi.actions/group/addChatRoom', {
+              contractID,
+              data: {
+                attributes: {
+                  name: CHATROOM_GENERAL_NAME,
+                  type: CHATROOM_TYPES.GROUP,
+                  description: '',
+                  privacyLevel: CHATROOM_PRIVACY_LEVEL.GROUP
+                }
+              },
+              signingKeyId: CSKid,
+              encryptionKeyId: CEKid,
+              // The #General chatroom does not have an inner signature as it's part
+              // of the group creation process
+              innerSigningContractID: null
+            })
+          }).catch((e) => {
+            console.error(`[gi.contracts/group/sideEffect] Error creating #General chatroom for ${contractID}`, e)
+          })
+        }
       }
     },
     'gi.contracts/group/payment': {
@@ -1051,11 +1087,14 @@ sbp('chelonia/defineContract', {
     },
     'gi.contracts/group/inviteAccept': {
       validate: Boolean,
-      process ({ data, meta, innerSigningKeyId }, { state }) {
+      process ({ data, meta }, { state }) {
         // TODO: ensure `meta.username` is unique for the lifetime of the username
         //       since we are making it possible for the same username to leave and
         //       rejoin the group. All of their past posts will be re-associated with
         //       them upon re-joining.
+        if (state.profiles[meta.username]?.status === PROFILE_STATUS.ACTIVE) {
+          throw new Error(`[gi.contracts/group/inviteAccept] Existing members can't accept invites: ${meta.username}`)
+        }
         Vue.set(state.profiles, meta.username, initGroupProfile(meta.createdDate))
         // If we're triggered by handleEvent in state.js (and not latestContractState)
         // then the asynchronous sideEffect function will get called next
@@ -1113,18 +1152,20 @@ sbp('chelonia/defineContract', {
                     }
                   }
                 }).catch((e) => {
-                  console.error('Error while joining the #General chatroom', e)
-                  // setTimeout to avoid blocking the main thread
-                  setTimeout(() => {
+                  console.error('Error while joining the #General chatroom', e);
+                  // avoid blocking the main thread
+                  // eslint-disable-next-line require-await
+                  (async () => {
                     alert(L("Couldn't join the #{chatroomName} in the group. An error occurred: #{error}.", { chatroomName: CHATROOM_GENERAL_NAME, error: e?.message || e }))
-                  }, 0)
+                  })()
                 })
               }
             } else {
-              // setTimeout to avoid blocking the main thread
-              setTimeout(() => {
+              // avoid blocking the main thread
+              // eslint-disable-next-line require-await
+              (async () => {
                 alert(L("Couldn't join the #{chatroomName} in the group. Doesn't exist.", { chatroomName: CHATROOM_GENERAL_NAME }))
-              }, 0)
+              })()
             }
 
             // subscribe to founder's IdentityContract & everyone else's
@@ -1167,8 +1208,8 @@ sbp('chelonia/defineContract', {
             // 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/sync', meta.identityContractID).catch(() => {
-              console.error(`Error subscribing to identity contract ${meta.identityContractID} of group member for group ${contractID}`)
+            sbp('chelonia/contract/sync', innerSigningContractID).catch(() => {
+              console.error(`Error subscribing to identity contract ${innerSigningContractID} of group member for group ${contractID}`)
             })
           }
         }).catch(e => {

frontend/model/contracts/identity.js

@@ -180,17 +180,27 @@ sbp('chelonia/defineContract', {
             return
           }
 
+          return inviteSecretId
+        }).then((inviteSecretId) => {
+          // Calling 'gi.actions/group/join' here _after_ queueInvoication
+          // and not inside of it.
+          // This is because 'gi.actions/group/join' might (depending on
+          // where we are at in the process of joining a group) call
+          // 'chelonia/out/keyRequest'. If this happens, it will block
+          // on the group contract queue (as normal and expected), but it
+          // will **ALSO** block on the current identity contract, which
+          // is already blocked by queueInvocation. This would result in
+          // a deadlock.
+          if (!inviteSecretId) return
+
           return sbp('gi.actions/group/join', {
             originatingContractID: contractID,
             originatingContractName: 'gi.contracts/identity',
             contractID: data.groupContractID,
             contractName: 'gi.contracts/group',
             signingKeyId: inviteSecretId,
             innerSigningKeyId: sbp('chelonia/contract/currentKeyIdByName', state, 'csk'),
-            encryptionKeyId: sbp('chelonia/contract/currentKeyIdByName', state, 'cek'),
-            blockOriginatingContract: false
-          }).catch(e => {
-            console.error(`[gi.contracts/identity/joinGroup/sideEffect] Error joining group ${data.groupContractID}`, e)
+            encryptionKeyId: sbp('chelonia/contract/currentKeyIdByName', state, 'cek')
           })
         }).catch(e => {
           console.error(`[gi.contracts/identity/joinGroup/sideEffect] Error at queueInvocation group ${data.groupContractID}`, e)

frontend/model/contracts/manifests.json

@@ -1,7 +1,7 @@
 {
   "manifests": {
     "gi.contracts/chatroom": "21XWnNHH2e67b1HMpFupf1EmSWH6ATaEN8yDL73d7nHKyduJru",
-    "gi.contracts/group": "21XWnNR7i79dR8DsefmNoGPbMW7ksg489BRymvqd3beRBCunoG",
-    "gi.contracts/identity": "21XWnNK49ogfNcAQfanvYPyEbGHRyekriVhAegxAvnWgZyLL8b"
+    "gi.contracts/group": "21XWnNRWSjMTNiutChd8uVkoxjhjTgFbBrtCxKdnYm5RdkUZUK",
+    "gi.contracts/identity": "21XWnNWEA9YxpSEVPr2cYhQfLELmz3PwiBwH89qFnzBfG5oS36"
   }
 }

frontend/views/pages/PendingApproval.vue

@@ -19,6 +19,7 @@ import sbp from '@sbp/sbp'
 import GroupWelcome from '@components/GroupWelcome.vue'
 import { PROFILE_STATUS } from '@model/contracts/shared/constants'
 import SvgInvitation from '@svgs/invitation.svg'
+import { JOINED_GROUP } from '@utils/events.js'
 import { mapGetters, mapState } from 'vuex'
 
 export default ({
@@ -32,6 +33,7 @@ export default ({
       ephemeral: {
         groupIdWhenMounted: null,
         groupJoined: false,
+        isJoining: false,
         settings: {}
       }
     }
@@ -43,6 +45,9 @@ export default ({
       if (!this.ephemeral.groupIdWhenMounted) return
       return this.$store.state[this.ephemeral.groupIdWhenMounted]
     },
+    isJoining () {
+      return this.ephemeral.isJoining && sbp('okTurtles.data/get', 'JOINING_GROUP-' + this.ephemeral.groupIdWhenMounted)
+    },
     haveActiveGroupProfile () {
       const state = this.groupState
       return (
@@ -52,13 +57,24 @@ export default ({
         // the state after receiving new private keys)
         !sbp('chelonia/contract/isResyncing', state) &&
         // And finally, we want the join process to be complete
-        !sbp('okTurtles.data/get', 'JOINING_GROUP-' + this.ephemeral.groupIdWhenMounted)
+        !this.isJoining
       )
     }
   },
   mounted () {
     this.ephemeral.groupIdWhenMounted = this.currentGroupId
     this.ephemeral.groupJoined = !!this.haveActiveGroupProfile
+    this.ephemeral.isJoining = sbp('okTurtles.data/get', 'JOINING_GROUP-' + this.ephemeral.groupIdWhenMounted)
+    if (this.ephemeral.isJoining) {
+      const handler = ({ contractID }) => {
+        if (contractID === this.ephemeral.groupIdWhenMounted) {
+          this.ephemeral.isJoining = false
+          // TODO: Remove when unmounted
+          sbp('okTurtles.events/off', handler)
+        }
+      }
+      sbp('okTurtles.events/on', JOINED_GROUP, handler)
+    }
   },
   watch: {
     groupState (to) {

shared/domains/chelonia/chelonia.js

@@ -127,7 +127,7 @@ export type ChelKeyRequestParams = {
   innerSigningKeyId: string;
   encryptionKeyId: string;
   innerEncryptionKeyId: string;
-  fullEncryption?: boolean;
+  encryptKeyRequestMetadata?: boolean;
   permissions?: '*' | string[];
   allowedActions?: '*' | string[];
   hooks?: {
@@ -431,7 +431,10 @@ export default (sbp('sbp/selectors/register', {
   },
   // The purpose of the 'chelonia/crypto/*' selectors is so that they can be called
   // from contracts without including the crypto code (i.e., importing crypto.js)
-  'chelonia/crypto/keyId': (inKeyFn: () => Key | string) => {
+  // This function takes a function as a parameter that returns a string
+  // It does not a string directly to prevent accidentally logging the value,
+  // which is a secret
+  'chelonia/crypto/keyId': (inKeyFn: { (): Key | string }) => {
     return keyId(inKeyFn())
   },
   // TODO: allow connecting to multiple servers at once
@@ -985,7 +988,7 @@ export default (sbp('sbp/selectors/register', {
     return msg
   },
   'chelonia/out/keyRequest': async function (params: ChelKeyRequestParams): Promise<?GIMessage> {
-    const { originatingContractID, originatingContractName, contractID, contractName, hooks, publishOptions, innerSigningKeyId, encryptionKeyId, innerEncryptionKeyId, fullEncryption } = params
+    const { originatingContractID, originatingContractName, contractID, contractName, hooks, publishOptions, innerSigningKeyId, encryptionKeyId, innerEncryptionKeyId, encryptKeyRequestMetadata } = params
     const manifestHash = this.config.contracts.manifests[contractName]
     const originatingManifestHash = this.config.contracts.manifests[originatingContractName]
     const contract = this.manifestToContract[manifestHash]?.contract
@@ -1038,7 +1041,7 @@ export default (sbp('sbp/selectors/register', {
               shareable: false
             },
             keyRequest: {
-              contractID: fullEncryption ? encryptedOutgoingData(originatingContractID, encryptionKeyId, contractID) : contractID
+              contractID: encryptKeyRequestMetadata ? encryptedOutgoingData(originatingContractID, encryptionKeyId, contractID) : contractID
             }
           },
           data: keyRequestReplyKeyP
@@ -1058,7 +1061,7 @@ export default (sbp('sbp/selectors/register', {
         op: [
           GIMessage.OP_KEY_REQUEST,
           signedOutgoingData(contractID, params.signingKeyId,
-            fullEncryption
+            encryptKeyRequestMetadata
               ? (encryptedOutgoingData(contractID, innerEncryptionKeyId, payload): any)
               : payload, this.transientSecretKeys
           )