search

status-im / status-go

The Status module that consumes go-ethereum
https://status.im
Mozilla Public License 2.0
722 stars 242 forks source link

Message outgoing status changes from `delivered` to `sent` #5464

Closed igor-sirotin closed 2 weeks ago

igor-sirotin commented 3 weeks ago

Problem

For messages on status-desktop I see this outgoing status change flow: sending -> delivered -> sent

https://github.com/status-im/status-go/assets/25482501/13519ee7-616f-4d52-b330-a262971914e8

After restart I can see it as delivered. And then it goes back to sent again.

From the logs it seems that this is happening due to message resending. MessageID from the video above: 0x158f207a2dd15b22a7ba3280494845245d192d02385c4c1799069a5ff7c18bba:

DEBUG[07-01|17:02:47.436|github.com/status-im/status-go/protocol/messenger_peersyncing.go:414] sent private messages                 messageIDs=[0x158f207a2dd15b22a7ba3280494845245d192d02385c4c1799069a5ff7c18bba] hashes=[0x67bc602b273d8de326068f1f39140af78ebb10f80e0391e6e315ebc46852bc60]
DEBUG[07-01|17:02:47.437|github.com/status-im/status-go/protocol/messenger_peersyncing.go:414] sent private messages                 messageIDs=[0x158f207a2dd15b22a7ba3280494845245d192d02385c4c1799069a5ff7c18bba] hashes=[0xfb7399cca4c5a069e147f5739c68622cb21713f9186bf9b67ea396b3ab4436eb]
DEBUG[07-01|17:02:52.647|github.com/status-im/status-go/protocol/messenger_peersyncing.go:41]  got datasync acknowledge for message  ack=f3b8b21b88aedad7b98088a609cffec527373cc22177ff4ac2d4b7f939173684 messageID=0x158f207a2dd15b22a7ba3280494845245d192d02385c4c1799069a5ff7c18bba
DEBUG[07-01|17:03:21.135|github.com/status-im/status-go/protocol/messenger_peersyncing.go:414] sent private messages                 messageIDs=[0x158f207a2dd15b22a7ba3280494845245d192d02385c4c1799069a5ff7c18bba] hashes=[0x0ede05b89dd3a5af8f41523e1b28a4941c876d85e02a9a61c52928f1032b7eac]
DEBUG[07-01|17:04:19.232|github.com/status-im/status-go/protocol/messenger_peersyncing.go:414] sent private messages                 messageIDs=[0x158f207a2dd15b22a7ba3280494845245d192d02385c4c1799069a5ff7c18bba] hashes=[0x4cc0fdd3c33e2f52b910e2d69f64d49b8655140e8def46e0f3474be377370cdb]
DEBUG[07-01|17:06:11.408|github.com/status-im/status-go/protocol/messenger_peersyncing.go:414] sent private messages                 messageIDs=[0x158f207a2dd15b22a7ba3280494845245d192d02385c4c1799069a5ff7c18bba] hashes=[0x4ca24c719659a2309a3910c35892ccb48017d366673a31b4375f8998b8d1e24e]

Implementation

We don't overwrite delivered in the db: https://github.com/status-im/status-go/blob/3b5eab3bf194e3d9f56fd98e1eb65b52f7683132/protocol/message_persistence.go#L2085-L2090

But there's no condition on notifying the client: https://github.com/status-im/status-go/blob/47899fd0456826f8da79e5cdbe5bebe490742c88/protocol/messenger_peersyncing.go#L62

Acceptance Criteria

  1. Strict outgoing message status flow: sending -> delivered -> sent. No way back.
  2. Message resending is not done when message is in delivered or sent state.
kaichaosun commented 3 weeks ago

Which branch are you using for this delivered mark on messages? I have tried status-desktop develop branch, but no this feature. @igor-sirotin

igor-sirotin commented 3 weeks ago

@kaichaosun sorry I didn't push it yet. I'll let you know when a PR is opened. But so far It seems to me that the it's the desktop side issue, because of the way we consider events from status-go.

Though resending the message is still weird, that is something you can check without my status-desktop changes.

igor-sirotin commented 2 weeks ago

I added a lock for Delivered state on desktop side: https://github.com/status-im/status-desktop/pull/15450 But why do we receive the envelope.sent event from status-go is still weird, this should be checked. Looks like it's being resent though it was delivered.

I'm able to reproduce it like this:

  1. Send a 1-1 message
  2. Wait until it's delivered
  3. Wait about ~30 seconds, it becomes sent again.
  4. If (3) didn't happen, restart the app and wait till the message is set to sent.

NOTE: You'll need to remove those lines to see it in desktop UI: https://github.com/status-im/status-desktop/blob/db359c622a176fa28b63bf263cd6fad8210b107b/src/app/modules/shared_models/message_model.nim#L549-L550

cc @kaichaosun

kaichaosun commented 2 weeks ago

Thanks @igor-sirotin, will look into the status-go code for the fix!

kaichaosun commented 2 weeks ago

@igor-sirotin I'm not able to reproduce the scenario you pointed out (probably because I don't have fast connection and delivered is always comes later than sent), but do find one improvement, could you please check this change https://github.com/status-im/status-go/pull/5502 fix it?