dtr-org / unit-e

A digital currency for a new era of decentralized trust
https://unit-e.io
MIT License
45 stars 15 forks source link

generate/generatetoaddress should use block builder #907

Open scravy opened 5 years ago

scravy commented 5 years ago

generate and generatetoaddress use a half-baked version of proposing blocks but they should use the proposer logic / block builder components.

The code in question is the following:

CValidationState state;
    const staking::ActiveChain *active_chain = GetComponent<staking::ActiveChain>();
    for (const staking::Coin &coin : stakeable_coins) {
      proposer::EligibleCoin eligible_coin = {
          staking::Coin(active_chain->GetBlockIndex(coin.GetTransactionId()),
              COutPoint(coin.GetTransactionId(), coin.GetOutputIndex()),
              CTxOut(coin.GetAmount(), scriptPubKeyIn)),
          GetRandHash(), //TODO UNIT-E: At the moment is not used, since we still have PoW here
          GetComponent<blockchain::Behavior>()->CalculateBlockReward(nHeight),
          0, //TODO UNIT-E: At the moment is not used, since we still have PoW here
          0, //TODO UNIT-E: At the moment is not used, since we still have PoW here
          0 //TODO UNIT-E: At the moment is not used, since we still have PoW here
      };

This code replaces the spending outpoint (!) with the target pubkey.

Using the components ready made for staking the proposing code should read:

    LOCK(m_chain->GetLock());
    const CBlockIndex *current_tip = m_chain->GetTip();
    if (!current_tip) {
      throw JSONRPCError(RPC_IN_WARMUP, "Genesis block not loaded yet (current active chain does not have a tip).");
    }
    const uint256 snapshot_hash = m_chain->ComputeSnapshotHash();
    const staking::CoinSet coins = staking_wallet.GetStakeableCoins();
    if (coins.empty()) {
      throw JSONRPCError(RPC_INTERNAL_ERROR, "No stakeable coins.");
    }
    staking::TransactionPicker::PickTransactionsParameters parameters{};
    const staking::TransactionPicker::PickTransactionsResult txs = m_transaction_picker->PickTransactions(parameters);
    if (!txs) {
      throw JSONRPCError(RPC_INTERNAL_ERROR, txs.error);
    }
    const CAmount fees = std::accumulate(txs.fees.begin(), txs.fees.end(), CAmount(0));
    boost::optional<EligibleCoin> coin = m_proposer_logic->TryPropose(coins);
    if (!coin) {
      throw JSONRPCError(RPC_INTERNAL_ERROR, "No eligible coin available.");
    }
    std::shared_ptr<const CBlock> block =
        m_block_builder->BuildBlock(*current_tip, snapshot_hash, *coin, coins, txs.transactions, fees, staking_wallet);
    if (!block) {
      throw JSONRPCError(RPC_INTERNAL_ERROR, "Failed to build block.");
    }
    if (!ProcessNewBlock(Params(), block, true, nullptr)) {
      throw JSONRPCError(RPC_INTERNAL_ERROR, "ProcessNewBlock: block not accepted.");
    }
scravy commented 5 years ago

Update: generate is correct in that it does create the right reference, but still there is something fishy. The logic proposed in the second code block here still is the logic which should go here. I had played around with passing the reward address and the stake return address separately to control this in https://github.com/dtr-org/unit-e/pull/886 which broke stuff, which should not happen if both were doing the same thing (which apparently they are not).

Anyways, postponing as generate/generatetoaddress are good enough for creating stake validation (All I need for that is the RPCs from #900 to properly assert on the staking state).