Refactored config line parsing logic.

Author: ray-kast

Cargo.lock

@@ -64,7 +64,6 @@ name = "holaplex-indexer-search"
 required-features = ["search"]
 
 [dependencies]
-arrayref = "0.3.6"
 async-trait = "0.1.52"
 crossbeam = { version = "0.8.1", optional = true }
 futures-util = "0.3.21"

crates/indexer/Cargo.toml

@@ -94,12 +94,13 @@ async fn process_config_lines(
 
     for config_line in &config_lines {
         let db_config_line = CMConfigLine {
-            candy_machine_address: Owned(bs58::encode(key).into_string()),
-            name: Owned(config_line.0.name.trim_matches(char::from(0)).to_owned()),
-            uri: Owned(config_line.0.uri.trim_matches(char::from(0)).to_owned()),
-            idx: i32::try_from(config_line.1).unwrap_or(-1i32),
+            candy_machine_address: Owned(key.to_string()),
+            name: Owned(config_line.0.name.trim_matches('\0').to_owned()),
+            uri: Owned(config_line.0.uri.trim_matches('\0').to_owned()),
+            idx: i32::try_from(config_line.1).unwrap_or(-1),
             taken: config_line.2,
         };
+
         db_config_lines.push(db_config_line);
     }
 
@@ -124,7 +125,7 @@ async fn process_config_lines(
             })
         })
         .await
-        .context("failed to insert candy machine config lines")?;
+        .context("Failed to insert candy machine config lines")?;
 
     Ok(())
 }

crates/indexer/src/geyser/accounts/candy_machine.rs

@@ -1,5 +1,4 @@
 use anchor_lang_v0_21::{AccountDeserialize, AnchorDeserialize};
-use arrayref::array_ref;
 use mpl_candy_machine::{
     CandyMachine, CollectionPDA, ConfigLine, CONFIG_ARRAY_START, CONFIG_LINE_SIZE,
 };
@@ -15,13 +14,13 @@ const COLLECTION_PDA_SIZE: usize = 8 + 64;
 /// returns a vector containing tuples: (`config_line`, index, taken)
 ///
 /// it is important that this not be called if the candy machine has hidden settings
