From b51107a0f5b1043a91b12dd34ab6237e11f46b31 Mon Sep 17 00:00:00 2001 From: Andrew Morgan Date: Mon, 6 Jun 2022 16:35:23 +0100 Subject: [PATCH 01/10] Prevent local quarantined media from being claimed by media retention --- synapse/rest/media/v1/media_repository.py | 12 ++++++--- .../databases/main/media_repository.py | 27 +++++++++++++++++++ 2 files changed, 35 insertions(+), 4 deletions(-) diff --git a/synapse/rest/media/v1/media_repository.py b/synapse/rest/media/v1/media_repository.py index a551458a9fef..5ee4312ba13a 100644 --- a/synapse/rest/media/v1/media_repository.py +++ b/synapse/rest/media/v1/media_repository.py @@ -919,6 +919,7 @@ async def _apply_media_retention_rules(self) -> None: await self.delete_old_local_media( before_ts=local_media_threshold_timestamp_ms, keep_profiles=True, + delete_quarantined_media=False, ) async def delete_old_remote_media(self, before_ts: int) -> Dict[str, int]: @@ -975,6 +976,7 @@ async def delete_old_local_media( before_ts: int, size_gt: int = 0, keep_profiles: bool = True, + delete_quarantined_media: bool = False, ) -> Tuple[List[str], int]: """ Delete local or remote media from this server by size and timestamp. Removes @@ -982,11 +984,12 @@ async def delete_old_local_media( Args: before_ts: Unix timestamp in ms. - Files that were last used before this timestamp will be deleted - size_gt: Size of the media in bytes. Files that are larger will be deleted + Files that were last used before this timestamp will be deleted. + size_gt: Size of the media in bytes. Files that are larger will be deleted. keep_profiles: Switch to delete also files that are still used in image data - (e.g user profile, room avatar) - If false these files will be deleted + (e.g user profile, room avatar). If false these files will be deleted. + delete_quarantined_media: If True, media marked as quarantined will be deleted. + Returns: A tuple of (list of deleted media IDs, total deleted media IDs). """ @@ -994,6 +997,7 @@ async def delete_old_local_media( before_ts, size_gt, keep_profiles, + include_quarantined_media=delete_quarantined_media, ) return await self._remove_local_media_from_disk(old_media) diff --git a/synapse/storage/databases/main/media_repository.py b/synapse/storage/databases/main/media_repository.py index deffdc19ce9f..e12aabfffc14 100644 --- a/synapse/storage/databases/main/media_repository.py +++ b/synapse/storage/databases/main/media_repository.py @@ -256,7 +256,28 @@ async def get_local_media_before( before_ts: int, size_gt: int, keep_profiles: bool, + include_quarantined_media: bool, ) -> List[str]: + """ + Retrieve a list of media IDs from the local media store. + + Args: + before_ts: Only retrieve IDs from media that was either last accessed + (or if never accessed, created) before the given UNIX timestamp in ms. + size_gt: Only retrieve IDs from media that has a size (in bytes) greater than + the given integer. + keep_profiles: If True, exclude media IDs from the results that are used in the + following situations: + * global profile user avatar + * per-room profile user avatar + * room avatar + * a user's avatar in the user directory + include_quarantined_media: If False, exclude media IDs from the results that have + been marked as quarantined. + + Returns: + A list of local media IDs. + """ # to find files that have never been accessed (last_access_ts IS NULL) # compare with `created_ts` @@ -294,6 +315,12 @@ async def get_local_media_before( ) sql += sql_keep + if include_quarantined_media is False: + # Only include media that has not been quarantined + sql += """ + AND quarantined_by IS NULL + """ + def _get_local_media_before_txn(txn: LoggingTransaction) -> List[str]: txn.execute(sql, (before_ts, before_ts, size_gt)) return [row[0] for row in txn] From d0d3a69633f872e3852244e99ca916e1bda349c4 Mon Sep 17 00:00:00 2001 From: Andrew Morgan Date: Mon, 6 Jun 2022 19:16:07 +0100 Subject: [PATCH 02/10] Protect remote quarantined media from media retention --- synapse/rest/media/v1/media_repository.py | 4 +++- .../databases/main/media_repository.py | 24 ++++++++++++++++++- 2 files changed, 26 insertions(+), 2 deletions(-) diff --git a/synapse/rest/media/v1/media_repository.py b/synapse/rest/media/v1/media_repository.py index 5ee4312ba13a..afc93601ea2d 100644 --- a/synapse/rest/media/v1/media_repository.py +++ b/synapse/rest/media/v1/media_repository.py @@ -923,7 +923,9 @@ async def _apply_media_retention_rules(self) -> None: ) async def delete_old_remote_media(self, before_ts: int) -> Dict[str, int]: - old_media = await self.store.get_remote_media_before(before_ts) + old_media = await self.store.get_remote_media_before( + before_ts, include_quarantined_media=False + ) deleted = 0 diff --git a/synapse/storage/databases/main/media_repository.py b/synapse/storage/databases/main/media_repository.py index e12aabfffc14..6eadce9d73a9 100644 --- a/synapse/storage/databases/main/media_repository.py +++ b/synapse/storage/databases/main/media_repository.py @@ -626,13 +626,35 @@ async def store_remote_media_thumbnail( desc="store_remote_media_thumbnail", ) - async def get_remote_media_before(self, before_ts: int) -> List[Dict[str, str]]: + async def get_remote_media_before( + self, before_ts: int, include_quarantined_media: bool + ) -> List[Dict[str, str]]: + """ + Retrieve a list of server name, media ID tuples from the remote media cache. + + Args: + before_ts: Only retrieve IDs from media that was either last accessed + (or if never accessed, created) before the given UNIX timestamp in ms. + include_quarantined_media: If False, exclude media IDs from the results that have + been marked as quarantined. + + Returns: + A list of tuples containing: + * The server name of homeserver where the media originates from, + * The ID of the media. + """ sql = ( "SELECT media_origin, media_id, filesystem_id" " FROM remote_media_cache" " WHERE last_access_ts < ?" ) + if include_quarantined_media is False: + # Only include media that has not been quarantined + sql += """ + AND quarantined_by IS NULL + """ + return await self.db_pool.execute( "get_remote_media_before", self.db_pool.cursor_to_dict, sql, before_ts ) From 62b17cddc03fb1206207033689a76c2fab68c518 Mon Sep 17 00:00:00 2001 From: Andrew Morgan Date: Mon, 6 Jun 2022 19:15:48 +0100 Subject: [PATCH 03/10] Test that media retention does not remove quarantined media --- tests/rest/media/test_media_retention.py | 80 ++++++++++++++++++++---- 1 file changed, 69 insertions(+), 11 deletions(-) diff --git a/tests/rest/media/test_media_retention.py b/tests/rest/media/test_media_retention.py index b98a5cd586f0..c675deda8e94 100644 --- a/tests/rest/media/test_media_retention.py +++ b/tests/rest/media/test_media_retention.py @@ -58,8 +58,9 @@ def prepare(self, reactor: MemoryReactor, clock: Clock, hs: HomeServer) -> None: media_repository = hs.get_media_repository() test_media_content = b"example string" - def _create_media_and_set_last_accessed( + def _create_media_and_set_attributes( last_accessed_ms: Optional[int], + is_quarantined: Optional[bool] = False, ) -> str: # "Upload" some media to the local media store mxc_uri = self.get_success( @@ -84,10 +85,22 @@ def _create_media_and_set_last_accessed( ) ) + if is_quarantined: + # Mark this media as quarantined + self.get_success( + self.store.quarantine_media_by_id( + server_name=self.hs.config.server.server_name, + media_id=media_id, + quarantined_by="@theadmin:test", + ) + ) + return media_id - def _cache_remote_media_and_set_last_accessed( - media_id: str, last_accessed_ms: Optional[int] + def _cache_remote_media_and_set_attributes( + media_id: str, + last_accessed_ms: Optional[int], + is_quarantined: Optional[bool] = False, ) -> str: # Pretend to cache some remote media self.get_success( @@ -112,23 +125,52 @@ def _cache_remote_media_and_set_last_accessed( ) ) + if is_quarantined: + # Mark this media as quarantined + self.get_success( + self.store.quarantine_media_by_id( + server_name=self.remote_server_name, + media_id=media_id, + quarantined_by="@theadmin:test", + ) + ) + return media_id # Start with the local media store - self.local_recently_accessed_media = _create_media_and_set_last_accessed( - self.THIRTY_DAYS_IN_MS + self.local_recently_accessed_media = _create_media_and_set_attributes( + last_accessed_ms=self.THIRTY_DAYS_IN_MS, + ) + self.local_not_recently_accessed_media = _create_media_and_set_attributes( + last_accessed_ms=self.ONE_DAY_IN_MS, ) - self.local_not_recently_accessed_media = _create_media_and_set_last_accessed( - self.ONE_DAY_IN_MS + self.local_not_recently_accessed_quarantined_media = ( + _create_media_and_set_attributes( + last_accessed_ms=self.ONE_DAY_IN_MS, + is_quarantined=True, + ) + ) + self.local_never_accessed_media = _create_media_and_set_attributes( + last_accessed_ms=None, ) - self.local_never_accessed_media = _create_media_and_set_last_accessed(None) # And now the remote media store - self.remote_recently_accessed_media = _cache_remote_media_and_set_last_accessed( - "a", self.THIRTY_DAYS_IN_MS + self.remote_recently_accessed_media = _cache_remote_media_and_set_attributes( + media_id="a", + last_accessed_ms=self.THIRTY_DAYS_IN_MS, ) self.remote_not_recently_accessed_media = ( - _cache_remote_media_and_set_last_accessed("b", self.ONE_DAY_IN_MS) + _cache_remote_media_and_set_attributes( + media_id="b", + last_accessed_ms=self.ONE_DAY_IN_MS, + ) + ) + self.remote_not_recently_accessed_quarantined_media = ( + _cache_remote_media_and_set_attributes( + media_id="c", + last_accessed_ms=self.ONE_DAY_IN_MS, + is_quarantined=True, + ) ) # Remote media will always have a "last accessed" attribute, as it would not # be fetched from the remote homeserver unless instigated by a user. @@ -163,8 +205,16 @@ def test_local_media_retention(self) -> None: ], not_purged=[ (self.hs.config.server.server_name, self.local_recently_accessed_media), + ( + self.hs.config.server.server_name, + self.local_not_recently_accessed_quarantined_media, + ), (self.remote_server_name, self.remote_recently_accessed_media), (self.remote_server_name, self.remote_not_recently_accessed_media), + ( + self.remote_server_name, + self.remote_not_recently_accessed_quarantined_media, + ), ], ) @@ -199,6 +249,14 @@ def test_remote_media_cache_retention(self) -> None: self.hs.config.server.server_name, self.local_not_recently_accessed_media, ), + ( + self.hs.config.server.server_name, + self.local_not_recently_accessed_quarantined_media, + ), + ( + self.remote_server_name, + self.remote_not_recently_accessed_quarantined_media, + ), (self.hs.config.server.server_name, self.local_never_accessed_media), ], ) From 2d837fc9077923e54614ed0873a7a0e7e170cd5e Mon Sep 17 00:00:00 2001 From: Andrew Morgan Date: Mon, 6 Jun 2022 19:21:09 +0100 Subject: [PATCH 04/10] Rename *_before methods, as they do more these days --- synapse/rest/media/v1/media_repository.py | 4 ++-- synapse/storage/databases/main/media_repository.py | 10 +++++----- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/synapse/rest/media/v1/media_repository.py b/synapse/rest/media/v1/media_repository.py index afc93601ea2d..7ef1b779dcec 100644 --- a/synapse/rest/media/v1/media_repository.py +++ b/synapse/rest/media/v1/media_repository.py @@ -923,7 +923,7 @@ async def _apply_media_retention_rules(self) -> None: ) async def delete_old_remote_media(self, before_ts: int) -> Dict[str, int]: - old_media = await self.store.get_remote_media_before( + old_media = await self.store.get_remote_media_ids( before_ts, include_quarantined_media=False ) @@ -995,7 +995,7 @@ async def delete_old_local_media( Returns: A tuple of (list of deleted media IDs, total deleted media IDs). """ - old_media = await self.store.get_local_media_before( + old_media = await self.store.get_local_media_ids( before_ts, size_gt, keep_profiles, diff --git a/synapse/storage/databases/main/media_repository.py b/synapse/storage/databases/main/media_repository.py index 6eadce9d73a9..5cb302864ae2 100644 --- a/synapse/storage/databases/main/media_repository.py +++ b/synapse/storage/databases/main/media_repository.py @@ -251,7 +251,7 @@ def get_local_media_by_user_paginate_txn( "get_local_media_by_user_paginate_txn", get_local_media_by_user_paginate_txn ) - async def get_local_media_before( + async def get_local_media_ids( self, before_ts: int, size_gt: int, @@ -321,12 +321,12 @@ async def get_local_media_before( AND quarantined_by IS NULL """ - def _get_local_media_before_txn(txn: LoggingTransaction) -> List[str]: + def _get_local_media_ids_txn(txn: LoggingTransaction) -> List[str]: txn.execute(sql, (before_ts, before_ts, size_gt)) return [row[0] for row in txn] return await self.db_pool.runInteraction( - "get_local_media_before", _get_local_media_before_txn + "get_local_media_ids", _get_local_media_ids_txn ) async def store_local_media( @@ -626,7 +626,7 @@ async def store_remote_media_thumbnail( desc="store_remote_media_thumbnail", ) - async def get_remote_media_before( + async def get_remote_media_ids( self, before_ts: int, include_quarantined_media: bool ) -> List[Dict[str, str]]: """ @@ -656,7 +656,7 @@ async def get_remote_media_before( """ return await self.db_pool.execute( - "get_remote_media_before", self.db_pool.cursor_to_dict, sql, before_ts + "get_remote_media_ids", self.db_pool.cursor_to_dict, sql, before_ts ) async def delete_remote_media(self, media_origin: str, media_id: str) -> None: From e60beb9e49572ca9bd87a0f5303dc3e98b389f64 Mon Sep 17 00:00:00 2001 From: Andrew Morgan Date: Mon, 6 Jun 2022 19:22:27 +0100 Subject: [PATCH 05/10] Remove 'local' from log line about potentially remote media --- synapse/rest/admin/media.py | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/synapse/rest/admin/media.py b/synapse/rest/admin/media.py index 8ca57bdb2830..19d4a008e89c 100644 --- a/synapse/rest/admin/media.py +++ b/synapse/rest/admin/media.py @@ -83,7 +83,7 @@ async def on_POST( requester = await self.auth.get_user_by_req(request) await assert_user_is_admin(self.auth, requester.user) - logging.info("Quarantining local media by user: %s", user_id) + logging.info("Quarantining media by user: %s", user_id) # Quarantine all media this user has uploaded num_quarantined = await self.store.quarantine_media_ids_by_user( @@ -112,7 +112,7 @@ async def on_POST( requester = await self.auth.get_user_by_req(request) await assert_user_is_admin(self.auth, requester.user) - logging.info("Quarantining local media by ID: %s/%s", server_name, media_id) + logging.info("Quarantining media by ID: %s/%s", server_name, media_id) # Quarantine this media id await self.store.quarantine_media_by_id( @@ -140,9 +140,7 @@ async def on_POST( ) -> Tuple[int, JsonDict]: await assert_requester_is_admin(self.auth, request) - logging.info( - "Remove from quarantine local media by ID: %s/%s", server_name, media_id - ) + logging.info("Remove from quarantine media by ID: %s/%s", server_name, media_id) # Remove from quarantine this media id await self.store.quarantine_media_by_id(server_name, media_id, None) From e4837e6fe13a2f45b6f7d87861d3aaea1f9ce727 Mon Sep 17 00:00:00 2001 From: Andrew Morgan Date: Mon, 6 Jun 2022 19:25:47 +0100 Subject: [PATCH 06/10] Changelog --- changelog.d/12972.feature | 1 + 1 file changed, 1 insertion(+) create mode 100644 changelog.d/12972.feature diff --git a/changelog.d/12972.feature b/changelog.d/12972.feature new file mode 100644 index 000000000000..3c73363d28d2 --- /dev/null +++ b/changelog.d/12972.feature @@ -0,0 +1 @@ +Add new `media_retention` options to the homeserver config for routinely cleaning up non-recently accessed media. \ No newline at end of file From 97f0c5d1ad178d01e9610533bb74431ddefe8fa6 Mon Sep 17 00:00:00 2001 From: Andrew Morgan Date: Tue, 7 Jun 2022 10:12:36 +0100 Subject: [PATCH 07/10] Additionally prevent protected media from being purged --- docs/usage/configuration/config_documentation.md | 6 ++++++ synapse/rest/media/v1/media_repository.py | 3 +++ synapse/storage/databases/main/media_repository.py | 11 ++++++++++- 3 files changed, 19 insertions(+), 1 deletion(-) diff --git a/docs/usage/configuration/config_documentation.md b/docs/usage/configuration/config_documentation.md index 1c75a23a3628..392ae80a759a 100644 --- a/docs/usage/configuration/config_documentation.md +++ b/docs/usage/configuration/config_documentation.md @@ -1583,6 +1583,12 @@ been accessed, the media's creation time is used instead. Both thumbnails and the original media will be removed. If either of these options are unset, then media of that type will not be purged. +Local or cached remote media that has been +[quarantined](../../admin_api/media_admin_api.md#quarantining-media-in-a-room) +will not be deleted. Similarly, local media that has been marked as +[protected from quarantine](../../admin_api/media_admin_api.md#protecting-media-from-being-quarantined) +will not be deleted. + Example configuration: ```yaml media_retention: diff --git a/synapse/rest/media/v1/media_repository.py b/synapse/rest/media/v1/media_repository.py index 7ef1b779dcec..e0c8ae8f1e79 100644 --- a/synapse/rest/media/v1/media_repository.py +++ b/synapse/rest/media/v1/media_repository.py @@ -920,6 +920,7 @@ async def _apply_media_retention_rules(self) -> None: before_ts=local_media_threshold_timestamp_ms, keep_profiles=True, delete_quarantined_media=False, + delete_protected_media=False, ) async def delete_old_remote_media(self, before_ts: int) -> Dict[str, int]: @@ -979,6 +980,7 @@ async def delete_old_local_media( size_gt: int = 0, keep_profiles: bool = True, delete_quarantined_media: bool = False, + delete_protected_media: bool = False, ) -> Tuple[List[str], int]: """ Delete local or remote media from this server by size and timestamp. Removes @@ -1000,6 +1002,7 @@ async def delete_old_local_media( size_gt, keep_profiles, include_quarantined_media=delete_quarantined_media, + include_protected_media=delete_protected_media, ) return await self._remove_local_media_from_disk(old_media) diff --git a/synapse/storage/databases/main/media_repository.py b/synapse/storage/databases/main/media_repository.py index 5cb302864ae2..cad11b6b05a9 100644 --- a/synapse/storage/databases/main/media_repository.py +++ b/synapse/storage/databases/main/media_repository.py @@ -257,6 +257,7 @@ async def get_local_media_ids( size_gt: int, keep_profiles: bool, include_quarantined_media: bool, + include_protected_media: bool, ) -> List[str]: """ Retrieve a list of media IDs from the local media store. @@ -274,6 +275,8 @@ async def get_local_media_ids( * a user's avatar in the user directory include_quarantined_media: If False, exclude media IDs from the results that have been marked as quarantined. + include_protected_media: If False, exclude media IDs from the results that have + been marked as protected from quarantine. Returns: A list of local media IDs. @@ -316,11 +319,17 @@ async def get_local_media_ids( sql += sql_keep if include_quarantined_media is False: - # Only include media that has not been quarantined + # Do not include media that has been quarantined sql += """ AND quarantined_by IS NULL """ + if include_protected_media is False: + # Do not include media that has been protected from quarantine + sql += """ + AND safe_from_quarantine = 0 + """ + def _get_local_media_ids_txn(txn: LoggingTransaction) -> List[str]: txn.execute(sql, (before_ts, before_ts, size_gt)) return [row[0] for row in txn] From bfba18b598ef9c048dc481122dcb3c4caf6308fb Mon Sep 17 00:00:00 2001 From: Andrew Morgan Date: Tue, 7 Jun 2022 10:17:06 +0100 Subject: [PATCH 08/10] Update media retention tests to include protected media --- tests/rest/media/test_media_retention.py | 29 ++++++++++++++++++++++-- 1 file changed, 27 insertions(+), 2 deletions(-) diff --git a/tests/rest/media/test_media_retention.py b/tests/rest/media/test_media_retention.py index c675deda8e94..14af07c5af96 100644 --- a/tests/rest/media/test_media_retention.py +++ b/tests/rest/media/test_media_retention.py @@ -53,14 +53,16 @@ def prepare(self, reactor: MemoryReactor, clock: Clock, hs: HomeServer) -> None: # Create a user to upload media with test_user_id = self.register_user("alice", "password") - # Inject media (3 images each; recently accessed, old access, never accessed) - # into both the local store and the remote cache + # Inject media (recently accessed, old access, never accessed, old access + # quarantined media) into both the local store and the remote cache, plus + # one additional local media that is marked as protected from quarantine. media_repository = hs.get_media_repository() test_media_content = b"example string" def _create_media_and_set_attributes( last_accessed_ms: Optional[int], is_quarantined: Optional[bool] = False, + is_protected: Optional[bool] = False, ) -> str: # "Upload" some media to the local media store mxc_uri = self.get_success( @@ -95,6 +97,15 @@ def _create_media_and_set_attributes( ) ) + if is_protected: + # Mark this media as protected from quarantine + self.get_success( + self.store.mark_local_media_as_safe( + media_id=media_id, + safe=True, + ) + ) + return media_id def _cache_remote_media_and_set_attributes( @@ -150,6 +161,12 @@ def _cache_remote_media_and_set_attributes( is_quarantined=True, ) ) + self.local_not_recently_accessed_protected_media = ( + _create_media_and_set_attributes( + last_accessed_ms=self.ONE_DAY_IN_MS, + is_protected=True, + ) + ) self.local_never_accessed_media = _create_media_and_set_attributes( last_accessed_ms=None, ) @@ -209,6 +226,10 @@ def test_local_media_retention(self) -> None: self.hs.config.server.server_name, self.local_not_recently_accessed_quarantined_media, ), + ( + self.hs.config.server.server_name, + self.local_not_recently_accessed_protected_media, + ), (self.remote_server_name, self.remote_recently_accessed_media), (self.remote_server_name, self.remote_not_recently_accessed_media), ( @@ -253,6 +274,10 @@ def test_remote_media_cache_retention(self) -> None: self.hs.config.server.server_name, self.local_not_recently_accessed_quarantined_media, ), + ( + self.hs.config.server.server_name, + self.local_not_recently_accessed_protected_media, + ), ( self.remote_server_name, self.remote_not_recently_accessed_quarantined_media, From 66c33bf5875f790b79d57624fba6eddff819f58f Mon Sep 17 00:00:00 2001 From: Andrew Morgan Date: Tue, 7 Jun 2022 11:26:54 +0100 Subject: [PATCH 09/10] Add missing arg to docstring --- synapse/rest/media/v1/media_repository.py | 1 + 1 file changed, 1 insertion(+) diff --git a/synapse/rest/media/v1/media_repository.py b/synapse/rest/media/v1/media_repository.py index e0c8ae8f1e79..7435fd9130c5 100644 --- a/synapse/rest/media/v1/media_repository.py +++ b/synapse/rest/media/v1/media_repository.py @@ -993,6 +993,7 @@ async def delete_old_local_media( keep_profiles: Switch to delete also files that are still used in image data (e.g user profile, room avatar). If false these files will be deleted. delete_quarantined_media: If True, media marked as quarantined will be deleted. + delete_protected_media: If True, media marked as protected will be deleted. Returns: A tuple of (list of deleted media IDs, total deleted media IDs). From 45a8d383afb0cf982dd3239da53ac1d9a3050bb8 Mon Sep 17 00:00:00 2001 From: Andrew Morgan Date: Tue, 7 Jun 2022 11:30:00 +0100 Subject: [PATCH 10/10] Use false, not 0, in SQL to check booleans --- synapse/storage/databases/main/media_repository.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/synapse/storage/databases/main/media_repository.py b/synapse/storage/databases/main/media_repository.py index cad11b6b05a9..3c585c555ad3 100644 --- a/synapse/storage/databases/main/media_repository.py +++ b/synapse/storage/databases/main/media_repository.py @@ -327,7 +327,7 @@ async def get_local_media_ids( if include_protected_media is False: # Do not include media that has been protected from quarantine sql += """ - AND safe_from_quarantine = 0 + AND safe_from_quarantine = false """ def _get_local_media_ids_txn(txn: LoggingTransaction) -> List[str]: