From d9acbdd5398c2d764e0b0b726c98b0744303a2aa Mon Sep 17 00:00:00 2001
From: "guillaume.bouvignies" <guillaume.bouvignies@kurtosistech.com>
Date: Fri, 11 Aug 2023 16:18:54 +0200
Subject: [PATCH 1/7] Add possibility to configure a subfolder inside an S3
 bucket

---
 .../hyperlane-base/src/types/checkpoint_syncer.rs | 12 ++++++++++--
 rust/hyperlane-base/src/types/s3_storage.rs       | 15 +++++++++++++--
 2 files changed, 23 insertions(+), 4 deletions(-)

diff --git a/rust/hyperlane-base/src/types/checkpoint_syncer.rs b/rust/hyperlane-base/src/types/checkpoint_syncer.rs
index fdfd9190aa..044bfa39e8 100644
--- a/rust/hyperlane-base/src/types/checkpoint_syncer.rs
+++ b/rust/hyperlane-base/src/types/checkpoint_syncer.rs
@@ -23,6 +23,8 @@ pub enum CheckpointSyncerConf {
     S3 {
         /// Bucket name
         bucket: String,
+        /// Folder name inside bucket
+        folder: String
         /// S3 Region
         region: Region,
     },
@@ -41,6 +43,8 @@ pub enum RawCheckpointSyncerConf {
     S3 {
         /// Bucket name
         bucket: Option<String>,
+        // Folder name inside bucket - defaults to the root of the bucket (i.e. empty string)
+        folder: Option<String>
         /// S3 Region
         region: Option<String>,
     },
@@ -79,10 +83,13 @@ impl FromRawConf<'_, RawCheckpointSyncerConf> for CheckpointSyncerConf {
                 }
                 Ok(Self::LocalStorage { path })
             }
