From 1fd1c2edbdc8c533466d76623622866e783afdae Mon Sep 17 00:00:00 2001
From: Dirk McCormick <dirkmdev@gmail.com>
Date: Mon, 29 Jun 2020 18:34:24 -0400
Subject: [PATCH] feat: allow nil deal state change (deal removed)

---
 chain/events/state/predicates.go      | 18 ++++++++++------
 chain/events/state/predicates_test.go | 31 ++++++++++++++++++---------
 2 files changed, 32 insertions(+), 17 deletions(-)

diff --git a/chain/events/state/predicates.go b/chain/events/state/predicates.go
index 3245d5c03..7bcf6c7cd 100644
--- a/chain/events/state/predicates.go
+++ b/chain/events/state/predicates.go
@@ -2,7 +2,6 @@ package state
 
 import (
 	"context"
-
 	"github.com/filecoin-project/go-address"
 	"github.com/filecoin-project/go-amt-ipld/v2"
 	"github.com/filecoin-project/lotus/api/apibstore"
@@ -107,8 +106,8 @@ type ChangedDeals map[abi.DealID]DealStateChange
 
 // DealStateChange is a change in deal state from -> to
 type DealStateChange struct {
-	From market.DealState
-	To   market.DealState
+	From *market.DealState
+	To   *market.DealState
 }
 
 // DealStateChangedForIDs detects changes in the deal state AMT for the given deal IDs
@@ -116,17 +115,22 @@ func (sp *StatePredicates) DealStateChangedForIDs(dealIds []abi.DealID) DiffDeal
 	return func(ctx context.Context, oldDealStateRoot *amt.Root, newDealStateRoot *amt.Root) (changed bool, user UserData, err error) {
 		changedDeals := make(ChangedDeals)
 		for _, dealID := range dealIds {
+			var oldDealPtr, newDealPtr *market.DealState
 			var oldDeal, newDeal market.DealState
 			err := oldDealStateRoot.Get(ctx, uint64(dealID), &oldDeal)
-			if err != nil {
+			if err == nil {
+				oldDealPtr = &oldDeal
+			} else if _, ok := err.(*amt.ErrNotFound); !ok {
 				return false, nil, err
 			}
 			err = newDealStateRoot.Get(ctx, uint64(dealID), &newDeal)
-			if err != nil {
+			if err == nil {
+				newDealPtr = &newDeal
+			} else if _, ok := err.(*amt.ErrNotFound); !ok {
 				return false, nil, err
 			}
-			if oldDeal != newDeal {
-				changedDeals[dealID] = DealStateChange{oldDeal, newDeal}
+			if oldDealPtr == nil || newDealPtr == nil || oldDeal != newDeal {
+				changedDeals[dealID] = DealStateChange{oldDealPtr, newDealPtr}
 			}
 		}
 		if len(changedDeals) > 0 {
diff --git a/chain/events/state/predicates_test.go b/chain/events/state/predicates_test.go
index 56387f8b5..c966e622e 100644
--- a/chain/events/state/predicates_test.go
+++ b/chain/events/state/predicates_test.go
@@ -71,12 +71,10 @@ func TestPredicates(t *testing.T) {
 		abi.DealID(1): {
 			SectorStartEpoch: 1,
 			LastUpdatedEpoch: 2,
-			SlashEpoch:       0,
 		},
 		abi.DealID(2): {
 			SectorStartEpoch: 4,
 			LastUpdatedEpoch: 5,
-			SlashEpoch:       0,
 		},
 	}
 	oldStateC := createMarketState(ctx, t, store, oldDeals)
@@ -85,12 +83,6 @@ func TestPredicates(t *testing.T) {
 		abi.DealID(1): {
 			SectorStartEpoch: 1,
 			LastUpdatedEpoch: 3,
-			SlashEpoch:       0,
-		},
-		abi.DealID(2): {
-			SectorStartEpoch: 4,
-			LastUpdatedEpoch: 6,
-			SlashEpoch:       6,
 		},
 	}
 	newStateC := createMarketState(ctx, t, store, newDeals)
@@ -131,8 +123,27 @@ func TestPredicates(t *testing.T) {
 		t.Fatal("Unexpected change to LastUpdatedEpoch")
 	}
 	deal2 := changedDeals[abi.DealID(2)]
-	if deal2.From.SlashEpoch != 0 || deal2.To.SlashEpoch != 6 {
-		t.Fatal("Unexpected change to SlashEpoch")
+	if deal2.From.LastUpdatedEpoch != 5 || deal2.To != nil {
+		t.Fatal("Expected To to be nil")
+	}
+
+	// Diff with non-existent deal.
+	// Note that for non existent deals we expect a change with
+	// From and To set to nil. This is to ensure that if a deal expires and is
+	// removed from state we will still find out about the change.
+	noDeal := []abi.DealID{3}
+	diffNoDealFn := preds.OnStorageMarketActorChanged(preds.OnDealStateChanged(preds.DealStateChangedForIDs(noDeal)))
+	changed, val, err = diffNoDealFn(ctx, oldState, newState)
+	require.NoError(t, err)
+	require.True(t, changed)
+
+	changedDeals, ok = val.(ChangedDeals)
+	require.True(t, ok)
+	require.Len(t, changedDeals, 1)
+	require.Contains(t, changedDeals, noDeal[0])
+	deal1 = changedDeals[abi.DealID(1)]
+	if deal1.From != nil || deal1.To != nil {
+		t.Fatal("Expected both from and to to be nil")
 	}
 
 	// Test that OnActorStateChanged does not call the callback if the state has not changed