From 504d329fa4d83d1c942a88c83319dd7372670899 Mon Sep 17 00:00:00 2001 From: Hamish Peebles Date: Fri, 20 Sep 2024 09:36:11 +0100 Subject: [PATCH] Refund prize messages that are removed due to disappearing messages (#6427) --- backend/canisters/community/CHANGELOG.md | 4 ++ backend/canisters/community/impl/src/lib.rs | 16 +++++++- .../community/impl/src/timer_job_types.rs | 6 +-- backend/canisters/group/CHANGELOG.md | 4 ++ backend/canisters/group/impl/src/lib.rs | 14 ++++++- .../group/impl/src/timer_job_types.rs | 4 +- .../src/prize_message_tests.rs | 40 ++++++++++++++----- .../libraries/chat_events/src/chat_events.rs | 13 ++++-- .../src/message_content_internal.rs | 9 ++++- backend/libraries/group_chat_core/src/lib.rs | 15 ++++--- 10 files changed, 95 insertions(+), 30 deletions(-) diff --git a/backend/canisters/community/CHANGELOG.md b/backend/canisters/community/CHANGELOG.md index 15bcdad36a..97ae66afe8 100644 --- a/backend/canisters/community/CHANGELOG.md +++ b/backend/canisters/community/CHANGELOG.md @@ -10,6 +10,10 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/). - Update `send_message` args to work with MessagePack ([#6425](https://github.com/open-chat-labs/open-chat/pull/6315)) - Add `winner_count` to prizes enabling us to stop sending all winners ([#6426](https://github.com/open-chat-labs/open-chat/pull/6426)) +### Fixed + +- Refund prize messages that are removed due to disappearing messages ([#6427](https://github.com/open-chat-labs/open-chat/pull/6427)) + ## [[2.0.1349](https://github.com/open-chat-labs/open-chat/releases/tag/v2.0.1349-community)] - 2024-09-16 ### Added diff --git a/backend/canisters/community/impl/src/lib.rs b/backend/canisters/community/impl/src/lib.rs index 8194720e30..843276e595 100644 --- a/backend/canisters/community/impl/src/lib.rs +++ b/backend/canisters/community/impl/src/lib.rs @@ -2,7 +2,7 @@ use crate::memory::{get_instruction_counts_data_memory, get_instruction_counts_i use crate::model::channels::Channels; use crate::model::groups_being_imported::{GroupBeingImportedSummary, GroupsBeingImported}; use crate::model::members::CommunityMembers; -use crate::timer_job_types::{RemoveExpiredEventsJob, TimerJob}; +use crate::timer_job_types::{MakeTransferJob, RemoveExpiredEventsJob, TimerJob}; use activity_notification_state::ActivityNotificationState; use candid::Principal; use canister_state_macros::canister_state; @@ -226,13 +226,15 @@ impl RuntimeState { pub fn run_event_expiry_job(&mut self) { let now = self.env.now(); let mut next_event_expiry = None; + let mut prize_refunds = Vec::new(); for channel in self.data.channels.iter_mut() { - channel.chat.remove_expired_events(now); + let result = channel.chat.remove_expired_events(now); if let Some(expiry) = channel.chat.events.next_event_expiry() { if next_event_expiry.map_or(true, |current| expiry < current) { next_event_expiry = Some(expiry); } } + prize_refunds.extend(result.prize_refunds); } self.data.next_event_expiry = next_event_expiry; @@ -241,6 +243,16 @@ impl RuntimeState { .timer_jobs .enqueue_job(TimerJob::RemoveExpiredEvents(RemoveExpiredEventsJob), expiry, now); } + for pending_transaction in prize_refunds { + self.data.timer_jobs.enqueue_job( + TimerJob::MakeTransfer(MakeTransferJob { + pending_transaction, + attempt: 0, + }), + now, + now, + ); + } } pub fn metrics(&self) -> Metrics { diff --git a/backend/canisters/community/impl/src/timer_job_types.rs b/backend/canisters/community/impl/src/timer_job_types.rs index 4e13820760..2f11c43a49 100644 --- a/backend/canisters/community/impl/src/timer_job_types.rs +++ b/backend/canisters/community/impl/src/timer_job_types.rs @@ -180,7 +180,7 @@ impl Job for HardDeleteMessageContentJob { }); ic_cdk::spawn(storage_bucket_client::delete_files(files_to_delete)); } - if let MessageContentInternal::Prize(prize) = content { + if let MessageContentInternal::Prize(mut prize) = content { if let Some(message_index) = channel .chat .events @@ -270,8 +270,8 @@ impl Job for MarkGroupImportCompleteJob { impl Job for RefundPrizeJob { fn execute(self) { - if let Some(pending_transaction) = read_state(|state| { - if let Some(channel) = state.data.channels.get(&self.channel_id) { + if let Some(pending_transaction) = mutate_state(|state| { + if let Some(channel) = state.data.channels.get_mut(&self.channel_id) { channel.chat.events.prize_refund( self.thread_root_message_index, self.message_index, diff --git a/backend/canisters/group/CHANGELOG.md b/backend/canisters/group/CHANGELOG.md index 339b1ffe0e..8f8036e8d7 100644 --- a/backend/canisters/group/CHANGELOG.md +++ b/backend/canisters/group/CHANGELOG.md @@ -10,6 +10,10 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/). - Update `send_message` args to work with MessagePack ([#6425](https://github.com/open-chat-labs/open-chat/pull/6315)) - Add `winner_count` to prizes enabling us to stop sending all winners ([#6426](https://github.com/open-chat-labs/open-chat/pull/6426)) +### Fixed + +- Refund prize messages that are removed due to disappearing messages ([#6427](https://github.com/open-chat-labs/open-chat/pull/6427)) + ## [[2.0.1350](https://github.com/open-chat-labs/open-chat/releases/tag/v2.0.1350-group)] - 2024-09-16 ### Changed diff --git a/backend/canisters/group/impl/src/lib.rs b/backend/canisters/group/impl/src/lib.rs index e8ac686bfe..0114f37a65 100644 --- a/backend/canisters/group/impl/src/lib.rs +++ b/backend/canisters/group/impl/src/lib.rs @@ -1,7 +1,7 @@ use crate::memory::{get_instruction_counts_data_memory, get_instruction_counts_index_memory}; use crate::model::new_joiner_rewards::{NewJoinerRewardMetrics, NewJoinerRewardStatus, NewJoinerRewards}; use crate::new_joiner_rewards::process_new_joiner_reward; -use crate::timer_job_types::{RemoveExpiredEventsJob, TimerJob}; +use crate::timer_job_types::{MakeTransferJob, RemoveExpiredEventsJob, TimerJob}; use crate::updates::c2c_freeze_group::freeze_group_impl; use activity_notification_state::ActivityNotificationState; use candid::Principal; @@ -342,7 +342,7 @@ impl RuntimeState { pub fn run_event_expiry_job(&mut self) { let now = self.env.now(); - self.data.chat.remove_expired_events(now); + let result = self.data.chat.remove_expired_events(now); self.data.next_event_expiry = self.data.chat.events.next_event_expiry(); if let Some(expiry) = self.data.next_event_expiry { @@ -350,6 +350,16 @@ impl RuntimeState { .timer_jobs .enqueue_job(TimerJob::RemoveExpiredEvents(RemoveExpiredEventsJob), expiry, now); } + for pending_transaction in result.prize_refunds { + self.data.timer_jobs.enqueue_job( + TimerJob::MakeTransfer(MakeTransferJob { + pending_transaction, + attempt: 0, + }), + now, + now, + ); + } } pub fn metrics(&self) -> Metrics { diff --git a/backend/canisters/group/impl/src/timer_job_types.rs b/backend/canisters/group/impl/src/timer_job_types.rs index 38d86b4b50..11df101b62 100644 --- a/backend/canisters/group/impl/src/timer_job_types.rs +++ b/backend/canisters/group/impl/src/timer_job_types.rs @@ -148,7 +148,7 @@ impl Job for HardDeleteMessageContentJob { ic_cdk::spawn(storage_bucket_client::delete_files(files_to_delete)); } match content { - MessageContentInternal::Prize(prize) => { + MessageContentInternal::Prize(mut prize) => { if let Some(message_index) = state .data .chat @@ -223,7 +223,7 @@ impl Job for EndPollJob { impl Job for RefundPrizeJob { fn execute(self) { - if let Some(pending_transaction) = read_state(|state| { + if let Some(pending_transaction) = mutate_state(|state| { state.data.chat.events.prize_refund( self.thread_root_message_index, self.message_index, diff --git a/backend/integration_tests/src/prize_message_tests.rs b/backend/integration_tests/src/prize_message_tests.rs index 1fc8ead20c..0bff9f1a35 100644 --- a/backend/integration_tests/src/prize_message_tests.rs +++ b/backend/integration_tests/src/prize_message_tests.rs @@ -6,7 +6,7 @@ use std::time::Duration; use test_case::test_case; use testing::rng::{random_message_id, random_string}; use types::{ - icrc1, ChatEvent, CryptoTransaction, Cryptocurrency, EventIndex, MessageContent, MessageContentInitial, + icrc1, ChatEvent, CryptoTransaction, Cryptocurrency, EventIndex, MessageContent, MessageContentInitial, OptionUpdate, PendingCryptoTransaction, PrizeContentInitial, }; use utils::time::{HOUR_IN_MS, MINUTE_IN_MS}; @@ -102,9 +102,10 @@ fn prize_messages_can_be_claimed_successfully() { } } -#[test_case(false)] -#[test_case(true)] -fn unclaimed_prizes_get_refunded(delete_message: bool) { +#[test_case(1; "Prize expires")] +#[test_case(2; "Message deleted")] +#[test_case(3; "Message removed due to disappearing messages")] +fn unclaimed_prizes_get_refunded(case: u32) { let mut wrapper = ENV.deref().get(); let TestEnv { env, @@ -116,6 +117,20 @@ fn unclaimed_prizes_get_refunded(delete_message: bool) { let user1 = client::register_diamond_user(env, canister_ids, *controller); let user2 = client::register_user(env, canister_ids); let group_id = client::user::happy_path::create_group(env, &user1, random_string().as_str(), true, true); + + if case == 3 { + // Set disappearing messages to 5 minutes + client::group::happy_path::update_group( + env, + user1.principal, + group_id, + &group_canister::update_group_v2::Args { + events_ttl: OptionUpdate::SetToSome(5 * MINUTE_IN_MS), + ..Default::default() + }, + ); + } + client::local_user_index::happy_path::join_group(env, user2.principal, canister_ids.local_user_index, group_id); // Send user1 some ICP @@ -163,20 +178,23 @@ fn unclaimed_prizes_get_refunded(delete_message: bool) { client::group::happy_path::claim_prize(env, user2.principal, group_id, message_id); - let interval = if delete_message { - client::group::happy_path::delete_messages(env, user1.principal, group_id, None, vec![message_id]); - 5 * MINUTE_IN_MS - } else { - HOUR_IN_MS + let interval = match case { + 1 => HOUR_IN_MS, + 2 => { + client::group::happy_path::delete_messages(env, user1.principal, group_id, None, vec![message_id]); + 5 * MINUTE_IN_MS + } + 3 => 5 * MINUTE_IN_MS, + _ => unreachable!(), }; env.advance_time(Duration::from_millis(interval - 1)); - env.tick(); + tick_many(env, 3); let user1_balance_before_refund = client::ledger::happy_path::balance_of(env, canister_ids.icp_ledger, user1.user_id); env.advance_time(Duration::from_millis(1)); - tick_many(env, 2); + tick_many(env, 3); let user1_balance_after_refund = client::ledger::happy_path::balance_of(env, canister_ids.icp_ledger, user1.user_id); diff --git a/backend/libraries/chat_events/src/chat_events.rs b/backend/libraries/chat_events/src/chat_events.rs index 871fa6947e..31cc26b4fd 100644 --- a/backend/libraries/chat_events/src/chat_events.rs +++ b/backend/libraries/chat_events/src/chat_events.rs @@ -25,6 +25,7 @@ use types::{ }; pub const OPENCHAT_BOT_USER_ID: UserId = UserId::new(Principal::from_slice(&[228, 104, 142, 9, 133, 211, 135, 217, 129, 1])); +const MEMO_PRIZE_REFUND: [u8; 8] = [0x4f, 0x43, 0x5f, 0x50, 0x52, 0x5a, 0x52, 0x46]; // OC_PRZRF #[derive(Serialize, Deserialize)] pub struct ChatEvents { @@ -499,16 +500,16 @@ impl ChatEvents { } pub fn prize_refund( - &self, + &mut self, thread_root_message_index: Option, message_index: MessageIndex, memo: &[u8], now_nanos: TimestampNanos, ) -> Option { if let Some((message, _)) = - self.message_internal(EventIndex::default(), thread_root_message_index, message_index.into()) + self.message_internal_mut(EventIndex::default(), thread_root_message_index, message_index.into()) { - if let MessageContentInternal::Prize(p) = &message.content { + if let MessageContentInternal::Prize(p) = &mut message.content { return p.prize_refund(message.sender, memo, now_nanos); } } @@ -1550,6 +1551,11 @@ impl ChatEvents { .threads .push((m.message_index, thread.participants_and_followers(true))); } + if let MessageContentInternal::Prize(mut p) = m.content { + if let Some(refund) = p.prize_refund(m.sender, &MEMO_PRIZE_REFUND, now * 1_000_000) { + result.prize_refunds.push(refund); + } + } } } } @@ -2010,6 +2016,7 @@ pub enum UnfollowThreadResult { pub struct RemoveExpiredEventsResult { pub events: Vec, pub threads: Vec<(MessageIndex, Vec)>, + pub prize_refunds: Vec, } #[derive(Copy, Clone)] diff --git a/backend/libraries/chat_events/src/message_content_internal.rs b/backend/libraries/chat_events/src/message_content_internal.rs index 1538a9f0d8..ba6a257848 100644 --- a/backend/libraries/chat_events/src/message_content_internal.rs +++ b/backend/libraries/chat_events/src/message_content_internal.rs @@ -815,6 +815,8 @@ pub struct PrizeContentInternal { pub caption: Option, #[serde(rename = "d", default, skip_serializing_if = "is_default")] pub diamond_only: bool, + #[serde(rename = "f", default, skip_serializing_if = "is_default")] + pub refund_started: bool, } impl PrizeContentInternal { @@ -827,13 +829,18 @@ impl PrizeContentInternal { end_date: content.end_date, caption: content.caption, diamond_only: content.diamond_only, + refund_started: false, } } - pub fn prize_refund(&self, sender: UserId, memo: &[u8], now_nanos: TimestampNanos) -> Option { + pub fn prize_refund(&mut self, sender: UserId, memo: &[u8], now_nanos: TimestampNanos) -> Option { + if self.refund_started { + return None; + } let fee = self.transaction.fee(); let unclaimed = self.prizes_remaining.iter().map(|p| p + fee).sum::(); if unclaimed > 0 { + self.refund_started = true; Some(create_pending_transaction( self.transaction.token(), self.transaction.ledger_canister_id(), diff --git a/backend/libraries/group_chat_core/src/lib.rs b/backend/libraries/group_chat_core/src/lib.rs index 8b5cad29d9..829d649b51 100644 --- a/backend/libraries/group_chat_core/src/lib.rs +++ b/backend/libraries/group_chat_core/src/lib.rs @@ -1,6 +1,7 @@ use chat_events::{ AddRemoveReactionArgs, ChatEventInternal, ChatEvents, ChatEventsListReader, DeleteMessageResult, - DeleteUndeleteMessagesArgs, MessageContentInternal, PushMessageArgs, Reader, TipMessageArgs, UndeleteMessageResult, + DeleteUndeleteMessagesArgs, MessageContentInternal, PushMessageArgs, Reader, RemoveExpiredEventsResult, TipMessageArgs, + UndeleteMessageResult, }; use event_store_producer::{EventStoreClient, Runtime}; use itertools::Itertools; @@ -1680,17 +1681,19 @@ impl GroupChatCore { } } - pub fn remove_expired_events(&mut self, now: TimestampMillis) { + pub fn remove_expired_events(&mut self, now: TimestampMillis) -> RemoveExpiredEventsResult { let result = self.events.remove_expired_events(now); - for (thread_root_message_index, users) in result.threads { + for (thread_root_message_index, users) in result.threads.iter() { for user_id in users { - if let Some(member) = self.members.get_mut(&user_id) { - member.threads.remove(&thread_root_message_index); - member.unfollowed_threads.retain(|&m| m != thread_root_message_index); + if let Some(member) = self.members.get_mut(user_id) { + member.threads.remove(thread_root_message_index); + member.unfollowed_threads.retain(|&m| m != *thread_root_message_index); } } } + + result } fn events_reader(&self, user_id: Option, thread_root_message_index: Option) -> EventsReaderResult {