-            RawCheckpointSyncerConf::S3 { bucket, region } => Ok(Self::S3 {
+            RawCheckpointSyncerConf::S3 { bucket, folder, region } => Ok(Self::S3 {
                 bucket: bucket
                     .ok_or_else(|| eyre!("Missing `bucket` for S3 checkpoint syncer"))
                     .into_config_result(|| cwp + "bucket")?,
+                folder: folder
+                    .ok_or_else("")
+                    .into_config_result(|| cwp + "folder")?,
                 region: region
                     .ok_or_else(|| eyre!("Missing `region` for S3 checkpoint syncer"))
                     .into_config_result(|| cwp + "region")?
@@ -136,8 +143,9 @@ impl CheckpointSyncerConf {
             CheckpointSyncerConf::LocalStorage { path } => {
                 Box::new(LocalStorage::new(path.clone(), latest_index_gauge)?)
             }
-            CheckpointSyncerConf::S3 { bucket, region } => Box::new(S3Storage::new(
+            CheckpointSyncerConf::S3 { bucket, folder, region } => Box::new(S3Storage::new(
                 bucket.clone(),
+                folder.clone(),
                 region.clone(),
                 latest_index_gauge,
             )),
diff --git a/rust/hyperlane-base/src/types/s3_storage.rs b/rust/hyperlane-base/src/types/s3_storage.rs
index cc2c43cbb3..20359e3e5d 100644
--- a/rust/hyperlane-base/src/types/s3_storage.rs
+++ b/rust/hyperlane-base/src/types/s3_storage.rs
@@ -28,6 +28,8 @@ const S3_REQUEST_TIMEOUT_SECONDS: u64 = 30;
 pub struct S3Storage {
     /// The name of the bucket.
     bucket: String,
+    /// A specific folder inside the above repo - set to empty string to use the root of the bucket
+    folder: String,
     /// The region of the bucket.
     region: Region,
     /// A client with AWS credentials.
@@ -44,6 +46,7 @@ impl fmt::Debug for S3Storage {
     fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
         f.debug_struct("S3Storage")
             .field("bucket", &self.bucket)
+            .field("folder", &self.folder)
             .field("region", &self.region)
             .finish()
     }
@@ -52,7 +55,7 @@ impl fmt::Debug for S3Storage {
 impl S3Storage {
     async fn write_to_bucket(&self, key: String, body: &str) -> Result<()> {
         let req = PutObjectRequest {
-            key,
+            key: self.get_composite_key(key),
             bucket: self.bucket.clone(),
             body: Some(Vec::from(body).into()),
             content_type: Some("application/json".to_owned()),
@@ -69,7 +72,7 @@ impl S3Storage {
     /// Uses an anonymous client. This should only be used for publicly accessible buckets.
     async fn anonymously_read_from_bucket(&self, key: String) -> Result<Option<Vec<u8>>> {
         let req = GetObjectRequest {
-            key,
+            key: self.get_composite_key(key),
             bucket: self.bucket.clone(),
             ..Default::default()
         };
@@ -120,6 +123,14 @@ impl S3Storage {
         })
     }
 
+    fn get_composite_key(&self, key: String) -> String {
+        if self.folder != "" {
+            key
+        } else {
+            format!("{}/{}", self.folder, key)
+        }
+    }
+
     fn legacy_checkpoint_key(index: u32) -> String {
         format!("checkpoint_{index}.json")
     }

From a523729301d778f7daa020a42d554d45cbe892e4 Mon Sep 17 00:00:00 2001
From: "guillaume.bouvignies" <guillaume.bouvignies@kurtosistech.com>
Date: Wed, 16 Aug 2023 11:34:17 +0200
Subject: [PATCH 2/7] Update announcement

---
 .../src/types/checkpoint_syncer.rs            | 20 +++++++++++--------
 rust/hyperlane-base/src/types/s3_storage.rs   |  2 +-
 2 files changed, 13 insertions(+), 9 deletions(-)

diff --git a/rust/hyperlane-base/src/types/checkpoint_syncer.rs b/rust/hyperlane-base/src/types/checkpoint_syncer.rs
index 044bfa39e8..93c05d0720 100644
--- a/rust/hyperlane-base/src/types/checkpoint_syncer.rs
+++ b/rust/hyperlane-base/src/types/checkpoint_syncer.rs
@@ -24,7 +24,7 @@ pub enum CheckpointSyncerConf {
         /// Bucket name
         bucket: String,
         /// Folder name inside bucket
-        folder: String
+        folder: String,
         /// S3 Region
         region: Region,
     },
@@ -43,8 +43,8 @@ pub enum RawCheckpointSyncerConf {
     S3 {
         /// Bucket name
         bucket: Option<String>,
-        // Folder name inside bucket - defaults to the root of the bucket (i.e. empty string)
-        folder: Option<String>
+        /// Folder name inside bucket - defaults to the root of the bucket (i.e. empty string)
+        folder: Option<String>,
         /// S3 Region
         region: Option<String>,
     },
@@ -88,7 +88,7 @@ impl FromRawConf<'_, RawCheckpointSyncerConf> for CheckpointSyncerConf {
                     .ok_or_else(|| eyre!("Missing `bucket` for S3 checkpoint syncer"))
                     .into_config_result(|| cwp + "bucket")?,
                 folder: folder
-                    .ok_or_else("")
+                    .map_or(Ok::<String, Report>(String::from("")), Result::Ok)
                     .into_config_result(|| cwp + "folder")?,
                 region: region
                     .ok_or_else(|| eyre!("Missing `region` for S3 checkpoint syncer"))
@@ -113,13 +113,17 @@ impl FromStr for CheckpointSyncerConf {
 
         match prefix {
             "s3" => {
-                let [bucket, region]: [&str; 2] = suffix
+                let url_components = suffix
                     .split('/')
-                    .collect::<Vec<_>>()
-                    .try_into()
-                    .map_err(|_| eyre!("Error parsing storage location; could not split bucket and region ({suffix})"))?;
+                    .collect::<Vec<&str>>();
+                let [bucket, region, folder]: [&str; 3] = match url_components[..] {
+                    [bucket, region] => Ok([bucket, region, ""]), // no folder means empty folder path
+                    [bucket, region, folder] => Ok([bucket, region, folder]),
+                    _ => Err(eyre!("Error parsing storage location; could not split bucket, region and folder ({suffix})"))
+                }?;
                 Ok(CheckpointSyncerConf::S3 {
                     bucket: bucket.into(),
+                    folder: folder.into(),
                     region: region
                         .parse()
                         .context("Invalid region when parsing storage location")?,
diff --git a/rust/hyperlane-base/src/types/s3_storage.rs b/rust/hyperlane-base/src/types/s3_storage.rs
index 20359e3e5d..4b84145d2e 100644
--- a/rust/hyperlane-base/src/types/s3_storage.rs
+++ b/rust/hyperlane-base/src/types/s3_storage.rs
@@ -220,6 +220,6 @@ impl CheckpointSyncer for S3Storage {
     }
 
     fn announcement_location(&self) -> String {
-        format!("s3://{}/{}", self.bucket, self.region.name())
+        format!("s3://{}/{}/{}", self.bucket, self.region.name(), self.folder)
     }
 }

From 1147537c02738b1cabd4753a43f4adef3e2ed866 Mon Sep 17 00:00:00 2001
From: "guillaume.bouvignies" <guillaume.bouvignies@kurtosistech.com>
Date: Mon, 21 Aug 2023 14:29:32 +0200
Subject: [PATCH 3/7] Handle case of nested folders and fix condition

---
 rust/hyperlane-base/src/types/checkpoint_syncer.rs | 11 ++++++++---
 rust/hyperlane-base/src/types/s3_storage.rs        |  8 ++++++--
 2 files changed, 14 insertions(+), 5 deletions(-)

diff --git a/rust/hyperlane-base/src/types/checkpoint_syncer.rs b/rust/hyperlane-base/src/types/checkpoint_syncer.rs
index 93c05d0720..9b61ae9c01 100644
--- a/rust/hyperlane-base/src/types/checkpoint_syncer.rs
+++ b/rust/hyperlane-base/src/types/checkpoint_syncer.rs
@@ -8,6 +8,7 @@ use rusoto_core::Region;
 use serde::Deserialize;
 
 use hyperlane_core::{config::*, H160};
+use tracing::info;
 
 use crate::{CheckpointSyncer, LocalStorage, MultisigCheckpointSyncer, S3Storage};
 
@@ -116,9 +117,13 @@ impl FromStr for CheckpointSyncerConf {
                 let url_components = suffix
                     .split('/')
                     .collect::<Vec<&str>>();
-                let [bucket, region, folder]: [&str; 3] = match url_components[..] {
-                    [bucket, region] => Ok([bucket, region, ""]), // no folder means empty folder path
-                    [bucket, region, folder] => Ok([bucket, region, folder]),
+                let mut folder = String::from("");
+                let [bucket, region]: [&str; 2] = match url_components.len() {
+                    2 => Ok([url_components[0], url_components[1]]),
+                    3 .. => {
+                        folder.push_str(url_components[2..].join("/").as_str());
+                        Ok([url_components[0], url_components[1]])
+                    },
                     _ => Err(eyre!("Error parsing storage location; could not split bucket, region and folder ({suffix})"))
                 }?;
                 Ok(CheckpointSyncerConf::S3 {
diff --git a/rust/hyperlane-base/src/types/s3_storage.rs b/rust/hyperlane-base/src/types/s3_storage.rs
index 4b84145d2e..91df2de693 100644
--- a/rust/hyperlane-base/src/types/s3_storage.rs
+++ b/rust/hyperlane-base/src/types/s3_storage.rs
@@ -124,7 +124,7 @@ impl S3Storage {
     }
 
     fn get_composite_key(&self, key: String) -> String {
-        if self.folder != "" {
+        if self.folder == "" {
             key
         } else {
             format!("{}/{}", self.folder, key)
@@ -220,6 +220,10 @@ impl CheckpointSyncer for S3Storage {
     }
 
     fn announcement_location(&self) -> String {
-        format!("s3://{}/{}/{}", self.bucket, self.region.name(), self.folder)
+        if self.folder == "" {
+            format!("s3://{}/{}", self.bucket, self.region.name())
+        } else {
+            format!("s3://{}/{}/{}", self.bucket, self.region.name(), self.folder)
+        }
     }
 }

From 2c46d7d0293bf19aa148c650755317f0c951cb5a Mon Sep 17 00:00:00 2001
From: "guillaume.bouvignies" <guillaume.bouvignies@kurtosistech.com>
Date: Mon, 21 Aug 2023 19:10:39 +0200
Subject: [PATCH 4/7] make folder an Option<String>

---
 .../src/types/checkpoint_syncer.rs              | 17 +++++------------
 rust/hyperlane-base/src/types/s3_storage.rs     | 16 +++++++---------
 2 files changed, 12 insertions(+), 21 deletions(-)

diff --git a/rust/hyperlane-base/src/types/checkpoint_syncer.rs b/rust/hyperlane-base/src/types/checkpoint_syncer.rs
index 9b61ae9c01..39ae2f410a 100644
--- a/rust/hyperlane-base/src/types/checkpoint_syncer.rs
+++ b/rust/hyperlane-base/src/types/checkpoint_syncer.rs
@@ -8,7 +8,6 @@ use rusoto_core::Region;
 use serde::Deserialize;
 
 use hyperlane_core::{config::*, H160};
-use tracing::info;
 
 use crate::{CheckpointSyncer, LocalStorage, MultisigCheckpointSyncer, S3Storage};
 
@@ -25,7 +24,7 @@ pub enum CheckpointSyncerConf {
         /// Bucket name
         bucket: String,
         /// Folder name inside bucket
-        folder: String,
+        folder: Option<String>,
         /// S3 Region
         region: Region,
     },
@@ -88,9 +87,7 @@ impl FromRawConf<'_, RawCheckpointSyncerConf> for CheckpointSyncerConf {
                 bucket: bucket
                     .ok_or_else(|| eyre!("Missing `bucket` for S3 checkpoint syncer"))
                     .into_config_result(|| cwp + "bucket")?,
-                folder: folder
-                    .map_or(Ok::<String, Report>(String::from("")), Result::Ok)
-                    .into_config_result(|| cwp + "folder")?,
+                folder,
                 region: region
                     .ok_or_else(|| eyre!("Missing `region` for S3 checkpoint syncer"))
                     .into_config_result(|| cwp + "region")?
@@ -117,13 +114,9 @@ impl FromStr for CheckpointSyncerConf {
                 let url_components = suffix
                     .split('/')
                     .collect::<Vec<&str>>();
-                let mut folder = String::from("");
-                let [bucket, region]: [&str; 2] = match url_components.len() {
-                    2 => Ok([url_components[0], url_components[1]]),
-                    3 .. => {
-                        folder.push_str(url_components[2..].join("/").as_str());
-                        Ok([url_components[0], url_components[1]])
-                    },
+                let (bucket, region, folder): (&str, &str, Option<String>) = match url_components.len() {
+                    2 => Ok((url_components[0], url_components[1], None)),
+                    3 .. => Ok((url_components[0], url_components[1], Some(url_components[2..].join("/")))),
                     _ => Err(eyre!("Error parsing storage location; could not split bucket, region and folder ({suffix})"))
                 }?;
                 Ok(CheckpointSyncerConf::S3 {
diff --git a/rust/hyperlane-base/src/types/s3_storage.rs b/rust/hyperlane-base/src/types/s3_storage.rs
index 91df2de693..2c048725a6 100644
--- a/rust/hyperlane-base/src/types/s3_storage.rs
+++ b/rust/hyperlane-base/src/types/s3_storage.rs
@@ -29,7 +29,7 @@ pub struct S3Storage {
     /// The name of the bucket.
     bucket: String,
     /// A specific folder inside the above repo - set to empty string to use the root of the bucket
-    folder: String,
+    folder: Option<String>,
     /// The region of the bucket.
     region: Region,
     /// A client with AWS credentials.
@@ -124,10 +124,9 @@ impl S3Storage {
     }
 
     fn get_composite_key(&self, key: String) -> String {
-        if self.folder == "" {
-            key
-        } else {
-            format!("{}/{}", self.folder, key)
+        match self.folder.as_deref() {
+            None | Some("") => key,
+            Some(folder_str) => format!("{}/{}", folder_str, key),
         }
     }
 
@@ -220,10 +219,9 @@ impl CheckpointSyncer for S3Storage {
     }
 
     fn announcement_location(&self) -> String {
-        if self.folder == "" {
-            format!("s3://{}/{}", self.bucket, self.region.name())
-        } else {
-            format!("s3://{}/{}/{}", self.bucket, self.region.name(), self.folder)
+        match self.folder.as_deref() {
+            None | Some("") => format!("s3://{}/{}", self.bucket, self.region.name()),
+            Some(folder_str) => format!("s3://{}/{}/{}", self.bucket, self.region.name(), folder_str),
         }
     }
 }

From a39f48d71300233e71b44536c047f0ff18d57baf Mon Sep 17 00:00:00 2001
From: "guillaume.bouvignies" <guillaume.bouvignies@kurtosistech.com>
Date: Mon, 21 Aug 2023 20:06:54 +0200
Subject: [PATCH 5/7] Update RawCheckpointSyncerConf in parser.rs

---
 rust/hyperlane-base/src/settings/checkpoint_syncer.rs | 2 +-
 rust/hyperlane-base/src/settings/parser.rs            | 5 ++++-
 2 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/rust/hyperlane-base/src/settings/checkpoint_syncer.rs b/rust/hyperlane-base/src/settings/checkpoint_syncer.rs
index df643cdc24..538ad81bfb 100644
--- a/rust/hyperlane-base/src/settings/checkpoint_syncer.rs
+++ b/rust/hyperlane-base/src/settings/checkpoint_syncer.rs
@@ -20,7 +20,7 @@ pub enum CheckpointSyncerConf {
     S3 {
         /// Bucket name
         bucket: String,
-        /// Folder name inside bucket
+        /// Folder name inside bucket - defaults to the root of the bucket
         folder: Option<String>,
         /// S3 Region
         region: Region,
diff --git a/rust/hyperlane-base/src/settings/parser.rs b/rust/hyperlane-base/src/settings/parser.rs
index 37320a6065..42afa9c513 100644
--- a/rust/hyperlane-base/src/settings/parser.rs
+++ b/rust/hyperlane-base/src/settings/parser.rs
@@ -182,6 +182,8 @@ pub enum RawCheckpointSyncerConf {
     S3 {
         /// Bucket name
         bucket: Option<String>,
+        /// Folder name inside bucket - defaults to the root of the bucket
+        folder: Option<String>,
         /// S3 Region
         region: Option<String>,
     },
@@ -642,10 +644,11 @@ impl FromRawConf<RawCheckpointSyncerConf> for CheckpointSyncerConf {
                 }
                 Ok(Self::LocalStorage { path })
             }
-            RawCheckpointSyncerConf::S3 { bucket, region } => Ok(Self::S3 {
+            RawCheckpointSyncerConf::S3 { bucket, folder, region } => Ok(Self::S3 {
                 bucket: bucket
                     .ok_or_else(|| eyre!("Missing `bucket` for S3 checkpoint syncer"))
                     .into_config_result(|| cwp + "bucket")?,
+                folder,
                 region: region
                     .ok_or_else(|| eyre!("Missing `region` for S3 checkpoint syncer"))
                     .into_config_result(|| cwp + "region")?

From 6ea14bc5492550b196829fb267dcfc9927b3152a Mon Sep 17 00:00:00 2001
From: "guillaume.bouvignies" <guillaume.bouvignies@kurtosistech.com>
Date: Thu, 24 Aug 2023 09:50:28 +0200
Subject: [PATCH 6/7] Fix linter

---
 rust/hyperlane-base/src/settings/checkpoint_syncer.rs | 10 ++++++----
 rust/hyperlane-base/src/settings/parser.rs            |  6 +++++-
 rust/hyperlane-base/src/types/s3_storage.rs           |  4 +++-
 3 files changed, 14 insertions(+), 6 deletions(-)

diff --git a/rust/hyperlane-base/src/settings/checkpoint_syncer.rs b/rust/hyperlane-base/src/settings/checkpoint_syncer.rs
index 538ad81bfb..ec8be86de6 100644
--- a/rust/hyperlane-base/src/settings/checkpoint_syncer.rs
+++ b/rust/hyperlane-base/src/settings/checkpoint_syncer.rs
@@ -38,9 +38,7 @@ impl FromStr for CheckpointSyncerConf {
 
         match prefix {
             "s3" => {
-                let url_components = suffix
-                    .split('/')
-                    .collect::<Vec<&str>>();
+                let url_components = suffix.split('/').collect::<Vec<&str>>();
                 let (bucket, region, folder): (&str, &str, Option<String>) = match url_components.len() {
                     2 => Ok((url_components[0], url_components[1], None)),
                     3 .. => Ok((url_components[0], url_components[1], Some(url_components[2..].join("/")))),
@@ -72,7 +70,11 @@ impl CheckpointSyncerConf {
             CheckpointSyncerConf::LocalStorage { path } => {
                 Box::new(LocalStorage::new(path.clone(), latest_index_gauge)?)
             }
-            CheckpointSyncerConf::S3 { bucket, folder, region } => Box::new(S3Storage::new(
+            CheckpointSyncerConf::S3 {
+                bucket,
+                folder,
+                region,
+            } => Box::new(S3Storage::new(
                 bucket.clone(),
                 folder.clone(),
                 region.clone(),
diff --git a/rust/hyperlane-base/src/settings/parser.rs b/rust/hyperlane-base/src/settings/parser.rs
index 42afa9c513..d1aa162909 100644
--- a/rust/hyperlane-base/src/settings/parser.rs
+++ b/rust/hyperlane-base/src/settings/parser.rs
@@ -644,7 +644,11 @@ impl FromRawConf<RawCheckpointSyncerConf> for CheckpointSyncerConf {
                 }
                 Ok(Self::LocalStorage { path })
             }
-            RawCheckpointSyncerConf::S3 { bucket, folder, region } => Ok(Self::S3 {
+            RawCheckpointSyncerConf::S3 {
+                bucket,
+                folder,
+                region,
+            } => Ok(Self::S3 {
                 bucket: bucket
                     .ok_or_else(|| eyre!("Missing `bucket` for S3 checkpoint syncer"))
                     .into_config_result(|| cwp + "bucket")?,
diff --git a/rust/hyperlane-base/src/types/s3_storage.rs b/rust/hyperlane-base/src/types/s3_storage.rs
index 2c048725a6..544bf6e229 100644
--- a/rust/hyperlane-base/src/types/s3_storage.rs
+++ b/rust/hyperlane-base/src/types/s3_storage.rs
@@ -221,7 +221,9 @@ impl CheckpointSyncer for S3Storage {
     fn announcement_location(&self) -> String {
         match self.folder.as_deref() {
             None | Some("") => format!("s3://{}/{}", self.bucket, self.region.name()),
-            Some(folder_str) => format!("s3://{}/{}/{}", self.bucket, self.region.name(), folder_str),
+            Some(folder_str) => {
+                format!("s3://{}/{}/{}", self.bucket, self.region.name(), folder_str)
+            }
         }
     }
 }

From a83aa6e20255d6b48ba3ff513e5ffacdcdfbc215 Mon Sep 17 00:00:00 2001
From: "guillaume.bouvignies" <guillaume.bouvignies@kurtosistech.com>
Date: Mon, 28 Aug 2023 10:17:08 +0200
Subject: [PATCH 7/7] Fix cargo clippy

---
 rust/hyperlane-base/src/settings/checkpoint_syncer.rs | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/rust/hyperlane-base/src/settings/checkpoint_syncer.rs b/rust/hyperlane-base/src/settings/checkpoint_syncer.rs
index ec8be86de6..342f44732c 100644
--- a/rust/hyperlane-base/src/settings/checkpoint_syncer.rs
+++ b/rust/hyperlane-base/src/settings/checkpoint_syncer.rs
@@ -46,7 +46,7 @@ impl FromStr for CheckpointSyncerConf {
                 }?;
                 Ok(CheckpointSyncerConf::S3 {
                     bucket: bucket.into(),
-                    folder: folder.into(),
+                    folder,
                     region: region
                         .parse()
                         .context("Invalid region when parsing storage location")?,