Populate memo in blockstore signatures-for-address (#19515)

* Add TransactionMemos column family

* Traitify extract_memos

* Write TransactionMemos in TransactionStatusService

* Populate memos from column

* Dedupe and add unit test

ledger/src/blockstore.rs

@@ -140,6 +140,7 @@ pub struct Blockstore {
     code_shred_cf: LedgerColumn<cf::ShredCode>,
     transaction_status_cf: LedgerColumn<cf::TransactionStatus>,
     address_signatures_cf: LedgerColumn<cf::AddressSignatures>,
+    transaction_memos_cf: LedgerColumn<cf::TransactionMemos>,
     transaction_status_index_cf: LedgerColumn<cf::TransactionStatusIndex>,
     active_transaction_status_index: RwLock<u64>,
     rewards_cf: LedgerColumn<cf::Rewards>,
@@ -342,6 +343,7 @@ impl Blockstore {
         let code_shred_cf = db.column();
         let transaction_status_cf = db.column();
         let address_signatures_cf = db.column();
+        let transaction_memos_cf = db.column();
         let transaction_status_index_cf = db.column();
         let rewards_cf = db.column();
         let blocktime_cf = db.column();
@@ -391,6 +393,7 @@ impl Blockstore {
             code_shred_cf,
             transaction_status_cf,
             address_signatures_cf,
+            transaction_memos_cf,
             transaction_status_index_cf,
             active_transaction_status_index: RwLock::new(active_transaction_status_index),
             rewards_cf,
@@ -2112,6 +2115,14 @@ impl Blockstore {
         Ok(())
     }
 
+    pub fn read_transaction_memos(&self, signature: Signature) -> Result<Option<String>> {
+        self.transaction_memos_cf.get(signature)
+    }
+
+    pub fn write_transaction_memos(&self, signature: &Signature, memos: String) -> Result<()> {
+        self.transaction_memos_cf.put(*signature, &memos)
+    }
+
     fn check_lowest_cleanup_slot(&self, slot: Slot) -> Result<std::sync::RwLockReadGuard<Slot>> {
         // lowest_cleanup_slot is the last slot that was not cleaned up by LedgerCleanupService
         let lowest_cleanup_slot = self.lowest_cleanup_slot.read().unwrap();
@@ -2612,12 +2623,13 @@ impl Blockstore {
             let transaction_status =
                 self.get_transaction_status(signature, &confirmed_unrooted_slots)?;
             let err = transaction_status.and_then(|(_slot, status)| status.status.err());
+            let memo = self.read_transaction_memos(signature)?;
             let block_time = self.get_block_time(slot)?;
             infos.push(ConfirmedTransactionStatusWithSignature {
                 signature,
                 slot,
                 err,
-                memo: None,
+                memo,
                 block_time,
             });
         }

ledger/src/blockstore_db.rs

@@ -61,6 +61,8 @@ const CODE_SHRED_CF: &str = "code_shred";
 const TRANSACTION_STATUS_CF: &str = "transaction_status";
 /// Column family for Address Signatures
 const ADDRESS_SIGNATURES_CF: &str = "address_signatures";
+/// Column family for TransactionMemos
+const TRANSACTION_MEMOS_CF: &str = "transaction_memos";
 /// Column family for the Transaction Status Index.
 /// This column family is used for tracking the active primary index for columns that for
 /// query performance reasons should not be indexed by Slot.
@@ -164,6 +166,10 @@ pub mod columns {
     /// The address signatures column
     pub struct AddressSignatures;
 
+    #[derive(Debug)]
+    // The transaction memos column
+    pub struct TransactionMemos;
+
     #[derive(Debug)]
     /// The transaction status index column
     pub struct TransactionStatusIndex;
@@ -333,6 +339,10 @@ impl Rocks {
             AddressSignatures::NAME,
             get_cf_options::<AddressSignatures>(&access_type, &oldest_slot),
         );
+        let transaction_memos_cf_descriptor = ColumnFamilyDescriptor::new(
+            TransactionMemos::NAME,
+            get_cf_options::<TransactionMemos>(&access_type, &oldest_slot),
+        );
         let transaction_status_index_cf_descriptor = ColumnFamilyDescriptor::new(
             TransactionStatusIndex::NAME,
             get_cf_options::<TransactionStatusIndex>(&access_type, &oldest_slot),
@@ -373,6 +383,7 @@ impl Rocks {
             (ShredCode::NAME, shred_code_cf_descriptor),
             (TransactionStatus::NAME, transaction_status_cf_descriptor),
             (AddressSignatures::NAME, address_signatures_cf_descriptor),
+            (TransactionMemos::NAME, transaction_memos_cf_descriptor),
             (
                 TransactionStatusIndex::NAME,
                 transaction_status_index_cf_descriptor,
@@ -495,6 +506,7 @@ impl Rocks {
             ShredCode::NAME,
             TransactionStatus::NAME,
             AddressSignatures::NAME,
+            TransactionMemos::NAME,
             TransactionStatusIndex::NAME,
             Rewards::NAME,
             Blocktime::NAME,
@@ -595,6 +607,10 @@ impl TypedColumn for columns::AddressSignatures {
     type Type = blockstore_meta::AddressSignatureMeta;
 }
 
+impl TypedColumn for columns::TransactionMemos {
+    type Type = String;
+}
+
 impl TypedColumn for columns::TransactionStatusIndex {
     type Type = blockstore_meta::TransactionStatusIndexMeta;
 }
@@ -709,6 +725,37 @@ impl ColumnName for columns::AddressSignatures {
     const NAME: &'static str = ADDRESS_SIGNATURES_CF;
 }
 
+impl Column for columns::TransactionMemos {
+    type Index = Signature;
+
+    fn key(signature: Signature) -> Vec<u8> {
+        let mut key = vec![0; 64]; // size_of Signature
+        key[0..64].clone_from_slice(&signature.as_ref()[0..64]);
+        key
+    }
+
+    fn index(key: &[u8]) -> Signature {
+        Signature::new(&key[0..64])
+    }
+
+    fn primary_index(_index: Self::Index) -> u64 {
+        unimplemented!()
+    }
+
+    fn slot(_index: Self::Index) -> Slot {
+        unimplemented!()
+    }
+
+    #[allow(clippy::wrong_self_convention)]
+    fn as_index(_index: u64) -> Self::Index {
+        Signature::default()
+    }
+}
+
+impl ColumnName for columns::TransactionMemos {
+    const NAME: &'static str = TRANSACTION_MEMOS_CF;
+}
+
 impl Column for columns::TransactionStatusIndex {
     type Index = u64;
 
@@ -1374,6 +1421,7 @@ fn excludes_from_compaction(cf_name: &str) -> bool {
     let no_compaction_cfs: HashSet<&'static str> = vec![
         columns::TransactionStatusIndex::NAME,
         columns::ProgramCosts::NAME,
+        columns::TransactionMemos::NAME,
     ]
     .into_iter()
     .collect();
@@ -1441,6 +1489,7 @@ pub mod tests {
             columns::TransactionStatusIndex::NAME
         ));
         assert!(excludes_from_compaction(columns::ProgramCosts::NAME));
+        assert!(excludes_from_compaction(columns::TransactionMemos::NAME));
         assert!(!excludes_from_compaction("something else"));
     }
 }

rpc/src/transaction_status_service.rs

@@ -8,7 +8,9 @@ use {
     solana_runtime::bank::{
         Bank, InnerInstructionsList, NonceRollbackInfo, TransactionLogMessages,
     },
-    solana_transaction_status::{InnerInstructions, Reward, TransactionStatusMeta},
+    solana_transaction_status::{
+        extract_and_fmt_memos, InnerInstructions, Reward, TransactionStatusMeta,
+    },
     std::{
         sync::{
             atomic::{AtomicBool, AtomicU64, Ordering},
@@ -141,6 +143,12 @@ impl TransactionStatusService {
                                 .collect(),
                         );
 
+                        if let Some(memos) = extract_and_fmt_memos(transaction.message()) {
+                            blockstore
+                                .write_transaction_memos(transaction.signature(), memos)
+                                .expect("Expect database write to succeed: TransactionMemos");
+                        }
+
                         blockstore
                             .write_transaction_status(
                                 slot,
@@ -159,7 +167,7 @@ impl TransactionStatusService {
                                     rewards,
                                 },
                             )
-                            .expect("Expect database write to succeed");
+                            .expect("Expect database write to succeed: TransactionStatus");
                     }
                 }
             }

transaction-status/src/extract_memos.rs

@@ -1,6 +1,9 @@
 use {
     crate::parse_instruction::parse_memo_data,
-    solana_sdk::{message::Message, pubkey::Pubkey},
+    solana_sdk::{
+        message::{Message, SanitizedMessage},
+        pubkey::Pubkey,
+    },
 };
 
 // A helper function to convert spl_memo::v1::id() as spl_sdk::pubkey::Pubkey to
@@ -15,8 +18,8 @@ pub fn spl_memo_id_v3() -> Pubkey {
     Pubkey::new_from_array(spl_memo::id().to_bytes())
 }
 
-pub fn extract_and_fmt_memos(message: &Message) -> Option<String> {
+pub fn extract_and_fmt_memos<T: ExtractMemos>(message: &T) -> Option<String> {
-    let memos = extract_memos(message);
+    let memos = message.extract_memos();
     if memos.is_empty() {
         None
     } else {
@@ -24,20 +27,121 @@ pub fn extract_and_fmt_memos(message: &Message) -> Option<String> {
     }
 }
 
-fn extract_memos(message: &Message) -> Vec<String> {
+fn maybe_push_parsed_memo(memos: &mut Vec<String>, program_id: Pubkey, data: &[u8]) {
-    let mut memos = vec![];
-    if message.account_keys.contains(&spl_memo_id_v1())
-        || message.account_keys.contains(&spl_memo_id_v3())
-    {
-        for instruction in &message.instructions {
-            let program_id = message.account_keys[instruction.program_id_index as usize];
     if program_id == spl_memo_id_v1() || program_id == spl_memo_id_v3() {
-                let memo_len = instruction.data.len();
+        let memo_len = data.len();
-                let parsed_memo = parse_memo_data(&instruction.data)
+        let parsed_memo = parse_memo_data(data).unwrap_or_else(|_| "(unparseable)".to_string());
-                    .unwrap_or_else(|_| "(unparseable)".to_string());
         memos.push(format!("[{}] {}", memo_len, parsed_memo));
     }
 }
+
+pub trait ExtractMemos {
+    fn extract_memos(&self) -> Vec<String>;
+}
+
+impl ExtractMemos for Message {
+    fn extract_memos(&self) -> Vec<String> {
+        let mut memos = vec![];
+        if self.account_keys.contains(&spl_memo_id_v1())
+            || self.account_keys.contains(&spl_memo_id_v3())
+        {
+            for instruction in &self.instructions {
+                let program_id = self.account_keys[instruction.program_id_index as usize];
+                maybe_push_parsed_memo(&mut memos, program_id, &instruction.data);
+            }
         }
         memos
     }
+}
+
+impl ExtractMemos for SanitizedMessage {
+    fn extract_memos(&self) -> Vec<String> {
+        let mut memos = vec![];
+        if self
+            .account_keys_iter()
+            .any(|&pubkey| pubkey == spl_memo_id_v1() || pubkey == spl_memo_id_v3())
+        {
+            for (program_id, instruction) in self.program_instructions_iter() {
+                maybe_push_parsed_memo(&mut memos, *program_id, &instruction.data);
+            }
+        }
+        memos
+    }
+}
+
+#[cfg(test)]
+mod test {
+    use {
+        super::*,
+        solana_sdk::{
+            hash::Hash,
+            instruction::CompiledInstruction,
+            message::{v0, MappedAddresses, MappedMessage, MessageHeader},
+        },
+    };
+
+    #[test]
+    fn test_extract_memos() {
+        let fee_payer = Pubkey::new_unique();
+        let another_program_id = Pubkey::new_unique();
+        let memo0 = "Test memo";
+        let memo1 = "🦖";
+        let expected_memos = vec![
+            format!("[{}] {}", memo0.len(), memo0),
+            format!("[{}] {}", memo1.len(), memo1),
+        ];
+        let memo_instructions = vec![
+            CompiledInstruction {
+                program_id_index: 1,
+                accounts: vec![],
+                data: memo0.as_bytes().to_vec(),
+            },
+            CompiledInstruction {
+                program_id_index: 2,
+                accounts: vec![],
+                data: memo1.as_bytes().to_vec(),
+            },
+            CompiledInstruction {
+                program_id_index: 3,
+                accounts: vec![],
+                data: memo1.as_bytes().to_vec(),
+            },
+        ];
+        let message = Message::new_with_compiled_instructions(
+            1,
+            0,
+            3,
+            vec![
+                fee_payer,
+                spl_memo_id_v1(),
+                another_program_id,
+                spl_memo_id_v3(),
+            ],
+            Hash::default(),
+            memo_instructions.clone(),
+        );
+        assert_eq!(message.extract_memos(), expected_memos);
+
+        let sanitized_message = SanitizedMessage::Legacy(message);
+        assert_eq!(sanitized_message.extract_memos(), expected_memos);
+
+        let mapped_message = MappedMessage {
+            message: v0::Message {
+                header: MessageHeader {
+                    num_required_signatures: 1,
+                    num_readonly_signed_accounts: 0,
+                    num_readonly_unsigned_accounts: 3,
+                },
+                account_keys: vec![fee_payer],
+                instructions: memo_instructions,
+                ..v0::Message::default()
+            },
+            mapped_addresses: MappedAddresses {
+                writable: vec![],
+                readonly: vec![spl_memo_id_v1(), another_program_id, spl_memo_id_v3()],
+            },
+        };
+        let sanitized_mapped_message = SanitizedMessage::V0(mapped_message);
+        assert_eq!(sanitized_mapped_message.extract_memos(), expected_memos);
+    }
+}