Skip to content

Commit

Permalink
Remove BlockBuilder dependency on FinalizationRewardLogic
Browse files Browse the repository at this point in the history
Signed-off-by: Azat Nizametdinov <[email protected]>
  • Loading branch information
Nizametdinov committed May 27, 2019
1 parent 5320157 commit cde2613
Show file tree
Hide file tree
Showing 11 changed files with 104 additions and 61 deletions.
6 changes: 3 additions & 3 deletions src/injector.h
Original file line number Diff line number Diff line change
Expand Up @@ -136,8 +136,7 @@ class UnitEInjector : public Injector<UnitEInjector> {
COMPONENT(MultiWallet, proposer::MultiWallet, proposer::MultiWallet::New)

COMPONENT(BlockBuilder, proposer::BlockBuilder, proposer::BlockBuilder::New,
Settings,
proposer::FinalizationRewardLogic)
Settings)

COMPONENT(ProposerRPC, proposer::ProposerRPC, proposer::ProposerRPC::New,
Settings,
Expand All @@ -160,7 +159,8 @@ class UnitEInjector : public Injector<UnitEInjector> {
staking::ActiveChain,
staking::TransactionPicker,
proposer::BlockBuilder,
proposer::Logic)
proposer::Logic,
proposer::FinalizationRewardLogic)

#endif

Expand Down
5 changes: 4 additions & 1 deletion src/miner.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -181,7 +181,10 @@ std::unique_ptr<CBlockTemplate> BlockAssembler::CreateNewBlock(const CScript& sc
0 //TODO UNIT-E: At the moment is not used, since we still have PoW here
};

const CTransactionRef coinbase = GetComponent<proposer::BlockBuilder>()->BuildCoinbaseTransaction(*active_chain->GetTip(), uint256(snapshot_hash), eligible_coin, staking::CoinSet(), nFees, scriptPubKeyIn, pwallet->GetWalletExtension());
const CBlockIndex &tip = *active_chain->GetTip();
std::vector<CTxOut> finalization_rewards = GetComponent<proposer::FinalizationRewardLogic>()->GetFinalizationRewards(tip);
const CTransactionRef coinbase = GetComponent<proposer::BlockBuilder>()->BuildCoinbaseTransaction(
uint256(snapshot_hash), eligible_coin, staking::CoinSet(), nFees, finalization_rewards, scriptPubKeyIn, pwallet->GetWalletExtension());
pblocktemplate->block.vtx[0] = coinbase;

