Null Piece data (#560)

* Add null piece_data.

Signed-off-by: Markuu-s <[email protected]>

* Add test for null piece_data.

Signed-off-by: Markuu-s <[email protected]>

* Fix sealing_test. Fix comments from pull request.

Signed-off-by: Markuu-s <[email protected]>

* Fixed constructors. Added test.

Signed-off-by: Markuu-s <[email protected]>

* Changed the getting of Null PieceData.

Signed-off-by: Markuu-s <[email protected]>

Author: Markuu-s

core/miner/storage_fsm/impl/sealing_impl.cpp

@@ -223,7 +223,7 @@ namespace fc::mining {
       for (const auto &pad : sector_and_padding.pads) {
         OUTCOME_TRY(addPiece(sector_and_padding.sector,
                              pad.unpadded(),
-                             PieceData("/dev/zero"),
+                             PieceData::makeNull(),
                              boost::none));
       }
 
@@ -900,7 +900,7 @@ namespace fc::mining {
       OUTCOME_TRY(
           piece_info,
           sealer_->addPieceSync(
-              sector, existing_piece_sizes, size, PieceData("/dev/zero"), 0));
+              sector, existing_piece_sizes, size, PieceData::makeNull(), 0));
 
       existing_piece_sizes.push_back(size);
 

core/primitives/piece/impl/piece_data.cpp

@@ -8,11 +8,25 @@
 #include <boost/filesystem.hpp>
 
 namespace fc::primitives::piece {
-  PieceData::PieceData(const std::string &path_to_file, int flags) {
+  PieceData::PieceData(const std::string &path_to_file, int flags)
+      : is_null_data_(false) {
     fd_ = open(path_to_file.c_str(), flags, 0644);
   }
 
+  PieceData::PieceData(PieceData &&other) noexcept
+      : fd_(other.fd_), is_null_data_(other.is_null_data_) {
+    other.fd_ = kUnopenedFileDescriptor;
+    other.is_null_data_ = false;
+  }
+
+  PieceData::PieceData(int &pipe_fd) : fd_(pipe_fd), is_null_data_(false) {
+    pipe_fd = kUnopenedFileDescriptor;
+  }
+
+  PieceData::PieceData() : fd_(kUnopenedFileDescriptor), is_null_data_(false) {}
+
   int PieceData::getFd() const {
+    assert(not is_null_data_);
     return fd_;
   }
 
@@ -26,19 +40,25 @@ namespace fc::primitives::piece {
     return fd_ != kUnopenedFileDescriptor;
   }
 
-  PieceData::PieceData(PieceData &&other) noexcept
-      : fd_(kUnopenedFileDescriptor) {
-    fd_ = other.fd_;
-    other.fd_ = kUnopenedFileDescriptor;
+  bool PieceData::isNullData() const {
+    return is_null_data_;
   }
 
-  PieceData::PieceData(int &pipe_fd) : fd_(pipe_fd) {
-    pipe_fd = kUnopenedFileDescriptor;
+  PieceData PieceData::makeNull() {
+    PieceData temp = PieceData();
+    temp.is_null_data_ = true;
+    return temp;
   }
 
   PieceData &PieceData::operator=(PieceData &&other) noexcept {
+    if (fd_ != kUnopenedFileDescriptor) {
+      close(fd_);
+    }
     fd_ = other.fd_;
+    is_null_data_ = other.is_null_data_;
+
     other.fd_ = kUnopenedFileDescriptor;
+    other.is_null_data_ = false;
     return *this;
   }
 }  // namespace fc::primitives::piece

core/primitives/piece/piece_data.hpp

@@ -15,7 +15,8 @@ namespace fc::primitives::piece {
   class PieceData {
    public:
     explicit PieceData(const std::string &path_to_file, int flags = O_RDWR);
-    explicit PieceData(int &pipe_fd);
+    explicit PieceData(int &pipe_fd);  // pipe_fd is out parameter and is
+                                       // expected to be set -1 after call.
 
     PieceData(const PieceData &) = delete;
     PieceData &operator=(const PieceData &) = delete;
@@ -28,8 +29,15 @@ namespace fc::primitives::piece {
 
     bool isOpened() const;
 
+    bool isNullData() const;
+
+    static PieceData makeNull();
+
    private:
+    PieceData();
+
     int fd_;
+    bool is_null_data_;
   };
 
 }  // namespace fc::primitives::piece

core/primitives/sector_file/sector_file.cpp

@@ -15,6 +15,7 @@
 #include "primitives/rle_bitset/runs_utils.hpp"
 #include "primitives/sector/sector.hpp"
 #include "proofs/impl/proof_engine_impl.hpp"
+#include "sector_storage/zerocomm/zerocomm.hpp"
 
 namespace fc::primitives::sector_file {
 
@@ -350,6 +351,10 @@ namespace fc::primitives::sector_file {
       PaddedByteIndex offset,
       PaddedPieceSize size,
       const boost::optional<RegisteredSealProof> &maybe_seal_proof_type) {
+    if (data.isNullData()) {
+      return writeNull(offset, size, maybe_seal_proof_type.has_value());
+    }
+
     if (not data.isOpened()) {
       return SectorFileError::kPipeNotOpen;
     }
@@ -454,6 +459,32 @@ namespace fc::primitives::sector_file {
     };
   }
 
+  outcome::result<boost::optional<PieceInfo>> SectorFile::writeNull(
+      PaddedByteIndex offset, PaddedPieceSize size, bool need_piece_info) {
+    if (not file_.good()) {
+      return SectorFileError::kInvalidFile;
+    }
+
+    const auto piece = runsAnd(runs_, std::vector<uint64_t>({offset, size}));
+    OUTCOME_TRY(allocated_size, runsCount(piece));
+
+    assert(allocated_size == 0 && "Overwriting with zeros is not available");
+    // TODO (@Markuu-s) Overwriting with zeros
+    // here we write zeros. but file already zero-filled, so do nothing
+
+    OUTCOME_TRY(markAllocated(offset, size));
+
+    if (not need_piece_info) {
+      return boost::none;
+    }
+
+    OUTCOME_TRY(
+        cid,
+        fc::sector_storage::zerocomm::getZeroPieceCommitment(size.unpadded()));
+
+    return PieceInfo{.size = size, .cid = cid};
+  }
+
   outcome::result<bool> SectorFile::read(const PieceData &output,
                                          PaddedByteIndex offset,
                                          PaddedPieceSize size) {

core/primitives/sector_file/sector_file.hpp

@@ -6,6 +6,7 @@
 #pragma once
 
 #include <fstream>
+#include <map>
 #include <string>
 #include <unordered_map>
 
@@ -122,6 +123,9 @@ namespace fc::primitives::sector_file {
     outcome::result<void> markAllocated(PaddedByteIndex offset,
                                         PaddedPieceSize size);
 
+    outcome::result<boost::optional<PieceInfo>> writeNull(
+        PaddedByteIndex offset, PaddedPieceSize size, bool need_piece_info);
+
     struct PadWriter {
      public:
       explicit PadWriter(std::fstream &output);

test/core/miner/sealing_test.cpp

@@ -424,8 +424,9 @@ namespace fc::mining {
    * @then SealingError::kUpgradeSeveralPieces occurs
    */
   TEST_F(SealingTest, MarkForUpgradeSeveralPieces) {
-    UnpaddedPieceSize piece_size(127);
+    const UnpaddedPieceSize piece_size(127);
     PieceData piece("/dev/random");
+    PieceData piece2("/dev/random");
     DealInfo deal{
         .publish_cid = "010001020001"_cid,
         .deal_id = 0,
@@ -471,7 +472,7 @@ namespace fc::mining {
     EXPECT_OUTCOME_TRUE_1(
         sealing_->addPieceToAnySector(piece_size, std::move(piece), deal));
     EXPECT_OUTCOME_TRUE_1(
-        sealing_->addPieceToAnySector(piece_size, std::move(piece), deal));
+        sealing_->addPieceToAnySector(piece_size, std::move(piece2), deal));
 
     EXPECT_OUTCOME_TRUE_1(
         sealing_->forceSectorState(sector, SealingState::kProving));
@@ -887,12 +888,12 @@ namespace fc::mining {
 
     SectorRef sector_ref{.id = sector_id, .proof_type = seal_proof_type_};
     std::vector<UnpaddedPieceSize> exist_pieces = {};
-    EXPECT_CALL(*manager_,
-                doAddPieceSync(sector_ref,
-                               gsl::span<const UnpaddedPieceSize>(exist_pieces),
-                               PaddedPieceSize(sector_size_).unpadded(),
-                               _,
-                               0))
+    EXPECT_CALL(
+        *manager_,
+        doNullAddPieceSync(sector_ref,
+                           gsl::span<const UnpaddedPieceSize>(exist_pieces),
+                           PaddedPieceSize(sector_size_).unpadded(),
+                           0))
         .WillOnce(testing::Return(outcome::success(info)));
 
     ASSERT_EQ(sealing_->getListSectors().size(), 1);

test/core/primitives/sector_file/sector_file_test.cpp

@@ -6,6 +6,7 @@
 #include "primitives/sector_file/sector_file.hpp"
 
 #include <gtest/gtest.h>
+#include "sector_storage/zerocomm/zerocomm.hpp"
 #include "testutil/outcome.hpp"
 #include "testutil/read_file.hpp"
 #include "testutil/resources/resources.hpp"
@@ -240,9 +241,67 @@ namespace fc::primitives::sector_file {
     EXPECT_OUTCOME_TRUE(
         sector_size,
         fc::primitives::sector::getSectorSize(min_seal_proof_type));
-    uint64_t result =
+    const uint64_t result =
         kOverheadSeal.at(file_type) * sector_size / kOverheadDenominator;
     EXPECT_OUTCOME_TRUE(seal_size, sealSpaceUse(file_type, sector_size));
     ASSERT_EQ(result, seal_size);
   }
+
+  TEST_F(SectorFileTest, checkFlag) {
+    EXPECT_TRUE(PieceData::makeNull().isNullData());
+  }
+
+  /*
+   * TODO (@Markuu-s) add tests: CannotMoveCursor and checkAssertGetFd
+   * Problem: ASAN-build doesn't pass these tests
+   */
+
+  TEST_F(SectorFileTest, WriteNullPiece) {
+    EXPECT_OUTCOME_TRUE(
+        sector_size,
+        fc::primitives::sector::getSectorSize(min_seal_proof_type));
+    const SectorId sector{
+        .miner = 1,
+        .sector = 1,
+    };
+
+    const PaddedPieceSize piece_size(128);
+
+    EXPECT_OUTCOME_TRUE(result_cid,
+                        fc::sector_storage::zerocomm::getZeroPieceCommitment(
+                            piece_size.unpadded()));
+    EXPECT_OUTCOME_TRUE(
+        file,
+        SectorFile::createFile(
+            (base_path / fc::primitives::sector_file::sectorName(sector))
+                .string(),
+            PaddedPieceSize(sector_size)));
+
+    EXPECT_OUTCOME_TRUE(
+        piece_info,
+        file->write(PieceData::makeNull(), 0, piece_size, min_seal_proof_type));
+
+    ASSERT_EQ(piece_info->size, piece_size);
+    EXPECT_EQ(piece_info->cid, result_cid);
+    EXPECT_OUTCOME_EQ(file->hasAllocated(0, piece_size.unpadded()), true);
+
+    EXPECT_OUTCOME_TRUE(cid,
+                        fc::sector_storage::zerocomm::getZeroPieceCommitment(
+                            piece_size.unpadded()));
+
+    EXPECT_EQ(piece_info->cid, cid);
+
+    const boost::filesystem::path read_file_path =
+        base_path / fs::unique_path();
+    EXPECT_OUTCOME_TRUE_1(file->read(
+        PieceData(read_file_path.string(), O_WRONLY | O_CREAT), 0, piece_size));
+    const std::vector<unsigned char> read_data = readFile(read_file_path);
+
+    ASSERT_EQ(read_data.size(), piece_size.unpadded());
+    const std::string result_data = std::string(piece_size.unpadded(), '\0');
+    ASSERT_EQ(
+        read_data,
+        std::vector<unsigned char>(result_data.begin(), result_data.end()));
+  }
+
 }  // namespace fc::primitives::sector_file

test/testutil/mocks/sector_storage/manager_mock.hpp

@@ -172,12 +172,23 @@ namespace fc::sector_storage {
                      int,
                      uint64_t priority));
 
+    MOCK_METHOD4(doNullAddPieceSync,
+                 outcome::result<PieceInfo>(
+                     const SectorRef &sector,
+                     gsl::span<const UnpaddedPieceSize> piece_sizes,
+                     const UnpaddedPieceSize &new_piece_size,
+                     uint64_t priority));
+
     outcome::result<PieceInfo> addPieceSync(
         const SectorRef &sector,
         gsl::span<const UnpaddedPieceSize> piece_sizes,
         const UnpaddedPieceSize &new_piece_size,
         proofs::PieceData piece_data,
         uint64_t priority) override {
+      if (piece_data.isNullData()) {
+        return doNullAddPieceSync(
+            sector, piece_sizes, new_piece_size, priority);
+      }
       return doAddPieceSync(
           sector, piece_sizes, new_piece_size, piece_data.getFd(), priority);
     }