From 8e102cd58fcb737666b6f566c7e354f1a55f86d5 Mon Sep 17 00:00:00 2001 From: elestrias Date: Thu, 8 Jul 2021 13:44:01 +0300 Subject: [PATCH 01/17] PreCommitBatcher Signed-off-by: elestrias --- core/miner/storage_fsm/CMakeLists.txt | 8 + .../impl/precommit_batcher_impl.cpp | 134 +++++++++++++ .../impl/precommit_batcher_impl.hpp | 80 ++++++++ core/miner/storage_fsm/precommit_batcher.hpp | 17 ++ test/core/miner/CMakeLists.txt | 6 + test/core/miner/PreCommitBatcherTest.cpp | 188 ++++++++++++++++++ test/testutil/mocks/libp2p/scheduler_mock.hpp | 2 +- 7 files changed, 434 insertions(+), 1 deletion(-) create mode 100644 core/miner/storage_fsm/impl/precommit_batcher_impl.cpp create mode 100644 core/miner/storage_fsm/impl/precommit_batcher_impl.hpp create mode 100644 core/miner/storage_fsm/precommit_batcher.hpp create mode 100644 test/core/miner/PreCommitBatcherTest.cpp diff --git a/core/miner/storage_fsm/CMakeLists.txt b/core/miner/storage_fsm/CMakeLists.txt index c7e89a3e65..fc8606d23e 100644 --- a/core/miner/storage_fsm/CMakeLists.txt +++ b/core/miner/storage_fsm/CMakeLists.txt @@ -44,6 +44,14 @@ target_link_libraries(deal_info_manager api ) +add_library(batcher + impl/precommit_batcher_impl.cpp + ) +target_link_libraries(batcher + api + p2p::asio_scheduler + ) + add_library(storage_fsm impl/sealing_impl.cpp impl/checks.cpp diff --git a/core/miner/storage_fsm/impl/precommit_batcher_impl.cpp b/core/miner/storage_fsm/impl/precommit_batcher_impl.cpp new file mode 100644 index 0000000000..3cedc764e1 --- /dev/null +++ b/core/miner/storage_fsm/impl/precommit_batcher_impl.cpp @@ -0,0 +1,134 @@ +/** + * Copyright Soramitsu Co., Ltd. All Rights Reserved. + * SPDX-License-Identifier: Apache-2.0 + */ + +#include "miner/storage_fsm/impl/precommit_batcher_impl.hpp" + +#include + +namespace fc::mining { + + PreCommitBatcherImpl::PreCommitBatcherImpl( + size_t maxTime, + std::shared_ptr api, + std::shared_ptr scheduler) + : api_(std::move(api)), sch_(std::move(scheduler)){}; + + PreCommitBatcherImpl::preCommitEntry::preCommitEntry( + primitives::TokenAmount number, api::SectorPreCommitInfo info) + : deposit(std::move(number)), pci(std::move(info)){}; + + PreCommitBatcherImpl::preCommitEntry & + PreCommitBatcherImpl::preCommitEntry::operator=(const preCommitEntry &x) = default; + + outcome::result> + PreCommitBatcherImpl::makeBatcher( + size_t maxWait, + std::shared_ptr api, + std::shared_ptr scheduler, + Address &miner_address) { + + struct make_unique_enabler : public PreCommitBatcherImpl { + make_unique_enabler( + size_t MaxTime, + std::shared_ptr api, + std::shared_ptr scheduler) + : PreCommitBatcherImpl( + MaxTime, std::move(api), std::move(scheduler)){}; + }; + + std::shared_ptr batcher = + std::make_unique( + maxWait, std::move(api), std::move(scheduler)); + + batcher->miner_address_ = miner_address; + batcher->maxDelay = maxWait; + + batcher->handle_ = batcher->sch_->schedule(maxWait, [=](){ + auto maybe_send = batcher->sendBatch(); + if(maybe_send.has_error()){ + return maybe_send.error(); + } + }); + return batcher; + } + + outcome::result PreCommitBatcherImpl::sendBatch() { + std::unique_lock locker(mutex_, std::defer_lock); + + OUTCOME_TRY(head, api_->ChainHead()); + OUTCOME_TRY(minfo, api_->StateMinerInfo(miner_address_, head->key)); + + std::vector params; + for (auto &data : batchStorage) { + mutualDeposit += data.second.deposit; + params.push_back(data.second.pci); + } + // TODO: goodFunds = mutualDeposit + MaxFee; - for checking payable + + OUTCOME_TRY(encodedParams, codec::cbor::encode(params)); + + auto maybe_signed = api_->MpoolPushMessage( + vm::message::UnsignedMessage( + miner_address_, + minfo.worker, // TODO: handle worker + 0, + mutualDeposit, + {}, + {}, + // TODO: PreCommitSectorBatch + vm::actor::builtin::v0::miner::PreCommitSector::Number, + MethodParams{encodedParams}), + kPushNoSpec); + + if(maybe_signed.has_error()){ + handle_.reschedule(maxDelay); + return maybe_signed.error(); + } + + mutualDeposit = 0; + batchStorage.clear(); + handle_.reschedule(maxDelay); // TODO: maybe in gsl::finally + return outcome::success(); + } + + outcome::result PreCommitBatcherImpl::forceSend() { + handle_.reschedule(0); + return outcome::success(); + } + + void PreCommitBatcherImpl::getPreCommitCutoff( + primitives::ChainEpoch curEpoch, const SectorInfo& si) { + primitives::ChainEpoch cutoffEpoch = si.ticket_epoch + static_cast(fc::kEpochsInDay + 600); + primitives::ChainEpoch startEpoch{}; + for (auto p : si.pieces) { + if (!p.deal_info) { + continue; + } + startEpoch = p.deal_info->deal_schedule.start_epoch; + if (startEpoch < cutoffEpoch) { + cutoffEpoch = startEpoch ; + } + } + if (cutoffEpoch <= curEpoch) { + handle_.reschedule(0); + } else { + handle_.reschedule(toTicks(std::chrono::seconds( + (cutoffEpoch - curEpoch) * kEpochDurationSeconds))); + } + } + + outcome::result PreCommitBatcherImpl::addPreCommit( + SectorInfo secInf, + primitives::TokenAmount deposit, + api::SectorPreCommitInfo pcInfo) { + std::unique_lock locker(mutex_, std::defer_lock); + OUTCOME_TRY(head, api_->ChainHead()); + + //getPreCommitCutoff(head->epoch(), secInf); + auto sn = secInf.sector_number; + batchStorage[sn] = preCommitEntry(std::move(deposit), std::move(pcInfo)); + return outcome::success(); + } +} // namespace fc::mining \ No newline at end of file diff --git a/core/miner/storage_fsm/impl/precommit_batcher_impl.hpp b/core/miner/storage_fsm/impl/precommit_batcher_impl.hpp new file mode 100644 index 0000000000..3e645b22e4 --- /dev/null +++ b/core/miner/storage_fsm/impl/precommit_batcher_impl.hpp @@ -0,0 +1,80 @@ +/** + * Copyright Soramitsu Co., Ltd. All Rights Reserved. + * SPDX-License-Identifier: Apache-2.0 + */ + +#pragma once + +#include +#include +#include +#include +#include +#include +#include +#include "api/full_node/node_api.hpp" +#include "const.hpp" +#include "fsm/fsm.hpp" +#include "miner/storage_fsm/precommit_batcher.hpp" +#include "miner/storage_fsm/sealing_events.hpp" +#include "miner/storage_fsm/types.hpp" +#include "primitives/address/address.hpp" +#include "primitives/types.hpp" +#include "vm/actor/actor_method.hpp" +#include "vm/actor/builtin/types/miner/sector_info.hpp" +#include "vm/actor/builtin/v0/miner/miner_actor.hpp" + +namespace fc::mining { + using api::FullNodeApi; + using api::SectorNumber; + using api::SectorPreCommitInfo; + using api::kPushNoSpec; + + using boost::multiprecision::cpp_int; + using fc::mining::types::SectorInfo; + using libp2p::protocol::scheduler::Handle; + using libp2p::protocol::scheduler::toTicks; + using primitives::address::Address; + using vm::actor::MethodParams; + using StorageFSM = + fsm::FSM; + + class PreCommitBatcherImpl : public PreCommitBatcher { + public: + struct preCommitEntry { + preCommitEntry(primitives::TokenAmount number, + SectorPreCommitInfo info); + preCommitEntry() = default; + primitives::TokenAmount deposit; + SectorPreCommitInfo pci; + preCommitEntry& operator=(const preCommitEntry &x); + }; + + void getPreCommitCutoff(primitives::ChainEpoch curEpoch, + const SectorInfo & si); + + static outcome::result> makeBatcher(size_t maxWait, std::shared_ptr api, + std::shared_ptr scheduler, Address & miner_address); + + outcome::result addPreCommit(SectorInfo secInf, + primitives::TokenAmount deposit, + SectorPreCommitInfo pcInfo); + + outcome::result forceSend(); + + outcome::result sendBatch(); + + + private: + PreCommitBatcherImpl(size_t maxTime, std::shared_ptr api, std::shared_ptr scheduler); + std::mutex mutex_; + Address miner_address_; + cpp_int mutualDeposit; + std::map batchStorage; + std::shared_ptr api_; + std::shared_ptr sch_; + Handle handle_; + size_t maxDelay; + }; + +} // namespace fc::mining diff --git a/core/miner/storage_fsm/precommit_batcher.hpp b/core/miner/storage_fsm/precommit_batcher.hpp new file mode 100644 index 0000000000..b9fe6a5cf0 --- /dev/null +++ b/core/miner/storage_fsm/precommit_batcher.hpp @@ -0,0 +1,17 @@ +/** + * Copyright Soramitsu Co., Ltd. All Rights Reserved. + * SPDX-License-Identifier: Apache-2.0 + */ + +#pragma once +#include +#include + +namespace fc::mining { + class PreCommitBatcher { + public: + + virtual ~PreCommitBatcher() = default; + }; + +} // namespace fc::mining \ No newline at end of file diff --git a/test/core/miner/CMakeLists.txt b/test/core/miner/CMakeLists.txt index 3cc1198420..d65280ffab 100644 --- a/test/core/miner/CMakeLists.txt +++ b/test/core/miner/CMakeLists.txt @@ -9,6 +9,12 @@ addtest(events_test target_link_libraries(events_test events) +addtest(batcher_test + PreCommitBatcherTest.cpp + ) +target_link_libraries(batcher_test + batcher) + addtest(precommit_policy_test precommit_policy_test.cpp ) diff --git a/test/core/miner/PreCommitBatcherTest.cpp b/test/core/miner/PreCommitBatcherTest.cpp new file mode 100644 index 0000000000..1c6c2be609 --- /dev/null +++ b/test/core/miner/PreCommitBatcherTest.cpp @@ -0,0 +1,188 @@ +/** + * Copyright Soramitsu Co., Ltd. All Rights Reserved. + * SPDX-License-Identifier: Apache-2.0 + */ + +#include +#include "miner/storage_fsm/impl/precommit_batcher_impl.hpp" +#include "testutil/literals.hpp" +#include "testutil/mocks/libp2p/scheduler_mock.hpp" + +namespace fc::mining { + using api::MinerInfo; + using api::SignedMessage; + using api::UnsignedMessage; + using libp2p::protocol::SchedulerMock; + using libp2p::protocol::scheduler::Ticks; + using primitives::tipset::Tipset; + using primitives::tipset::TipsetCPtr; + using testing::Mock; + + class PreCommitBatcherTest : public testing::Test { + protected: + virtual void SetUp() { + seal_proof_type_ = RegisteredSealProof::kStackedDrg2KiBV1; + api_ = std::make_shared(); + sch_ = std::make_shared(); + miner_id = 42; + miner_address_ = Address::makeFromId(miner_id); + current_time_ = toTicks(std::chrono::duration_cast( + std::chrono::system_clock::now().time_since_epoch())); + + api::BlockHeader block; + block.height = 9; + + auto tipset = std::make_shared( + TipsetKey(), std::vector({block})); + + api_->ChainHead = [&]() -> outcome::result { + return outcome::success(tipset); + }; + + api_->StateMinerInfo = + [&](const Address &address, + const TipsetKey &tipset_key) -> outcome::result { + if (address == miner_address_) { + MinerInfo info; + info.seal_proof_type = seal_proof_type_; + return info; + } + }; + EXPECT_CALL(*sch_, now()).WillOnce(testing::Return(current_time_)); + auto result = PreCommitBatcherImpl::makeBatcher( + toTicks(std::chrono::seconds(10)), api_, sch_, miner_address_); + batcher_ = result.value(); + ASSERT_FALSE(result.has_error()); + } + virtual void TearDown(){ + batcher_.reset(); + EXPECT_TRUE(Mock::VerifyAndClearExpectations(sch_.get())); + } + + + std::shared_ptr api_; + std::shared_ptr sch_; + std::shared_ptr batcher_; + Address miner_address_; + uint64_t miner_id; + RegisteredSealProof seal_proof_type_; + Ticks current_time_; + }; + + TEST_F(PreCommitBatcherTest, BatcherSetup) {} + + TEST_F(PreCommitBatcherTest, BatcherWrite) { + api::BlockHeader block; + block.height = 9; + auto tipset = std::make_shared( + TipsetKey(), std::vector({block})); + api_->ChainHead = [&]() -> outcome::result { + return outcome::success(tipset); + }; + + SectorInfo si = SectorInfo(); + api::SectorPreCommitInfo precInf; + primitives::TokenAmount deposit = 10; + ASSERT_FALSE(batcher_->addPreCommit(si, deposit, precInf).has_error()); + }; + + TEST_F(PreCommitBatcherTest, BatcherSend) { + api::BlockHeader block; + block.height = 9; + primitives::TokenAmount mutualDeposit = 0; + bool isCalled = false; + auto tipset = std::make_shared( + TipsetKey(), std::vector({block})); + + api_->ChainHead = [&]() -> outcome::result { + return outcome::success(tipset); + }; + + api_->MpoolPushMessage = [&isCalled, &mutualDeposit]( + const UnsignedMessage &msg, + const boost::optional &) + -> outcome::result { + std::cout << "testing\n"; + if (msg.method == vm::actor::builtin::v0::miner::PreCommitSector::Number + && msg.value == mutualDeposit) { + isCalled = true; + return SignedMessage{.message = msg, .signature = BlsSignature()}; + } + + return ERROR_TEXT("ERROR"); + }; + + SectorInfo si = SectorInfo(); + api::SectorPreCommitInfo precInf; + primitives::TokenAmount deposit = 10; + + precInf.sealed_cid = "010001020005"_cid; + si.sector_number = 2; + + auto maybe_result = batcher_->addPreCommit(si, deposit, precInf); + ASSERT_FALSE(maybe_result.has_error()); + mutualDeposit += 10; + + precInf.sealed_cid = "010001020006"_cid; + si.sector_number = 3; + + maybe_result = batcher_->addPreCommit(si, deposit, precInf); + ASSERT_FALSE(maybe_result.has_error()); + mutualDeposit += 10; + + auto maybe_force_send = batcher_->sendBatch(); + + if (maybe_force_send.has_error()) { + std::cout << maybe_force_send.error() << "\n"; + } + ASSERT_FALSE(maybe_force_send.has_error()); + ASSERT_TRUE(isCalled); + } + + TEST_F(PreCommitBatcherTest, CallbackSend) { + api::BlockHeader block; + block.height = 9; + primitives::TokenAmount mutualDeposit = 0; + bool isCalled = false; + auto tipset = std::make_shared( + TipsetKey(), std::vector({block})); + + api_->ChainHead = [&]() -> outcome::result { + return outcome::success(tipset); + }; + + api_->MpoolPushMessage = [&isCalled, &mutualDeposit]( + const UnsignedMessage &msg, + const boost::optional &) + -> outcome::result { + std::cout << "testing\n"; + if (msg.method == vm::actor::builtin::v0::miner::PreCommitSector::Number + && msg.value == mutualDeposit) { + isCalled = true; + return SignedMessage{.message = msg, .signature = BlsSignature()}; + } + + return ERROR_TEXT("ERROR"); + }; + + SectorInfo si = SectorInfo(); + api::SectorPreCommitInfo precInf; + primitives::TokenAmount deposit = 10; + + precInf.sealed_cid = "010001020005"_cid; + si.sector_number = 2; + + auto maybe_result = batcher_->addPreCommit(si, deposit, precInf); + ASSERT_FALSE(maybe_result.has_error()); + mutualDeposit += 10; + + EXPECT_CALL(*sch_, now()) + .WillOnce( + testing::Return(current_time_ + toTicks(std::chrono::seconds(11)))) + .WillOnce( testing::Return(current_time_ + toTicks(std::chrono::seconds(11)))); + sch_->next_clock(); + ASSERT_TRUE(isCalled); + + } + +} // namespace fc::mining diff --git a/test/testutil/mocks/libp2p/scheduler_mock.hpp b/test/testutil/mocks/libp2p/scheduler_mock.hpp index ea8952f7c7..96f485dde3 100644 --- a/test/testutil/mocks/libp2p/scheduler_mock.hpp +++ b/test/testutil/mocks/libp2p/scheduler_mock.hpp @@ -17,7 +17,7 @@ namespace libp2p::protocol { void next_clock() { pulse(false); } - + ~SchedulerMock() override = default; protected: void scheduleImmediate() override { pulse(true); From 38a3227fe724125dc0b9fc450a06d436a081a059 Mon Sep 17 00:00:00 2001 From: elestrias Date: Thu, 8 Jul 2021 16:02:24 +0300 Subject: [PATCH 02/17] PreCommitBatcher: Cutoff_update Signed-off-by: elestrias --- .../impl/precommit_batcher_impl.cpp | 56 ++++++++++-------- .../impl/precommit_batcher_impl.hpp | 8 ++- test/core/miner/PreCommitBatcherTest.cpp | 59 ++----------------- 3 files changed, 40 insertions(+), 83 deletions(-) diff --git a/core/miner/storage_fsm/impl/precommit_batcher_impl.cpp b/core/miner/storage_fsm/impl/precommit_batcher_impl.cpp index 3cedc764e1..4f59b5d60d 100644 --- a/core/miner/storage_fsm/impl/precommit_batcher_impl.cpp +++ b/core/miner/storage_fsm/impl/precommit_batcher_impl.cpp @@ -9,18 +9,17 @@ namespace fc::mining { - PreCommitBatcherImpl::PreCommitBatcherImpl( - size_t maxTime, - std::shared_ptr api, - std::shared_ptr scheduler) - : api_(std::move(api)), sch_(std::move(scheduler)){}; + PreCommitBatcherImpl::PreCommitBatcherImpl(size_t maxTime, + std::shared_ptr api) + : api_(std::move(api)){}; PreCommitBatcherImpl::preCommitEntry::preCommitEntry( primitives::TokenAmount number, api::SectorPreCommitInfo info) : deposit(std::move(number)), pci(std::move(info)){}; PreCommitBatcherImpl::preCommitEntry & - PreCommitBatcherImpl::preCommitEntry::operator=(const preCommitEntry &x) = default; + PreCommitBatcherImpl::preCommitEntry::operator=(const preCommitEntry &x) = + default; outcome::result> PreCommitBatcherImpl::makeBatcher( @@ -28,33 +27,28 @@ namespace fc::mining { std::shared_ptr api, std::shared_ptr scheduler, Address &miner_address) { - struct make_unique_enabler : public PreCommitBatcherImpl { - make_unique_enabler( - size_t MaxTime, - std::shared_ptr api, - std::shared_ptr scheduler) - : PreCommitBatcherImpl( - MaxTime, std::move(api), std::move(scheduler)){}; + make_unique_enabler(size_t MaxTime, std::shared_ptr api) + : PreCommitBatcherImpl(MaxTime, std::move(api)){}; }; std::shared_ptr batcher = - std::make_unique( - maxWait, std::move(api), std::move(scheduler)); + std::make_unique(maxWait, std::move(api)); batcher->miner_address_ = miner_address; batcher->maxDelay = maxWait; - batcher->handle_ = batcher->sch_->schedule(maxWait, [=](){ + batcher->cutoffStart = std::chrono::system_clock::now(); + batcher->handle_ = scheduler->schedule(maxWait, [=]() { auto maybe_send = batcher->sendBatch(); - if(maybe_send.has_error()){ + if (maybe_send.has_error()) { return maybe_send.error(); } }); return batcher; } - outcome::result PreCommitBatcherImpl::sendBatch() { + outcome::result PreCommitBatcherImpl::sendBatch() { std::unique_lock locker(mutex_, std::defer_lock); OUTCOME_TRY(head, api_->ChainHead()); @@ -82,7 +76,7 @@ namespace fc::mining { MethodParams{encodedParams}), kPushNoSpec); - if(maybe_signed.has_error()){ + if (maybe_signed.has_error()) { handle_.reschedule(maxDelay); return maybe_signed.error(); } @@ -90,6 +84,7 @@ namespace fc::mining { mutualDeposit = 0; batchStorage.clear(); handle_.reschedule(maxDelay); // TODO: maybe in gsl::finally + cutoffStart = std::chrono::system_clock::now(); return outcome::success(); } @@ -98,9 +93,10 @@ namespace fc::mining { return outcome::success(); } - void PreCommitBatcherImpl::getPreCommitCutoff( - primitives::ChainEpoch curEpoch, const SectorInfo& si) { - primitives::ChainEpoch cutoffEpoch = si.ticket_epoch + static_cast(fc::kEpochsInDay + 600); + void PreCommitBatcherImpl::getPreCommitCutoff(primitives::ChainEpoch curEpoch, + const SectorInfo &si) { + primitives::ChainEpoch cutoffEpoch = + si.ticket_epoch + static_cast(fc::kEpochsInDay + 600); primitives::ChainEpoch startEpoch{}; for (auto p : si.pieces) { if (!p.deal_info) { @@ -108,14 +104,22 @@ namespace fc::mining { } startEpoch = p.deal_info->deal_schedule.start_epoch; if (startEpoch < cutoffEpoch) { - cutoffEpoch = startEpoch ; + cutoffEpoch = startEpoch; } } if (cutoffEpoch <= curEpoch) { handle_.reschedule(0); } else { - handle_.reschedule(toTicks(std::chrono::seconds( - (cutoffEpoch - curEpoch) * kEpochDurationSeconds))); + Ticks tempCutoff = toTicks(std::chrono::seconds((cutoffEpoch - curEpoch) + * kEpochDurationSeconds)); + if ((closestCutoff + - toTicks(std::chrono::duration_cast( + std::chrono::system_clock::now() - cutoffStart)) + > tempCutoff)) { + handle_.reschedule(tempCutoff); + closestCutoff = tempCutoff; + cutoffStart = std::chrono::system_clock::now(); + } } } @@ -126,7 +130,7 @@ namespace fc::mining { std::unique_lock locker(mutex_, std::defer_lock); OUTCOME_TRY(head, api_->ChainHead()); - //getPreCommitCutoff(head->epoch(), secInf); + getPreCommitCutoff(head->epoch(), secInf); auto sn = secInf.sector_number; batchStorage[sn] = preCommitEntry(std::move(deposit), std::move(pcInfo)); return outcome::success(); diff --git a/core/miner/storage_fsm/impl/precommit_batcher_impl.hpp b/core/miner/storage_fsm/impl/precommit_batcher_impl.hpp index 3e645b22e4..1c5f7ef88d 100644 --- a/core/miner/storage_fsm/impl/precommit_batcher_impl.hpp +++ b/core/miner/storage_fsm/impl/precommit_batcher_impl.hpp @@ -29,7 +29,7 @@ namespace fc::mining { using api::SectorNumber; using api::SectorPreCommitInfo; using api::kPushNoSpec; - + using libp2p::protocol::scheduler::Ticks; using boost::multiprecision::cpp_int; using fc::mining::types::SectorInfo; using libp2p::protocol::scheduler::Handle; @@ -66,15 +66,17 @@ namespace fc::mining { private: - PreCommitBatcherImpl(size_t maxTime, std::shared_ptr api, std::shared_ptr scheduler); + PreCommitBatcherImpl(size_t maxTime, std::shared_ptr api); std::mutex mutex_; Address miner_address_; cpp_int mutualDeposit; std::map batchStorage; std::shared_ptr api_; - std::shared_ptr sch_; Handle handle_; size_t maxDelay; + Ticks closestCutoff; + std::chrono::system_clock::time_point cutoffStart; + }; } // namespace fc::mining diff --git a/test/core/miner/PreCommitBatcherTest.cpp b/test/core/miner/PreCommitBatcherTest.cpp index 1c6c2be609..fb8528af95 100644 --- a/test/core/miner/PreCommitBatcherTest.cpp +++ b/test/core/miner/PreCommitBatcherTest.cpp @@ -54,11 +54,6 @@ namespace fc::mining { batcher_ = result.value(); ASSERT_FALSE(result.has_error()); } - virtual void TearDown(){ - batcher_.reset(); - EXPECT_TRUE(Mock::VerifyAndClearExpectations(sch_.get())); - } - std::shared_ptr api_; std::shared_ptr sch_; @@ -86,7 +81,7 @@ namespace fc::mining { ASSERT_FALSE(batcher_->addPreCommit(si, deposit, precInf).has_error()); }; - TEST_F(PreCommitBatcherTest, BatcherSend) { + TEST_F(PreCommitBatcherTest, CallbackSend) { api::BlockHeader block; block.height = 9; primitives::TokenAmount mutualDeposit = 0; @@ -130,59 +125,15 @@ namespace fc::mining { ASSERT_FALSE(maybe_result.has_error()); mutualDeposit += 10; - auto maybe_force_send = batcher_->sendBatch(); - - if (maybe_force_send.has_error()) { - std::cout << maybe_force_send.error() << "\n"; - } - ASSERT_FALSE(maybe_force_send.has_error()); - ASSERT_TRUE(isCalled); - } - - TEST_F(PreCommitBatcherTest, CallbackSend) { - api::BlockHeader block; - block.height = 9; - primitives::TokenAmount mutualDeposit = 0; - bool isCalled = false; - auto tipset = std::make_shared( - TipsetKey(), std::vector({block})); - - api_->ChainHead = [&]() -> outcome::result { - return outcome::success(tipset); - }; - - api_->MpoolPushMessage = [&isCalled, &mutualDeposit]( - const UnsignedMessage &msg, - const boost::optional &) - -> outcome::result { - std::cout << "testing\n"; - if (msg.method == vm::actor::builtin::v0::miner::PreCommitSector::Number - && msg.value == mutualDeposit) { - isCalled = true; - return SignedMessage{.message = msg, .signature = BlsSignature()}; - } - - return ERROR_TEXT("ERROR"); - }; - - SectorInfo si = SectorInfo(); - api::SectorPreCommitInfo precInf; - primitives::TokenAmount deposit = 10; - - precInf.sealed_cid = "010001020005"_cid; - si.sector_number = 2; - - auto maybe_result = batcher_->addPreCommit(si, deposit, precInf); - ASSERT_FALSE(maybe_result.has_error()); - mutualDeposit += 10; - EXPECT_CALL(*sch_, now()) .WillOnce( testing::Return(current_time_ + toTicks(std::chrono::seconds(11)))) - .WillOnce( testing::Return(current_time_ + toTicks(std::chrono::seconds(11)))); + .WillOnce( + testing::Return(current_time_ + toTicks(std::chrono::seconds(11)))) + .WillRepeatedly( + testing::Return(current_time_ + toTicks(std::chrono::seconds(11)))); sch_->next_clock(); ASSERT_TRUE(isCalled); - } } // namespace fc::mining From b5e0f5efc555fd884051f8fae1f4431d94a7203b Mon Sep 17 00:00:00 2001 From: elestrias Date: Fri, 9 Jul 2021 12:23:30 +0300 Subject: [PATCH 03/17] PrecommitBatch: ActrorMethod + cutoff tests Signed-off-by: elestrias --- .../impl/precommit_batcher_impl.cpp | 8 +- .../impl/precommit_batcher_impl.hpp | 8 +- core/miner/storage_fsm/precommit_batcher.hpp | 10 +- .../vm/actor/builtin/v0/miner/miner_actor.hpp | 14 +++ test/core/miner/PreCommitBatcherTest.cpp | 117 ++++++++++++++---- 5 files changed, 124 insertions(+), 33 deletions(-) diff --git a/core/miner/storage_fsm/impl/precommit_batcher_impl.cpp b/core/miner/storage_fsm/impl/precommit_batcher_impl.cpp index 4f59b5d60d..19c60ce0de 100644 --- a/core/miner/storage_fsm/impl/precommit_batcher_impl.cpp +++ b/core/miner/storage_fsm/impl/precommit_batcher_impl.cpp @@ -39,6 +39,7 @@ namespace fc::mining { batcher->maxDelay = maxWait; batcher->cutoffStart = std::chrono::system_clock::now(); + batcher -> closestCutoff = maxWait; batcher->handle_ = scheduler->schedule(maxWait, [=]() { auto maybe_send = batcher->sendBatch(); if (maybe_send.has_error()) { @@ -71,7 +72,6 @@ namespace fc::mining { mutualDeposit, {}, {}, - // TODO: PreCommitSectorBatch vm::actor::builtin::v0::miner::PreCommitSector::Number, MethodParams{encodedParams}), kPushNoSpec); @@ -93,11 +93,11 @@ namespace fc::mining { return outcome::success(); } - void PreCommitBatcherImpl::getPreCommitCutoff(primitives::ChainEpoch curEpoch, + void PreCommitBatcherImpl::getPreCommitCutoff(ChainEpoch curEpoch, const SectorInfo &si) { primitives::ChainEpoch cutoffEpoch = si.ticket_epoch + static_cast(fc::kEpochsInDay + 600); - primitives::ChainEpoch startEpoch{}; + ChainEpoch startEpoch{}; for (auto p : si.pieces) { if (!p.deal_info) { continue; @@ -116,9 +116,9 @@ namespace fc::mining { - toTicks(std::chrono::duration_cast( std::chrono::system_clock::now() - cutoffStart)) > tempCutoff)) { + cutoffStart = std::chrono::system_clock::now(); handle_.reschedule(tempCutoff); closestCutoff = tempCutoff; - cutoffStart = std::chrono::system_clock::now(); } } } diff --git a/core/miner/storage_fsm/impl/precommit_batcher_impl.hpp b/core/miner/storage_fsm/impl/precommit_batcher_impl.hpp index 1c5f7ef88d..62c6dc9097 100644 --- a/core/miner/storage_fsm/impl/precommit_batcher_impl.hpp +++ b/core/miner/storage_fsm/impl/precommit_batcher_impl.hpp @@ -30,11 +30,13 @@ namespace fc::mining { using api::SectorPreCommitInfo; using api::kPushNoSpec; using libp2p::protocol::scheduler::Ticks; + using libp2p::protocol::Scheduler; using boost::multiprecision::cpp_int; - using fc::mining::types::SectorInfo; using libp2p::protocol::scheduler::Handle; using libp2p::protocol::scheduler::toTicks; using primitives::address::Address; + using primitives::TokenAmount; + using primitives::ChainEpoch; using vm::actor::MethodParams; using StorageFSM = fsm::FSM; @@ -54,10 +56,10 @@ namespace fc::mining { const SectorInfo & si); static outcome::result> makeBatcher(size_t maxWait, std::shared_ptr api, - std::shared_ptr scheduler, Address & miner_address); + std::shared_ptr scheduler, Address & miner_address); outcome::result addPreCommit(SectorInfo secInf, - primitives::TokenAmount deposit, + TokenAmount deposit, SectorPreCommitInfo pcInfo); outcome::result forceSend(); diff --git a/core/miner/storage_fsm/precommit_batcher.hpp b/core/miner/storage_fsm/precommit_batcher.hpp index b9fe6a5cf0..4cc6f1efbd 100644 --- a/core/miner/storage_fsm/precommit_batcher.hpp +++ b/core/miner/storage_fsm/precommit_batcher.hpp @@ -6,11 +6,19 @@ #pragma once #include #include +#include "miner/storage_fsm/types.hpp" namespace fc::mining { + using api::SectorPreCommitInfo; + using primitives::TokenAmount; + using fc::mining::types::SectorInfo; + class PreCommitBatcher { public: - + virtual outcome::result addPreCommit(SectorInfo secInf, + TokenAmount deposit, + SectorPreCommitInfo pcInfo) = 0; + virtual outcome::result forceSend()=0; virtual ~PreCommitBatcher() = default; }; diff --git a/core/vm/actor/builtin/v0/miner/miner_actor.hpp b/core/vm/actor/builtin/v0/miner/miner_actor.hpp index 05d24ef4a4..d6540fd0b2 100644 --- a/core/vm/actor/builtin/v0/miner/miner_actor.hpp +++ b/core/vm/actor/builtin/v0/miner/miner_actor.hpp @@ -335,4 +335,18 @@ namespace fc::vm::actor::builtin::v0::miner { extern const ActorExports exports; + /** + * + * + * + */ + + struct PreCommitBatch: ActorMethodBase<25> { + struct Params { + std::vector sectors; + }; + ACTOR_METHOD_DECL(); + }; + CBOR_TUPLE(PreCommitBatch::Params, sectors); + } // namespace fc::vm::actor::builtin::v0::miner diff --git a/test/core/miner/PreCommitBatcherTest.cpp b/test/core/miner/PreCommitBatcherTest.cpp index fb8528af95..f3b49a84f0 100644 --- a/test/core/miner/PreCommitBatcherTest.cpp +++ b/test/core/miner/PreCommitBatcherTest.cpp @@ -4,6 +4,7 @@ */ #include +#include #include "miner/storage_fsm/impl/precommit_batcher_impl.hpp" #include "testutil/literals.hpp" #include "testutil/mocks/libp2p/scheduler_mock.hpp" @@ -12,6 +13,10 @@ namespace fc::mining { using api::MinerInfo; using api::SignedMessage; using api::UnsignedMessage; + using fc::mining::types::DealInfo; + using fc::mining::types::PaddedPieceSize; + using fc::mining::types::Piece; + using fc::mining::types::PieceInfo; using libp2p::protocol::SchedulerMock; using libp2p::protocol::scheduler::Ticks; using primitives::tipset::Tipset; @@ -30,17 +35,17 @@ namespace fc::mining { std::chrono::system_clock::now().time_since_epoch())); api::BlockHeader block; - block.height = 9; + block.height = 2; auto tipset = std::make_shared( TipsetKey(), std::vector({block})); - api_->ChainHead = [&]() -> outcome::result { + api_->ChainHead = [=]() -> outcome::result { return outcome::success(tipset); }; api_->StateMinerInfo = - [&](const Address &address, + [=](const Address &address, const TipsetKey &tipset_key) -> outcome::result { if (address == miner_address_) { MinerInfo info; @@ -50,7 +55,7 @@ namespace fc::mining { }; EXPECT_CALL(*sch_, now()).WillOnce(testing::Return(current_time_)); auto result = PreCommitBatcherImpl::makeBatcher( - toTicks(std::chrono::seconds(10)), api_, sch_, miner_address_); + toTicks(std::chrono::seconds(60)), api_, sch_, miner_address_); batcher_ = result.value(); ASSERT_FALSE(result.has_error()); } @@ -62,30 +67,30 @@ namespace fc::mining { uint64_t miner_id; RegisteredSealProof seal_proof_type_; Ticks current_time_; + primitives::TokenAmount mutualDeposit = 0; }; - TEST_F(PreCommitBatcherTest, BatcherSetup) {} + /** BatcherWrite checking that tgeё TEST_F(PreCommitBatcherTest, BatcherWrite) { - api::BlockHeader block; - block.height = 9; - auto tipset = std::make_shared( - TipsetKey(), std::vector({block})); - api_->ChainHead = [&]() -> outcome::result { - return outcome::success(tipset); - }; - SectorInfo si = SectorInfo(); api::SectorPreCommitInfo precInf; primitives::TokenAmount deposit = 10; ASSERT_FALSE(batcher_->addPreCommit(si, deposit, precInf).has_error()); }; + /** + * CallbackSend test is checking that after the scheduled time for a Precommit + * collecting, all the stored batcher's data will be published in a + * messagePool + */ TEST_F(PreCommitBatcherTest, CallbackSend) { + mutualDeposit = 0; + bool isCall = false; + api::BlockHeader block; - block.height = 9; - primitives::TokenAmount mutualDeposit = 0; - bool isCalled = false; + block.height = 2; + auto tipset = std::make_shared( TipsetKey(), std::vector({block})); @@ -93,14 +98,13 @@ namespace fc::mining { return outcome::success(tipset); }; - api_->MpoolPushMessage = [&isCalled, &mutualDeposit]( - const UnsignedMessage &msg, + api_->MpoolPushMessage = [&](const UnsignedMessage &msg, const boost::optional &) -> outcome::result { - std::cout << "testing\n"; + std::cout << "testing Sending\n"; if (msg.method == vm::actor::builtin::v0::miner::PreCommitSector::Number && msg.value == mutualDeposit) { - isCalled = true; + isCall = true; return SignedMessage{.message = msg, .signature = BlsSignature()}; } @@ -127,13 +131,76 @@ namespace fc::mining { EXPECT_CALL(*sch_, now()) .WillOnce( - testing::Return(current_time_ + toTicks(std::chrono::seconds(11)))) - .WillOnce( - testing::Return(current_time_ + toTicks(std::chrono::seconds(11)))) + testing::Return(current_time_ + toTicks(std::chrono::seconds(61)))) + .WillRepeatedly( + testing::Return(current_time_ + toTicks(std::chrono::seconds(61)))); + sch_->next_clock(); + ASSERT_TRUE(isCall); + } + + /** + * ShortDistanceSending checking cutoff functionality + * that makes PreCommitBatcher rescheduling to be sure, + * that PreCommits with a short scheduled deals will be published + * in message pool before the deadline. + **/ + TEST_F(PreCommitBatcherTest, ShortDistanceSending) { + mutualDeposit = 0; + bool isCall = false; + EXPECT_CALL(*sch_, now()).WillOnce(testing::Return(current_time_)); + + api::BlockHeader block; + block.height = 2; + + auto tipset = std::make_shared( + TipsetKey(), std::vector({block})); + + api_->ChainHead = [&]() -> outcome::result { + return outcome::success(tipset); + }; + + api_->MpoolPushMessage = [&](const UnsignedMessage &msg, + const boost::optional &) + -> outcome::result { + if (msg.method == vm::actor::builtin::v0::miner::PreCommitSector::Number + && msg.value == mutualDeposit) { + isCall = true; + return SignedMessage{.message = msg, .signature = BlsSignature()}; + } + + return ERROR_TEXT("ERROR"); + }; + + SectorInfo si = SectorInfo(); + api::SectorPreCommitInfo precInf; + primitives::TokenAmount deposit = 10; + si.ticket_epoch = 5; + Piece p{.piece = PieceInfo{.size = PaddedPieceSize(128), + .cid = "010001020008"_cid}, + .deal_info = boost::none}; + si.pieces.push_back(p); + DealInfo deal{}; + deal.deal_schedule.start_epoch = 3; + Piece p2 = {.piece = PieceInfo{.size = PaddedPieceSize(128), + .cid = "010001020009"_cid}, + .deal_info = deal}; + si.pieces.push_back(p2); + + precInf.sealed_cid = "010001020005"_cid; + si.sector_number = 2; + + auto maybe_result = batcher_->addPreCommit(si, deposit, precInf); + ASSERT_FALSE(maybe_result.has_error()); + mutualDeposit += 10; + + EXPECT_CALL(*sch_, now()) + .WillOnce(testing::Return( + current_time_ + + toTicks(std::chrono::seconds(kEpochDurationSeconds + 10)))) .WillRepeatedly( - testing::Return(current_time_ + toTicks(std::chrono::seconds(11)))); + testing::Return(current_time_ + toTicks(std::chrono::seconds(10)))); sch_->next_clock(); - ASSERT_TRUE(isCalled); + EXPECT_TRUE(isCall); } } // namespace fc::mining From cf003376fb5d0d05d8c058086849a820ce2c1b65 Mon Sep 17 00:00:00 2001 From: elestrias Date: Fri, 9 Jul 2021 18:49:53 +0300 Subject: [PATCH 04/17] PreCommitBatcherCutoff tests Signed-off-by: elestrias --- .../impl/precommit_batcher_impl.cpp | 9 ++-- test/core/miner/PreCommitBatcherTest.cpp | 49 +++++++++++++++---- 2 files changed, 45 insertions(+), 13 deletions(-) diff --git a/core/miner/storage_fsm/impl/precommit_batcher_impl.cpp b/core/miner/storage_fsm/impl/precommit_batcher_impl.cpp index 19c60ce0de..3220c1ce4c 100644 --- a/core/miner/storage_fsm/impl/precommit_batcher_impl.cpp +++ b/core/miner/storage_fsm/impl/precommit_batcher_impl.cpp @@ -38,13 +38,15 @@ namespace fc::mining { batcher->miner_address_ = miner_address; batcher->maxDelay = maxWait; - batcher->cutoffStart = std::chrono::system_clock::now(); + batcher -> cutoffStart = std::chrono::system_clock::now(); batcher -> closestCutoff = maxWait; - batcher->handle_ = scheduler->schedule(maxWait, [=]() { + batcher -> handle_ = scheduler->schedule(maxWait, [=]() { auto maybe_send = batcher->sendBatch(); if (maybe_send.has_error()) { + batcher->handle_.reschedule(maxWait); return maybe_send.error(); } + batcher->handle_.reschedule(maxWait); // TODO: maybe in gsl::finally }); return batcher; } @@ -77,13 +79,12 @@ namespace fc::mining { kPushNoSpec); if (maybe_signed.has_error()) { - handle_.reschedule(maxDelay); return maybe_signed.error(); } mutualDeposit = 0; batchStorage.clear(); - handle_.reschedule(maxDelay); // TODO: maybe in gsl::finally + cutoffStart = std::chrono::system_clock::now(); return outcome::success(); } diff --git a/test/core/miner/PreCommitBatcherTest.cpp b/test/core/miner/PreCommitBatcherTest.cpp index f3b49a84f0..033ca3dac7 100644 --- a/test/core/miner/PreCommitBatcherTest.cpp +++ b/test/core/miner/PreCommitBatcherTest.cpp @@ -53,6 +53,7 @@ namespace fc::mining { return info; } }; + EXPECT_CALL(*sch_, now()).WillOnce(testing::Return(current_time_)); auto result = PreCommitBatcherImpl::makeBatcher( toTicks(std::chrono::seconds(60)), api_, sch_, miner_address_); @@ -70,8 +71,6 @@ namespace fc::mining { primitives::TokenAmount mutualDeposit = 0; }; - /** BatcherWrite checking that tgeё - TEST_F(PreCommitBatcherTest, BatcherWrite) { SectorInfo si = SectorInfo(); api::SectorPreCommitInfo precInf; @@ -79,6 +78,7 @@ namespace fc::mining { ASSERT_FALSE(batcher_->addPreCommit(si, deposit, precInf).has_error()); }; + /** * CallbackSend test is checking that after the scheduled time for a Precommit * collecting, all the stored batcher's data will be published in a @@ -147,7 +147,16 @@ namespace fc::mining { TEST_F(PreCommitBatcherTest, ShortDistanceSending) { mutualDeposit = 0; bool isCall = false; - EXPECT_CALL(*sch_, now()).WillOnce(testing::Return(current_time_)); + EXPECT_CALL(*sch_, now()).WillOnce(testing::Return(current_time_)) + .WillOnce(testing::Return( + current_time_ + + toTicks(std::chrono::seconds(kEpochDurationSeconds)) + 10)) + .WillOnce(testing::Return( + current_time_ + + toTicks(std::chrono::seconds(kEpochDurationSeconds )) + 11)) + .WillRepeatedly(testing::Return( + current_time_ + + toTicks(std::chrono::seconds(kEpochDurationSeconds)) + 12)); api::BlockHeader block; block.height = 2; @@ -193,14 +202,36 @@ namespace fc::mining { ASSERT_FALSE(maybe_result.has_error()); mutualDeposit += 10; - EXPECT_CALL(*sch_, now()) - .WillOnce(testing::Return( - current_time_ - + toTicks(std::chrono::seconds(kEpochDurationSeconds + 10)))) - .WillRepeatedly( - testing::Return(current_time_ + toTicks(std::chrono::seconds(10)))); sch_->next_clock(); EXPECT_TRUE(isCall); + + block.height = 3; + tipset = std::make_shared( + TipsetKey(), std::vector({block})); + + si.pieces = {}; + mutualDeposit = 0; + deal.deal_schedule.start_epoch = 1; + Piece p3 = {.piece = PieceInfo{.size = PaddedPieceSize(128), + .cid = "010001020010"_cid}, + .deal_info = deal}; + si.pieces.push_back(p3); + + + maybe_result = batcher_->addPreCommit(si, deposit, precInf); + ASSERT_FALSE(maybe_result.has_error()); + mutualDeposit += 10; + + sch_->next_clock(); + ASSERT_TRUE(isCall); + + + + + + + + } } // namespace fc::mining From f9fe0944f8cbc8a4f26a87cdc8a875725bce7d78 Mon Sep 17 00:00:00 2001 From: elestrias Date: Mon, 12 Jul 2021 05:15:04 +0300 Subject: [PATCH 05/17] Precommit batch: final tests Signed-off-by: elestrias --- .../impl/precommit_batcher_impl.cpp | 62 +++++++++---------- .../impl/precommit_batcher_impl.hpp | 31 +++++----- .../vm/actor/builtin/v0/miner/miner_actor.hpp | 12 ++-- test/core/miner/PreCommitBatcherTest.cpp | 55 ++++++++-------- 4 files changed, 83 insertions(+), 77 deletions(-) diff --git a/core/miner/storage_fsm/impl/precommit_batcher_impl.cpp b/core/miner/storage_fsm/impl/precommit_batcher_impl.cpp index 3220c1ce4c..7e6c0fa118 100644 --- a/core/miner/storage_fsm/impl/precommit_batcher_impl.cpp +++ b/core/miner/storage_fsm/impl/precommit_batcher_impl.cpp @@ -38,15 +38,11 @@ namespace fc::mining { batcher->miner_address_ = miner_address; batcher->maxDelay = maxWait; - batcher -> cutoffStart = std::chrono::system_clock::now(); - batcher -> closestCutoff = maxWait; - batcher -> handle_ = scheduler->schedule(maxWait, [=]() { + batcher->cutoffStart = std::chrono::system_clock::now(); + batcher->closestCutoff = maxWait; + batcher->handle_ = scheduler->schedule(maxWait, [=]() { auto maybe_send = batcher->sendBatch(); - if (maybe_send.has_error()) { - batcher->handle_.reschedule(maxWait); - return maybe_send.error(); - } - batcher->handle_.reschedule(maxWait); // TODO: maybe in gsl::finally + batcher->handle_.reschedule(maxWait); // TODO: maybe in gsl::finally }); return batcher; } @@ -57,40 +53,42 @@ namespace fc::mining { OUTCOME_TRY(head, api_->ChainHead()); OUTCOME_TRY(minfo, api_->StateMinerInfo(miner_address_, head->key)); - std::vector params; + PreCommitBatch::Params params = {}; for (auto &data : batchStorage) { mutualDeposit += data.second.deposit; - params.push_back(data.second.pci); + params.sectors.push_back(data.second.pci); } // TODO: goodFunds = mutualDeposit + MaxFee; - for checking payable + if (params.sectors.size() != 0) { + OUTCOME_TRY(encodedParams, codec::cbor::encode(params)); + + auto maybe_signed = api_->MpoolPushMessage( + vm::message::UnsignedMessage( + miner_address_, + minfo.worker, // TODO: handle worker + 0, + mutualDeposit, + {}, + {}, + vm::actor::builtin::v0::miner::PreCommitSector::Number, + MethodParams{encodedParams}), + kPushNoSpec); + + if (maybe_signed.has_error()) { + return maybe_signed.error(); //TODO: maybe logs + } - OUTCOME_TRY(encodedParams, codec::cbor::encode(params)); - - auto maybe_signed = api_->MpoolPushMessage( - vm::message::UnsignedMessage( - miner_address_, - minfo.worker, // TODO: handle worker - 0, - mutualDeposit, - {}, - {}, - vm::actor::builtin::v0::miner::PreCommitSector::Number, - MethodParams{encodedParams}), - kPushNoSpec); - - if (maybe_signed.has_error()) { - return maybe_signed.error(); + mutualDeposit = 0; + batchStorage.clear(); } - - mutualDeposit = 0; - batchStorage.clear(); - cutoffStart = std::chrono::system_clock::now(); return outcome::success(); } outcome::result PreCommitBatcherImpl::forceSend() { - handle_.reschedule(0); + auto error = sendBatch(); + assert(not error.has_error()); + handle_.reschedule(maxDelay); return outcome::success(); } @@ -109,7 +107,7 @@ namespace fc::mining { } } if (cutoffEpoch <= curEpoch) { - handle_.reschedule(0); + auto maybe_error = forceSend(); } else { Ticks tempCutoff = toTicks(std::chrono::seconds((cutoffEpoch - curEpoch) * kEpochDurationSeconds)); diff --git a/core/miner/storage_fsm/impl/precommit_batcher_impl.hpp b/core/miner/storage_fsm/impl/precommit_batcher_impl.hpp index 62c6dc9097..451af31c9c 100644 --- a/core/miner/storage_fsm/impl/precommit_batcher_impl.hpp +++ b/core/miner/storage_fsm/impl/precommit_batcher_impl.hpp @@ -7,11 +7,11 @@ #include #include +#include #include #include #include #include -#include #include "api/full_node/node_api.hpp" #include "const.hpp" #include "fsm/fsm.hpp" @@ -26,17 +26,18 @@ namespace fc::mining { using api::FullNodeApi; + using api::kPushNoSpec; using api::SectorNumber; using api::SectorPreCommitInfo; - using api::kPushNoSpec; - using libp2p::protocol::scheduler::Ticks; - using libp2p::protocol::Scheduler; using boost::multiprecision::cpp_int; + using fc::vm::actor::builtin::v0::miner::PreCommitBatch; + using libp2p::protocol::Scheduler; using libp2p::protocol::scheduler::Handle; + using libp2p::protocol::scheduler::Ticks; using libp2p::protocol::scheduler::toTicks; - using primitives::address::Address; - using primitives::TokenAmount; using primitives::ChainEpoch; + using primitives::TokenAmount; + using primitives::address::Address; using vm::actor::MethodParams; using StorageFSM = fsm::FSM; @@ -44,19 +45,21 @@ namespace fc::mining { class PreCommitBatcherImpl : public PreCommitBatcher { public: struct preCommitEntry { - preCommitEntry(primitives::TokenAmount number, - SectorPreCommitInfo info); + preCommitEntry(primitives::TokenAmount number, SectorPreCommitInfo info); preCommitEntry() = default; primitives::TokenAmount deposit; SectorPreCommitInfo pci; - preCommitEntry& operator=(const preCommitEntry &x); + preCommitEntry &operator=(const preCommitEntry &x); }; void getPreCommitCutoff(primitives::ChainEpoch curEpoch, - const SectorInfo & si); + const SectorInfo &si); - static outcome::result> makeBatcher(size_t maxWait, std::shared_ptr api, - std::shared_ptr scheduler, Address & miner_address); + static outcome::result> makeBatcher( + size_t maxWait, + std::shared_ptr api, + std::shared_ptr scheduler, + Address &miner_address); outcome::result addPreCommit(SectorInfo secInf, TokenAmount deposit, @@ -66,9 +69,8 @@ namespace fc::mining { outcome::result sendBatch(); - private: - PreCommitBatcherImpl(size_t maxTime, std::shared_ptr api); + PreCommitBatcherImpl(size_t maxTime, std::shared_ptr api); std::mutex mutex_; Address miner_address_; cpp_int mutualDeposit; @@ -78,7 +80,6 @@ namespace fc::mining { size_t maxDelay; Ticks closestCutoff; std::chrono::system_clock::time_point cutoffStart; - }; } // namespace fc::mining diff --git a/core/vm/actor/builtin/v0/miner/miner_actor.hpp b/core/vm/actor/builtin/v0/miner/miner_actor.hpp index d6540fd0b2..c7581bbb28 100644 --- a/core/vm/actor/builtin/v0/miner/miner_actor.hpp +++ b/core/vm/actor/builtin/v0/miner/miner_actor.hpp @@ -18,6 +18,7 @@ namespace fc::vm::actor::builtin::v0::miner { using common::Buffer; using crypto::randomness::Randomness; + using fc::primitives::sector::SectorInfo; using libp2p::multi::Multiaddress; using primitives::ChainEpoch; using primitives::RleBitset; @@ -336,14 +337,15 @@ namespace fc::vm::actor::builtin::v0::miner { extern const ActorExports exports; /** - * - * - * + * Collects and stores precommit messages to make + * a packaged sending of a several messages within one transaction + * which reduces the general amount of transactions in the network + * with reduction of a gas fee for transactions. */ - struct PreCommitBatch: ActorMethodBase<25> { + struct PreCommitBatch : ActorMethodBase<25> { struct Params { - std::vector sectors; + std::vector sectors; }; ACTOR_METHOD_DECL(); }; diff --git a/test/core/miner/PreCommitBatcherTest.cpp b/test/core/miner/PreCommitBatcherTest.cpp index 033ca3dac7..f933752cf2 100644 --- a/test/core/miner/PreCommitBatcherTest.cpp +++ b/test/core/miner/PreCommitBatcherTest.cpp @@ -52,6 +52,7 @@ namespace fc::mining { info.seal_proof_type = seal_proof_type_; return info; } + return ERROR_TEXT("Error"); }; EXPECT_CALL(*sch_, now()).WillOnce(testing::Return(current_time_)); @@ -78,10 +79,9 @@ namespace fc::mining { ASSERT_FALSE(batcher_->addPreCommit(si, deposit, precInf).has_error()); }; - /** * CallbackSend test is checking that after the scheduled time for a Precommit - * collecting, all the stored batcher's data will be published in a + * collecting, all the stored batcher's data will be published in a * messagePool */ TEST_F(PreCommitBatcherTest, CallbackSend) { @@ -101,7 +101,6 @@ namespace fc::mining { api_->MpoolPushMessage = [&](const UnsignedMessage &msg, const boost::optional &) -> outcome::result { - std::cout << "testing Sending\n"; if (msg.method == vm::actor::builtin::v0::miner::PreCommitSector::Number && msg.value == mutualDeposit) { isCall = true; @@ -132,8 +131,22 @@ namespace fc::mining { EXPECT_CALL(*sch_, now()) .WillOnce( testing::Return(current_time_ + toTicks(std::chrono::seconds(61)))) - .WillRepeatedly( - testing::Return(current_time_ + toTicks(std::chrono::seconds(61)))); + .WillOnce( + testing::Return(current_time_ + toTicks(std::chrono::seconds(123)))) + .WillRepeatedly(testing::Return(current_time_ + + toTicks(std::chrono::seconds(300)))); + sch_->next_clock(); + ASSERT_TRUE(isCall); + + isCall = false; + mutualDeposit = 0; + + precInf.sealed_cid = "010001020008"_cid; + si.sector_number = 6; + + maybe_result = batcher_->addPreCommit(si, deposit, precInf); + ASSERT_FALSE(maybe_result.has_error()); + mutualDeposit += 10; sch_->next_clock(); ASSERT_TRUE(isCall); } @@ -147,16 +160,17 @@ namespace fc::mining { TEST_F(PreCommitBatcherTest, ShortDistanceSending) { mutualDeposit = 0; bool isCall = false; - EXPECT_CALL(*sch_, now()).WillOnce(testing::Return(current_time_)) + EXPECT_CALL(*sch_, now()) + .WillOnce(testing::Return(current_time_)) .WillOnce(testing::Return( - current_time_ - + toTicks(std::chrono::seconds(kEpochDurationSeconds)) + 10)) + current_time_ + toTicks(std::chrono::seconds(kEpochDurationSeconds)) + + 10)) .WillOnce(testing::Return( - current_time_ - + toTicks(std::chrono::seconds(kEpochDurationSeconds )) + 11)) + current_time_ + toTicks(std::chrono::seconds(kEpochDurationSeconds)) + + 11)) .WillRepeatedly(testing::Return( - current_time_ - + toTicks(std::chrono::seconds(kEpochDurationSeconds)) + 12)); + current_time_ + toTicks(std::chrono::seconds(kEpochDurationSeconds)) + + 12)); api::BlockHeader block; block.height = 2; @@ -206,32 +220,23 @@ namespace fc::mining { EXPECT_TRUE(isCall); block.height = 3; - tipset = std::make_shared( - TipsetKey(), std::vector({block})); + tipset = std::make_shared(TipsetKey(), + std::vector({block})); si.pieces = {}; mutualDeposit = 0; deal.deal_schedule.start_epoch = 1; Piece p3 = {.piece = PieceInfo{.size = PaddedPieceSize(128), - .cid = "010001020010"_cid}, - .deal_info = deal}; + .cid = "010001020010"_cid}, + .deal_info = deal}; si.pieces.push_back(p3); - maybe_result = batcher_->addPreCommit(si, deposit, precInf); ASSERT_FALSE(maybe_result.has_error()); mutualDeposit += 10; sch_->next_clock(); ASSERT_TRUE(isCall); - - - - - - - - } } // namespace fc::mining From fb09856b8253946430cde365137d3391c94aad2e Mon Sep 17 00:00:00 2001 From: elestrias Date: Mon, 12 Jul 2021 11:48:46 +0300 Subject: [PATCH 06/17] sealing event fix Signed-off-by: elestrias --- core/miner/storage_fsm/sealing_events.hpp | 2 ++ 1 file changed, 2 insertions(+) diff --git a/core/miner/storage_fsm/sealing_events.hpp b/core/miner/storage_fsm/sealing_events.hpp index 461f9ef7ac..96ad9bb4bf 100644 --- a/core/miner/storage_fsm/sealing_events.hpp +++ b/core/miner/storage_fsm/sealing_events.hpp @@ -13,6 +13,8 @@ namespace fc::mining { /** * SealingEventId is an id event that occurs in a sealing lifecycle */ + + using api::SectorNumber; enum class SealingEvent { kSectorStart = 1, kSectorStartWithPieces, From 1c5d1f93a2e0c063498b5bfbac6dac72fb642c64 Mon Sep 17 00:00:00 2001 From: elestrias Date: Tue, 13 Jul 2021 16:22:17 +0300 Subject: [PATCH 07/17] PreCommitBatch: fixes snake_case_naming, overriding, dry principles, cosmetic changes Signed-off-by: elestrias --- .../impl/precommit_batcher_impl.cpp | 54 ++++----- .../impl/precommit_batcher_impl.hpp | 21 ++-- core/miner/storage_fsm/precommit_batcher.hpp | 10 +- core/miner/storage_fsm/sealing_events.hpp | 4 +- .../vm/actor/builtin/v0/miner/miner_actor.hpp | 16 --- test/core/miner/CMakeLists.txt | 2 +- ...herTest.cpp => precommit_batcher_test.cpp} | 114 +++++++----------- 7 files changed, 85 insertions(+), 136 deletions(-) rename test/core/miner/{PreCommitBatcherTest.cpp => precommit_batcher_test.cpp} (65%) diff --git a/core/miner/storage_fsm/impl/precommit_batcher_impl.cpp b/core/miner/storage_fsm/impl/precommit_batcher_impl.cpp index 7e6c0fa118..6cb21ef169 100644 --- a/core/miner/storage_fsm/impl/precommit_batcher_impl.cpp +++ b/core/miner/storage_fsm/impl/precommit_batcher_impl.cpp @@ -13,13 +13,9 @@ namespace fc::mining { std::shared_ptr api) : api_(std::move(api)){}; - PreCommitBatcherImpl::preCommitEntry::preCommitEntry( - primitives::TokenAmount number, api::SectorPreCommitInfo info) - : deposit(std::move(number)), pci(std::move(info)){}; - - PreCommitBatcherImpl::preCommitEntry & - PreCommitBatcherImpl::preCommitEntry::operator=(const preCommitEntry &x) = - default; + PreCommitBatcherImpl::PreCommitEntry::PreCommitEntry( + TokenAmount number, api::SectorPreCommitInfo info) + : deposit(std::move(number)), precommit_info(std::move(info)){}; outcome::result> PreCommitBatcherImpl::makeBatcher( @@ -49,17 +45,16 @@ namespace fc::mining { outcome::result PreCommitBatcherImpl::sendBatch() { std::unique_lock locker(mutex_, std::defer_lock); - - OUTCOME_TRY(head, api_->ChainHead()); - OUTCOME_TRY(minfo, api_->StateMinerInfo(miner_address_, head->key)); - - PreCommitBatch::Params params = {}; - for (auto &data : batchStorage) { - mutualDeposit += data.second.deposit; - params.sectors.push_back(data.second.pci); - } - // TODO: goodFunds = mutualDeposit + MaxFee; - for checking payable - if (params.sectors.size() != 0) { + // TODO(Elestrias): [FIL-398] goodFunds = mutualDeposit + MaxFee; - for checking payable + if (batchStorage.size() != 0) { + auto head = api_->ChainHead().value(); + auto minfo = api_->StateMinerInfo(miner_address_, head->key).value(); + + PreCommitBatch::Params params = {}; + for (const auto &data : batchStorage) { + mutualDeposit += data.second.deposit; + params.sectors.push_back(data.second.precommit_info); + } OUTCOME_TRY(encodedParams, codec::cbor::encode(params)); auto maybe_signed = api_->MpoolPushMessage( @@ -70,7 +65,7 @@ namespace fc::mining { mutualDeposit, {}, {}, - vm::actor::builtin::v0::miner::PreCommitSector::Number, + 25, // TODO (m.tagirov) Miner actor v5 PreCommitBatch MethodParams{encodedParams}), kPushNoSpec); @@ -86,16 +81,15 @@ namespace fc::mining { } outcome::result PreCommitBatcherImpl::forceSend() { - auto error = sendBatch(); - assert(not error.has_error()); + OUTCOME_TRY(sendBatch()); handle_.reschedule(maxDelay); return outcome::success(); } - void PreCommitBatcherImpl::getPreCommitCutoff(ChainEpoch curEpoch, + outcome::result PreCommitBatcherImpl::setPreCommitCutoff(ChainEpoch curEpoch, const SectorInfo &si) { - primitives::ChainEpoch cutoffEpoch = - si.ticket_epoch + static_cast(fc::kEpochsInDay + 600); + ChainEpoch cutoffEpoch = + si.ticket_epoch + static_cast(fc::kEpochsInDay + kChainFinality); ChainEpoch startEpoch{}; for (auto p : si.pieces) { if (!p.deal_info) { @@ -107,7 +101,7 @@ namespace fc::mining { } } if (cutoffEpoch <= curEpoch) { - auto maybe_error = forceSend(); + OUTCOME_TRY(forceSend()); } else { Ticks tempCutoff = toTicks(std::chrono::seconds((cutoffEpoch - curEpoch) * kEpochDurationSeconds)); @@ -120,18 +114,20 @@ namespace fc::mining { closestCutoff = tempCutoff; } } + return outcome::success(); } outcome::result PreCommitBatcherImpl::addPreCommit( SectorInfo secInf, - primitives::TokenAmount deposit, + TokenAmount deposit, api::SectorPreCommitInfo pcInfo) { std::unique_lock locker(mutex_, std::defer_lock); OUTCOME_TRY(head, api_->ChainHead()); - getPreCommitCutoff(head->epoch(), secInf); auto sn = secInf.sector_number; - batchStorage[sn] = preCommitEntry(std::move(deposit), std::move(pcInfo)); + batchStorage[sn] = PreCommitEntry(std::move(deposit), std::move(pcInfo)); + + setPreCommitCutoff(head->epoch(), secInf); return outcome::success(); } -} // namespace fc::mining \ No newline at end of file +} // namespace fc::mining diff --git a/core/miner/storage_fsm/impl/precommit_batcher_impl.hpp b/core/miner/storage_fsm/impl/precommit_batcher_impl.hpp index 451af31c9c..2fa42bdd39 100644 --- a/core/miner/storage_fsm/impl/precommit_batcher_impl.hpp +++ b/core/miner/storage_fsm/impl/precommit_batcher_impl.hpp @@ -39,21 +39,22 @@ namespace fc::mining { using primitives::TokenAmount; using primitives::address::Address; using vm::actor::MethodParams; + using vm::actor::builtin::types::miner::kChainFinality; using StorageFSM = fsm::FSM; class PreCommitBatcherImpl : public PreCommitBatcher { public: - struct preCommitEntry { - preCommitEntry(primitives::TokenAmount number, SectorPreCommitInfo info); - preCommitEntry() = default; + struct PreCommitEntry { + PreCommitEntry(primitives::TokenAmount number, SectorPreCommitInfo info); + PreCommitEntry() = default; primitives::TokenAmount deposit; - SectorPreCommitInfo pci; - preCommitEntry &operator=(const preCommitEntry &x); + SectorPreCommitInfo precommit_info; + PreCommitEntry &operator=(const PreCommitEntry & other) = default; }; - void getPreCommitCutoff(primitives::ChainEpoch curEpoch, - const SectorInfo &si); + outcome::result setPreCommitCutoff(primitives::ChainEpoch curEpoch, + const SectorInfo &si); static outcome::result> makeBatcher( size_t maxWait, @@ -63,9 +64,9 @@ namespace fc::mining { outcome::result addPreCommit(SectorInfo secInf, TokenAmount deposit, - SectorPreCommitInfo pcInfo); + SectorPreCommitInfo pcInfo) override; - outcome::result forceSend(); + outcome::result forceSend() override; outcome::result sendBatch(); @@ -74,7 +75,7 @@ namespace fc::mining { std::mutex mutex_; Address miner_address_; cpp_int mutualDeposit; - std::map batchStorage; + std::map batchStorage; std::shared_ptr api_; Handle handle_; size_t maxDelay; diff --git a/core/miner/storage_fsm/precommit_batcher.hpp b/core/miner/storage_fsm/precommit_batcher.hpp index 4cc6f1efbd..22a0ad1115 100644 --- a/core/miner/storage_fsm/precommit_batcher.hpp +++ b/core/miner/storage_fsm/precommit_batcher.hpp @@ -10,16 +10,16 @@ namespace fc::mining { using api::SectorPreCommitInfo; - using primitives::TokenAmount; using fc::mining::types::SectorInfo; + using primitives::TokenAmount; class PreCommitBatcher { public: virtual outcome::result addPreCommit(SectorInfo secInf, - TokenAmount deposit, - SectorPreCommitInfo pcInfo) = 0; - virtual outcome::result forceSend()=0; + TokenAmount deposit, + SectorPreCommitInfo pcInfo) = 0; + virtual outcome::result forceSend() = 0; virtual ~PreCommitBatcher() = default; }; -} // namespace fc::mining \ No newline at end of file +} // namespace fc::mining diff --git a/core/miner/storage_fsm/sealing_events.hpp b/core/miner/storage_fsm/sealing_events.hpp index 96ad9bb4bf..9e0b9c81b2 100644 --- a/core/miner/storage_fsm/sealing_events.hpp +++ b/core/miner/storage_fsm/sealing_events.hpp @@ -10,11 +10,11 @@ #include "miner/storage_fsm/types.hpp" namespace fc::mining { + using api::SectorNumber; + /** * SealingEventId is an id event that occurs in a sealing lifecycle */ - - using api::SectorNumber; enum class SealingEvent { kSectorStart = 1, kSectorStartWithPieces, diff --git a/core/vm/actor/builtin/v0/miner/miner_actor.hpp b/core/vm/actor/builtin/v0/miner/miner_actor.hpp index c7581bbb28..e1840b2037 100644 --- a/core/vm/actor/builtin/v0/miner/miner_actor.hpp +++ b/core/vm/actor/builtin/v0/miner/miner_actor.hpp @@ -335,20 +335,4 @@ namespace fc::vm::actor::builtin::v0::miner { CBOR_TUPLE(CompactSectorNumbers::Params, mask_sector_numbers) extern const ActorExports exports; - - /** - * Collects and stores precommit messages to make - * a packaged sending of a several messages within one transaction - * which reduces the general amount of transactions in the network - * with reduction of a gas fee for transactions. - */ - - struct PreCommitBatch : ActorMethodBase<25> { - struct Params { - std::vector sectors; - }; - ACTOR_METHOD_DECL(); - }; - CBOR_TUPLE(PreCommitBatch::Params, sectors); - } // namespace fc::vm::actor::builtin::v0::miner diff --git a/test/core/miner/CMakeLists.txt b/test/core/miner/CMakeLists.txt index d65280ffab..9859682561 100644 --- a/test/core/miner/CMakeLists.txt +++ b/test/core/miner/CMakeLists.txt @@ -10,7 +10,7 @@ target_link_libraries(events_test events) addtest(batcher_test - PreCommitBatcherTest.cpp + precommit_batcher_test.cpp ) target_link_libraries(batcher_test batcher) diff --git a/test/core/miner/PreCommitBatcherTest.cpp b/test/core/miner/precommit_batcher_test.cpp similarity index 65% rename from test/core/miner/PreCommitBatcherTest.cpp rename to test/core/miner/precommit_batcher_test.cpp index f933752cf2..31ed37e666 100644 --- a/test/core/miner/PreCommitBatcherTest.cpp +++ b/test/core/miner/precommit_batcher_test.cpp @@ -8,6 +8,7 @@ #include "miner/storage_fsm/impl/precommit_batcher_impl.hpp" #include "testutil/literals.hpp" #include "testutil/mocks/libp2p/scheduler_mock.hpp" +#include "testutil/outcome.hpp" namespace fc::mining { using api::MinerInfo; @@ -26,22 +27,23 @@ namespace fc::mining { class PreCommitBatcherTest : public testing::Test { protected: virtual void SetUp() { + mutualDeposit = 0; seal_proof_type_ = RegisteredSealProof::kStackedDrg2KiBV1; api_ = std::make_shared(); sch_ = std::make_shared(); - miner_id = 42; - miner_address_ = Address::makeFromId(miner_id); + miner_id_ = 42; + miner_address_ = Address::makeFromId(miner_id_); current_time_ = toTicks(std::chrono::duration_cast( std::chrono::system_clock::now().time_since_epoch())); api::BlockHeader block; block.height = 2; - auto tipset = std::make_shared( + tipset_ = std::make_shared( TipsetKey(), std::vector({block})); api_->ChainHead = [=]() -> outcome::result { - return outcome::success(tipset); + return tipset_; }; api_->StateMinerInfo = @@ -55,6 +57,17 @@ namespace fc::mining { return ERROR_TEXT("Error"); }; + api_->MpoolPushMessage = [&](const UnsignedMessage &msg, + const boost::optional &) + -> outcome::result { + if (msg.method == 25) { + isCall = true; + return SignedMessage{.message = msg, .signature = BlsSignature()}; + } + + return ERROR_TEXT("ERROR"); + }; + EXPECT_CALL(*sch_, now()).WillOnce(testing::Return(current_time_)); auto result = PreCommitBatcherImpl::makeBatcher( toTicks(std::chrono::seconds(60)), api_, sch_, miner_address_); @@ -65,17 +78,20 @@ namespace fc::mining { std::shared_ptr api_; std::shared_ptr sch_; std::shared_ptr batcher_; + std::shared_ptr tipset_; Address miner_address_; - uint64_t miner_id; + uint64_t miner_id_; RegisteredSealProof seal_proof_type_; Ticks current_time_; - primitives::TokenAmount mutualDeposit = 0; + TokenAmount mutualDeposit; + bool isCall; + }; TEST_F(PreCommitBatcherTest, BatcherWrite) { SectorInfo si = SectorInfo(); api::SectorPreCommitInfo precInf; - primitives::TokenAmount deposit = 10; + TokenAmount deposit = 10; ASSERT_FALSE(batcher_->addPreCommit(si, deposit, precInf).has_error()); }; @@ -86,46 +102,22 @@ namespace fc::mining { */ TEST_F(PreCommitBatcherTest, CallbackSend) { mutualDeposit = 0; - bool isCall = false; - - api::BlockHeader block; - block.height = 2; - - auto tipset = std::make_shared( - TipsetKey(), std::vector({block})); - - api_->ChainHead = [&]() -> outcome::result { - return outcome::success(tipset); - }; - - api_->MpoolPushMessage = [&](const UnsignedMessage &msg, - const boost::optional &) - -> outcome::result { - if (msg.method == vm::actor::builtin::v0::miner::PreCommitSector::Number - && msg.value == mutualDeposit) { - isCall = true; - return SignedMessage{.message = msg, .signature = BlsSignature()}; - } - - return ERROR_TEXT("ERROR"); - }; + isCall = false; SectorInfo si = SectorInfo(); api::SectorPreCommitInfo precInf; - primitives::TokenAmount deposit = 10; + TokenAmount deposit = 10; precInf.sealed_cid = "010001020005"_cid; si.sector_number = 2; - auto maybe_result = batcher_->addPreCommit(si, deposit, precInf); - ASSERT_FALSE(maybe_result.has_error()); + EXPECT_OUTCOME_TRUE_1(batcher_->addPreCommit(si, deposit, precInf)); mutualDeposit += 10; precInf.sealed_cid = "010001020006"_cid; si.sector_number = 3; - maybe_result = batcher_->addPreCommit(si, deposit, precInf); - ASSERT_FALSE(maybe_result.has_error()); + EXPECT_OUTCOME_TRUE_1(batcher_->addPreCommit(si, deposit, precInf)); mutualDeposit += 10; EXPECT_CALL(*sch_, now()) @@ -144,11 +136,11 @@ namespace fc::mining { precInf.sealed_cid = "010001020008"_cid; si.sector_number = 6; - maybe_result = batcher_->addPreCommit(si, deposit, precInf); - ASSERT_FALSE(maybe_result.has_error()); + EXPECT_OUTCOME_TRUE_1( batcher_->addPreCommit(si, deposit, precInf)); mutualDeposit += 10; sch_->next_clock(); ASSERT_TRUE(isCall); + isCall = false; } /** @@ -159,44 +151,20 @@ namespace fc::mining { **/ TEST_F(PreCommitBatcherTest, ShortDistanceSending) { mutualDeposit = 0; - bool isCall = false; + isCall = false; + EXPECT_CALL(*sch_, now()) .WillOnce(testing::Return(current_time_)) .WillOnce(testing::Return( current_time_ + toTicks(std::chrono::seconds(kEpochDurationSeconds)) + 10)) - .WillOnce(testing::Return( - current_time_ + toTicks(std::chrono::seconds(kEpochDurationSeconds)) - + 11)) .WillRepeatedly(testing::Return( current_time_ + toTicks(std::chrono::seconds(kEpochDurationSeconds)) + 12)); - api::BlockHeader block; - block.height = 2; - - auto tipset = std::make_shared( - TipsetKey(), std::vector({block})); - - api_->ChainHead = [&]() -> outcome::result { - return outcome::success(tipset); - }; - - api_->MpoolPushMessage = [&](const UnsignedMessage &msg, - const boost::optional &) - -> outcome::result { - if (msg.method == vm::actor::builtin::v0::miner::PreCommitSector::Number - && msg.value == mutualDeposit) { - isCall = true; - return SignedMessage{.message = msg, .signature = BlsSignature()}; - } - - return ERROR_TEXT("ERROR"); - }; - SectorInfo si = SectorInfo(); api::SectorPreCommitInfo precInf; - primitives::TokenAmount deposit = 10; + TokenAmount deposit = 10; si.ticket_epoch = 5; Piece p{.piece = PieceInfo{.size = PaddedPieceSize(128), .cid = "010001020008"_cid}, @@ -212,31 +180,31 @@ namespace fc::mining { precInf.sealed_cid = "010001020005"_cid; si.sector_number = 2; - auto maybe_result = batcher_->addPreCommit(si, deposit, precInf); - ASSERT_FALSE(maybe_result.has_error()); + EXPECT_OUTCOME_TRUE_1(batcher_->addPreCommit(si, deposit, precInf)); mutualDeposit += 10; sch_->next_clock(); EXPECT_TRUE(isCall); - block.height = 3; - tipset = std::make_shared(TipsetKey(), - std::vector({block})); + + isCall = false; si.pieces = {}; mutualDeposit = 0; + deal.deal_schedule.start_epoch = 1; Piece p3 = {.piece = PieceInfo{.size = PaddedPieceSize(128), .cid = "010001020010"_cid}, .deal_info = deal}; si.pieces.push_back(p3); - maybe_result = batcher_->addPreCommit(si, deposit, precInf); - ASSERT_FALSE(maybe_result.has_error()); - mutualDeposit += 10; + precInf.sealed_cid = "010001020013"_cid; + si.sector_number = 4; - sch_->next_clock(); + EXPECT_OUTCOME_TRUE_1( batcher_->addPreCommit(si, deposit, precInf)); + mutualDeposit += 10; ASSERT_TRUE(isCall); } + } // namespace fc::mining From 99d5fe00a8f35905640ad6658c1aeab2a6fa0600 Mon Sep 17 00:00:00 2001 From: elestrias Date: Tue, 13 Jul 2021 16:44:22 +0300 Subject: [PATCH 08/17] Build fix, miner actor parameter Signed-off-by: elestrias --- core/vm/actor/builtin/v0/miner/miner_actor.hpp | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/core/vm/actor/builtin/v0/miner/miner_actor.hpp b/core/vm/actor/builtin/v0/miner/miner_actor.hpp index e1840b2037..2a9d7df813 100644 --- a/core/vm/actor/builtin/v0/miner/miner_actor.hpp +++ b/core/vm/actor/builtin/v0/miner/miner_actor.hpp @@ -335,4 +335,19 @@ namespace fc::vm::actor::builtin::v0::miner { CBOR_TUPLE(CompactSectorNumbers::Params, mask_sector_numbers) extern const ActorExports exports; + + /** + * Collects and stores precommit messages to make + * a packaged sending of a several messages within one transaction + * which reduces the general amount of transactions in the network + * with reduction of a gas fee for transactions. + */ + struct PreCommitBatch : ActorMethodBase<25> { + struct Params { + std::vector sectors; + }; + ACTOR_METHOD_DECL(); + }; + CBOR_TUPLE(PreCommitBatch::Params, sectors); + } // namespace fc::vm::actor::builtin::v0::miner From 38530e6c8720aa7f47e6d193ebed5e6958477031 Mon Sep 17 00:00:00 2001 From: elestrias Date: Wed, 14 Jul 2021 08:35:38 +0300 Subject: [PATCH 09/17] snake_case_names, copy mechanism changes, logs Signed-off-by: elestrias --- .../impl/precommit_batcher_impl.cpp | 109 ++++++++++-------- .../impl/precommit_batcher_impl.hpp | 49 ++++---- core/miner/storage_fsm/precommit_batcher.hpp | 6 +- test/core/miner/precommit_batcher_test.cpp | 2 + 4 files changed, 87 insertions(+), 79 deletions(-) diff --git a/core/miner/storage_fsm/impl/precommit_batcher_impl.cpp b/core/miner/storage_fsm/impl/precommit_batcher_impl.cpp index 6cb21ef169..8220411d7a 100644 --- a/core/miner/storage_fsm/impl/precommit_batcher_impl.cpp +++ b/core/miner/storage_fsm/impl/precommit_batcher_impl.cpp @@ -6,12 +6,22 @@ #include "miner/storage_fsm/impl/precommit_batcher_impl.hpp" #include - +#include "vm/actor/builtin/v0/miner/miner_actor.hpp" +#include "const.hpp" namespace fc::mining { - - PreCommitBatcherImpl::PreCommitBatcherImpl(size_t maxTime, - std::shared_ptr api) - : api_(std::move(api)){}; + using fc::vm::actor::builtin::v0::miner::PreCommitBatch; + using api::kPushNoSpec; + using vm::actor::builtin::types::miner::kChainFinality; + using libp2p::protocol::scheduler::toTicks; + using primitives::ChainEpoch; + PreCommitBatcherImpl::PreCommitBatcherImpl(const Ticks &max_time, + std::shared_ptr api, + const Address &miner_address, + const Ticks &closest_cutoff) + : max_delay_(max_time), + api_(std::move(api)), + miner_address_(miner_address), + closest_cutoff_(closest_cutoff){}; PreCommitBatcherImpl::PreCommitEntry::PreCommitEntry( TokenAmount number, api::SectorPreCommitInfo info) @@ -19,77 +29,78 @@ namespace fc::mining { outcome::result> PreCommitBatcherImpl::makeBatcher( - size_t maxWait, + const Ticks &max_wait, std::shared_ptr api, std::shared_ptr scheduler, - Address &miner_address) { - struct make_unique_enabler : public PreCommitBatcherImpl { - make_unique_enabler(size_t MaxTime, std::shared_ptr api) - : PreCommitBatcherImpl(MaxTime, std::move(api)){}; - }; - + const Address &miner_address) { std::shared_ptr batcher = - std::make_unique(maxWait, std::move(api)); - - batcher->miner_address_ = miner_address; - batcher->maxDelay = maxWait; - - batcher->cutoffStart = std::chrono::system_clock::now(); - batcher->closestCutoff = maxWait; - batcher->handle_ = scheduler->schedule(maxWait, [=]() { - auto maybe_send = batcher->sendBatch(); - batcher->handle_.reschedule(maxWait); // TODO: maybe in gsl::finally + std::make_shared( + max_wait, std::move(api), miner_address, max_wait); + + batcher->cutoff_start_ = std::chrono::system_clock::now(); + batcher->logger_ = common::createLogger("batcher"); + batcher->logger_->info("Bather have been started"); + batcher->handle_ = scheduler->schedule(max_wait, [=]()->outcome::result { + batcher->sendBatch(); + batcher->handle_.reschedule(max_wait); // TODO: maybe in gsl::finally }); return batcher; } - outcome::result PreCommitBatcherImpl::sendBatch() { + void PreCommitBatcherImpl::sendBatch() { std::unique_lock locker(mutex_, std::defer_lock); - // TODO(Elestrias): [FIL-398] goodFunds = mutualDeposit + MaxFee; - for checking payable - if (batchStorage.size() != 0) { + // TODO(Elestrias): [FIL-398] goodFunds = mutualDeposit + MaxFee; - for + // checking payable + + if (batch_storage_.size() != 0) { + logger_ ->info("Sending procedure started"); auto head = api_->ChainHead().value(); auto minfo = api_->StateMinerInfo(miner_address_, head->key).value(); PreCommitBatch::Params params = {}; - for (const auto &data : batchStorage) { - mutualDeposit += data.second.deposit; + for (const auto &data : batch_storage_) { + mutual_deposit_ += data.second.deposit; params.sectors.push_back(data.second.precommit_info); } - OUTCOME_TRY(encodedParams, codec::cbor::encode(params)); - + auto encodedParams = codec::cbor::encode(params); + if (encodedParams.has_error()){ + logger_->error("Error has occurred during parameters encoding: {}", encodedParams.error().message()); + } + logger_->info("Sending data to the network..."); auto maybe_signed = api_->MpoolPushMessage( vm::message::UnsignedMessage( miner_address_, minfo.worker, // TODO: handle worker 0, - mutualDeposit, + mutual_deposit_, {}, {}, 25, // TODO (m.tagirov) Miner actor v5 PreCommitBatch - MethodParams{encodedParams}), + MethodParams{encodedParams.value()}), kPushNoSpec); if (maybe_signed.has_error()) { - return maybe_signed.error(); //TODO: maybe logs + logger_->error("Error has occurred during batch sending: {}", maybe_signed.error().message()); // TODO: maybe logs } - mutualDeposit = 0; - batchStorage.clear(); + mutual_deposit_ = 0; + batch_storage_.clear(); } - cutoffStart = std::chrono::system_clock::now(); - return outcome::success(); + cutoff_start_ = std::chrono::system_clock::now(); + logger_ ->info("Sending procedure completed"); } outcome::result PreCommitBatcherImpl::forceSend() { - OUTCOME_TRY(sendBatch()); - handle_.reschedule(maxDelay); + sendBatch(); + handle_.reschedule(max_delay_); return outcome::success(); } - outcome::result PreCommitBatcherImpl::setPreCommitCutoff(ChainEpoch curEpoch, - const SectorInfo &si) { + outcome::result PreCommitBatcherImpl::setPreCommitCutoff( + const ChainEpoch &curEpoch, const SectorInfo &si) { ChainEpoch cutoffEpoch = - si.ticket_epoch + static_cast(fc::kEpochsInDay + kChainFinality); + si.ticket_epoch + + static_cast(fc::kEpochsInDay + kChainFinality); ChainEpoch startEpoch{}; for (auto p : si.pieces) { if (!p.deal_info) { @@ -105,27 +116,27 @@ namespace fc::mining { } else { Ticks tempCutoff = toTicks(std::chrono::seconds((cutoffEpoch - curEpoch) * kEpochDurationSeconds)); - if ((closestCutoff + if ((closest_cutoff_ - toTicks(std::chrono::duration_cast( - std::chrono::system_clock::now() - cutoffStart)) + std::chrono::system_clock::now() - cutoff_start_)) > tempCutoff)) { - cutoffStart = std::chrono::system_clock::now(); + cutoff_start_ = std::chrono::system_clock::now(); handle_.reschedule(tempCutoff); - closestCutoff = tempCutoff; + closest_cutoff_ = tempCutoff; } } return outcome::success(); } outcome::result PreCommitBatcherImpl::addPreCommit( - SectorInfo secInf, - TokenAmount deposit, - api::SectorPreCommitInfo pcInfo) { + const SectorInfo &secInf, + const TokenAmount &deposit, + const api::SectorPreCommitInfo &pcInfo) { std::unique_lock locker(mutex_, std::defer_lock); OUTCOME_TRY(head, api_->ChainHead()); auto sn = secInf.sector_number; - batchStorage[sn] = PreCommitEntry(std::move(deposit), std::move(pcInfo)); + batch_storage_[sn] = PreCommitEntry(deposit, pcInfo); setPreCommitCutoff(head->epoch(), secInf); return outcome::success(); diff --git a/core/miner/storage_fsm/impl/precommit_batcher_impl.hpp b/core/miner/storage_fsm/impl/precommit_batcher_impl.hpp index 2fa42bdd39..75600fa05b 100644 --- a/core/miner/storage_fsm/impl/precommit_batcher_impl.hpp +++ b/core/miner/storage_fsm/impl/precommit_batcher_impl.hpp @@ -5,7 +5,6 @@ #pragma once -#include #include #include #include @@ -13,35 +12,26 @@ #include #include #include "api/full_node/node_api.hpp" -#include "const.hpp" -#include "fsm/fsm.hpp" #include "miner/storage_fsm/precommit_batcher.hpp" -#include "miner/storage_fsm/sealing_events.hpp" #include "miner/storage_fsm/types.hpp" #include "primitives/address/address.hpp" #include "primitives/types.hpp" #include "vm/actor/actor_method.hpp" #include "vm/actor/builtin/types/miner/sector_info.hpp" -#include "vm/actor/builtin/v0/miner/miner_actor.hpp" + namespace fc::mining { using api::FullNodeApi; - using api::kPushNoSpec; using api::SectorNumber; using api::SectorPreCommitInfo; using boost::multiprecision::cpp_int; - using fc::vm::actor::builtin::v0::miner::PreCommitBatch; using libp2p::protocol::Scheduler; using libp2p::protocol::scheduler::Handle; using libp2p::protocol::scheduler::Ticks; - using libp2p::protocol::scheduler::toTicks; - using primitives::ChainEpoch; using primitives::TokenAmount; using primitives::address::Address; using vm::actor::MethodParams; - using vm::actor::builtin::types::miner::kChainFinality; - using StorageFSM = - fsm::FSM; + class PreCommitBatcherImpl : public PreCommitBatcher { public: @@ -50,37 +40,42 @@ namespace fc::mining { PreCommitEntry() = default; primitives::TokenAmount deposit; SectorPreCommitInfo precommit_info; - PreCommitEntry &operator=(const PreCommitEntry & other) = default; + PreCommitEntry &operator=(const PreCommitEntry &other) = default; }; - outcome::result setPreCommitCutoff(primitives::ChainEpoch curEpoch, + outcome::result setPreCommitCutoff(const ChainEpoch &curEpoch, const SectorInfo &si); static outcome::result> makeBatcher( - size_t maxWait, + const Ticks &maxWait, std::shared_ptr api, std::shared_ptr scheduler, - Address &miner_address); + const Address &miner_address); - outcome::result addPreCommit(SectorInfo secInf, - TokenAmount deposit, - SectorPreCommitInfo pcInfo) override; + outcome::result addPreCommit( + const SectorInfo &secInf, + const TokenAmount &deposit, + const SectorPreCommitInfo &pcInfo) override; outcome::result forceSend() override; - outcome::result sendBatch(); + void sendBatch(); + PreCommitBatcherImpl(const Ticks &maxTime, + std::shared_ptr api, + const Address &miner_address, + const Ticks &closest_cutoff); private: - PreCommitBatcherImpl(size_t maxTime, std::shared_ptr api); std::mutex mutex_; - Address miner_address_; - cpp_int mutualDeposit; - std::map batchStorage; + cpp_int mutual_deposit_; + std::map batch_storage_; + Ticks max_delay_; std::shared_ptr api_; + Address miner_address_; Handle handle_; - size_t maxDelay; - Ticks closestCutoff; - std::chrono::system_clock::time_point cutoffStart; + Ticks closest_cutoff_; + std::chrono::system_clock::time_point cutoff_start_; + common::Logger logger_; }; } // namespace fc::mining diff --git a/core/miner/storage_fsm/precommit_batcher.hpp b/core/miner/storage_fsm/precommit_batcher.hpp index 22a0ad1115..226345e9d9 100644 --- a/core/miner/storage_fsm/precommit_batcher.hpp +++ b/core/miner/storage_fsm/precommit_batcher.hpp @@ -15,9 +15,9 @@ namespace fc::mining { class PreCommitBatcher { public: - virtual outcome::result addPreCommit(SectorInfo secInf, - TokenAmount deposit, - SectorPreCommitInfo pcInfo) = 0; + virtual outcome::result addPreCommit(const SectorInfo & secInf, + const TokenAmount & deposit, + const SectorPreCommitInfo & pcInfo) = 0; virtual outcome::result forceSend() = 0; virtual ~PreCommitBatcher() = default; }; diff --git a/test/core/miner/precommit_batcher_test.cpp b/test/core/miner/precommit_batcher_test.cpp index 31ed37e666..186d42cdc1 100644 --- a/test/core/miner/precommit_batcher_test.cpp +++ b/test/core/miner/precommit_batcher_test.cpp @@ -23,6 +23,8 @@ namespace fc::mining { using primitives::tipset::Tipset; using primitives::tipset::TipsetCPtr; using testing::Mock; + using libp2p::protocol::scheduler::toTicks; + class PreCommitBatcherTest : public testing::Test { protected: From adcaf31161d04e77add12efe0f68e266e312cf5c Mon Sep 17 00:00:00 2001 From: elestrias Date: Wed, 14 Jul 2021 08:58:07 +0300 Subject: [PATCH 10/17] cosmetic changes + build fix Signed-off-by: elestrias --- .../impl/precommit_batcher_impl.cpp | 22 ++++++------ .../impl/precommit_batcher_impl.hpp | 2 -- test/core/miner/precommit_batcher_test.cpp | 34 +++++++++---------- 3 files changed, 28 insertions(+), 30 deletions(-) diff --git a/core/miner/storage_fsm/impl/precommit_batcher_impl.cpp b/core/miner/storage_fsm/impl/precommit_batcher_impl.cpp index 8220411d7a..9c4da8f9d1 100644 --- a/core/miner/storage_fsm/impl/precommit_batcher_impl.cpp +++ b/core/miner/storage_fsm/impl/precommit_batcher_impl.cpp @@ -6,14 +6,14 @@ #include "miner/storage_fsm/impl/precommit_batcher_impl.hpp" #include -#include "vm/actor/builtin/v0/miner/miner_actor.hpp" #include "const.hpp" +#include "vm/actor/builtin/v0/miner/miner_actor.hpp" namespace fc::mining { - using fc::vm::actor::builtin::v0::miner::PreCommitBatch; using api::kPushNoSpec; - using vm::actor::builtin::types::miner::kChainFinality; + using fc::vm::actor::builtin::v0::miner::PreCommitBatch; using libp2p::protocol::scheduler::toTicks; using primitives::ChainEpoch; + using vm::actor::builtin::types::miner::kChainFinality; PreCommitBatcherImpl::PreCommitBatcherImpl(const Ticks &max_time, std::shared_ptr api, const Address &miner_address, @@ -40,20 +40,20 @@ namespace fc::mining { batcher->cutoff_start_ = std::chrono::system_clock::now(); batcher->logger_ = common::createLogger("batcher"); batcher->logger_->info("Bather have been started"); - batcher->handle_ = scheduler->schedule(max_wait, [=]()->outcome::result { + batcher->handle_ = scheduler->schedule(max_wait, [=]() { batcher->sendBatch(); batcher->handle_.reschedule(max_wait); // TODO: maybe in gsl::finally }); return batcher; } - void PreCommitBatcherImpl::sendBatch() { + void PreCommitBatcherImpl::sendBatch() { std::unique_lock locker(mutex_, std::defer_lock); // TODO(Elestrias): [FIL-398] goodFunds = mutualDeposit + MaxFee; - for // checking payable if (batch_storage_.size() != 0) { - logger_ ->info("Sending procedure started"); + logger_->info("Sending procedure started"); auto head = api_->ChainHead().value(); auto minfo = api_->StateMinerInfo(miner_address_, head->key).value(); @@ -63,8 +63,9 @@ namespace fc::mining { params.sectors.push_back(data.second.precommit_info); } auto encodedParams = codec::cbor::encode(params); - if (encodedParams.has_error()){ - logger_->error("Error has occurred during parameters encoding: {}", encodedParams.error().message()); + if (encodedParams.has_error()) { + logger_->error("Error has occurred during parameters encoding: {}", + encodedParams.error().message()); } logger_->info("Sending data to the network..."); auto maybe_signed = api_->MpoolPushMessage( @@ -80,14 +81,15 @@ namespace fc::mining { kPushNoSpec); if (maybe_signed.has_error()) { - logger_->error("Error has occurred during batch sending: {}", maybe_signed.error().message()); // TODO: maybe logs + logger_->error("Error has occurred during batch sending: {}", + maybe_signed.error().message()); // TODO: maybe logs } mutual_deposit_ = 0; batch_storage_.clear(); } cutoff_start_ = std::chrono::system_clock::now(); - logger_ ->info("Sending procedure completed"); + logger_->info("Sending procedure completed"); } outcome::result PreCommitBatcherImpl::forceSend() { diff --git a/core/miner/storage_fsm/impl/precommit_batcher_impl.hpp b/core/miner/storage_fsm/impl/precommit_batcher_impl.hpp index 75600fa05b..5f9f971ca2 100644 --- a/core/miner/storage_fsm/impl/precommit_batcher_impl.hpp +++ b/core/miner/storage_fsm/impl/precommit_batcher_impl.hpp @@ -19,7 +19,6 @@ #include "vm/actor/actor_method.hpp" #include "vm/actor/builtin/types/miner/sector_info.hpp" - namespace fc::mining { using api::FullNodeApi; using api::SectorNumber; @@ -32,7 +31,6 @@ namespace fc::mining { using primitives::address::Address; using vm::actor::MethodParams; - class PreCommitBatcherImpl : public PreCommitBatcher { public: struct PreCommitEntry { diff --git a/test/core/miner/precommit_batcher_test.cpp b/test/core/miner/precommit_batcher_test.cpp index 186d42cdc1..edb3d8fe38 100644 --- a/test/core/miner/precommit_batcher_test.cpp +++ b/test/core/miner/precommit_batcher_test.cpp @@ -20,11 +20,10 @@ namespace fc::mining { using fc::mining::types::PieceInfo; using libp2p::protocol::SchedulerMock; using libp2p::protocol::scheduler::Ticks; + using libp2p::protocol::scheduler::toTicks; using primitives::tipset::Tipset; using primitives::tipset::TipsetCPtr; using testing::Mock; - using libp2p::protocol::scheduler::toTicks; - class PreCommitBatcherTest : public testing::Test { protected: @@ -41,7 +40,7 @@ namespace fc::mining { api::BlockHeader block; block.height = 2; - tipset_ = std::make_shared( + tipset_ = std::make_shared( TipsetKey(), std::vector({block})); api_->ChainHead = [=]() -> outcome::result { @@ -59,8 +58,9 @@ namespace fc::mining { return ERROR_TEXT("Error"); }; - api_->MpoolPushMessage = [&](const UnsignedMessage &msg, - const boost::optional &) + api_->MpoolPushMessage = + [&](const UnsignedMessage &msg, + const boost::optional &) -> outcome::result { if (msg.method == 25) { isCall = true; @@ -87,7 +87,6 @@ namespace fc::mining { Ticks current_time_; TokenAmount mutualDeposit; bool isCall; - }; TEST_F(PreCommitBatcherTest, BatcherWrite) { @@ -98,9 +97,10 @@ namespace fc::mining { }; /** - * CallbackSend test is checking that after the scheduled time for a Precommit - * collecting, all the stored batcher's data will be published in a - * messagePool + * CallbackSend have 4 unic precommits sent in pairs + * a PrecommitBatcher should send the first two and after the rescheduling + * next pair the result should be 2 messages in message pool with pair of + * precommits in each */ TEST_F(PreCommitBatcherTest, CallbackSend) { mutualDeposit = 0; @@ -138,7 +138,7 @@ namespace fc::mining { precInf.sealed_cid = "010001020008"_cid; si.sector_number = 6; - EXPECT_OUTCOME_TRUE_1( batcher_->addPreCommit(si, deposit, precInf)); + EXPECT_OUTCOME_TRUE_1(batcher_->addPreCommit(si, deposit, precInf)); mutualDeposit += 10; sch_->next_clock(); ASSERT_TRUE(isCall); @@ -146,10 +146,11 @@ namespace fc::mining { } /** - * ShortDistanceSending checking cutoff functionality - * that makes PreCommitBatcher rescheduling to be sure, - * that PreCommits with a short scheduled deals will be published - * in message pool before the deadline. + * ShortDistanceSending have 3 Precommits + * Test should send first two after 30 sec instead of 60 and one more + *immediately after the addition The result should be 2 new messages in + *message pool 1st: 2 precommits, have been sent after 30 sec, and 2nd: one + *precommmit, have been sent after the first one immediately **/ TEST_F(PreCommitBatcherTest, ShortDistanceSending) { mutualDeposit = 0; @@ -188,8 +189,6 @@ namespace fc::mining { sch_->next_clock(); EXPECT_TRUE(isCall); - - isCall = false; si.pieces = {}; mutualDeposit = 0; @@ -203,10 +202,9 @@ namespace fc::mining { precInf.sealed_cid = "010001020013"_cid; si.sector_number = 4; - EXPECT_OUTCOME_TRUE_1( batcher_->addPreCommit(si, deposit, precInf)); + EXPECT_OUTCOME_TRUE_1(batcher_->addPreCommit(si, deposit, precInf)); mutualDeposit += 10; ASSERT_TRUE(isCall); } - } // namespace fc::mining From 67c3fcaad5a261b2cff81b6ae85d4fe6f70d1574 Mon Sep 17 00:00:00 2001 From: elestrias Date: Thu, 15 Jul 2021 03:01:57 +0300 Subject: [PATCH 11/17] callback system for sealing integration, cosmetic fixes Signed-off-by: elestrias --- .../impl/precommit_batcher_impl.cpp | 96 ++++++++++--------- .../impl/precommit_batcher_impl.hpp | 29 +++--- core/miner/storage_fsm/precommit_batcher.hpp | 11 ++- test/core/miner/precommit_batcher_test.cpp | 12 +-- 4 files changed, 78 insertions(+), 70 deletions(-) diff --git a/core/miner/storage_fsm/impl/precommit_batcher_impl.cpp b/core/miner/storage_fsm/impl/precommit_batcher_impl.cpp index 9c4da8f9d1..c12096eb62 100644 --- a/core/miner/storage_fsm/impl/precommit_batcher_impl.cpp +++ b/core/miner/storage_fsm/impl/precommit_batcher_impl.cpp @@ -7,12 +7,16 @@ #include #include "const.hpp" +#include "vm/actor/actor.hpp" +#include "vm/actor/builtin/types/miner/sector_info.hpp" #include "vm/actor/builtin/v0/miner/miner_actor.hpp" + namespace fc::mining { using api::kPushNoSpec; using fc::vm::actor::builtin::v0::miner::PreCommitBatch; using libp2p::protocol::scheduler::toTicks; using primitives::ChainEpoch; + using vm::actor::MethodParams; using vm::actor::builtin::types::miner::kChainFinality; PreCommitBatcherImpl::PreCommitBatcherImpl(const Ticks &max_time, std::shared_ptr api, @@ -21,11 +25,11 @@ namespace fc::mining { : max_delay_(max_time), api_(std::move(api)), miner_address_(miner_address), - closest_cutoff_(closest_cutoff){}; + closest_cutoff_(closest_cutoff) {} PreCommitBatcherImpl::PreCommitEntry::PreCommitEntry( - TokenAmount number, api::SectorPreCommitInfo info) - : deposit(std::move(number)), precommit_info(std::move(info)){}; + const TokenAmount &number, const SectorPreCommitInfo &info) + : deposit(number), precommit_info(info){}; outcome::result> PreCommitBatcherImpl::makeBatcher( @@ -41,65 +45,71 @@ namespace fc::mining { batcher->logger_ = common::createLogger("batcher"); batcher->logger_->info("Bather have been started"); batcher->handle_ = scheduler->schedule(max_wait, [=]() { - batcher->sendBatch(); + auto maybe_result = batcher->sendBatch(); + for (const auto &[key, cb] : batcher->callbacks_) { + cb(maybe_result); + } + batcher->callbacks_.clear(); batcher->handle_.reschedule(max_wait); // TODO: maybe in gsl::finally }); return batcher; } - void PreCommitBatcherImpl::sendBatch() { + outcome::result PreCommitBatcherImpl::sendBatch() { std::unique_lock locker(mutex_, std::defer_lock); // TODO(Elestrias): [FIL-398] goodFunds = mutualDeposit + MaxFee; - for // checking payable - + CID message_cid; if (batch_storage_.size() != 0) { logger_->info("Sending procedure started"); - auto head = api_->ChainHead().value(); - auto minfo = api_->StateMinerInfo(miner_address_, head->key).value(); + + OUTCOME_TRY(head, api_->ChainHead()); + OUTCOME_TRY(minfo, api_->StateMinerInfo(miner_address_, head->key)); PreCommitBatch::Params params = {}; + for (const auto &data : batch_storage_) { mutual_deposit_ += data.second.deposit; params.sectors.push_back(data.second.precommit_info); } - auto encodedParams = codec::cbor::encode(params); - if (encodedParams.has_error()) { - logger_->error("Error has occurred during parameters encoding: {}", - encodedParams.error().message()); - } - logger_->info("Sending data to the network..."); - auto maybe_signed = api_->MpoolPushMessage( - vm::message::UnsignedMessage( - miner_address_, - minfo.worker, // TODO: handle worker - 0, - mutual_deposit_, - {}, - {}, - 25, // TODO (m.tagirov) Miner actor v5 PreCommitBatch - MethodParams{encodedParams.value()}), - kPushNoSpec); - - if (maybe_signed.has_error()) { - logger_->error("Error has occurred during batch sending: {}", - maybe_signed.error().message()); // TODO: maybe logs - } + + OUTCOME_TRY(encodedParams, codec::cbor::encode(params)); + + OUTCOME_TRY(signed_message, + api_->MpoolPushMessage( + vm::message::UnsignedMessage( + miner_address_, + minfo.worker, // TODO: handle worker + 0, + mutual_deposit_, + {}, + {}, + 25, // TODO (m.tagirov) Miner actor v5 PreCommitBatch + MethodParams{encodedParams}), + kPushNoSpec)); mutual_deposit_ = 0; batch_storage_.clear(); + message_cid = signed_message.getCid(); + logger_->info("Sending procedure completed"); + cutoff_start_ = std::chrono::system_clock::now(); + return message_cid; } cutoff_start_ = std::chrono::system_clock::now(); - logger_->info("Sending procedure completed"); + return ERROR_TEXT("Empty Batcher"); } - outcome::result PreCommitBatcherImpl::forceSend() { - sendBatch(); + void PreCommitBatcherImpl::forceSend() { + auto maybe_result = sendBatch(); + for (const auto &[key, cb] : callbacks_) { + cb(maybe_result); + } + callbacks_.clear(); handle_.reschedule(max_delay_); - return outcome::success(); } - outcome::result PreCommitBatcherImpl::setPreCommitCutoff( - const ChainEpoch &curEpoch, const SectorInfo &si) { + void PreCommitBatcherImpl::setPreCommitCutoff(const ChainEpoch &curEpoch, + const SectorInfo &si) { ChainEpoch cutoffEpoch = si.ticket_epoch + static_cast(fc::kEpochsInDay + kChainFinality); @@ -114,10 +124,10 @@ namespace fc::mining { } } if (cutoffEpoch <= curEpoch) { - OUTCOME_TRY(forceSend()); + forceSend(); } else { - Ticks tempCutoff = toTicks(std::chrono::seconds((cutoffEpoch - curEpoch) - * kEpochDurationSeconds)); + const Ticks tempCutoff = toTicks(std::chrono::seconds( + (cutoffEpoch - curEpoch) * kEpochDurationSeconds)); if ((closest_cutoff_ - toTicks(std::chrono::duration_cast( std::chrono::system_clock::now() - cutoff_start_)) @@ -127,19 +137,19 @@ namespace fc::mining { closest_cutoff_ = tempCutoff; } } - return outcome::success(); } outcome::result PreCommitBatcherImpl::addPreCommit( const SectorInfo &secInf, const TokenAmount &deposit, - const api::SectorPreCommitInfo &pcInfo) { + const api::SectorPreCommitInfo &pcInfo, + const PrecommitCallback &callback) { std::unique_lock locker(mutex_, std::defer_lock); OUTCOME_TRY(head, api_->ChainHead()); - auto sn = secInf.sector_number; + const auto &sn = secInf.sector_number; batch_storage_[sn] = PreCommitEntry(deposit, pcInfo); - + callbacks_[sn] = callback; // TODO: batcher upper limit setPreCommitCutoff(head->epoch(), secInf); return outcome::success(); } diff --git a/core/miner/storage_fsm/impl/precommit_batcher_impl.hpp b/core/miner/storage_fsm/impl/precommit_batcher_impl.hpp index 5f9f971ca2..06fa92e1de 100644 --- a/core/miner/storage_fsm/impl/precommit_batcher_impl.hpp +++ b/core/miner/storage_fsm/impl/precommit_batcher_impl.hpp @@ -10,39 +10,33 @@ #include #include #include -#include #include "api/full_node/node_api.hpp" #include "miner/storage_fsm/precommit_batcher.hpp" #include "miner/storage_fsm/types.hpp" #include "primitives/address/address.hpp" #include "primitives/types.hpp" -#include "vm/actor/actor_method.hpp" -#include "vm/actor/builtin/types/miner/sector_info.hpp" +#include "vm/actor/actor.hpp" namespace fc::mining { using api::FullNodeApi; using api::SectorNumber; - using api::SectorPreCommitInfo; - using boost::multiprecision::cpp_int; using libp2p::protocol::Scheduler; using libp2p::protocol::scheduler::Handle; using libp2p::protocol::scheduler::Ticks; - using primitives::TokenAmount; using primitives::address::Address; - using vm::actor::MethodParams; class PreCommitBatcherImpl : public PreCommitBatcher { public: struct PreCommitEntry { - PreCommitEntry(primitives::TokenAmount number, SectorPreCommitInfo info); + PreCommitEntry(const TokenAmount &number, + const SectorPreCommitInfo &info); PreCommitEntry() = default; primitives::TokenAmount deposit; SectorPreCommitInfo precommit_info; PreCommitEntry &operator=(const PreCommitEntry &other) = default; }; - outcome::result setPreCommitCutoff(const ChainEpoch &curEpoch, - const SectorInfo &si); + void setPreCommitCutoff(const ChainEpoch &curEpoch, const SectorInfo &si); static outcome::result> makeBatcher( const Ticks &maxWait, @@ -50,14 +44,14 @@ namespace fc::mining { std::shared_ptr scheduler, const Address &miner_address); - outcome::result addPreCommit( - const SectorInfo &secInf, - const TokenAmount &deposit, - const SectorPreCommitInfo &pcInfo) override; + outcome::result addPreCommit(const SectorInfo &secInf, + const TokenAmount &deposit, + const SectorPreCommitInfo &pcInfo, + const PrecommitCallback &callback); - outcome::result forceSend() override; + void forceSend() override; - void sendBatch(); + outcome::result sendBatch(); PreCommitBatcherImpl(const Ticks &maxTime, std::shared_ptr api, const Address &miner_address, @@ -65,7 +59,7 @@ namespace fc::mining { private: std::mutex mutex_; - cpp_int mutual_deposit_; + TokenAmount mutual_deposit_; std::map batch_storage_; Ticks max_delay_; std::shared_ptr api_; @@ -74,6 +68,7 @@ namespace fc::mining { Ticks closest_cutoff_; std::chrono::system_clock::time_point cutoff_start_; common::Logger logger_; + std::map callbacks_; }; } // namespace fc::mining diff --git a/core/miner/storage_fsm/precommit_batcher.hpp b/core/miner/storage_fsm/precommit_batcher.hpp index 226345e9d9..d5bd1a59bc 100644 --- a/core/miner/storage_fsm/precommit_batcher.hpp +++ b/core/miner/storage_fsm/precommit_batcher.hpp @@ -12,13 +12,16 @@ namespace fc::mining { using api::SectorPreCommitInfo; using fc::mining::types::SectorInfo; using primitives::TokenAmount; + using PrecommitCallback = std::function)>; class PreCommitBatcher { public: - virtual outcome::result addPreCommit(const SectorInfo & secInf, - const TokenAmount & deposit, - const SectorPreCommitInfo & pcInfo) = 0; - virtual outcome::result forceSend() = 0; + virtual outcome::result addPreCommit( + const SectorInfo &secInf, + const TokenAmount &deposit, + const SectorPreCommitInfo &pcPrecommitResultInfo, + const PrecommitCallback &callback) = 0; + virtual void forceSend() = 0; virtual ~PreCommitBatcher() = default; }; diff --git a/test/core/miner/precommit_batcher_test.cpp b/test/core/miner/precommit_batcher_test.cpp index edb3d8fe38..f851b4c560 100644 --- a/test/core/miner/precommit_batcher_test.cpp +++ b/test/core/miner/precommit_batcher_test.cpp @@ -93,7 +93,7 @@ namespace fc::mining { SectorInfo si = SectorInfo(); api::SectorPreCommitInfo precInf; TokenAmount deposit = 10; - ASSERT_FALSE(batcher_->addPreCommit(si, deposit, precInf).has_error()); + EXPECT_OUTCOME_TRUE_1(batcher_->addPreCommit(si, deposit, precInf, [](outcome::result){})); }; /** @@ -113,13 +113,13 @@ namespace fc::mining { precInf.sealed_cid = "010001020005"_cid; si.sector_number = 2; - EXPECT_OUTCOME_TRUE_1(batcher_->addPreCommit(si, deposit, precInf)); + EXPECT_OUTCOME_TRUE_1(batcher_->addPreCommit(si, deposit, precInf, [](outcome::result){})); mutualDeposit += 10; precInf.sealed_cid = "010001020006"_cid; si.sector_number = 3; - EXPECT_OUTCOME_TRUE_1(batcher_->addPreCommit(si, deposit, precInf)); + EXPECT_OUTCOME_TRUE_1(batcher_->addPreCommit(si, deposit, precInf, [](outcome::result){})); mutualDeposit += 10; EXPECT_CALL(*sch_, now()) @@ -138,7 +138,7 @@ namespace fc::mining { precInf.sealed_cid = "010001020008"_cid; si.sector_number = 6; - EXPECT_OUTCOME_TRUE_1(batcher_->addPreCommit(si, deposit, precInf)); + EXPECT_OUTCOME_TRUE_1(batcher_->addPreCommit(si, deposit, precInf, [](outcome::result){})); mutualDeposit += 10; sch_->next_clock(); ASSERT_TRUE(isCall); @@ -183,7 +183,7 @@ namespace fc::mining { precInf.sealed_cid = "010001020005"_cid; si.sector_number = 2; - EXPECT_OUTCOME_TRUE_1(batcher_->addPreCommit(si, deposit, precInf)); + EXPECT_OUTCOME_TRUE_1(batcher_->addPreCommit(si, deposit, precInf, [](outcome::result){})); mutualDeposit += 10; sch_->next_clock(); @@ -202,7 +202,7 @@ namespace fc::mining { precInf.sealed_cid = "010001020013"_cid; si.sector_number = 4; - EXPECT_OUTCOME_TRUE_1(batcher_->addPreCommit(si, deposit, precInf)); + EXPECT_OUTCOME_TRUE_1(batcher_->addPreCommit(si, deposit, precInf, [](outcome::result){})); mutualDeposit += 10; ASSERT_TRUE(isCall); } From 798b473db066a7e6d1abf1c965ff7b88d23b23e2 Mon Sep 17 00:00:00 2001 From: elestrias Date: Thu, 15 Jul 2021 13:33:40 +0300 Subject: [PATCH 12/17] final cosmetic changes Signed-off-by: elestrias --- .../impl/precommit_batcher_impl.cpp | 44 +++++++++---------- .../impl/precommit_batcher_impl.hpp | 16 +++---- core/miner/storage_fsm/precommit_batcher.hpp | 8 ++-- core/miner/storage_fsm/sealing_events.hpp | 2 - .../vm/actor/builtin/v0/miner/miner_actor.hpp | 1 - 5 files changed, 34 insertions(+), 37 deletions(-) diff --git a/core/miner/storage_fsm/impl/precommit_batcher_impl.cpp b/core/miner/storage_fsm/impl/precommit_batcher_impl.cpp index c12096eb62..0b5b95b789 100644 --- a/core/miner/storage_fsm/impl/precommit_batcher_impl.cpp +++ b/core/miner/storage_fsm/impl/precommit_batcher_impl.cpp @@ -35,7 +35,7 @@ namespace fc::mining { PreCommitBatcherImpl::makeBatcher( const Ticks &max_wait, std::shared_ptr api, - std::shared_ptr scheduler, + const std::shared_ptr& scheduler, const Address &miner_address) { std::shared_ptr batcher = std::make_shared( @@ -50,7 +50,7 @@ namespace fc::mining { cb(maybe_result); } batcher->callbacks_.clear(); - batcher->handle_.reschedule(max_wait); // TODO: maybe in gsl::finally + batcher->handle_.reschedule(max_wait); }); return batcher; } @@ -108,49 +108,49 @@ namespace fc::mining { handle_.reschedule(max_delay_); } - void PreCommitBatcherImpl::setPreCommitCutoff(const ChainEpoch &curEpoch, - const SectorInfo &si) { - ChainEpoch cutoffEpoch = - si.ticket_epoch + void PreCommitBatcherImpl::setPreCommitCutoff(const ChainEpoch ¤t_epoch, + const SectorInfo §or_info) { + ChainEpoch cutoff_epoch = + sector_info.ticket_epoch + static_cast(fc::kEpochsInDay + kChainFinality); - ChainEpoch startEpoch{}; - for (auto p : si.pieces) { + ChainEpoch start_epoch{}; + for (auto p : sector_info.pieces) { if (!p.deal_info) { continue; } - startEpoch = p.deal_info->deal_schedule.start_epoch; - if (startEpoch < cutoffEpoch) { - cutoffEpoch = startEpoch; + start_epoch = p.deal_info->deal_schedule.start_epoch; + if (start_epoch < cutoff_epoch) { + cutoff_epoch = start_epoch; } } - if (cutoffEpoch <= curEpoch) { + if (cutoff_epoch <= current_epoch) { forceSend(); } else { - const Ticks tempCutoff = toTicks(std::chrono::seconds( - (cutoffEpoch - curEpoch) * kEpochDurationSeconds)); + const Ticks temp_cutoff = toTicks(std::chrono::seconds( + (cutoff_epoch - current_epoch) * kEpochDurationSeconds)); if ((closest_cutoff_ - toTicks(std::chrono::duration_cast( std::chrono::system_clock::now() - cutoff_start_)) - > tempCutoff)) { + > temp_cutoff)) { cutoff_start_ = std::chrono::system_clock::now(); - handle_.reschedule(tempCutoff); - closest_cutoff_ = tempCutoff; + handle_.reschedule(temp_cutoff); + closest_cutoff_ = temp_cutoff; } } } outcome::result PreCommitBatcherImpl::addPreCommit( - const SectorInfo &secInf, + const SectorInfo §or_info, const TokenAmount &deposit, - const api::SectorPreCommitInfo &pcInfo, + const api::SectorPreCommitInfo &precommit_info, const PrecommitCallback &callback) { std::unique_lock locker(mutex_, std::defer_lock); OUTCOME_TRY(head, api_->ChainHead()); - const auto &sn = secInf.sector_number; - batch_storage_[sn] = PreCommitEntry(deposit, pcInfo); + const auto &sn = sector_info.sector_number; + batch_storage_[sn] = PreCommitEntry(deposit, precommit_info); callbacks_[sn] = callback; // TODO: batcher upper limit - setPreCommitCutoff(head->epoch(), secInf); + setPreCommitCutoff(head->epoch(), sector_info); return outcome::success(); } } // namespace fc::mining diff --git a/core/miner/storage_fsm/impl/precommit_batcher_impl.hpp b/core/miner/storage_fsm/impl/precommit_batcher_impl.hpp index 06fa92e1de..962407c392 100644 --- a/core/miner/storage_fsm/impl/precommit_batcher_impl.hpp +++ b/core/miner/storage_fsm/impl/precommit_batcher_impl.hpp @@ -19,7 +19,7 @@ namespace fc::mining { using api::FullNodeApi; - using api::SectorNumber; + using primitives::SectorNumber; using libp2p::protocol::Scheduler; using libp2p::protocol::scheduler::Handle; using libp2p::protocol::scheduler::Ticks; @@ -31,28 +31,28 @@ namespace fc::mining { PreCommitEntry(const TokenAmount &number, const SectorPreCommitInfo &info); PreCommitEntry() = default; - primitives::TokenAmount deposit; + TokenAmount deposit; SectorPreCommitInfo precommit_info; PreCommitEntry &operator=(const PreCommitEntry &other) = default; }; - void setPreCommitCutoff(const ChainEpoch &curEpoch, const SectorInfo &si); + void setPreCommitCutoff(const ChainEpoch ¤t_epoch, const SectorInfo §or_info); static outcome::result> makeBatcher( - const Ticks &maxWait, + const Ticks &max_wait, std::shared_ptr api, - std::shared_ptr scheduler, + const std::shared_ptr & scheduler, const Address &miner_address); - outcome::result addPreCommit(const SectorInfo &secInf, + outcome::result addPreCommit(const SectorInfo §or_info, const TokenAmount &deposit, - const SectorPreCommitInfo &pcInfo, + const SectorPreCommitInfo &precommit_info, const PrecommitCallback &callback); void forceSend() override; outcome::result sendBatch(); - PreCommitBatcherImpl(const Ticks &maxTime, + PreCommitBatcherImpl(const Ticks &max_time, std::shared_ptr api, const Address &miner_address, const Ticks &closest_cutoff); diff --git a/core/miner/storage_fsm/precommit_batcher.hpp b/core/miner/storage_fsm/precommit_batcher.hpp index d5bd1a59bc..5e8e451e74 100644 --- a/core/miner/storage_fsm/precommit_batcher.hpp +++ b/core/miner/storage_fsm/precommit_batcher.hpp @@ -9,17 +9,17 @@ #include "miner/storage_fsm/types.hpp" namespace fc::mining { - using api::SectorPreCommitInfo; - using fc::mining::types::SectorInfo; + using vm::actor::builtin::types::miner::SectorPreCommitInfo; + using types::SectorInfo; using primitives::TokenAmount; using PrecommitCallback = std::function)>; class PreCommitBatcher { public: virtual outcome::result addPreCommit( - const SectorInfo &secInf, + const SectorInfo §or_info, const TokenAmount &deposit, - const SectorPreCommitInfo &pcPrecommitResultInfo, + const SectorPreCommitInfo &precommit_info, const PrecommitCallback &callback) = 0; virtual void forceSend() = 0; virtual ~PreCommitBatcher() = default; diff --git a/core/miner/storage_fsm/sealing_events.hpp b/core/miner/storage_fsm/sealing_events.hpp index 9e0b9c81b2..461f9ef7ac 100644 --- a/core/miner/storage_fsm/sealing_events.hpp +++ b/core/miner/storage_fsm/sealing_events.hpp @@ -10,8 +10,6 @@ #include "miner/storage_fsm/types.hpp" namespace fc::mining { - using api::SectorNumber; - /** * SealingEventId is an id event that occurs in a sealing lifecycle */ diff --git a/core/vm/actor/builtin/v0/miner/miner_actor.hpp b/core/vm/actor/builtin/v0/miner/miner_actor.hpp index 2a9d7df813..d70e731737 100644 --- a/core/vm/actor/builtin/v0/miner/miner_actor.hpp +++ b/core/vm/actor/builtin/v0/miner/miner_actor.hpp @@ -18,7 +18,6 @@ namespace fc::vm::actor::builtin::v0::miner { using common::Buffer; using crypto::randomness::Randomness; - using fc::primitives::sector::SectorInfo; using libp2p::multi::Multiaddress; using primitives::ChainEpoch; using primitives::RleBitset; From 5b55d03b717110eda484ee09c3593a3e49f22b3d Mon Sep 17 00:00:00 2001 From: elestrias Date: Thu, 15 Jul 2021 13:59:17 +0300 Subject: [PATCH 13/17] Batcher_precommit_test cosmetic changes Signed-off-by: elestrias --- test/core/miner/precommit_batcher_test.cpp | 44 +++++++++++----------- 1 file changed, 22 insertions(+), 22 deletions(-) diff --git a/test/core/miner/precommit_batcher_test.cpp b/test/core/miner/precommit_batcher_test.cpp index f851b4c560..b7dd479773 100644 --- a/test/core/miner/precommit_batcher_test.cpp +++ b/test/core/miner/precommit_batcher_test.cpp @@ -28,7 +28,7 @@ namespace fc::mining { class PreCommitBatcherTest : public testing::Test { protected: virtual void SetUp() { - mutualDeposit = 0; + mutual_deposit_ = 0; seal_proof_type_ = RegisteredSealProof::kStackedDrg2KiBV1; api_ = std::make_shared(); sch_ = std::make_shared(); @@ -63,7 +63,7 @@ namespace fc::mining { const boost::optional &) -> outcome::result { if (msg.method == 25) { - isCall = true; + is_called_ = true; return SignedMessage{.message = msg, .signature = BlsSignature()}; } @@ -85,8 +85,8 @@ namespace fc::mining { uint64_t miner_id_; RegisteredSealProof seal_proof_type_; Ticks current_time_; - TokenAmount mutualDeposit; - bool isCall; + TokenAmount mutual_deposit_; + bool is_called_; }; TEST_F(PreCommitBatcherTest, BatcherWrite) { @@ -103,8 +103,8 @@ namespace fc::mining { * precommits in each */ TEST_F(PreCommitBatcherTest, CallbackSend) { - mutualDeposit = 0; - isCall = false; + mutual_deposit_ = 0; + is_called_ = false; SectorInfo si = SectorInfo(); api::SectorPreCommitInfo precInf; @@ -114,13 +114,13 @@ namespace fc::mining { si.sector_number = 2; EXPECT_OUTCOME_TRUE_1(batcher_->addPreCommit(si, deposit, precInf, [](outcome::result){})); - mutualDeposit += 10; + mutual_deposit_ += 10; precInf.sealed_cid = "010001020006"_cid; si.sector_number = 3; EXPECT_OUTCOME_TRUE_1(batcher_->addPreCommit(si, deposit, precInf, [](outcome::result){})); - mutualDeposit += 10; + mutual_deposit_ += 10; EXPECT_CALL(*sch_, now()) .WillOnce( @@ -130,19 +130,19 @@ namespace fc::mining { .WillRepeatedly(testing::Return(current_time_ + toTicks(std::chrono::seconds(300)))); sch_->next_clock(); - ASSERT_TRUE(isCall); + ASSERT_TRUE(is_called_); - isCall = false; - mutualDeposit = 0; + is_called_ = false; + mutual_deposit_ = 0; precInf.sealed_cid = "010001020008"_cid; si.sector_number = 6; EXPECT_OUTCOME_TRUE_1(batcher_->addPreCommit(si, deposit, precInf, [](outcome::result){})); - mutualDeposit += 10; + mutual_deposit_ += 10; sch_->next_clock(); - ASSERT_TRUE(isCall); - isCall = false; + ASSERT_TRUE(is_called_); + is_called_ = false; } /** @@ -153,8 +153,8 @@ namespace fc::mining { *precommmit, have been sent after the first one immediately **/ TEST_F(PreCommitBatcherTest, ShortDistanceSending) { - mutualDeposit = 0; - isCall = false; + mutual_deposit_ = 0; + is_called_ = false; EXPECT_CALL(*sch_, now()) .WillOnce(testing::Return(current_time_)) @@ -184,14 +184,14 @@ namespace fc::mining { si.sector_number = 2; EXPECT_OUTCOME_TRUE_1(batcher_->addPreCommit(si, deposit, precInf, [](outcome::result){})); - mutualDeposit += 10; + mutual_deposit_ += 10; sch_->next_clock(); - EXPECT_TRUE(isCall); + EXPECT_TRUE(is_called_); - isCall = false; + is_called_ = false; si.pieces = {}; - mutualDeposit = 0; + mutual_deposit_ = 0; deal.deal_schedule.start_epoch = 1; Piece p3 = {.piece = PieceInfo{.size = PaddedPieceSize(128), @@ -203,8 +203,8 @@ namespace fc::mining { si.sector_number = 4; EXPECT_OUTCOME_TRUE_1(batcher_->addPreCommit(si, deposit, precInf, [](outcome::result){})); - mutualDeposit += 10; - ASSERT_TRUE(isCall); + mutual_deposit_ += 10; + ASSERT_TRUE(is_called_); } } // namespace fc::mining From 152e277511acafa4f513b9d9ff730431c0109e50 Mon Sep 17 00:00:00 2001 From: elestrias Date: Thu, 15 Jul 2021 16:15:31 +0300 Subject: [PATCH 14/17] additional mutex locks, cosmetic fixes Signed-off-by: elestrias --- .../impl/precommit_batcher_impl.cpp | 24 ++++++------- .../impl/precommit_batcher_impl.hpp | 25 +++++++------ test/core/miner/precommit_batcher_test.cpp | 36 +++++++++++-------- 3 files changed, 48 insertions(+), 37 deletions(-) diff --git a/core/miner/storage_fsm/impl/precommit_batcher_impl.cpp b/core/miner/storage_fsm/impl/precommit_batcher_impl.cpp index 0b5b95b789..d5e1c62519 100644 --- a/core/miner/storage_fsm/impl/precommit_batcher_impl.cpp +++ b/core/miner/storage_fsm/impl/precommit_batcher_impl.cpp @@ -13,11 +13,12 @@ namespace fc::mining { using api::kPushNoSpec; - using fc::vm::actor::builtin::v0::miner::PreCommitBatch; using libp2p::protocol::scheduler::toTicks; using primitives::ChainEpoch; - using vm::actor::MethodParams; using vm::actor::builtin::types::miner::kChainFinality; + using vm::actor::builtin::v0::miner::PreCommitBatch; + using vm::actor::MethodParams; + PreCommitBatcherImpl::PreCommitBatcherImpl(const Ticks &max_time, std::shared_ptr api, const Address &miner_address, @@ -35,7 +36,7 @@ namespace fc::mining { PreCommitBatcherImpl::makeBatcher( const Ticks &max_wait, std::shared_ptr api, - const std::shared_ptr& scheduler, + const std::shared_ptr &scheduler, const Address &miner_address) { std::shared_ptr batcher = std::make_shared( @@ -43,9 +44,10 @@ namespace fc::mining { batcher->cutoff_start_ = std::chrono::system_clock::now(); batcher->logger_ = common::createLogger("batcher"); - batcher->logger_->info("Bather have been started"); + batcher->logger_->info("Bather has been started"); batcher->handle_ = scheduler->schedule(max_wait, [=]() { - auto maybe_result = batcher->sendBatch(); + std::unique_lock locker(batcher->mutex_, std::defer_lock); + const auto maybe_result = batcher->sendBatch(); for (const auto &[key, cb] : batcher->callbacks_) { cb(maybe_result); } @@ -56,10 +58,8 @@ namespace fc::mining { } outcome::result PreCommitBatcherImpl::sendBatch() { - std::unique_lock locker(mutex_, std::defer_lock); // TODO(Elestrias): [FIL-398] goodFunds = mutualDeposit + MaxFee; - for // checking payable - CID message_cid; if (batch_storage_.size() != 0) { logger_->info("Sending procedure started"); @@ -90,17 +90,17 @@ namespace fc::mining { mutual_deposit_ = 0; batch_storage_.clear(); - message_cid = signed_message.getCid(); logger_->info("Sending procedure completed"); cutoff_start_ = std::chrono::system_clock::now(); - return message_cid; + return signed_message.getCid(); } cutoff_start_ = std::chrono::system_clock::now(); return ERROR_TEXT("Empty Batcher"); } void PreCommitBatcherImpl::forceSend() { - auto maybe_result = sendBatch(); + std::unique_lock locker(mutex_, std::defer_lock); + const auto maybe_result = sendBatch(); for (const auto &[key, cb] : callbacks_) { cb(maybe_result); } @@ -112,9 +112,9 @@ namespace fc::mining { const SectorInfo §or_info) { ChainEpoch cutoff_epoch = sector_info.ticket_epoch - + static_cast(fc::kEpochsInDay + kChainFinality); + + static_cast(kEpochsInDay + kChainFinality); ChainEpoch start_epoch{}; - for (auto p : sector_info.pieces) { + for (const auto &p : sector_info.pieces) { if (!p.deal_info) { continue; } diff --git a/core/miner/storage_fsm/impl/precommit_batcher_impl.hpp b/core/miner/storage_fsm/impl/precommit_batcher_impl.hpp index 962407c392..a45da876ad 100644 --- a/core/miner/storage_fsm/impl/precommit_batcher_impl.hpp +++ b/core/miner/storage_fsm/impl/precommit_batcher_impl.hpp @@ -19,14 +19,19 @@ namespace fc::mining { using api::FullNodeApi; - using primitives::SectorNumber; using libp2p::protocol::Scheduler; using libp2p::protocol::scheduler::Handle; using libp2p::protocol::scheduler::Ticks; + using primitives::SectorNumber; using primitives::address::Address; class PreCommitBatcherImpl : public PreCommitBatcher { public: + PreCommitBatcherImpl(const Ticks &max_time, + std::shared_ptr api, + const Address &miner_address, + const Ticks &closest_cutoff); + struct PreCommitEntry { PreCommitEntry(const TokenAmount &number, const SectorPreCommitInfo &info); @@ -36,26 +41,24 @@ namespace fc::mining { PreCommitEntry &operator=(const PreCommitEntry &other) = default; }; - void setPreCommitCutoff(const ChainEpoch ¤t_epoch, const SectorInfo §or_info); + void setPreCommitCutoff(const ChainEpoch ¤t_epoch, + const SectorInfo §or_info); static outcome::result> makeBatcher( const Ticks &max_wait, std::shared_ptr api, - const std::shared_ptr & scheduler, + const std::shared_ptr &scheduler, const Address &miner_address); - outcome::result addPreCommit(const SectorInfo §or_info, - const TokenAmount &deposit, - const SectorPreCommitInfo &precommit_info, - const PrecommitCallback &callback); + outcome::result addPreCommit( + const SectorInfo §or_info, + const TokenAmount &deposit, + const SectorPreCommitInfo &precommit_info, + const PrecommitCallback &callback); void forceSend() override; outcome::result sendBatch(); - PreCommitBatcherImpl(const Ticks &max_time, - std::shared_ptr api, - const Address &miner_address, - const Ticks &closest_cutoff); private: std::mutex mutex_; diff --git a/test/core/miner/precommit_batcher_test.cpp b/test/core/miner/precommit_batcher_test.cpp index b7dd479773..6eb562439a 100644 --- a/test/core/miner/precommit_batcher_test.cpp +++ b/test/core/miner/precommit_batcher_test.cpp @@ -93,13 +93,15 @@ namespace fc::mining { SectorInfo si = SectorInfo(); api::SectorPreCommitInfo precInf; TokenAmount deposit = 10; - EXPECT_OUTCOME_TRUE_1(batcher_->addPreCommit(si, deposit, precInf, [](outcome::result){})); + EXPECT_OUTCOME_TRUE_1(batcher_->addPreCommit( + si, deposit, precInf, [](outcome::result) {})); }; /** - * CallbackSend have 4 unic precommits sent in pairs - * a PrecommitBatcher should send the first two and after the rescheduling - * next pair the result should be 2 messages in message pool with pair of + * CallbackSend have 4 precommits sent in pairs + * @when PrecommitBatcher send the first pair and after the rescheduling + * next pair + * @then The result should be 2 messages in message pool with pair of * precommits in each */ TEST_F(PreCommitBatcherTest, CallbackSend) { @@ -113,13 +115,15 @@ namespace fc::mining { precInf.sealed_cid = "010001020005"_cid; si.sector_number = 2; - EXPECT_OUTCOME_TRUE_1(batcher_->addPreCommit(si, deposit, precInf, [](outcome::result){})); + EXPECT_OUTCOME_TRUE_1(batcher_->addPreCommit( + si, deposit, precInf, [](outcome::result) {})); mutual_deposit_ += 10; precInf.sealed_cid = "010001020006"_cid; si.sector_number = 3; - EXPECT_OUTCOME_TRUE_1(batcher_->addPreCommit(si, deposit, precInf, [](outcome::result){})); + EXPECT_OUTCOME_TRUE_1(batcher_->addPreCommit( + si, deposit, precInf, [](outcome::result) {})); mutual_deposit_ += 10; EXPECT_CALL(*sch_, now()) @@ -138,7 +142,8 @@ namespace fc::mining { precInf.sealed_cid = "010001020008"_cid; si.sector_number = 6; - EXPECT_OUTCOME_TRUE_1(batcher_->addPreCommit(si, deposit, precInf, [](outcome::result){})); + EXPECT_OUTCOME_TRUE_1(batcher_->addPreCommit( + si, deposit, precInf, [](outcome::result) {})); mutual_deposit_ += 10; sch_->next_clock(); ASSERT_TRUE(is_called_); @@ -146,11 +151,12 @@ namespace fc::mining { } /** - * ShortDistanceSending have 3 Precommits - * Test should send first two after 30 sec instead of 60 and one more - *immediately after the addition The result should be 2 new messages in - *message pool 1st: 2 precommits, have been sent after 30 sec, and 2nd: one - *precommmit, have been sent after the first one immediately + ShortDistanceSending have 3 Precommits + * @when First two are sent after 30 sec instead of 60 and one more + * immediately after the addition. + * @then The result should be 2 new messages in message pool 1st: + * 2 precommits, have been sent after 30 sec, and 2nd: one + * precommmit, have been sent after the first one immediately **/ TEST_F(PreCommitBatcherTest, ShortDistanceSending) { mutual_deposit_ = 0; @@ -183,7 +189,8 @@ namespace fc::mining { precInf.sealed_cid = "010001020005"_cid; si.sector_number = 2; - EXPECT_OUTCOME_TRUE_1(batcher_->addPreCommit(si, deposit, precInf, [](outcome::result){})); + EXPECT_OUTCOME_TRUE_1(batcher_->addPreCommit( + si, deposit, precInf, [](outcome::result) {})); mutual_deposit_ += 10; sch_->next_clock(); @@ -202,7 +209,8 @@ namespace fc::mining { precInf.sealed_cid = "010001020013"_cid; si.sector_number = 4; - EXPECT_OUTCOME_TRUE_1(batcher_->addPreCommit(si, deposit, precInf, [](outcome::result){})); + EXPECT_OUTCOME_TRUE_1(batcher_->addPreCommit( + si, deposit, precInf, [](outcome::result) {})); mutual_deposit_ += 10; ASSERT_TRUE(is_called_); } From 8802740cff3bc1cdc06d1d2765b87dfd3a240d89 Mon Sep 17 00:00:00 2001 From: elestrias Date: Thu, 15 Jul 2021 18:46:00 +0300 Subject: [PATCH 15/17] factory method removal, cosmetic changes, callbackfunction results checks Signed-off-by: elestrias --- .../impl/precommit_batcher_impl.cpp | 51 ++++++++----------- .../impl/precommit_batcher_impl.hpp | 16 ++---- core/miner/storage_fsm/precommit_batcher.hpp | 6 +-- test/core/miner/precommit_batcher_test.cpp | 32 +++++++----- 4 files changed, 48 insertions(+), 57 deletions(-) diff --git a/core/miner/storage_fsm/impl/precommit_batcher_impl.cpp b/core/miner/storage_fsm/impl/precommit_batcher_impl.cpp index d5e1c62519..e592d4ee0d 100644 --- a/core/miner/storage_fsm/impl/precommit_batcher_impl.cpp +++ b/core/miner/storage_fsm/impl/precommit_batcher_impl.cpp @@ -15,48 +15,37 @@ namespace fc::mining { using api::kPushNoSpec; using libp2p::protocol::scheduler::toTicks; using primitives::ChainEpoch; + using vm::actor::MethodParams; using vm::actor::builtin::types::miner::kChainFinality; using vm::actor::builtin::v0::miner::PreCommitBatch; - using vm::actor::MethodParams; - PreCommitBatcherImpl::PreCommitBatcherImpl(const Ticks &max_time, - std::shared_ptr api, - const Address &miner_address, - const Ticks &closest_cutoff) + PreCommitBatcherImpl::PreCommitBatcherImpl( + const Ticks &max_time, + std::shared_ptr api, + const Address &miner_address, + const std::shared_ptr &scheduler) : max_delay_(max_time), api_(std::move(api)), miner_address_(miner_address), - closest_cutoff_(closest_cutoff) {} - - PreCommitBatcherImpl::PreCommitEntry::PreCommitEntry( - const TokenAmount &number, const SectorPreCommitInfo &info) - : deposit(number), precommit_info(info){}; - - outcome::result> - PreCommitBatcherImpl::makeBatcher( - const Ticks &max_wait, - std::shared_ptr api, - const std::shared_ptr &scheduler, - const Address &miner_address) { - std::shared_ptr batcher = - std::make_shared( - max_wait, std::move(api), miner_address, max_wait); - - batcher->cutoff_start_ = std::chrono::system_clock::now(); - batcher->logger_ = common::createLogger("batcher"); - batcher->logger_->info("Bather has been started"); - batcher->handle_ = scheduler->schedule(max_wait, [=]() { - std::unique_lock locker(batcher->mutex_, std::defer_lock); - const auto maybe_result = batcher->sendBatch(); - for (const auto &[key, cb] : batcher->callbacks_) { + closest_cutoff_(max_time) { + cutoff_start_ = std::chrono::system_clock::now(); + logger_ = common::createLogger("batcher"); + logger_->info("Bather has been started"); + handle_ = scheduler->schedule(max_delay_, [this]() { + std::unique_lock locker(mutex_, std::defer_lock); + const auto maybe_result = sendBatch(); + for (const auto &[key, cb] : callbacks_) { cb(maybe_result); } - batcher->callbacks_.clear(); - batcher->handle_.reschedule(max_wait); + callbacks_.clear(); + handle_.reschedule(max_delay_); }); - return batcher; } + PreCommitBatcherImpl::PreCommitEntry::PreCommitEntry( + const TokenAmount &number, const SectorPreCommitInfo &info) + : deposit(number), precommit_info(info){}; + outcome::result PreCommitBatcherImpl::sendBatch() { // TODO(Elestrias): [FIL-398] goodFunds = mutualDeposit + MaxFee; - for // checking payable diff --git a/core/miner/storage_fsm/impl/precommit_batcher_impl.hpp b/core/miner/storage_fsm/impl/precommit_batcher_impl.hpp index a45da876ad..1bded848d4 100644 --- a/core/miner/storage_fsm/impl/precommit_batcher_impl.hpp +++ b/core/miner/storage_fsm/impl/precommit_batcher_impl.hpp @@ -27,11 +27,6 @@ namespace fc::mining { class PreCommitBatcherImpl : public PreCommitBatcher { public: - PreCommitBatcherImpl(const Ticks &max_time, - std::shared_ptr api, - const Address &miner_address, - const Ticks &closest_cutoff); - struct PreCommitEntry { PreCommitEntry(const TokenAmount &number, const SectorPreCommitInfo &info); @@ -41,15 +36,14 @@ namespace fc::mining { PreCommitEntry &operator=(const PreCommitEntry &other) = default; }; + PreCommitBatcherImpl(const Ticks &max_time, + std::shared_ptr api, + const Address &miner_address, + const std::shared_ptr &scheduler); + void setPreCommitCutoff(const ChainEpoch ¤t_epoch, const SectorInfo §or_info); - static outcome::result> makeBatcher( - const Ticks &max_wait, - std::shared_ptr api, - const std::shared_ptr &scheduler, - const Address &miner_address); - outcome::result addPreCommit( const SectorInfo §or_info, const TokenAmount &deposit, diff --git a/core/miner/storage_fsm/precommit_batcher.hpp b/core/miner/storage_fsm/precommit_batcher.hpp index 5e8e451e74..b5f5429be9 100644 --- a/core/miner/storage_fsm/precommit_batcher.hpp +++ b/core/miner/storage_fsm/precommit_batcher.hpp @@ -9,10 +9,10 @@ #include "miner/storage_fsm/types.hpp" namespace fc::mining { - using vm::actor::builtin::types::miner::SectorPreCommitInfo; - using types::SectorInfo; using primitives::TokenAmount; - using PrecommitCallback = std::function)>; + using types::SectorInfo; + using vm::actor::builtin::types::miner::SectorPreCommitInfo; + using PrecommitCallback = std::function &)>; class PreCommitBatcher { public: diff --git a/test/core/miner/precommit_batcher_test.cpp b/test/core/miner/precommit_batcher_test.cpp index 6eb562439a..ac63f9136b 100644 --- a/test/core/miner/precommit_batcher_test.cpp +++ b/test/core/miner/precommit_batcher_test.cpp @@ -71,10 +71,8 @@ namespace fc::mining { }; EXPECT_CALL(*sch_, now()).WillOnce(testing::Return(current_time_)); - auto result = PreCommitBatcherImpl::makeBatcher( - toTicks(std::chrono::seconds(60)), api_, sch_, miner_address_); - batcher_ = result.value(); - ASSERT_FALSE(result.has_error()); + batcher_ = std::make_shared( + toTicks(std::chrono::seconds(60)), api_, miner_address_, sch_); } std::shared_ptr api_; @@ -94,7 +92,7 @@ namespace fc::mining { api::SectorPreCommitInfo precInf; TokenAmount deposit = 10; EXPECT_OUTCOME_TRUE_1(batcher_->addPreCommit( - si, deposit, precInf, [](outcome::result) {})); + si, deposit, precInf, [](const outcome::result &cid) {})); }; /** @@ -116,14 +114,19 @@ namespace fc::mining { si.sector_number = 2; EXPECT_OUTCOME_TRUE_1(batcher_->addPreCommit( - si, deposit, precInf, [](outcome::result) {})); + si, deposit, precInf, [](const outcome::result &cid) { + EXPECT_OUTCOME_TRUE_1(cid); + ASSERT_TRUE(cid.has_value()); + })); mutual_deposit_ += 10; precInf.sealed_cid = "010001020006"_cid; si.sector_number = 3; EXPECT_OUTCOME_TRUE_1(batcher_->addPreCommit( - si, deposit, precInf, [](outcome::result) {})); + si, deposit, precInf, [](const outcome::result &cid) { + ASSERT_TRUE(cid.has_value()); + })); mutual_deposit_ += 10; EXPECT_CALL(*sch_, now()) @@ -143,7 +146,9 @@ namespace fc::mining { si.sector_number = 6; EXPECT_OUTCOME_TRUE_1(batcher_->addPreCommit( - si, deposit, precInf, [](outcome::result) {})); + si, deposit, precInf, [](const outcome::result &cid) { + ASSERT_TRUE(cid.has_value()); + })); mutual_deposit_ += 10; sch_->next_clock(); ASSERT_TRUE(is_called_); @@ -151,7 +156,7 @@ namespace fc::mining { } /** - ShortDistanceSending have 3 Precommits + * ShortDistanceSending have 3 Precommits * @when First two are sent after 30 sec instead of 60 and one more * immediately after the addition. * @then The result should be 2 new messages in message pool 1st: @@ -190,7 +195,9 @@ namespace fc::mining { si.sector_number = 2; EXPECT_OUTCOME_TRUE_1(batcher_->addPreCommit( - si, deposit, precInf, [](outcome::result) {})); + si, deposit, precInf, [](const outcome::result &cid) { + ASSERT_TRUE(cid.has_value()); + })); mutual_deposit_ += 10; sch_->next_clock(); @@ -210,9 +217,10 @@ namespace fc::mining { si.sector_number = 4; EXPECT_OUTCOME_TRUE_1(batcher_->addPreCommit( - si, deposit, precInf, [](outcome::result) {})); + si, deposit, precInf, [](const outcome::result &cid) { + ASSERT_TRUE(cid.has_value()); + })); mutual_deposit_ += 10; ASSERT_TRUE(is_called_); } - } // namespace fc::mining From b6ac747fb0006c92cc44b6e47a1ec16c81be7316 Mon Sep 17 00:00:00 2001 From: Mikhail Tagirov Date: Thu, 15 Jul 2021 20:14:43 +0300 Subject: [PATCH 16/17] PreCommitBatch actor method Signed-off-by: Mikhail Tagirov --- .../impl/precommit_batcher_impl.cpp | 51 ++++++++--------- .../impl/precommit_batcher_impl.hpp | 20 +++---- core/miner/storage_fsm/precommit_batcher.hpp | 7 ++- core/miner/storage_fsm/types.hpp | 1 - .../vm/actor/builtin/v0/miner/miner_actor.hpp | 14 ----- .../vm/actor/builtin/v4/miner/miner_actor.hpp | 40 +++++++++++++ .../vm/actor/builtin/v5/miner/miner_actor.hpp | 56 +++++++++++++++++++ 7 files changed, 134 insertions(+), 55 deletions(-) create mode 100644 core/vm/actor/builtin/v4/miner/miner_actor.hpp create mode 100644 core/vm/actor/builtin/v5/miner/miner_actor.hpp diff --git a/core/miner/storage_fsm/impl/precommit_batcher_impl.cpp b/core/miner/storage_fsm/impl/precommit_batcher_impl.cpp index e592d4ee0d..7412d420e0 100644 --- a/core/miner/storage_fsm/impl/precommit_batcher_impl.cpp +++ b/core/miner/storage_fsm/impl/precommit_batcher_impl.cpp @@ -5,11 +5,8 @@ #include "miner/storage_fsm/impl/precommit_batcher_impl.hpp" -#include -#include "const.hpp" #include "vm/actor/actor.hpp" -#include "vm/actor/builtin/types/miner/sector_info.hpp" -#include "vm/actor/builtin/v0/miner/miner_actor.hpp" +#include "vm/actor/builtin/v5/miner/miner_actor.hpp" namespace fc::mining { using api::kPushNoSpec; @@ -17,7 +14,11 @@ namespace fc::mining { using primitives::ChainEpoch; using vm::actor::MethodParams; using vm::actor::builtin::types::miner::kChainFinality; - using vm::actor::builtin::v0::miner::PreCommitBatch; + using vm::actor::builtin::v5::miner::PreCommitBatch; + + PreCommitBatcherImpl::PreCommitEntry::PreCommitEntry( + const TokenAmount &number, const SectorPreCommitInfo &info) + : deposit(number), precommit_info(info){}; PreCommitBatcherImpl::PreCommitBatcherImpl( const Ticks &max_time, @@ -30,8 +31,8 @@ namespace fc::mining { closest_cutoff_(max_time) { cutoff_start_ = std::chrono::system_clock::now(); logger_ = common::createLogger("batcher"); - logger_->info("Bather has been started"); - handle_ = scheduler->schedule(max_delay_, [this]() { + logger_->info("Batcher has been started"); + handle_ = scheduler->schedule(max_delay_, [&]() { std::unique_lock locker(mutex_, std::defer_lock); const auto maybe_result = sendBatch(); for (const auto &[key, cb] : callbacks_) { @@ -42,10 +43,6 @@ namespace fc::mining { }); } - PreCommitBatcherImpl::PreCommitEntry::PreCommitEntry( - const TokenAmount &number, const SectorPreCommitInfo &info) - : deposit(number), precommit_info(info){}; - outcome::result PreCommitBatcherImpl::sendBatch() { // TODO(Elestrias): [FIL-398] goodFunds = mutualDeposit + MaxFee; - for // checking payable @@ -55,7 +52,7 @@ namespace fc::mining { OUTCOME_TRY(head, api_->ChainHead()); OUTCOME_TRY(minfo, api_->StateMinerInfo(miner_address_, head->key)); - PreCommitBatch::Params params = {}; + PreCommitBatch::Params params; for (const auto &data : batch_storage_) { mutual_deposit_ += data.second.deposit; @@ -64,18 +61,18 @@ namespace fc::mining { OUTCOME_TRY(encodedParams, codec::cbor::encode(params)); - OUTCOME_TRY(signed_message, - api_->MpoolPushMessage( - vm::message::UnsignedMessage( - miner_address_, - minfo.worker, // TODO: handle worker - 0, - mutual_deposit_, - {}, - {}, - 25, // TODO (m.tagirov) Miner actor v5 PreCommitBatch - MethodParams{encodedParams}), - kPushNoSpec)); + OUTCOME_TRY( + signed_message, + api_->MpoolPushMessage( + vm::message::UnsignedMessage(miner_address_, + minfo.worker, // TODO: handle worker + 0, + mutual_deposit_, + {}, + {}, + PreCommitBatch::Number, + MethodParams{encodedParams}), + kPushNoSpec)); mutual_deposit_ = 0; batch_storage_.clear(); @@ -103,11 +100,11 @@ namespace fc::mining { sector_info.ticket_epoch + static_cast(kEpochsInDay + kChainFinality); ChainEpoch start_epoch{}; - for (const auto &p : sector_info.pieces) { - if (!p.deal_info) { + for (const auto &piece : sector_info.pieces) { + if (!piece.deal_info) { continue; } - start_epoch = p.deal_info->deal_schedule.start_epoch; + start_epoch = piece.deal_info->deal_schedule.start_epoch; if (start_epoch < cutoff_epoch) { cutoff_epoch = start_epoch; } diff --git a/core/miner/storage_fsm/impl/precommit_batcher_impl.hpp b/core/miner/storage_fsm/impl/precommit_batcher_impl.hpp index 1bded848d4..89c8811037 100644 --- a/core/miner/storage_fsm/impl/precommit_batcher_impl.hpp +++ b/core/miner/storage_fsm/impl/precommit_batcher_impl.hpp @@ -5,17 +5,14 @@ #pragma once -#include +#include "miner/storage_fsm/precommit_batcher.hpp" + #include -#include #include #include +#include #include "api/full_node/node_api.hpp" -#include "miner/storage_fsm/precommit_batcher.hpp" -#include "miner/storage_fsm/types.hpp" #include "primitives/address/address.hpp" -#include "primitives/types.hpp" -#include "vm/actor/actor.hpp" namespace fc::mining { using api::FullNodeApi; @@ -28,12 +25,15 @@ namespace fc::mining { class PreCommitBatcherImpl : public PreCommitBatcher { public: struct PreCommitEntry { + PreCommitEntry() = default; + PreCommitEntry(const TokenAmount &number, const SectorPreCommitInfo &info); - PreCommitEntry() = default; - TokenAmount deposit; - SectorPreCommitInfo precommit_info; + PreCommitEntry &operator=(const PreCommitEntry &other) = default; + + TokenAmount deposit{}; + SectorPreCommitInfo precommit_info; }; PreCommitBatcherImpl(const Ticks &max_time, @@ -48,7 +48,7 @@ namespace fc::mining { const SectorInfo §or_info, const TokenAmount &deposit, const SectorPreCommitInfo &precommit_info, - const PrecommitCallback &callback); + const PrecommitCallback &callback) override; void forceSend() override; diff --git a/core/miner/storage_fsm/precommit_batcher.hpp b/core/miner/storage_fsm/precommit_batcher.hpp index b5f5429be9..dd3ec6f53f 100644 --- a/core/miner/storage_fsm/precommit_batcher.hpp +++ b/core/miner/storage_fsm/precommit_batcher.hpp @@ -4,8 +4,7 @@ */ #pragma once -#include -#include + #include "miner/storage_fsm/types.hpp" namespace fc::mining { @@ -16,13 +15,15 @@ namespace fc::mining { class PreCommitBatcher { public: + virtual ~PreCommitBatcher() = default; + virtual outcome::result addPreCommit( const SectorInfo §or_info, const TokenAmount &deposit, const SectorPreCommitInfo &precommit_info, const PrecommitCallback &callback) = 0; + virtual void forceSend() = 0; - virtual ~PreCommitBatcher() = default; }; } // namespace fc::mining diff --git a/core/miner/storage_fsm/types.hpp b/core/miner/storage_fsm/types.hpp index 0c3398aaf1..280b9aa4fc 100644 --- a/core/miner/storage_fsm/types.hpp +++ b/core/miner/storage_fsm/types.hpp @@ -6,7 +6,6 @@ #pragma once #include "miner/storage_fsm/sealing_states.hpp" -#include "miner/storage_fsm/types.hpp" #include "primitives/piece/piece.hpp" #include "primitives/sector/sector.hpp" #include "primitives/tipset/tipset_key.hpp" diff --git a/core/vm/actor/builtin/v0/miner/miner_actor.hpp b/core/vm/actor/builtin/v0/miner/miner_actor.hpp index d70e731737..05d24ef4a4 100644 --- a/core/vm/actor/builtin/v0/miner/miner_actor.hpp +++ b/core/vm/actor/builtin/v0/miner/miner_actor.hpp @@ -335,18 +335,4 @@ namespace fc::vm::actor::builtin::v0::miner { extern const ActorExports exports; - /** - * Collects and stores precommit messages to make - * a packaged sending of a several messages within one transaction - * which reduces the general amount of transactions in the network - * with reduction of a gas fee for transactions. - */ - struct PreCommitBatch : ActorMethodBase<25> { - struct Params { - std::vector sectors; - }; - ACTOR_METHOD_DECL(); - }; - CBOR_TUPLE(PreCommitBatch::Params, sectors); - } // namespace fc::vm::actor::builtin::v0::miner diff --git a/core/vm/actor/builtin/v4/miner/miner_actor.hpp b/core/vm/actor/builtin/v4/miner/miner_actor.hpp new file mode 100644 index 0000000000..d98380c958 --- /dev/null +++ b/core/vm/actor/builtin/v4/miner/miner_actor.hpp @@ -0,0 +1,40 @@ +/** + * Copyright Soramitsu Co., Ltd. All Rights Reserved. + * SPDX-License-Identifier: Apache-2.0 + */ + +#pragma once + +#include "vm/actor/builtin/v3/miner/miner_actor.hpp" + +namespace fc::vm::actor::builtin::v4::miner { + + // TODO implement + using Construct = v3::miner::Construct; + using ControlAddresses = v3::miner::ControlAddresses; + using ChangeWorkerAddress = v3::miner::ChangeWorkerAddress; + using ChangePeerId = v3::miner::ChangePeerId; + using SubmitWindowedPoSt = v3::miner::SubmitWindowedPoSt; + using PreCommitSector = v3::miner::PreCommitSector; + using ProveCommitSector = v3::miner::ProveCommitSector; + using ExtendSectorExpiration = v3::miner::ExtendSectorExpiration; + using TerminateSectors = v3::miner::TerminateSectors; + using DeclareFaults = v3::miner::DeclareFaults; + using DeclareFaultsRecovered = v3::miner::DeclareFaultsRecovered; + using OnDeferredCronEvent = v3::miner::OnDeferredCronEvent; + using CheckSectorProven = v3::miner::CheckSectorProven; + using ApplyRewards = v3::miner::ApplyRewards; + using ReportConsensusFault = v3::miner::ReportConsensusFault; + using WithdrawBalance = v3::miner::WithdrawBalance; + using ConfirmSectorProofsValid = v3::miner::ConfirmSectorProofsValid; + using ChangeMultiaddresses = v3::miner::ChangeMultiaddresses; + using CompactPartitions = v3::miner::CompactPartitions; + using CompactSectorNumbers = v3::miner::CompactSectorNumbers; + using ConfirmUpdateWorkerKey = v3::miner::ConfirmUpdateWorkerKey; + using RepayDebt = v3::miner::RepayDebt; + using ChangeOwnerAddress = v3::miner::ChangeOwnerAddress; + using DisputeWindowedPoSt = v3::miner::DisputeWindowedPoSt; + + // extern const ActorExports exports; + +} // namespace fc::vm::actor::builtin::v4::miner diff --git a/core/vm/actor/builtin/v5/miner/miner_actor.hpp b/core/vm/actor/builtin/v5/miner/miner_actor.hpp new file mode 100644 index 0000000000..f2486929cb --- /dev/null +++ b/core/vm/actor/builtin/v5/miner/miner_actor.hpp @@ -0,0 +1,56 @@ +/** + * Copyright Soramitsu Co., Ltd. All Rights Reserved. + * SPDX-License-Identifier: Apache-2.0 + */ + +#pragma once + +#include "vm/actor/builtin/v4/miner/miner_actor.hpp" + +#include "vm/actor/builtin/types/miner/sector_info.hpp" + +namespace fc::vm::actor::builtin::v5::miner { + using types::miner::SectorPreCommitInfo; + + // TODO implement + using Construct = v4::miner::Construct; + using ControlAddresses = v4::miner::ControlAddresses; + using ChangeWorkerAddress = v4::miner::ChangeWorkerAddress; + using ChangePeerId = v4::miner::ChangePeerId; + using SubmitWindowedPoSt = v4::miner::SubmitWindowedPoSt; + using PreCommitSector = v4::miner::PreCommitSector; + using ProveCommitSector = v4::miner::ProveCommitSector; + using ExtendSectorExpiration = v4::miner::ExtendSectorExpiration; + using TerminateSectors = v4::miner::TerminateSectors; + using DeclareFaults = v4::miner::DeclareFaults; + using DeclareFaultsRecovered = v4::miner::DeclareFaultsRecovered; + using OnDeferredCronEvent = v4::miner::OnDeferredCronEvent; + using CheckSectorProven = v4::miner::CheckSectorProven; + using ApplyRewards = v4::miner::ApplyRewards; + using ReportConsensusFault = v4::miner::ReportConsensusFault; + using WithdrawBalance = v4::miner::WithdrawBalance; + using ConfirmSectorProofsValid = v4::miner::ConfirmSectorProofsValid; + using ChangeMultiaddresses = v4::miner::ChangeMultiaddresses; + using CompactPartitions = v4::miner::CompactPartitions; + using CompactSectorNumbers = v4::miner::CompactSectorNumbers; + using ConfirmUpdateWorkerKey = v4::miner::ConfirmUpdateWorkerKey; + using RepayDebt = v4::miner::RepayDebt; + using ChangeOwnerAddress = v4::miner::ChangeOwnerAddress; + using DisputeWindowedPoSt = v4::miner::DisputeWindowedPoSt; + + /** + * Collects and stores precommit messages to make a packaged sending of a + * several messages within one transaction which reduces the general amount of + * transactions in the network with reduction of a gas fee for transactions. + */ + struct PreCommitBatch : ActorMethodBase<25> { + struct Params { + std::vector sectors; + }; + ACTOR_METHOD_DECL(); + }; + CBOR_TUPLE(PreCommitBatch::Params, sectors); + + // extern const ActorExports exports; + +} // namespace fc::vm::actor::builtin::v5::miner From c05f9286e8e7353898b8663517b7dc698f0f2573 Mon Sep 17 00:00:00 2001 From: elestrias Date: Fri, 16 Jul 2021 14:03:11 +0300 Subject: [PATCH 17/17] mutex fixes, cosmetic changes Signed-off-by: elestrias --- .../impl/precommit_batcher_impl.cpp | 14 ++++++-- .../impl/precommit_batcher_impl.hpp | 36 ++++++++++--------- test/core/miner/precommit_batcher_test.cpp | 18 +++++----- 3 files changed, 40 insertions(+), 28 deletions(-) diff --git a/core/miner/storage_fsm/impl/precommit_batcher_impl.cpp b/core/miner/storage_fsm/impl/precommit_batcher_impl.cpp index 7412d420e0..d325759add 100644 --- a/core/miner/storage_fsm/impl/precommit_batcher_impl.cpp +++ b/core/miner/storage_fsm/impl/precommit_batcher_impl.cpp @@ -33,12 +33,14 @@ namespace fc::mining { logger_ = common::createLogger("batcher"); logger_->info("Batcher has been started"); handle_ = scheduler->schedule(max_delay_, [&]() { - std::unique_lock locker(mutex_, std::defer_lock); + std::unique_lock locker(mutex_); const auto maybe_result = sendBatch(); for (const auto &[key, cb] : callbacks_) { cb(maybe_result); } callbacks_.clear(); + cutoff_start_ = std::chrono::system_clock::now(); + closest_cutoff_ = max_delay_; handle_.reschedule(max_delay_); }); } @@ -85,12 +87,18 @@ namespace fc::mining { } void PreCommitBatcherImpl::forceSend() { - std::unique_lock locker(mutex_, std::defer_lock); + std::unique_lock locker(mutex_); + forceSendWithoutLock(); + } + + void PreCommitBatcherImpl::forceSendWithoutLock(){ const auto maybe_result = sendBatch(); for (const auto &[key, cb] : callbacks_) { cb(maybe_result); } callbacks_.clear(); + cutoff_start_ = std::chrono::system_clock::now(); + closest_cutoff_ = max_delay_; handle_.reschedule(max_delay_); } @@ -110,7 +118,7 @@ namespace fc::mining { } } if (cutoff_epoch <= current_epoch) { - forceSend(); + forceSendWithoutLock(); } else { const Ticks temp_cutoff = toTicks(std::chrono::seconds( (cutoff_epoch - current_epoch) * kEpochDurationSeconds)); diff --git a/core/miner/storage_fsm/impl/precommit_batcher_impl.hpp b/core/miner/storage_fsm/impl/precommit_batcher_impl.hpp index 89c8811037..a8ee98cf0c 100644 --- a/core/miner/storage_fsm/impl/precommit_batcher_impl.hpp +++ b/core/miner/storage_fsm/impl/precommit_batcher_impl.hpp @@ -24,26 +24,11 @@ namespace fc::mining { class PreCommitBatcherImpl : public PreCommitBatcher { public: - struct PreCommitEntry { - PreCommitEntry() = default; - - PreCommitEntry(const TokenAmount &number, - const SectorPreCommitInfo &info); - - PreCommitEntry &operator=(const PreCommitEntry &other) = default; - - TokenAmount deposit{}; - SectorPreCommitInfo precommit_info; - }; - PreCommitBatcherImpl(const Ticks &max_time, std::shared_ptr api, const Address &miner_address, const std::shared_ptr &scheduler); - void setPreCommitCutoff(const ChainEpoch ¤t_epoch, - const SectorInfo §or_info); - outcome::result addPreCommit( const SectorInfo §or_info, const TokenAmount &deposit, @@ -52,9 +37,19 @@ namespace fc::mining { void forceSend() override; - outcome::result sendBatch(); - private: + struct PreCommitEntry { + PreCommitEntry() = default; + + PreCommitEntry(const TokenAmount &number, + const SectorPreCommitInfo &info); + + PreCommitEntry &operator=(const PreCommitEntry &other) = default; + + TokenAmount deposit{}; + SectorPreCommitInfo precommit_info; + }; + std::mutex mutex_; TokenAmount mutual_deposit_; std::map batch_storage_; @@ -66,6 +61,13 @@ namespace fc::mining { std::chrono::system_clock::time_point cutoff_start_; common::Logger logger_; std::map callbacks_; + + void forceSendWithoutLock(); + + void setPreCommitCutoff(const ChainEpoch ¤t_epoch, + const SectorInfo §or_info); + + outcome::result sendBatch(); }; } // namespace fc::mining diff --git a/test/core/miner/precommit_batcher_test.cpp b/test/core/miner/precommit_batcher_test.cpp index ac63f9136b..7fa4b78903 100644 --- a/test/core/miner/precommit_batcher_test.cpp +++ b/test/core/miner/precommit_batcher_test.cpp @@ -9,6 +9,7 @@ #include "testutil/literals.hpp" #include "testutil/mocks/libp2p/scheduler_mock.hpp" #include "testutil/outcome.hpp" +#include "vm/actor/builtin/v5/miner/miner_actor.hpp" namespace fc::mining { using api::MinerInfo; @@ -24,6 +25,7 @@ namespace fc::mining { using primitives::tipset::Tipset; using primitives::tipset::TipsetCPtr; using testing::Mock; + using vm::actor::builtin::v5::miner::PreCommitBatch; class PreCommitBatcherTest : public testing::Test { protected: @@ -62,7 +64,7 @@ namespace fc::mining { [&](const UnsignedMessage &msg, const boost::optional &) -> outcome::result { - if (msg.method == 25) { + if (msg.method == PreCommitBatch::Number) { is_called_ = true; return SignedMessage{.message = msg, .signature = BlsSignature()}; } @@ -96,8 +98,8 @@ namespace fc::mining { }; /** - * CallbackSend have 4 precommits sent in pairs - * @when PrecommitBatcher send the first pair and after the rescheduling + * @given 4 precommits + * @when send the first pair and after the rescheduling * next pair * @then The result should be 2 messages in message pool with pair of * precommits in each @@ -156,12 +158,12 @@ namespace fc::mining { } /** - * ShortDistanceSending have 3 Precommits - * @when First two are sent after 30 sec instead of 60 and one more + * @given 3 precommits + * @when first two are sent after 30 sec instead of 60 and one more * immediately after the addition. - * @then The result should be 2 new messages in message pool 1st: - * 2 precommits, have been sent after 30 sec, and 2nd: one - * precommmit, have been sent after the first one immediately + * @then The result should be 2 new messages in message pool 1st pair: + * have been sent after 30 sec, and 2nd pair: + * have been sent after the first one immediately **/ TEST_F(PreCommitBatcherTest, ShortDistanceSending) { mutual_deposit_ = 0;