winpost: Add more defenses against slashing (#190)

Author: magik6k

tasks/winning/winning_task.go

@@ -105,16 +105,22 @@ func (t *WinPostTask) Do(taskID harmonytask.TaskID, stillOwned func() bool) (don
 		Epoch     uint64
 		BlockCIDs []BlockCID
 		CompTime  time.Time
+		Won       bool
 	}
 
 	var details MiningTaskDetails
 
 	// First query to fetch from mining_tasks
-	err = t.db.QueryRow(ctx, `SELECT sp_id, epoch, base_compute_time FROM mining_tasks WHERE task_id = $1`, taskID).Scan(&details.SpID, &details.Epoch, &details.CompTime)
+	err = t.db.QueryRow(ctx, `SELECT sp_id, epoch, base_compute_time, won FROM mining_tasks WHERE task_id = $1`, taskID).Scan(&details.SpID, &details.Epoch, &details.CompTime, &details.Won)
 	if err != nil {
 		return false, xerrors.Errorf("query mining base info fail: %w", err)
 	}
 
+	if details.Won {
+		log.Errorw("WinPostTask.Do() task already won?!", "taskID", taskID)
+		return true, nil
+	}
+
 	// Second query to fetch from mining_base_block
 	rows, err := t.db.Query(ctx, `SELECT block_cid FROM mining_base_block WHERE task_id = $1`, taskID)
 	if err != nil {
@@ -415,12 +421,17 @@ func (t *WinPostTask) Do(taskID harmonytask.TaskID, stillOwned func() bool) (don
 			return false, xerrors.Errorf("failed to marshal block header: %w", err)
 		}
 
-		_, err = t.db.Exec(ctx, `UPDATE mining_tasks
+		n, err := t.db.Exec(ctx, `UPDATE mining_tasks
             SET won = true, mined_cid = $2, mined_header = $3, mined_at = $4
-            WHERE task_id = $1`, taskID, blockMsg.Header.Cid(), string(bhjson), time.Now().UTC())
+            WHERE task_id = $1 AND won = false`, taskID, blockMsg.Header.Cid(), string(bhjson), time.Now().UTC())
 		if err != nil {
 			return false, xerrors.Errorf("failed to update mining task: %w", err)
 		}
+
+		if n == 0 {
+			log.Warnw("WinPostTask task already mined?", "tipset", types.LogCids(base.TipSet.Cids()), "miner", maddr, "round", round, "block", blockMsg.Header.Cid())
+			return true, xerrors.Errorf("block already mined?")
+		}
 	}
 
 	// wait until block timestamp
@@ -429,6 +440,12 @@ func (t *WinPostTask) Do(taskID harmonytask.TaskID, stillOwned func() bool) (don
 		time.Sleep(time.Until(time.Unix(int64(blockMsg.Header.Timestamp), 0)))
 	}
 
+	if !stillOwned() {
+		// make sure no one else got their hands on the task
+		log.Warnw("WinPostTask task no longer owned", "tipset", types.LogCids(base.TipSet.Cids()), "miner", maddr, "round", round, "block", blockMsg.Header.Cid())
+		return true, xerrors.Errorf("task no longer owned")
+	}
+
 	// submit block!!
 	{
 		log.Infow("WinPostTask submitting block", "tipset", types.LogCids(base.TipSet.Cids()), "miner", maddr, "round", round, "block", blockMsg.Header.Cid())
@@ -522,10 +539,13 @@ func (t *WinPostTask) TypeDetails() harmonytask.TaskTypeDetails {
 	}
 
 	return harmonytask.TaskTypeDetails{
-		Name:        "WinPost",
-		Max:         t.max,
-		MaxFailures: 3,
-		Follows:     nil,
+		Name: "WinPost",
+		Max:  t.max,
+
+		// We're not allowing retry to be conservative. Retry in winningPoSt done badly can lead to slashing, and
+		// that is generally worse than not mining a block. In general the task code is heavily defensive, and
+		// retry should be fine, but not allowing it is just another layer preventing slashing.
+		MaxFailures: 1,
 		Cost: resources.Resources{
 			Cpu: 1,