Add Protobuf typed/defined Penumbra ABCI Events (#3492)

* skeleton for Protobuf generated Penumbra defined ABCI events

* first attempt at filling out ProtoEvent. Added EventSpend protobuf def with generated code

* explicitly added Serde's serialization traits on ProtoEvent, directly using serde_json::{to/from_value}

* proto: tweaks to proto-events

- use DeserializeOwned to avoid lifetime bounds
- use the fully qualified proto name for the event type, to avoid conflicts
- add a hardcoded test vector (generated with the current code and manually inspected)
- run rustfmt

* proto: fix proto-event encoding on more complex examples

This adds the EventOutput case, which exposed some issues with the
(de)serialization logic since it has nested objects.

* shielded-pool: try using record_proto for spend, output events

* fill in missing comment

---------

Co-authored-by: Henry de Valence <[email protected]>

crates/core/component/shielded-pool/src/component/action_handler/output.rs

@@ -4,6 +4,7 @@ use anyhow::Result;
 use async_trait::async_trait;
 use penumbra_component::ActionHandler;
 use penumbra_proof_params::OUTPUT_PROOF_VERIFICATION_KEY;
+use penumbra_proto::StateWriteProto as _;
 use penumbra_storage::{StateRead, StateWrite};
 
 use crate::{component::NoteManager, event, Output};
@@ -34,7 +35,7 @@ impl ActionHandler for Output {
             .add_note_payload(self.body.note_payload.clone(), source)
             .await;
 
-        state.record(event::output(&self.body.note_payload));
+        state.record_proto(event::output(&self.body.note_payload));
 
         Ok(())
     }

crates/core/component/shielded-pool/src/component/action_handler/spend.rs

@@ -5,6 +5,7 @@ use async_trait::async_trait;
 use penumbra_chain::TransactionContext;
 use penumbra_component::ActionHandler;
 use penumbra_proof_params::SPEND_PROOF_VERIFICATION_KEY;
+use penumbra_proto::StateWriteProto as _;
 use penumbra_storage::{StateRead, StateWrite};
 
 use crate::{
@@ -51,7 +52,7 @@ impl ActionHandler for Spend {
         state.spend_nullifier(self.body.nullifier, source).await;
 
         // Also record an ABCI event for transaction indexing.
-        state.record(event::spend(&self.body.nullifier));
+        state.record_proto(event::spend(&self.body.nullifier));
 
         Ok(())
     }

crates/core/component/shielded-pool/src/event.rs

@@ -1,18 +1,20 @@
 use penumbra_sct::Nullifier;
-use tendermint::abci::{Event, EventAttributeIndexExt};
+
+use penumbra_proto::core::component::shielded_pool::v1alpha1::{EventOutput, EventSpend};
 
 use crate::NotePayload;
 
-pub fn spend(nullifier: &Nullifier) -> Event {
-    Event::new(
-        "action_spend",
-        [("nullifier", nullifier.to_string()).index()],
-    )
+// These are sort of like the proto/domain type From impls, because
+// we don't have separate domain types for the events (yet, possibly ever).
+
+pub fn spend(nullifier: &Nullifier) -> EventSpend {
+    EventSpend {
+        nullifier: nullifier.to_bytes().to_vec(),
+    }
 }
 
-pub fn output(note_payload: &NotePayload) -> Event {
-    Event::new(
-        "action_output",
-        [("note_commitment", note_payload.note_commitment.to_string()).index()],
-    )
+pub fn output(note_payload: &NotePayload) -> EventOutput {
+    EventOutput {
+        note_commitment: Some(note_payload.note_commitment.into()),
+    }
 }

crates/proto/Cargo.toml

@@ -12,6 +12,7 @@ bytes = { version = "1", features = ["serde"] }
 prost = "0.12.3"
 tonic = { version = "0.10", optional = true }
 serde = { version = "1", features = ["derive"] }
+serde_json = "1"
 hex = "0.4"
 anyhow = "1.0"
 subtle-encoding = "0.5"

crates/proto/src/event.rs

@@ -0,0 +1,120 @@
+use crate::{Message, Name};
+use anyhow::{self, Context};
+use serde::{de::DeserializeOwned, Serialize};
+use std::collections::HashMap;
+use tendermint::abci::{self, EventAttribute};
+
+pub trait ProtoEvent: Message + Name + Serialize + DeserializeOwned + Sized {
+    fn into_event(&self) -> abci::Event {
+        let kind = Self::full_name();
+
+        let event_json = serde_json::to_value(&self)
+            .expect("ProtoEvent constrained values should be JSON serializeable.");
+
+        // WARNING: Assuming that Rust value will always serialize into a valid JSON Object value. This falls apart the moment that isn't true, so we fail hard if that turns out to be the case.
+        let mut attributes: Vec<EventAttribute> = event_json
+            .as_object()
+            .expect("serde_json Serialized ProtoEvent should not be empty.")
+            .into_iter()
+            .map(|(key, v)| abci::EventAttribute {
+                value: serde_json::to_string(v).expect("must be able to serialize value as JSON"),
+                key: key.to_string(),
+                index: true,
+            })
+            .collect();
+
+        // NOTE: cosmo-sdk sorts the attribute list so that it's deterministic every time.[0] I don't know if that is actually conformant but continuing that pattern here for now.
+        // [0]: https://github.com/cosmos/cosmos-sdk/blob/8fb62054c59e580c0ae0c898751f8dc46044499a/types/events.go#L102-L104
+        attributes.sort_by(|a, b| (&a.key).cmp(&b.key));
+
+        return abci::Event::new(kind, attributes);
+    }
+
+    fn from_event(event: &abci::Event) -> anyhow::Result<Self> {
+        // Check that we're dealing with the right type of event.
+        if Self::full_name() != event.kind {
+            return Err(anyhow::anyhow!(format!(
+                "ABCI Event {} not expected for {}",
+                event.kind,
+                Self::full_name()
+            )));
+        }
+
+        // NOTE: Is there any condition where there would be duplicate EventAttributes and problems that fall out of that?
+        let mut attributes = HashMap::<String, serde_json::Value>::new();
+        for attr in &event.attributes {
+            let value = serde_json::from_str(&attr.value)
+                .with_context(|| format!("could not parse JSON for attribute {:?}", attr))?;
+            attributes.insert(attr.key.clone(), value);
+        }
+
+        let json = serde_json::to_value(attributes)
+            .expect("HashMap of String, serde_json::Value should be serializeable.");
+
+        return Ok(
+            serde_json::from_value(json).context("could not deserialise ProtoJSON into event")?
+        );
+    }
+}
+
+impl<E: Message + Name + Serialize + DeserializeOwned + Sized> ProtoEvent for E {}
+
+#[cfg(test)]
+mod tests {
+    #[test]
+    fn event_round_trip() {
+        use super::*;
+        use crate::core::component::shielded_pool::v1alpha1::{EventOutput, EventSpend};
+        use crate::crypto::tct::v1alpha1::StateCommitment;
+
+        let proto_spend = EventSpend {
+            nullifier: vec![
+                148, 190, 149, 23, 86, 113, 152, 145, 104, 242, 142, 162, 233, 239, 137, 141, 140,
+                164, 180, 98, 154, 55, 168, 255, 163, 228, 179, 176, 26, 25, 219, 211,
+            ],
+        };
+
+        let abci_spend = proto_spend.into_event();
+
+        let expected_abci_spend = abci::Event::new(
+            "penumbra.core.component.shielded_pool.v1alpha1.EventSpend",
+            vec![abci::EventAttribute {
+                key: "nullifier".to_string(),
+                value: "\"lL6VF1ZxmJFo8o6i6e+JjYyktGKaN6j/o+SzsBoZ29M=\"".to_string(),
+                index: true,
+            }],
+        );
+        assert_eq!(abci_spend, expected_abci_spend);
+
+        let proto_spend2 = EventSpend::from_event(&abci_spend).unwrap();
+
+        assert_eq!(proto_spend, proto_spend2);
+
+        let proto_output = EventOutput {
+            // This is the same bytes as the nullifier above, we just care about the data format, not the value.
+            note_commitment: Some(StateCommitment {
+                inner: vec![
+                    148, 190, 149, 23, 86, 113, 152, 145, 104, 242, 142, 162, 233, 239, 137, 141,
+                    140, 164, 180, 98, 154, 55, 168, 255, 163, 228, 179, 176, 26, 25, 219, 211,
+                ],
+            }),
+        };
+
+        let abci_output = proto_output.into_event();
+
+        let expected_abci_output = abci::Event::new(
+            "penumbra.core.component.shielded_pool.v1alpha1.EventOutput",
+            vec![abci::EventAttribute {
+                // note: attribute keys become camelCase because ProtoJSON...
+                key: "noteCommitment".to_string(),
+                // note: attribute values are JSON objects, potentially nested as here
+                value: "{\"inner\":\"lL6VF1ZxmJFo8o6i6e+JjYyktGKaN6j/o+SzsBoZ29M=\"}".to_string(),
+                index: true,
+            }],
+        );
+        assert_eq!(abci_output, expected_abci_output);
+
+        let proto_output2 = EventOutput::from_event(&abci_output).unwrap();
+        assert_eq!(proto_output, proto_output2);
+    }
+}

crates/proto/src/gen/penumbra.core.component.shielded_pool.v1alpha1.rs

@@ -155,6 +155,40 @@ impl ::prost::Name for Spend {
         )
     }
 }
+/// ABCI Event recording a spend.
+#[allow(clippy::derive_partial_eq_without_eq)]
+#[derive(Clone, PartialEq, ::prost::Message)]
+pub struct EventSpend {
+    #[prost(bytes = "vec", tag = "1")]
+    pub nullifier: ::prost::alloc::vec::Vec<u8>,
+}
+impl ::prost::Name for EventSpend {
+    const NAME: &'static str = "EventSpend";
+    const PACKAGE: &'static str = "penumbra.core.component.shielded_pool.v1alpha1";
+    fn full_name() -> ::prost::alloc::string::String {
+        ::prost::alloc::format!(
+            "penumbra.core.component.shielded_pool.v1alpha1.{}", Self::NAME
+        )
+    }
+}
+/// ABCI Event recording an output.
+#[allow(clippy::derive_partial_eq_without_eq)]
+#[derive(Clone, PartialEq, ::prost::Message)]
+pub struct EventOutput {
+    #[prost(message, optional, tag = "1")]
+    pub note_commitment: ::core::option::Option<
+        super::super::super::super::crypto::tct::v1alpha1::StateCommitment,
+    >,
+}
+impl ::prost::Name for EventOutput {
+    const NAME: &'static str = "EventOutput";
+    const PACKAGE: &'static str = "penumbra.core.component.shielded_pool.v1alpha1";
+    fn full_name() -> ::prost::alloc::string::String {
+        ::prost::alloc::format!(
+            "penumbra.core.component.shielded_pool.v1alpha1.{}", Self::NAME
+        )
+    }
+}
 /// The body of a spend description, containing only the effecting data
 /// describing changes to the ledger, and not the authorizing data that allows
 /// those changes to be performed.