Updates to Actor events based on community feedback for NV22 (#1531)

* update to Actor events

* changes as per review

* remvoe clippy

* changes as per new FIP update

* Update to latest PR, remove emit:: use from testing

* all itests passing and green CI

* address review feedback, minimise diff

* feat: add term_start to claim events

* fix: claim term_start at current epoch, not at deal_start

* fix: s/deal_term/claim_term

---------

Co-authored-by: Rod Vagg <rod@vagg.org>

actors/verifreg/src/emit.rs

@@ -1,10 +1,12 @@
 // A namespace for helpers that build and emit verified registry events.
 
-use crate::{ActorError, AllocationID};
+use crate::{ActorError, Allocation, AllocationID, Claim};
 use crate::{ClaimID, DataCap};
+use cid::Cid;
 use fil_actors_runtime::runtime::Runtime;
 use fil_actors_runtime::EventBuilder;
 use fvm_shared::bigint::bigint_ser::BigIntSer;
+use fvm_shared::clock::ChainEpoch;
 use fvm_shared::ActorID;
 
 /// Indicates a new value for a verifier's datacap balance.
@@ -28,60 +30,75 @@ pub fn verifier_balance(
 pub fn allocation(
     rt: &impl Runtime,
     id: AllocationID,
-    client: ActorID,
-    provider: ActorID,
+    alloc: &Allocation,
 ) -> Result<(), ActorError> {
     rt.emit_event(
-        &EventBuilder::new().typ("allocation").with_parties(id, client, provider).build()?,
+        &EventBuilder::new()
+            .typ("allocation")
+            .with_parties(id, alloc.client, alloc.provider)
+            .with_piece(&alloc.data, alloc.size.0)
+            .with_term(alloc.term_min, alloc.term_max)
+            .field("expiration", &alloc.expiration)
+            .build()?,
     )
 }
 
 /// Indicates an expired allocation has been removed.
 pub fn allocation_removed(
     rt: &impl Runtime,
     id: AllocationID,
-    client: ActorID,
-    provider: ActorID,
+    alloc: &Allocation,
 ) -> Result<(), ActorError> {
     rt.emit_event(
         &EventBuilder::new()
             .typ("allocation-removed")
-            .with_parties(id, client, provider)
+            .with_parties(id, alloc.client, alloc.provider)
+            .with_piece(&alloc.data, alloc.size.0)
+            .with_term(alloc.term_min, alloc.term_max)
+            .field("expiration", &alloc.expiration)
             .build()?,
     )
 }
 
 /// Indicates an allocation has been claimed.
-pub fn claim(
-    rt: &impl Runtime,
-    id: ClaimID,
-    client: ActorID,
-    provider: ActorID,
-) -> Result<(), ActorError> {
-    rt.emit_event(&EventBuilder::new().typ("claim").with_parties(id, client, provider).build()?)
+pub fn claim(rt: &impl Runtime, id: ClaimID, claim: &Claim) -> Result<(), ActorError> {
+    rt.emit_event(
+        &EventBuilder::new()
+            .typ("claim")
+            .with_parties(id, claim.client, claim.provider)
+            .with_piece(&claim.data, claim.size.0)
+            .with_term(claim.term_min, claim.term_max)
+            .field("term-start", &claim.term_start)
+            .field_indexed("sector", &claim.sector)
+            .build()?,
+    )
 }
 
 /// Indicates an existing claim has been updated (e.g. with a longer term).
-pub fn claim_updated(
-    rt: &impl Runtime,
-    id: ClaimID,
-    client: ActorID,
-    provider: ActorID,
-) -> Result<(), ActorError> {
+pub fn claim_updated(rt: &impl Runtime, id: ClaimID, claim: &Claim) -> Result<(), ActorError> {
     rt.emit_event(
-        &EventBuilder::new().typ("claim-updated").with_parties(id, client, provider).build()?,
+        &EventBuilder::new()
+            .typ("claim-updated")
+            .with_parties(id, claim.client, claim.provider)
+            .with_piece(&claim.data, claim.size.0)
+            .with_term(claim.term_min, claim.term_max)
+            .field("term-start", &claim.term_start)
+            .field_indexed("sector", &claim.sector)
+            .build()?,
     )
 }
 
 /// Indicates an expired claim has been removed.
-pub fn claim_removed(
-    rt: &impl Runtime,
-    id: ClaimID,
-    client: ActorID,
-    provider: ActorID,
-) -> Result<(), ActorError> {
+pub fn claim_removed(rt: &impl Runtime, id: ClaimID, claim: &Claim) -> Result<(), ActorError> {
     rt.emit_event(
-        &EventBuilder::new().typ("claim-removed").with_parties(id, client, provider).build()?,
+        &EventBuilder::new()
+            .typ("claim-removed")
+            .with_parties(id, claim.client, claim.provider)
+            .with_piece(&claim.data, claim.size.0)
+            .with_term(claim.term_min, claim.term_max)
+            .field("term-start", &claim.term_start)
+            .field_indexed("sector", &claim.sector)
+            .build()?,
     )
 }
 
@@ -97,3 +114,23 @@ impl WithParties for EventBuilder {
             .field_indexed("provider", &provider)
     }
 }
+
+trait WithPiece {
+    fn with_piece(self, piece_cid: &Cid, piece_size: u64) -> EventBuilder;
+}
+
+impl crate::emit::WithPiece for EventBuilder {
+    fn with_piece(self, piece_cid: &Cid, piece_size: u64) -> EventBuilder {
+        self.field_indexed("piece-cid", &piece_cid).field("piece-size", &piece_size)
+    }
+}
+
+trait WithTerm {
+    fn with_term(self, term_min: ChainEpoch, term_max: ChainEpoch) -> EventBuilder;
+}
+
+impl crate::emit::WithTerm for EventBuilder {
+    fn with_term(self, term_min: ChainEpoch, term_max: ChainEpoch) -> EventBuilder {
+        self.field("term-min", &term_min).field("term-max", &term_max)
+    }
+}

actors/verifreg/src/lib.rs

@@ -165,8 +165,8 @@ impl Actor {
             ));
         }
 
-        let client = resolve_to_actor_id(rt, &params.address, true)?;
-        let client = Address::new_id(client);
+        let client_id = resolve_to_actor_id(rt, &params.address, true)?;
+        let client = Address::new_id(client_id);
 
         rt.transaction(|st: &mut State, rt| {
             if client == st.root_key {
@@ -345,7 +345,7 @@ impl Actor {
                         )?
                         .unwrap(); // Unwrapping here as both paths to here should ensure the allocation exists.
 
-                    emit::allocation_removed(rt, *id, existing.client, existing.provider)?;
+                    emit::allocation_removed(rt, *id, &existing)?;
 
                     // Unwrapping here as both paths to here should ensure the allocation exists.
                     recovered_datacap += existing.size.0;
@@ -448,7 +448,8 @@ impl Actor {
                         return Err(actor_error!(illegal_argument, "claim {} already exists", id));
                     }
 
-                    emit::claim(rt, id, new_claim.client, new_claim.provider)?;
+                    // Emit a claim event below
+                    emit::claim(rt, id, &new_claim)?;
 
                     allocs.remove(new_claim.client, id).context_code(
                         ExitCode::USR_ILLEGAL_STATE,
@@ -565,7 +566,7 @@ impl Actor {
                         "HAMT put failure storing new claims",
                     )?;
                     batch_gen.add_success();
-                    emit::claim_updated(rt, term.claim_id, new_claim.client, new_claim.provider)?;
+                    emit::claim_updated(rt, term.claim_id, &new_claim)?;
                 } else {
                     batch_gen.add_fail(ExitCode::USR_NOT_FOUND);
                     info!("no claim {} for provider {}", term.claim_id, term.provider);
@@ -617,7 +618,7 @@ impl Actor {
                     )?
                     .unwrap();
 
-                emit::claim_removed(rt, *id, removed.client, removed.provider)?;
+                emit::claim_removed(rt, *id, &removed)?;
             }
 
             st.save_claims(&mut claims)?;
@@ -717,13 +718,13 @@ impl Actor {
             let ids = st.insert_allocations(rt.store(), client, new_allocs.clone())?;
 
             for (id, alloc) in ids.iter().zip(new_allocs.iter()) {
-                emit::allocation(rt, *id, alloc.client, alloc.provider)?;
+                emit::allocation(rt, *id, alloc)?;
             }
 
             st.put_claims(rt.store(), updated_claims.clone())?;
 
             for (id, claim) in updated_claims {
-                emit::claim_updated(rt, id, claim.client, claim.provider)?;
+                emit::claim_updated(rt, id, &claim)?;
             }
 
             Ok(ids)