From ecb2bac6e50efb59a73c7a26ea99cd460e00ac73 Mon Sep 17 00:00:00 2001 From: Sale Djenic Date: Mon, 12 Jul 2021 21:30:17 +0200 Subject: [PATCH] fix(@desktop/chat): sign and send appears for both recipient and sender when sharing the address The reason for this issue is a message where recipient accepted to share his address with sender. In that message recipient's public key is set as a "from" property of a "Message" object and we cannot determine which of two users has initiated transaction actually. This is fixed checking if the "from" address from the "commandParameters" object of the "Message" is contained as an address in the wallet of logged in user. If yes, means that currently logged in user has initiated a transaction (he is a sender), otherwise currently logged in user is a recipient. We were just sending a transaction, without notifying message about that. Now we call callPrivateRPC("acceptRequestTransaction".prefix, %* [transactionHash, messageId, signature]) and that notifies message about the change, but only on the sender side. Appropriate message on the recipient side was not notified about the change. That need to be checked. --- src/app/chat/event_handling.nim | 10 +++- src/app/chat/view.nim | 7 ++- src/app/chat/views/message_list.nim | 12 +++-- src/app/chat/views/messages.nim | 21 ++++++-- src/app/chat/views/transactions.nim | 3 ++ src/status/chat.nim | 4 ++ src/status/chat/message.nim | 3 ++ src/status/chat/utils.nim | 8 +-- src/status/libstatus/chat.nim | 9 ++-- src/status/libstatus/chatCommands.nim | 3 ++ src/status/signals/messages.nim | 51 ++++++++++++++----- .../ChatComponents/SignTransactionModal.qml | 29 +++++------ .../MessageComponents/TransactionBubble.qml | 28 +++------- 13 files changed, 115 insertions(+), 73 deletions(-) diff --git a/src/app/chat/event_handling.nim b/src/app/chat/event_handling.nim index bdb2b82f41..6d046005ad 100644 --- a/src/app/chat/event_handling.nim +++ b/src/app/chat/event_handling.nim @@ -36,7 +36,13 @@ proc handleChatEvents(self: ChatController) = for message in evArgs.messages: if (message.replace != ""): # Delete the message that this message replaces - self.view.deleteMessage(message.chatId, message.replace) + if (not self.view.deleteMessage(message.chatId, message.replace)): + # In cases where new message need to replace a message which already replaced initial message + # "replace" property contains id of the initial message, but not the id of the message which + # replaced the initial one. That's why we call this proce here in case message with "message.replace" + # was not deleted. + discard self.view.deleteMessageWhichReplacedMessageWithId(message.chatId, message.replace) + self.view.reactions.push(evArgs.emojiReactions) if (evArgs.communities.len > 0): for community in evArgs.communities.mitems: @@ -66,7 +72,7 @@ proc handleChatEvents(self: ChatController) = self.status.events.on("messageDeleted") do(e: Args): var evArgs = MessageArgs(e) - self.view.deleteMessage(evArgs.channel, evArgs.id) + discard self.view.deleteMessage(evArgs.channel, evArgs.id) self.status.events.on("chatHistoryCleared") do(e: Args): var args = ChannelArgs(e) diff --git a/src/app/chat/view.nim b/src/app/chat/view.nim index 11c59b00b5..40fd7ac5cd 100644 --- a/src/app/chat/view.nim +++ b/src/app/chat/view.nim @@ -430,8 +430,11 @@ QtObject: proc hideLoadingIndicator*(self: ChatsView) {.slot.} = self.messageView.hideLoadingIndicator() - proc deleteMessage*(self: ChatsView, channelId: string, messageId: string) = - self.messageView.deleteMessage(channelId, messageId) + proc deleteMessage*(self: ChatsView, channelId: string, messageId: string): bool = + result = self.messageView.deleteMessage(channelId, messageId) + + proc deleteMessageWhichReplacedMessageWithId*(self: ChatsView, channelId: string, messageId: string): bool = + result = self.messageView.deleteMessageWhichReplacedMessageWithId(channelId, messageId) proc addPinnedMessages*(self: ChatsView, pinnedMessages: seq[Message]) = self.messageView.addPinnedMessages(pinnedMessages) diff --git a/src/app/chat/views/message_list.nim b/src/app/chat/views/message_list.nim index e5089a9dc7..01d004c8a3 100644 --- a/src/app/chat/views/message_list.nim +++ b/src/app/chat/views/message_list.nim @@ -103,8 +103,10 @@ QtObject: proc getMessage*(self: ChatMessageList, messageId: string): Message = return self.messages[self.messageIndex[messageId]] - proc deleteMessage*(self: ChatMessageList, messageId: string) = - if not self.messageIndex.hasKey(messageId): return + proc deleteMessage*(self: ChatMessageList, messageId: string): bool = + if not self.messageIndex.hasKey(messageId): + return false + let messageIndex = self.messageIndex[messageId] self.beginRemoveRows(newQModelIndex(), messageIndex, messageIndex) self.messages.delete(messageIndex) @@ -115,10 +117,12 @@ QtObject: self.messageIndex[self.messages[i].id] = i self.endRemoveRows() + return true + proc deleteMessagesByChatId*(self: ChatMessageList, chatId: string) = let messages = self.messages.filter(m => m.chatId == chatId) for message in messages: - self.deleteMessage(message.id) + discard self.deleteMessage(message.id) proc replaceMessage*(self: ChatMessageList, message: Message) = let msgIdx = self.messageIndex[message.id] @@ -382,7 +386,7 @@ QtObject: if m.fromAuthor == publicKey: # Can't delete on a loop msgIdxToDelete.add(m.id) for m in msgIdxToDelete: - self.deleteMessage(m) + discard self.deleteMessage(m) proc getID*(self: ChatMessageList):string {.slot.} = self.id diff --git a/src/app/chat/views/messages.nim b/src/app/chat/views/messages.nim index 28192d0192..78b6012996 100644 --- a/src/app/chat/views/messages.nim +++ b/src/app/chat/views/messages.nim @@ -415,9 +415,24 @@ QtObject: self.unreadDirectMessagesAndMentionsCount = currentUnreadDirectMessagesAndMentionsCount self.unreadDirectMessagesAndMentionsCountChanged() - proc deleteMessage*(self: MessageView, channelId: string, messageId: string) = - self.messageList[channelId].deleteMessage(messageId) - self.hideMessage(messageId) + proc deleteMessage*(self: MessageView, channelId: string, messageId: string): bool = + result = self.messageList[channelId].deleteMessage(messageId) + if (result): + self.hideMessage(messageId) + + proc deleteMessageWhichReplacedMessageWithId*(self: MessageView, channelId: string, messageId: string): bool = + var msgIdToBeDeleted: string + for message in self.messageList[channelId].messages: + if (message.replace == messageId): + msgIdToBeDeleted = message.id + break + + if (msgIdToBeDeleted.len == 0): + return false + + result = self.messageList[channelId].deleteMessage(msgIdToBeDeleted) + if (result): + self.hideMessage(msgIdToBeDeleted) proc removeMessagesByUserId(self: MessageView, publicKey: string) {.slot.} = for k in self.messageList.keys: diff --git a/src/app/chat/views/transactions.nim b/src/app/chat/views/transactions.nim index a8ec7962b1..c884e0ba74 100644 --- a/src/app/chat/views/transactions.nim +++ b/src/app/chat/views/transactions.nim @@ -34,3 +34,6 @@ QtObject: proc declineRequest*(self: TransactionsView, messageId: string) {.slot.} = self.status.chat.declineRequestTransaction(messageId) + + proc acceptRequestTransaction*(self: TransactionsView, transactionHash: string, messageId: string, signature: string) {.slot.} = + self.status.chat.acceptRequestTransaction(transactionHash, messageId, signature) \ No newline at end of file diff --git a/src/status/chat.nim b/src/status/chat.nim index 9359008c7a..14d410e8d9 100644 --- a/src/status/chat.nim +++ b/src/status/chat.nim @@ -416,6 +416,10 @@ proc requestAddressForTransaction*(self: ChatModel, chatId: string, fromAddress: let response = status_chat_commands.requestAddressForTransaction(chatId, fromAddress, amount, address) discard self.processMessageUpdateAfterSend(response) +proc acceptRequestTransaction*(self: ChatModel, transactionHash: string, messageId: string, signature: string) = + let response = status_chat_commands.acceptRequestTransaction(transactionHash, messageId, signature) + discard self.processMessageUpdateAfterSend(response) + proc requestTransaction*(self: ChatModel, chatId: string, fromAddress: string, amount: string, tokenAddress: string) = let address = if (tokenAddress == ZERO_ADDRESS): "" else: tokenAddress let response = status_chat_commands.requestTransaction(chatId, fromAddress, amount, address) diff --git a/src/status/chat/message.nim b/src/status/chat/message.nim index 94061f8af9..14e01e0276 100644 --- a/src/status/chat/message.nim +++ b/src/status/chat/message.nim @@ -32,6 +32,9 @@ type CommandParameters* = object commandState*: int signature*: string +proc `$`*(self: CommandParameters): string = + result = fmt"CommandParameters(id:{self.id}, fromAddr:{self.fromAddress}, addr:{self.address}, contract:{self.contract}, value:{self.value}, transactionHash:{self.transactionHash}, commandState:{self.commandState}, signature:{self.signature})" + type Message* = object alias*: string userName*: string diff --git a/src/status/chat/utils.nim b/src/status/chat/utils.nim index cb27587aa2..78639ff35c 100644 --- a/src/status/chat/utils.nim +++ b/src/status/chat/utils.nim @@ -1,24 +1,20 @@ -import ../libstatus/settings as status_settings - proc formatChatUpdate(response: JsonNode): (seq[Chat], seq[Message]) = var chats: seq[Chat] = @[] var messages: seq[Message] = @[] - let pk = status_settings.getSetting[string](Setting.PublicKey, "0x0") if response["result"]{"messages"} != nil: for jsonMsg in response["result"]["messages"]: - messages.add(jsonMsg.toMessage(pk)) + messages.add(jsonMsg.toMessage()) if response["result"]{"chats"} != nil: for jsonChat in response["result"]["chats"]: chats.add(jsonChat.toChat) result = (chats, messages) proc processChatUpdate(self: ChatModel, response: JsonNode): (seq[Chat], seq[Message]) = - let pk = status_settings.getSetting[string](Setting.PublicKey, "0x0") var chats: seq[Chat] = @[] var messages: seq[Message] = @[] if response{"result"}{"messages"} != nil: for jsonMsg in response["result"]["messages"]: - messages.add(jsonMsg.toMessage(pk)) + messages.add(jsonMsg.toMessage()) if response{"result"}{"chats"} != nil: for jsonChat in response["result"]["chats"]: let chat = jsonChat.toChat diff --git a/src/status/libstatus/chat.nim b/src/status/libstatus/chat.nim index dd20ec366c..fe2188cf0e 100644 --- a/src/status/libstatus/chat.nim +++ b/src/status/libstatus/chat.nim @@ -77,18 +77,16 @@ proc parseChatMessagesResponse*(rpcResult: JsonNode): (string, seq[Message]) = var messages: seq[Message] = @[] let messagesObj = rpcResult{"messages"} if(messagesObj != nil and messagesObj.kind != JNull): - let pk = status_settings.getSetting[string](Setting.PublicKey, "0x0") for jsonMsg in messagesObj: - messages.add(jsonMsg.toMessage(pk)) + messages.add(jsonMsg.toMessage()) return (rpcResult{"cursor"}.getStr, messages) proc parseActivityCenterNotifications*(rpcResult: JsonNode): (string, seq[ActivityCenterNotification]) = - let pk = status_settings.getSetting[string](Setting.PublicKey, "0x0") var notifs: seq[ActivityCenterNotification] = @[] var msg: Message if rpcResult{"notifications"}.kind != JNull: for jsonMsg in rpcResult["notifications"]: - notifs.add(jsonMsg.toActivityCenterNotification(pk)) + notifs.add(jsonMsg.toActivityCenterNotification()) return (rpcResult{"cursor"}.getStr, notifs) proc rpcChatMessages*(chatId: string, cursorVal: JsonNode, limit: int, success: var bool): string = @@ -542,10 +540,9 @@ proc parseChatPinnedMessagesResponse*(rpcResult: JsonNode): (string, seq[Message var messages: seq[Message] = @[] let messagesObj = rpcResult{"pinnedMessages"} if(messagesObj != nil and messagesObj.kind != JNull): - let pk = status_settings.getSetting[string](Setting.PublicKey, "0x0") var msg: Message for jsonMsg in messagesObj: - msg = jsonMsg["message"].toMessage(pk) + msg = jsonMsg["message"].toMessage() msg.pinnedBy = $jsonMsg{"pinnedBy"}.getStr messages.add(msg) return (rpcResult{"cursor"}.getStr, messages) diff --git a/src/status/libstatus/chatCommands.nim b/src/status/libstatus/chatCommands.nim index b68ff73ecf..679bd14209 100644 --- a/src/status/libstatus/chatCommands.nim +++ b/src/status/libstatus/chatCommands.nim @@ -15,3 +15,6 @@ proc requestAddressForTransaction*(chatId: string, fromAddress: string, amount: proc requestTransaction*(chatId: string, fromAddress: string, amount: string, tokenAddress: string): string = result = callPrivateRPC("requestTransaction".prefix, %* [chatId, amount, tokenAddress, fromAddress]) + +proc acceptRequestTransaction*(transactionHash: string, messageId: string, signature: string): string = + result = callPrivateRPC("acceptRequestTransaction".prefix, %* [transactionHash, messageId, signature]) diff --git a/src/status/signals/messages.nim b/src/status/signals/messages.nim index 83d9459180..22ee1b3741 100644 --- a/src/status/signals/messages.nim +++ b/src/status/signals/messages.nim @@ -1,6 +1,8 @@ import json, random, strutils, sequtils, sugar, chronicles, tables import json_serialization import ../utils +import ../wallet/account +import ../libstatus/wallet as status_wallet import ../libstatus/accounts as status_accounts import ../libstatus/accounts/constants as constants import ../libstatus/settings as status_settings @@ -13,7 +15,7 @@ import types import web3/conversions from ../utils import parseAddress, wei2Eth -proc toMessage*(jsonMsg: JsonNode, pk: string): Message +proc toMessage*(jsonMsg: JsonNode): Message proc toChat*(jsonChat: JsonNode): Chat @@ -23,15 +25,13 @@ proc toCommunity*(jsonCommunity: JsonNode): Community proc toCommunityMembershipRequest*(jsonCommunityMembershipRequest: JsonNode): CommunityMembershipRequest -proc toActivityCenterNotification*(jsonNotification: JsonNode, pk: string): ActivityCenterNotification +proc toActivityCenterNotification*(jsonNotification: JsonNode): ActivityCenterNotification proc fromEvent*(event: JsonNode): Signal = var signal:MessageSignal = MessageSignal() signal.messages = @[] signal.contacts = @[] - let pk = status_settings.getSetting[string](Setting.PublicKey, "0x0") - if event["event"]{"contacts"} != nil: for jsonContact in event["event"]["contacts"]: signal.contacts.add(jsonContact.toProfileModel()) @@ -40,7 +40,7 @@ proc fromEvent*(event: JsonNode): Signal = if event["event"]{"messages"} != nil: for jsonMsg in event["event"]["messages"]: - var message = jsonMsg.toMessage(pk) + var message = jsonMsg.toMessage() if message.hasMention: chatsWithMentions.add(message.chatId) signal.messages.add(message) @@ -70,7 +70,7 @@ proc fromEvent*(event: JsonNode): Signal = if event["event"]{"activityCenterNotifications"} != nil: for jsonNotification in event["event"]["activityCenterNotifications"]: - signal.activityCenterNotification.add(jsonNotification.toActivityCenterNotification(pk)) + signal.activityCenterNotification.add(jsonNotification.toActivityCenterNotification()) if event["event"]{"pinMessages"} != nil: for jsonPinnedMessage in event["event"]["pinMessages"]: @@ -150,8 +150,6 @@ proc toChat*(jsonChat: JsonNode): Chat = let chatTypeInt = jsonChat{"chatType"}.getInt let chatType: ChatType = if chatTypeInt >= ord(low(ChatType)) or chatTypeInt <= ord(high(ChatType)): ChatType(chatTypeInt) else: ChatType.Unknown - let pk = status_settings.getSetting[string](Setting.PublicKey, "0x0") - result = Chat( id: jsonChat{"id"}.getStr, communityId: jsonChat{"communityId"}.getStr, @@ -176,7 +174,7 @@ proc toChat*(jsonChat: JsonNode): Chat = result.muted = jsonChat["muted"].getBool if jsonChat["lastMessage"].kind != JNull: - result.lastMessage = jsonChat{"lastMessage"}.toMessage(pk) + result.lastMessage = jsonChat{"lastMessage"}.toMessage() if jsonChat.hasKey("joined") and jsonChat["joined"].kind != JNull: result.joined = jsonChat{"joined"}.getInt @@ -279,8 +277,20 @@ proc toTextItem*(jsonText: JsonNode): TextItem = for child in jsonText["children"]: result.children.add(child.toTextItem) +proc currentUserWalletContainsAddress(address: string): bool = + if (address.len == 0): + return false + + let accounts = status_wallet.getWalletAccounts() + for acc in accounts: + if (acc.address == address): + return true + + return false + +proc toMessage*(jsonMsg: JsonNode): Message = + let publicChatKey = status_settings.getSetting[string](Setting.PublicKey, "0x0") -proc toMessage*(jsonMsg: JsonNode, pk: string): Message = var contentType: ContentType try: contentType = ContentType(jsonMsg{"contentType"}.getInt) @@ -311,7 +321,7 @@ proc toMessage*(jsonMsg: JsonNode, pk: string): Message = timestamp: $jsonMsg{"timestamp"}.getInt, whisperTimestamp: $jsonMsg{"whisperTimestamp"}.getInt, outgoingStatus: $jsonMsg{"outgoingStatus"}.getStr, - isCurrentUser: pk == jsonMsg{"from"}.getStr, + isCurrentUser: publicChatKey == jsonMsg{"from"}.getStr, stickerHash: "", stickerPackId: -1, parsedText: @[], @@ -362,7 +372,20 @@ proc toMessage*(jsonMsg: JsonNode, pk: string): Message = signature: jsonMsg["commandParameters"]["signature"].getStr ) - message.hasMention = concat(message.parsedText.map(t => t.children.filter(c => c.textType == "mention" and c.literal == pk))).len > 0 + # This is kind of a workaround in case we're processing a transaction message. The reason for + # that is a message where a recipient accepted to share his address with sender. In that message + # a recipient's public key is set as a "from" property of a "Message" object and we cannot + # determine which of two users has initiated transaction actually. + # + # To overcome this we're checking if the "from" address from the "commandParameters" object of + # the "Message" is contained as an address in the wallet of logged in user. If yes, means that + # currently logged in user has initiated a transaction (he is a sender), otherwise currently + # logged in user is a recipient. + message.isCurrentUser = currentUserWalletContainsAddress(message.commandParameters.fromAddress) + + message.hasMention = concat(message.parsedText.map( + t => t.children.filter( + c => c.textType == "mention" and c.literal == publicChatKey))).len > 0 result = message @@ -376,7 +399,7 @@ proc toReaction*(jsonReaction: JsonNode): Reaction = retracted: jsonReaction{"retracted"}.getBool ) -proc toActivityCenterNotification*(jsonNotification: JsonNode, pk: string): ActivityCenterNotification = +proc toActivityCenterNotification*(jsonNotification: JsonNode): ActivityCenterNotification = var activityCenterNotificationType: ActivityCenterNotificationType try: activityCenterNotificationType = ActivityCenterNotificationType(jsonNotification{"type"}.getInt) @@ -396,4 +419,4 @@ proc toActivityCenterNotification*(jsonNotification: JsonNode, pk: string): Acti ) if jsonNotification.contains("message") and jsonNotification{"message"}.kind != JNull: - result.message = jsonNotification{"message"}.toMessage(pk) + result.message = jsonNotification{"message"}.toMessage() diff --git a/ui/app/AppLayouts/Chat/ChatColumn/ChatComponents/SignTransactionModal.qml b/ui/app/AppLayouts/Chat/ChatColumn/ChatComponents/SignTransactionModal.qml index 44606f5062..7990a39a2f 100644 --- a/ui/app/AppLayouts/Chat/ChatColumn/ChatComponents/SignTransactionModal.qml +++ b/ui/app/AppLayouts/Chat/ChatColumn/ChatComponents/SignTransactionModal.qml @@ -45,9 +45,6 @@ ModalPopup { sendingError.text = qsTr("Invalid transaction parameters") sendingError.open() } - - - root.close() } id: root @@ -62,6 +59,7 @@ ModalPopup { title: qsTrId("error-sending-the-transaction") icon: StandardIcon.Critical standardButtons: StandardButton.Ok + onAccepted: root.close() } onClosed: { @@ -158,7 +156,7 @@ ModalPopup { if (!gasEstimate.success) { //% "Error estimating gas: %1" let message = qsTrId("error-estimating-gas---1").arg(gasEstimate.error.message) - console.warn(message) + //% ". The transaction will probably fail." gasEstimateErrorPopup.confirmationText = message + qsTrId("--the-transaction-will-probably-fail-") gasEstimateErrorPopup.open() @@ -296,28 +294,33 @@ ModalPopup { onTransactionWasSent: { try { let response = JSON.parse(txResult) + if (response.uuid !== stack.uuid) + return - if (response.uuid !== stack.uuid) return - - stack.currentGroup.isPending = false + let transactionId = response.result if (!response.success) { - if (Utils.isInvalidPasswordMessage(response.result)){ + if (Utils.isInvalidPasswordMessage(transactionId)){ //% "Wrong password" transactionSigner.validationError = qsTrId("wrong-password") return } - sendingError.text = response.result + sendingError.text = transactionId return sendingError.open() } + chatsModel.transactions.acceptRequestTransaction(transactionId, + messageId, + profileModel.profile.pubKey + transactionId.substr(2)) + //% "Transaction pending..." toastMessage.title = qsTrId("ens-transaction-pending") toastMessage.source = "../../../../img/loading.svg" toastMessage.iconColor = Style.current.primary toastMessage.iconRotates = true - toastMessage.link = `${walletModel.utilsView.etherscanLink}/${response.result}` + toastMessage.link = `${walletModel.utilsView.etherscanLink}/${transactionId}` toastMessage.open() + root.close() } catch (e) { console.error('Error parsing the response', e) @@ -327,9 +330,3 @@ ModalPopup { } } -/*##^## -Designer { - D{i:0;autoSize:true;height:480;width:640} -} -##^##*/ - diff --git a/ui/app/AppLayouts/Chat/ChatColumn/MessageComponents/TransactionBubble.qml b/ui/app/AppLayouts/Chat/ChatColumn/MessageComponents/TransactionBubble.qml index e55456fcb9..cf50976fb5 100644 --- a/ui/app/AppLayouts/Chat/ChatColumn/MessageComponents/TransactionBubble.qml +++ b/ui/app/AppLayouts/Chat/ChatColumn/MessageComponents/TransactionBubble.qml @@ -24,6 +24,7 @@ Item { } } } + property var token: JSON.parse(commandParametersObject.contract) // TODO: handle {} property string tokenAmount: commandParametersObject.value property string tokenSymbol: token.symbol || "" @@ -35,18 +36,10 @@ Item { return walletModel.balanceView.getFiatValue(tokenAmount, token.symbol, defaultFiatSymbol) + " " + defaultFiatSymbol.toUpperCase() } property int state: commandParametersObject.commandState - property bool outgoing: { - switch (root.state) { - case Constants.pending: - case Constants.confirmed: - case Constants.transactionRequested: - case Constants.addressRequested: return isCurrentUser - case Constants.declined: - case Constants.transactionDeclined: - case Constants.addressReceived: return !isCurrentUser - default: return false - } - } + + // Any transaction where isCurrentUser is true is actually outgoing transaction. + property bool outgoing: isCurrentUser + property int innerMargin: 12 property bool isError: commandParametersObject.contract === "{}" onTokenSymbolChanged: { @@ -74,11 +67,12 @@ Item { color: Style.current.secondaryText text: { if (root.state === Constants.transactionRequested) { - let prefix = root.outgoing ? "↓ ": "↑ " + let prefix = outgoing? "↑ " : "↓ " //% "Transaction request" return prefix + qsTrId("transaction-request") } - return root.outgoing ? + + return outgoing ? //% "↑ Outgoing transaction" qsTrId("--outgoing-transaction") : //% "↓ Incoming transaction" @@ -208,9 +202,3 @@ Item { } } } - -/*##^## -Designer { - D{i:0;formeditorColor:"#4c4e50";formeditorZoom:1.25} -} -##^##*/