-#[must_use]
 pub fn parse_cm_config_lines(
     data: &[u8],
     items_available: usize,
-) -> Vec<(ConfigLine, usize, bool)> {
-    let config_line_start = CONFIG_ARRAY_START + 4;
-    let available_bitmask_start = CONFIG_ARRAY_START + 4 + (items_available * CONFIG_LINE_SIZE) + 4;
+) -> Result<Vec<(ConfigLine, usize, bool)>> {
+    const CONFIG_LINE_START: usize = CONFIG_ARRAY_START + 4;
+    let available_bitmask_start = CONFIG_LINE_START + (items_available * CONFIG_LINE_SIZE) + 4;
+
     // NOTE(will): you would think that (items_available / 8) incorrectly computes the length of the
     // "available" bitmask. i.e. if there are 7 items available, this value would be 0.
     // however, this is how metaplex has coded it. It ends up working because there are 4 bytes of padding
@@ -35,11 +34,15 @@ pub fn parse_cm_config_lines(
     // Sanity check to make sure we aren't going to overflow data
     // This could occur if this function is called on a candy machine that uses hiddensettings instead of
     // config lines
-    let bytes_needed_for_taken_bitmask =
-        (items_available / 8) + if items_available % 8 == 0 { 0 } else { 1 };
-    if taken_bitmask_start + bytes_needed_for_taken_bitmask >= data.len() {
-        // TODO(will): Log warning
-        return Vec::new();
+    let bytes_needed_for_taken_bitmask = items_available / 8 + (items_available % 8).min(1);
+    let expected_taken_len = taken_bitmask_start + bytes_needed_for_taken_bitmask;
+
+    if expected_taken_len >= data.len() {
+        bail!(
+            "Config line bytes would overflow available data ({} vs {})",
+            expected_taken_len,
+            data.len()
+        );
     }
 
     // (config_line, index, taken)
@@ -52,23 +55,22 @@ pub fn parse_cm_config_lines(
 
         // NOTE(will): if the config line is not available, we simply ignore it
         if available {
-            let config_line_byte_offset = config_line_start + (idx * CONFIG_LINE_SIZE);
-            let config_line_data = array_ref![data, config_line_byte_offset, CONFIG_LINE_SIZE];
-            let config_line_result = ConfigLine::deserialize(&mut config_line_data.as_slice());
+            let config_line_byte_offset = CONFIG_LINE_START + (idx * CONFIG_LINE_SIZE);
+            let config_line = ConfigLine::deserialize(
+                &mut &data[config_line_byte_offset..config_line_byte_offset + CONFIG_LINE_SIZE],
+            )
+            .with_context(|| format!("Failed to deserialize config line at index {}", idx))?;
+
             let taken_bitmask_byte_offset = idx / 8;
             let taken_bitmask_bit_offset = 7 - (idx % 8);
             let taken_bitmask_value = data[taken_bitmask_start + taken_bitmask_byte_offset];
             let taken = taken_bitmask_value & (1 << taken_bitmask_bit_offset) != 0;
 
-            if let Ok(config_line) = config_line_result {
-                config_lines.push((config_line, idx, taken));
-            } else {
-                // TODO(will): log some warning here that might alert us to a problem with this code?
-            }
+            config_lines.push((config_line, idx, taken));
         }
     }
 
-    config_lines
+    Ok(config_lines)
 }
 
 pub async fn process_collection_pda(client: &Client, update: AccountUpdate) -> Result<()> {
@@ -82,19 +84,19 @@ pub async fn process_cm(client: &Client, update: AccountUpdate) -> Result<()> {
     let candy_machine: CandyMachine = CandyMachine::try_deserialize(&mut update.data.as_slice())
         .context("Failed to deserialize candy_machine")?;
 
-    let items_available = usize::try_from(candy_machine.data.items_available);
-    // TODO(will): log warning if conversion fails
-
-    match (
-        items_available,
-        candy_machine.data.hidden_settings.is_none(),
-    ) {
-        (Ok(items_available), true) => {
-            let config_lines = parse_cm_config_lines(&update.data, items_available);
-            candy_machine::process(client, update.key, candy_machine, Some(config_lines)).await
-        },
-        _ => candy_machine::process(client, update.key, candy_machine, None).await,
-    }
+    let items_available = usize::try_from(candy_machine.data.items_available)
+        .context("Failed to convert available item count")?;
+
+    let lines = if candy_machine.data.hidden_settings.is_some() {
+        Some(
+            parse_cm_config_lines(&update.data, items_available)
+                .context("Failed to parse candy machine lines")?,
+        )
+    } else {
+        None
+    };
+
+    candy_machine::process(client, update.key, candy_machine, lines).await
 }
 
 pub(crate) async fn process(client: &Client, update: AccountUpdate) -> Result<()> {
@@ -106,32 +108,27 @@ pub(crate) async fn process(client: &Client, update: AccountUpdate) -> Result<()
 
 #[cfg(test)]
 mod tests {
-    use std::{env, fs, io::Read, path::Path};
+    use std::{env, fs, io::prelude::*, path::Path};
 
     use anchor_lang_v0_21::AccountDeserialize;
     use mpl_candy_machine::CandyMachine;
 
-    use crate::geyser::programs::candy_machine::parse_cm_config_lines;
+    use super::parse_cm_config_lines;
+    use crate::prelude::*;
 
-    fn get_file_as_byte_vec<P: AsRef<Path>>(filename: P) -> Vec<u8> {
-        let mut f = fs::File::open(&filename).expect("no file found");
-        let mut buffer = vec![];
-        f.read_to_end(&mut buffer).expect("File read failed");
-        buffer
-    }
+    fn load_account_dump(filename: impl AsRef<Path>) -> Result<Vec<u8>> {
+        let mut path = env::current_dir().context("Failed to get working dir")?;
+        path.extend(["tests", "data"]);
+        path.push(filename);
 
-    fn get_data_dir() -> std::path::PathBuf {
-        let mut data_dir = env::current_dir().unwrap();
-        data_dir.push("tests");
-        data_dir.push("data");
-        data_dir.to_owned()
-    }
+        println!("Loading: {:?}", path);
+
+        let mut f = fs::File::open(&path).with_context(|| format!("Failed to open {:?}", path))?;
+        let mut buffer = vec![];
+        f.read_to_end(&mut buffer)
+            .with_context(|| format!("Failed to read {:?}", path))?;
 
-    fn load_account_dump<P: AsRef<Path>>(filename: P) -> Vec<u8> {
-        let data_dir = get_data_dir();
-        let full_path = data_dir.join(filename);
-        println!("Loading: {:?}", full_path);
-        return get_file_as_byte_vec(full_path);
+        Ok(buffer)
     }
 
     #[test]
@@ -146,15 +143,16 @@ mod tests {
 
         for filename in filenames {
             println!("Reading Candy Machine {:?}", filename);
-            let data = load_account_dump(filename);
+            let data = load_account_dump(filename).unwrap();
             let cm = CandyMachine::try_deserialize(&mut data.as_slice()).unwrap();
             println!("Candy Machine: {}", filename);
 
-            let results = parse_cm_config_lines(&data, cm.data.items_available as usize);
+            let avail = usize::try_from(cm.data.items_available).unwrap();
+            let results = parse_cm_config_lines(&data, avail).unwrap();
 
             let available_count = results.len();
             let mut taken_count = 0;
-            for (_, _, taken) in results.iter() {
+            for (_, _, taken) in &results {
                 if *taken {
                     taken_count += 1;
                 }
@@ -164,7 +162,7 @@ mod tests {
                 assert_eq!(available_count, 0);
                 assert_eq!(taken_count, 0);
             } else {
-                assert_eq!(available_count, cm.data.items_available as usize);
+                assert_eq!(available_count, avail);
                 assert_eq!(taken_count, cm.items_redeemed);
             }
         }