Fix ticket expiration (#603)

ortyomka · web-flow · commit bb21c6e54924 · 2022-02-23T17:01:31.000+03:00
* Fix precommit check

Signed-off-by: ortyomka &lt;iurin.art@gmail.com&gt;

* Fix states

Signed-off-by: ortyomka &lt;iurin.art@gmail.com&gt;
diff --git a/core/miner/impl/miner_impl.cpp b/core/miner/impl/miner_impl.cpp
@@ -102,6 +102,7 @@ namespace fc::miner {
                                                scheduler,
                                                SelectAddress,
                                                fee_config);
+
     OUTCOME_TRY(sealing,
                 SealingImpl::newSealing(api,
                                         events,
diff --git a/core/miner/storage_fsm/impl/checks.cpp b/core/miner/storage_fsm/impl/checks.cpp
@@ -33,6 +33,7 @@ namespace fc::mining::checks {
   using vm::actor::MethodParams;
   using vm::actor::builtin::states::MinerActorStatePtr;
   using vm::actor::builtin::types::miner::kChainFinality;
+  using vm::actor::builtin::types::miner::kMaxPreCommitRandomnessLookback;
   using vm::actor::builtin::types::miner::kMaxProveCommitDuration;
   using vm::actor::builtin::types::miner::kPreCommitChallengeDelay;
   using vm::actor::builtin::types::miner::maxSealDuration;
@@ -78,10 +79,13 @@ namespace fc::mining::checks {
         continue;
       }
 
-      OUTCOME_TRY(proposal,
-                  api->StateMarketStorageDeal(piece.deal_info->deal_id,
-                                              chain_head->key));
+      const auto maybe_proposal = api->StateMarketStorageDeal(
+          piece.deal_info->deal_id, chain_head->key);
+      if (maybe_proposal.has_error()) {
+        return ChecksError::kInvalidDeal;
+      }
 
+      const auto &proposal{maybe_proposal.value()};
       if (miner_address != proposal.proposal.provider) {
         return ChecksError::kInvalidDeal;
       }
@@ -192,15 +196,10 @@ namespace fc::mining::checks {
       return ChecksError::kBadCommD;
     }
 
-    OUTCOME_TRY(network, api->StateNetworkVersion(tipset_key));
-    OUTCOME_TRY(seal_duration, getMaxProveCommitDuration(network, sector_info));
-    if (height - (sector_info->ticket_epoch + kChainFinality) > seal_duration) {
-      return ChecksError::kExpiredTicket;
-    }
-
     OUTCOME_TRY(state_sector_precommit_info,
                 getStateSectorPreCommitInfo(
                     miner_address, sector_info, tipset_key, api));
+
     if (state_sector_precommit_info.has_value()) {
       if (state_sector_precommit_info->info.seal_epoch
           != sector_info->ticket_epoch) {
@@ -209,6 +208,12 @@ namespace fc::mining::checks {
       return ChecksError::kPrecommitOnChain;
     }
 
+    const ChainEpoch ticket_earliest = height - kMaxPreCommitRandomnessLookback;
+
+    if (sector_info->ticket_epoch < ticket_earliest) {
+      return ChecksError::kExpiredTicket;
+    }
+
     return outcome::success();
   }
 
diff --git a/core/miner/storage_fsm/impl/sealing_impl.cpp b/core/miner/storage_fsm/impl/sealing_impl.cpp
@@ -251,13 +251,13 @@ namespace fc::mining {
       auto sector_ref = minerSector(seal_proof_type, piece_location.sector);
 
       if (sector_and_padding.padding.size != 0) {
-        OUTCOME_TRY(sealer_->addPieceSync(
-            sector_ref,
-            VectorCoW(gsl::span<const UnpaddedPieceSize>(
-                unsealed_sector.piece_sizes)),
-            sector_and_padding.padding.size.unpadded(),
-            PieceData::makeNull(),
-            kDealSectorPriority));
+        OUTCOME_TRY(
+            sealer_->addPieceSync(sector_ref,
+                                  VectorCoW(gsl::span<const UnpaddedPieceSize>(
+                                      unsealed_sector.piece_sizes)),
+                                  sector_and_padding.padding.size.unpadded(),
+                                  PieceData::makeNull(),
+                                  kDealSectorPriority));
 
         unsealed_sector.stored += sector_and_padding.padding.size;
 
@@ -282,14 +282,14 @@ namespace fc::mining {
       piece_location.offset = unsealed_sector.stored;
 
       logger_->info("Add piece to sector {}", piece_location.sector);
-      OUTCOME_TRY(piece_info,
-                  sealer_->addPieceSync(sector_ref,
-                                        VectorCoW(
-                                            gsl::span<const UnpaddedPieceSize>(
-                                                unsealed_sector.piece_sizes)),
-                                        size,
-                                        std::move(piece_data),
-                                        kDealSectorPriority));
+      OUTCOME_TRY(
+          piece_info,
+          sealer_->addPieceSync(sector_ref,
+                                VectorCoW(gsl::span<const UnpaddedPieceSize>(
+                                    unsealed_sector.piece_sizes)),
+                                size,
+                                std::move(piece_data),
+                                kDealSectorPriority));
 
       context->pieces.push_back(Piece{
           .piece = piece_info,
@@ -623,14 +623,15 @@ namespace fc::mining {
             .to(SealingState::kPreCommit2)
             .action(CALLBACK_ACTION),
         SealingTransition(SealingEvent::kSectorSealPreCommit1Failed)
-            .fromMany(SealingState::kPreCommit1, SealingState::kPreCommitting)
+            .fromMany(SealingState::kPreCommit1,
+                      SealingState::kPreCommitting,
+                      SealingState::kPreCommitFail,
+                      SealingState::kCommitFail,
+                      SealingState::kComputeProofFail,
+                      SealingState::kSubmitPreCommitBatch)
             .to(SealingState::kSealPreCommit1Fail)
             .action(
                 [](auto info, auto event, auto context, auto from, auto to) {
-                  if (from == SealingState::kCommitting) {
-                    return;
-                  }
-
                   info->invalid_proofs = 0;
                   info->precommit2_fails = 0;
                 }),
diff --git a/test/core/miner/checks_test.cpp b/test/core/miner/checks_test.cpp
@@ -413,16 +413,18 @@ namespace fc::mining::checks {
   }
 
   /**
-   * @given info, valid pieces, expired ticket
+   * @given info, valid pieces, ticket, precommit on chain
    * @when try to check precommit
-   * @then ChecksError::kExpiredTicket occurs
+   * @then ChecksError::kPrecommitOnChain occurs
    */
-  TEST_F(CheckPrecommit, ExpiredTicket) {
+  TEST_F(CheckPrecommit, PrecommitOnChain) {
     std::shared_ptr<SectorInfo> info = std::make_shared<SectorInfo>();
     api::DealId deal_id = 1;
 
+    SectorNumber sector = 1;
     PieceInfo piece{.size = PaddedPieceSize(2048), .cid = "010001020001"_cid};
     info->comm_d = "010001020001"_cid;
+    info->sector_number = sector;
     info->pieces = {
         Piece{
             .piece = piece,
@@ -441,21 +443,28 @@ namespace fc::mining::checks {
         },
     };
 
-    MOCK_API(api_, ChainHead);
     TipsetKey head_key;
-    auto head_tipset =
-        std::make_shared<Tipset>(head_key, std::vector<api::BlockHeader>());
-    EXPECT_CALL(mock_ChainHead, Call())
-        .WillOnce(testing::Return(outcome::success(head_tipset)));
+    api_->ChainHead = [&head_key]() -> outcome::result<TipsetCPtr> {
+      auto tip =
+          std::make_shared<Tipset>(head_key, std::vector<api::BlockHeader>());
+      return tip;
+    };
 
-    MOCK_API(api_, StateMarketStorageDeal);
-    api::StorageDeal deal;
-    deal.proposal.provider = Address::makeFromId(miner_id_);
-    deal.proposal.piece_cid = piece.cid;
-    deal.proposal.piece_size = piece.size;
-    deal.proposal.start_epoch = 1;
-    EXPECT_CALL(mock_StateMarketStorageDeal, Call(deal_id, head_key))
-        .WillOnce(testing::Return(outcome::success(deal)));
+    api_->StateMarketStorageDeal =
+        [&piece, id{miner_id_}, deal_id, &head_key](
+            api::DealId did,
+            const TipsetKey &key) -> outcome::result<api::StorageDeal> {
+      if (did == deal_id and key == head_key) {
+        api::StorageDeal res;
+        res.proposal.provider = Address::makeFromId(id);
+        res.proposal.piece_cid = piece.cid;
+        res.proposal.piece_size = piece.size;
+        res.proposal.start_epoch = 1;
+        return res;
+      }
+
+      return ERROR_TEXT("ERROR");
+    };
 
     TipsetKey precommit_key{{CbCid::hash("01"_unhex),
                              CbCid::hash("02"_unhex),
@@ -468,23 +477,50 @@ namespace fc::mining::checks {
 
     MOCK_API(api_, StateNetworkVersion);
     EXPECT_CALL(mock_StateNetworkVersion, Call(precommit_key))
-        .WillOnce(testing::Return(outcome::success(version_)));
-    EXPECT_OUTCOME_TRUE(duration,
-                        checks::getMaxProveCommitDuration(version_, info));
-    ChainEpoch height = duration
-                        + vm::actor::builtin::types::miner::kChainFinality
-                        + info->ticket_epoch + 1;
+        .WillRepeatedly(testing::Return(outcome::success(version_)));
+
+    ChainEpoch height;
+
+    auto actor_key{"010001020003"_cid};
+    SectorPreCommitOnChainInfo some_info;
+    some_info.info.sealed_cid = "010001020006"_cid;
+    EXPECT_OUTCOME_TRUE_1(
+        actor_state_->precommitted_sectors.set(sector, some_info));
+    EXPECT_OUTCOME_TRUE(cid_root,
+                        actor_state_->precommitted_sectors.hamt.flush());
+
+    api_->ChainReadObj = [&](CID key) -> outcome::result<Bytes> {
+      if (key == actor_key) {
+        return codec::cbor::encode(actor_state_);
+      }
+      if (key == cid_root) {
+        EXPECT_OUTCOME_TRUE(root,
+                            getCbor<storage::hamt::Node>(ipld_, cid_root));
+        return codec::cbor::encode(root);
+      }
+      if (key == actor_state_->allocated_sectors) {
+        return codec::cbor::encode(primitives::RleBitset());
+      }
+      return ERROR_TEXT("ERROR");
+    };
+
+    Actor actor;
+    actor.code = vm::actor::builtin::v0::kStorageMinerCodeId;
+    actor.head = actor_key;
+    api_->StateGetActor = [&](const Address &addr, const TipsetKey &tipset_key)
+        -> outcome::result<Actor> { return actor; };
+
     EXPECT_OUTCOME_ERROR(
-        ChecksError::kExpiredTicket,
+        ChecksError::kPrecommitOnChain,
         checkPrecommit(miner_addr_, info, precommit_key, height, api_));
   }
 
   /**
    * @given info, valid pieces, ticket, precommit on chain
-   * @when try to check precommit
-   * @then ChecksError::kPrecommitOnChain occurs
+   * @when try to check precommit, but ticket with another epoch
+   * @then ChecksError::kBadTicketEpoch occurs
    */
-  TEST_F(CheckPrecommit, PrecommitOnChain) {
+  TEST_F(CheckPrecommit, BadTicketEpoch) {
     std::shared_ptr<SectorInfo> info = std::make_shared<SectorInfo>();
     api::DealId deal_id = 1;
 
@@ -544,17 +580,15 @@ namespace fc::mining::checks {
 
     MOCK_API(api_, StateNetworkVersion);
     EXPECT_CALL(mock_StateNetworkVersion, Call(precommit_key))
-        .Times(2)
         .WillRepeatedly(testing::Return(outcome::success(version_)));
-    EXPECT_OUTCOME_TRUE(duration,
-                        checks::getMaxProveCommitDuration(version_, info));
-    ChainEpoch height = duration
-                        + vm::actor::builtin::types::miner::kChainFinality
-                        + info->ticket_epoch;
+
+    ChainEpoch height;
 
     auto actor_key{"010001020003"_cid};
+
     SectorPreCommitOnChainInfo some_info;
     some_info.info.sealed_cid = "010001020006"_cid;
+    some_info.info.seal_epoch = info->ticket_epoch + 1;
     EXPECT_OUTCOME_TRUE_1(
         actor_state_->precommitted_sectors.set(sector, some_info));
     EXPECT_OUTCOME_TRUE(cid_root,
@@ -582,16 +616,16 @@ namespace fc::mining::checks {
         -> outcome::result<Actor> { return actor; };
 
     EXPECT_OUTCOME_ERROR(
-        ChecksError::kPrecommitOnChain,
+        ChecksError::kBadTicketEpoch,
         checkPrecommit(miner_addr_, info, precommit_key, height, api_));
   }
 
   /**
-   * @given info, valid pieces, ticket, precommit on chain
-   * @when try to check precommit, but ticket with another epoch
-   * @then ChecksError::kBadTicketEpoch occurs
+   * @given info, valid pieces, expired ticket
+   * @when try to check precommit
+   * @then ChecksError::kExpiredTicket occurs
    */
-  TEST_F(CheckPrecommit, BadTicketEpoch) {
+  TEST_F(CheckPrecommit, ExpiredTicket) {
     std::shared_ptr<SectorInfo> info = std::make_shared<SectorInfo>();
     api::DealId deal_id = 1;
 
@@ -651,21 +685,15 @@ namespace fc::mining::checks {
 
     MOCK_API(api_, StateNetworkVersion);
     EXPECT_CALL(mock_StateNetworkVersion, Call(precommit_key))
-        .Times(2)
         .WillRepeatedly(testing::Return(outcome::success(version_)));
-    EXPECT_OUTCOME_TRUE(duration,
-                        checks::getMaxProveCommitDuration(version_, info));
-    ChainEpoch height = duration
-                        + vm::actor::builtin::types::miner::kChainFinality
-                        + info->ticket_epoch;
 
     auto actor_key{"010001020003"_cid};
 
     SectorPreCommitOnChainInfo some_info;
     some_info.info.sealed_cid = "010001020006"_cid;
     some_info.info.seal_epoch = info->ticket_epoch + 1;
     EXPECT_OUTCOME_TRUE_1(
-        actor_state_->precommitted_sectors.set(sector, some_info));
+        actor_state_->precommitted_sectors.set(sector + 1, some_info));
     EXPECT_OUTCOME_TRUE(cid_root,
                         actor_state_->precommitted_sectors.hamt.flush());
 
@@ -690,8 +718,11 @@ namespace fc::mining::checks {
     api_->StateGetActor = [&](const Address &addr, const TipsetKey &tipset_key)
         -> outcome::result<Actor> { return actor; };
 
+    info->ticket_epoch = 0;
+    ChainEpoch height =
+        vm::actor::builtin::types::miner::kMaxPreCommitRandomnessLookback + 1;
     EXPECT_OUTCOME_ERROR(
-        ChecksError::kBadTicketEpoch,
+        ChecksError::kExpiredTicket,
         checkPrecommit(miner_addr_, info, precommit_key, height, api_));
   }
 