LogPrintf("%s: block weight=%u txs=%u fees=%ld sigops=%d\n", __func__, GetBlockWeight(*pblock), nBlockTx, nFees, nBlockSigOpsCost);
Expand Down
23 changes: 8 additions & 15 deletions src/proposer/block_builder.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@ namespace proposer {
class BlockBuilderImpl : public BlockBuilder {
private:
const Dependency<Settings> m_settings;
const Dependency<FinalizationRewardLogic> m_finalization_reward_logic;

std::vector<CAmount> SplitAmount(const CAmount amount, const CAmount threshold) const {
auto number_of_pieces = amount / threshold;
Expand Down Expand Up @@ -95,17 +94,15 @@ class BlockBuilderImpl : public BlockBuilder {

public:
explicit BlockBuilderImpl(
Dependency<Settings> settings,
Dependency<FinalizationRewardLogic> finalization_reward_logic)
: m_settings(settings),
m_finalization_reward_logic(finalization_reward_logic) {}
Dependency<Settings> settings)
: m_settings(settings) {}

const CTransactionRef BuildCoinbaseTransaction(
const CBlockIndex &prev_block,
const uint256 &snapshot_hash,
const EligibleCoin &eligible_coin,
const staking::CoinSet &coins,
const CAmount fees,
const std::vector<CTxOut> &finalization_rewards,
const boost::optional<CScript> &coinbase_script,
staking::StakingWallet &wallet) const override {
CMutableTransaction tx;
Expand Down Expand Up @@ -151,9 +148,6 @@ class BlockBuilderImpl : public BlockBuilder {

tx.vout.emplace_back(reward, reward_script);

std::vector<CTxOut> finalization_rewards =
m_finalization_reward_logic->GetFinalizationRewards(prev_block);

CAmount combined_reward = reward;
for (const auto &r : finalization_rewards) {
tx.vout.push_back(r);
Expand Down Expand Up @@ -194,6 +188,7 @@ class BlockBuilderImpl : public BlockBuilder {
const staking::CoinSet &coins,
const std::vector<CTransactionRef> &txs,
const CAmount fees,
const std::vector<CTxOut> &finalization_rewards,
const boost::optional<CScript> &coinbase_script,
staking::StakingWallet &wallet) const override {

Expand All @@ -206,8 +201,8 @@ class BlockBuilderImpl : public BlockBuilder {
// nonce will be removed and is not relevant in PoS, not setting it here

// add coinbase transaction first
const CTransactionRef coinbase_transaction =
BuildCoinbaseTransaction(prev_block, snapshot_hash, coin, coins, fees, coinbase_script, wallet);
const CTransactionRef coinbase_transaction = BuildCoinbaseTransaction(
snapshot_hash, coin, coins, fees, finalization_rewards, coinbase_script, wallet);
if (!coinbase_transaction) {
Log("Failed to create coinbase transaction.");
return nullptr;
Expand All @@ -227,10 +222,8 @@ class BlockBuilderImpl : public BlockBuilder {
}
};

std::unique_ptr<BlockBuilder> BlockBuilder::New(
const Dependency<Settings> settings,
const Dependency<FinalizationRewardLogic> finalization_reward_logic) {
return std::unique_ptr<BlockBuilder>(new BlockBuilderImpl(settings, finalization_reward_logic));
std::unique_ptr<BlockBuilder> BlockBuilder::New(const Dependency<Settings> settings) {
return MakeUnique<BlockBuilderImpl>(settings);
}

} // namespace proposer
7 changes: 3 additions & 4 deletions src/proposer/block_builder.h
Original file line number Diff line number Diff line change
Expand Up @@ -25,11 +25,11 @@ class BlockBuilder {
public:
//! \brief Builds a coinbase transaction.
virtual const CTransactionRef BuildCoinbaseTransaction(
const CBlockIndex &prev_block, //!< The previous block / current tip.
const uint256 &snapshot_hash, //!< The snapshot hash to be included.
const EligibleCoin &eligible_coin, //!< The eligible coin to reference as stake. Also contains the target height.
const staking::CoinSet &coins, //!< Any other coins that should be combined into the coinbase tx.
CAmount fees, //!< The amount of fees to be included (for the reward).
const std::vector<CTxOut> &finalization_rewards, //!< The finalization reward outputs to be added after block reward output.
const boost::optional<CScript> &coinbase_script, //!< The scriptpubkey to be used for the coinbase reward and fees.
staking::StakingWallet &wallet //!< The wallet to be used for signing the transaction.
) const = 0;
Expand All @@ -42,15 +42,14 @@ class BlockBuilder {
const staking::CoinSet &coins, //!< Other coins to combine with the stake.
const std::vector<CTransactionRef> &txs, //!< Transactions to include in the block.
CAmount fees, //!< The fees on the transactions.
const std::vector<CTxOut> &finalization_rewards, //!< The finalization reward outputs to be added to the coinbase transaction.
const boost::optional<CScript> &coinbase_script, //!< The scriptpubkey to be used for the coinbase reward and fees.
staking::StakingWallet &wallet //!< A wallet used to sign blocks and stake.
) const = 0;

virtual ~BlockBuilder() = default;

static std::unique_ptr<BlockBuilder> New(
Dependency<Settings>,
Dependency<FinalizationRewardLogic>);
static std::unique_ptr<BlockBuilder> New(Dependency<Settings>);
};

} // namespace proposer
Expand Down
28 changes: 21 additions & 7 deletions src/proposer/proposer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ std::shared_ptr<const CBlock> GenerateBlock(staking::ActiveChain &active_chain,
staking::TransactionPicker &transaction_picker,
proposer::BlockBuilder &block_builder,
proposer::Logic &logic,
proposer::FinalizationRewardLogic &fin_reward_logic,
staking::StakingWallet &wallet,
const staking::CoinSet &coins,
const boost::optional<CScript> &coinbase_script) {
Expand All @@ -59,8 +60,10 @@ std::shared_ptr<const CBlock> GenerateBlock(staking::ActiveChain &active_chain,
const CAmount fees = std::accumulate(result.fees.begin(), result.fees.end(), CAmount(0));
const uint256 snapshot_hash = active_chain.ComputeSnapshotHash();

const CBlockIndex &tip = *active_chain.GetTip();
const std::vector<CTxOut> fin_rewards = fin_reward_logic.GetFinalizationRewards(tip);
return block_builder.BuildBlock(
*active_chain.GetTip(), snapshot_hash, coin, coins, result.transactions, fees, coinbase_script, wallet);
tip, snapshot_hash, coin, coins, result.transactions, fees, fin_rewards, coinbase_script, wallet);
}

} // namespace
Expand All @@ -70,16 +73,19 @@ class PassiveProposerImpl : public Proposer {
const Dependency<proposer::BlockBuilder> m_block_builder;
const Dependency<staking::TransactionPicker> m_transaction_picker;
const Dependency<proposer::Logic> m_proposer_logic;
const Dependency<FinalizationRewardLogic> m_finalization_reward_logic;

public:
PassiveProposerImpl(Dependency<staking::ActiveChain> const active_chain,
Dependency<proposer::BlockBuilder> const block_builder,
Dependency<staking::TransactionPicker> const transaction_picker,
Dependency<proposer::Logic> const proposer_logic)
Dependency<proposer::Logic> const proposer_logic,
Dependency<FinalizationRewardLogic> const finalization_reward_logic)
: m_active_chain(active_chain),
m_block_builder(block_builder),
m_transaction_picker(transaction_picker),
m_proposer_logic(proposer_logic) {}
m_proposer_logic(proposer_logic),
m_finalization_reward_logic(finalization_reward_logic) {}

void Wake() override {}
void Start() override {}
Expand All @@ -93,6 +99,7 @@ class PassiveProposerImpl : public Proposer {
*m_transaction_picker,
*m_block_builder,
*m_proposer_logic,
*m_finalization_reward_logic,
wallet,
coins,
coinbase_script);
Expand All @@ -110,6 +117,7 @@ class ActiveProposerImpl : public Proposer {
const Dependency<blockchain::Behavior> m_blockchain_behavior;
const Dependency<MultiWallet> m_multi_wallet;
const Dependency<staking::Network> m_network;
const Dependency<FinalizationRewardLogic> m_finalization_reward_logic;

mutable CCriticalSection m_startstop_lock;

Expand Down Expand Up @@ -179,6 +187,7 @@ class ActiveProposerImpl : public Proposer {
*m_transaction_picker,
*m_block_builder,
*m_proposer_logic,
*m_finalization_reward_logic,
wallet_ext,
coins,
boost::none /* coinbase_script */);
Expand Down Expand Up @@ -211,14 +220,16 @@ class ActiveProposerImpl : public Proposer {
const Dependency<proposer::Logic> proposer_logic,
const Dependency<blockchain::Behavior> blockchain_behavior,
const Dependency<MultiWallet> multi_wallet,
const Dependency<staking::Network> network)
const Dependency<staking::Network> network,
const Dependency<FinalizationRewardLogic> finalization_reward_logic)
: m_active_chain(active_chain),
m_transaction_picker(transaction_picker),
m_block_builder(block_builder),
m_proposer_logic(proposer_logic),
m_blockchain_behavior(blockchain_behavior),
m_multi_wallet(multi_wallet),
m_network(network),
m_finalization_reward_logic(finalization_reward_logic),
m_interrupted(false) {
}

Expand Down Expand Up @@ -273,11 +284,14 @@ std::unique_ptr<Proposer> Proposer::New(
const Dependency<staking::ActiveChain> active_chain,
const Dependency<staking::TransactionPicker> transaction_picker,
const Dependency<BlockBuilder> block_builder,
const Dependency<Logic> proposer_logic) {
const Dependency<Logic> proposer_logic,
const Dependency<FinalizationRewardLogic> finalization_reward_logic) {
if (settings->node_is_proposer) {
return MakeUnique<ActiveProposerImpl>(active_chain, transaction_picker, block_builder, proposer_logic, behavior, multi_wallet, network);
return MakeUnique<ActiveProposerImpl>(active_chain, transaction_picker, block_builder, proposer_logic, behavior,
multi_wallet, network, finalization_reward_logic);
} else {
return MakeUnique<PassiveProposerImpl>(active_chain, block_builder, transaction_picker, proposer_logic);
return MakeUnique<PassiveProposerImpl>(active_chain, block_builder, transaction_picker, proposer_logic,
finalization_reward_logic);
}
}

Expand Down
4 changes: 3 additions & 1 deletion src/proposer/proposer.h
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ namespace proposer {
class BlockBuilder;
class Logic;
class MultiWallet;
class FinalizationRewardLogic;

class Proposer {

Expand All @@ -56,7 +57,8 @@ class Proposer {
Dependency<staking::ActiveChain>,
Dependency<staking::TransactionPicker>,
Dependency<BlockBuilder>,
Dependency<Logic>);
Dependency<Logic>,
Dependency<FinalizationRewardLogic>);
};

} // namespace proposer
Expand Down
7 changes: 2 additions & 5 deletions src/test/esperanza/walletextension_tests.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -77,10 +77,7 @@ BOOST_FIXTURE_TEST_CASE(sign_coinbase_transaction, WalletTestingSetup) {
const auto pubkey = key.GetPubKey();
const auto pubkeydata = std::vector<unsigned char>(pubkey.begin(), pubkey.end());

auto behavior = blockchain::Behavior::NewFromParameters(blockchain::Parameters::TestNet());
auto active_chain = staking::ActiveChain::New();
mocks::FinalizationRewardLogicMock finalization_reward_logic;
auto block_builder = proposer::BlockBuilder::New(&settings, &finalization_reward_logic);
auto block_builder = proposer::BlockBuilder::New(&settings);

{
LOCK(m_wallet->cs_wallet);
Expand Down Expand Up @@ -141,7 +138,7 @@ BOOST_FIXTURE_TEST_CASE(sign_coinbase_transaction, WalletTestingSetup) {

// BuildCoinbaseTransaction() will also sign it
CTransactionRef coinbase_transaction =
block_builder->BuildCoinbaseTransaction(*active_chain->GetTip(), uint256(), eligible_coin, coins, 700, boost::none, m_wallet->GetWalletExtension());
block_builder->BuildCoinbaseTransaction(uint256(), eligible_coin, coins, 700, {}, boost::none, m_wallet->GetWalletExtension());

// check that a coinbase transaction was built successfully
BOOST_REQUIRE(static_cast<bool>(coinbase_transaction));
Expand Down
Loading

0 comments on commit cde2613

Please sign in to comment.