Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Use tx_type in Coin and Snapshot instead of fIsCoinbase #856

Merged
merged 4 commits into from
Apr 2, 2019
Merged

Use tx_type in Coin and Snapshot instead of fIsCoinbase #856

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
5e7351f
Add transaction type to the Coin
frolosofsky Mar 27, 2019
1dab145
cr followup
frolosofsky Apr 1, 2019
4d0d022
Update help
frolosofsky Apr 1, 2019
93d3ca7
cr followup
frolosofsky Apr 1, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 3 additions & 4 deletions src/coins.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -99,13 +99,12 @@ void CCoinsViewCache::AddCoin(const COutPoint &outpoint, Coin&& coin, bool possi
}

void AddCoins(CCoinsViewCache& cache, const CTransaction &tx, int nHeight, bool check) {
bool fCoinbase = tx.IsCoinBase();
const uint256& txid = tx.GetHash();
for (size_t i = 0; i < tx.vout.size(); ++i) {
bool overwrite = check ? cache.HaveCoin(COutPoint(txid, i)) : fCoinbase;
const bool overwrite = check ? cache.HaveCoin(COutPoint(txid, i)) : tx.IsCoinBase();
// Always set the possible_overwrite flag to AddCoin for coinbase txn, in order to correctly
// deal with the pre-BIP30 occurrences of duplicate coinbase transactions.
frolosofsky marked this conversation as resolved.
Show resolved Hide resolved
cache.AddCoin(COutPoint(txid, i), Coin(tx.vout[i], nHeight, fCoinbase), overwrite);
cache.AddCoin(COutPoint(txid, i), Coin(tx.vout[i], nHeight, tx.GetType()), overwrite);
}
}

Expand Down Expand Up @@ -262,7 +261,7 @@ bool CCoinsViewCache::ApplySnapshot(std::unique_ptr<snapshot::Indexer> &&indexer
snapshot::UTXOSubset &subset = iter.GetUTXOSubset();
for (auto const &p : subset.outputs) {
COutPoint out(subset.tx_id, p.first);
Coin coin(p.second, subset.height, subset.is_coin_base);
Coin coin(p.second, subset.height, subset.tx_type);
AddCoin(out, std::move(coin), true);
}

Expand Down
35 changes: 18 additions & 17 deletions src/coins.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,8 @@
* A UTXO entry.
*
* Serialized format:
* - VARINT((coinbase ? 1 : 0) | (height << 1))
* - uint8_t for TxType
* - uint32_t for Height
* - the non-spent CTxOut (via CTxOutCompressor)
*/
class Coin
Expand All @@ -35,27 +36,26 @@ class Coin
//! unspent transaction output
CTxOut out;

//! whether containing transaction was a coinbase
unsigned int fCoinBase : 1;
TxType tx_type;

//! at which height this containing transaction was included in the active block chain
uint32_t nHeight : 31;
uint32_t nHeight;

//! construct a Coin from a CTxOut and height/coinbase information.
Coin(CTxOut&& outIn, int nHeightIn, bool fCoinBaseIn) : out(std::move(outIn)), fCoinBase(fCoinBaseIn), nHeight(nHeightIn) {}
Coin(const CTxOut& outIn, int nHeightIn, bool fCoinBaseIn) : out(outIn), fCoinBase(fCoinBaseIn),nHeight(nHeightIn) {}
Coin(CTxOut&& outIn, int nHeightIn, TxType tx_type) : out(std::move(outIn)), tx_type(tx_type), nHeight(nHeightIn) {}
Coin(const CTxOut& outIn, int nHeightIn, TxType tx_type) : out(outIn), tx_type(tx_type), nHeight(nHeightIn) {}

void Clear() {
out.SetNull();
fCoinBase = false;
tx_type = TxType::REGULAR;
nHeight = 0;
}

//! empty constructor
Coin() : fCoinBase(false), nHeight(0) { }
Coin() : tx_type(TxType::REGULAR), nHeight(0) { }

bool IsCoinBase() const {
return fCoinBase;
return tx_type == +TxType::COINBASE;
}

//! \brief checks if this transaction is a coinbase and the reward is still immature
Expand Down Expand Up @@ -89,17 +89,18 @@ class Coin
template<typename Stream>
void Serialize(Stream &s) const {
assert(!IsSpent());
uint32_t code = nHeight * 2 + fCoinBase;
::Serialize(s, VARINT(code));
uint8_t type = +tx_type;
::Serialize(s, type);
::Serialize(s, nHeight);
::Serialize(s, CTxOutCompressor(REF(out)));
}

template<typename Stream>
void Unserialize(Stream &s) {
uint32_t code = 0;
::Unserialize(s, VARINT(code));
nHeight = code >> 1;
fCoinBase = code & 1;
uint8_t type = 0;
::Unserialize(s, type);
tx_type = TxType::_from_integral(type);
::Unserialize(s, nHeight);
::Unserialize(s, REF(CTxOutCompressor(out)));
}

Expand Down Expand Up @@ -255,7 +256,7 @@ class CCoinsViewCache : public CCoinsViewBacked, public AccessibleCoinsView
protected:
/**
* Make mutable so that we can "fill the cache" even from Get-methods
* declared as "const".
* declared as "const".
*/
mutable uint256 hashBlock;
mutable snapshot::SnapshotHash snapshotHash;
Expand Down Expand Up @@ -341,7 +342,7 @@ class CCoinsViewCache : public CCoinsViewBacked, public AccessibleCoinsView
//! Calculate the size of the cache (in bytes)
size_t DynamicMemoryUsage() const;

/**
/**
* Amount of unites coming in to a transaction
* Note that lightweight clients may not know anything besides the hash of previous transactions,
* so may not be able to calculate this.
Expand Down
112 changes: 68 additions & 44 deletions src/esperanza/checks.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,36 +2,37 @@
// Distributed under the MIT software license, see the accompanying
// file COPYING or http://www.opensource.org/licenses/mit-license.php.

#include <chainparams.h>
#include <coins.h>
#include <esperanza/adminparams.h>
#include <esperanza/checks.h>
#include <esperanza/finalizationstate.h>
#include <finalization/vote_recorder.h>
#include <script/interpreter.h>
#include <script/standard.h>
#include <txmempool.h>
#include <util.h>
#include <validation.h>

namespace esperanza {

bool ContextualCheckFinalizerCommit(const CTransaction &tx,
CValidationState &err_state,
const Consensus::Params &params,
const FinalizationState &fin_state) {
const FinalizationState &fin_state,
const CCoinsView &view) {
switch (tx.GetType()) {
case +TxType::REGULAR:
case +TxType::COINBASE:
assert(not("Shouldn't be called on non-finalization transaction"));
case +TxType::DEPOSIT:
return ContextualCheckDepositTx(tx, err_state, fin_state);
case +TxType::VOTE:
return ContextualCheckVoteTx(tx, err_state, params, fin_state);
return ContextualCheckVoteTx(tx, err_state, fin_state, view);
case +TxType::LOGOUT:
return ContextualCheckLogoutTx(tx, err_state, params, fin_state);
return ContextualCheckLogoutTx(tx, err_state, fin_state, view);
case +TxType::SLASH:
return ContextualCheckSlashTx(tx, err_state, params, fin_state);
return ContextualCheckSlashTx(tx, err_state, fin_state);
case +TxType::WITHDRAW:
return ContextualCheckWithdrawTx(tx, err_state, params, fin_state);
return ContextualCheckWithdrawTx(tx, err_state, fin_state, view);
case +TxType::ADMIN:
return ContextualCheckAdminTx(tx, err_state, fin_state);
}
Expand Down Expand Up @@ -67,6 +68,37 @@ inline bool CheckValidatorAddress(const CTransaction &tx, uint160 *addr_out) {
}
return ExtractValidatorAddress(tx, *addr_out);
}

bool FindPrevOutData(const COutPoint &prevout,
const CCoinsView &view,
TxType *tx_type_out,
CScript *script_out) {
{
LOCK(mempool.cs);
CTransactionRef prev_tx = mempool.get(prevout.hash);
if (prev_tx != nullptr) {
if (tx_type_out != nullptr) {
*tx_type_out = prev_tx->GetType();
}
if (script_out != nullptr) {
*script_out = prev_tx->vout[prevout.n].scriptPubKey;
}
return true;
}
}
Coin prev_coin;
if (view.GetCoin(prevout, prev_coin)) {
if (tx_type_out != nullptr) {
*tx_type_out = prev_coin.tx_type;
}
if (script_out != nullptr) {
*script_out = prev_coin.out.scriptPubKey;
}
return true;
}
return false;
}

} // namespace

bool CheckDepositTx(const CTransaction &tx, CValidationState &err_state,
Expand Down Expand Up @@ -158,8 +190,8 @@ bool CheckLogoutTx(const CTransaction &tx, CValidationState &err_state,
}

bool ContextualCheckLogoutTx(const CTransaction &tx, CValidationState &err_state,
const Consensus::Params &consensus_params,
const FinalizationState &fin_state) {
const FinalizationState &fin_state,
const CCoinsView &view) {

uint160 validator_address = uint160();
if (!CheckLogoutTx(tx, err_state, &validator_address)) {
Expand All @@ -175,23 +207,20 @@ bool ContextualCheckLogoutTx(const CTransaction &tx, CValidationState &err_state
// check (potentially goes to disk) and there is a good chance that if the
// vote is not valid (i.e. outdated) then the function will return before
// reaching this point.
CTransactionRef prev_tx;
uint256 block_hash;
TxType prev_tx_type = TxType::REGULAR;
Copy link
Member

@Gnappuraz Gnappuraz Apr 2, 2019 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This might result quite confusing. Since we are passing a pointer as an out parameter I would init as nullptr and then also change the FindPrevOutData signature to make it more evident (prev_tx_type_out). I would rather init this to TxType::REGULAR inside FindPrevOutData.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more. 

bool FindPrevOutData(const COutPoint &prevout,
                     const CCoinsView &view,
                     TxType *tx_type_out,
                     CScript *script_out) {

tx_type_out refers exactly to prevout outpoint. I think appending prev_ prefix would make it much more ambiguous.

Regarding nullptr initialization, pointers doesn't work in this way. I cannot initialize TxType with nullptr as it is not a pointer. I cannot pass nullptr to the FindPrevOutData because otherwise I cannot assign its value. We can pass pointer-to-pointer to the function (TxType **prev_tx_type) but then we will face with a problem of returning pointer to temporary (stack) variable — the Coin.

I suppose I could not understand your suggestion, feel free to elaborate a bit, then I will fix them in follow-up PR.

Copy link
Member

@Gnappuraz Gnappuraz Apr 2, 2019 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would have created a TxType* and initialized to nullptr and passed it to the function.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Then it would not work unfortunately. We cannot write to the pointer, like *tx_type = TxType::REGULAR when tx_type is nullptr. We could use pointer-to-pointer (or reference-to-pointer), but I described why we can't.

CScript prev_out_script;

// We have to look into the tx database to find the prev tx, hence the
// use of fAllowSlow = true
if (!GetTransaction(tx.vin[0].prevout.hash, prev_tx, consensus_params,
block_hash, true)) {
if (!FindPrevOutData(tx.vin[0].prevout, view, &prev_tx_type, &prev_out_script)) {
return err_state.DoS(10, false, REJECT_INVALID,
"bad-logout-no-prev-tx-found");
}

if (!prev_tx->IsDeposit() && !prev_tx->IsVote()) {
if (prev_tx_type != +TxType::DEPOSIT && prev_tx_type != +TxType::VOTE) {
return err_state.DoS(10, false, REJECT_INVALID,
"bad-logout-prev-not-deposit-or-vote");
}

if (prev_tx->vout[0].scriptPubKey != tx.vout[0].scriptPubKey) {
if (prev_out_script != tx.vout[0].scriptPubKey) {
return err_state.DoS(10, false, REJECT_INVALID,
"bad-logout-not-same-payvoteslash-script");
}
Expand Down Expand Up @@ -229,29 +258,30 @@ bool CheckWithdrawTx(const CTransaction &tx, CValidationState &err_state,
}

bool ContextualCheckWithdrawTx(const CTransaction &tx, CValidationState &err_state,
const Consensus::Params &consensus_params,
const FinalizationState &fin_state) {
const FinalizationState &fin_state,
const CCoinsView &view) {

uint160 validator_address = uint160();
if (!CheckWithdrawTx(tx, err_state, &validator_address)) {
return false;
}

CTransactionRef prev_tx;
uint256 block_hash;

// We have to look into the tx database to find the prev tx, hence the
// use of fAllowSlow = true
if (!GetTransaction(tx.vin[0].prevout.hash, prev_tx, consensus_params,
block_hash, true)) {
TxType prev_tx_type = TxType::REGULAR;
CScript prev_out_script;

if (!FindPrevOutData(tx.vin[0].prevout, view, &prev_tx_type, &prev_out_script)) {
return err_state.DoS(10, false, REJECT_INVALID,
"bad-withdraw-no-prev-tx-found");
}

if (prev_tx_type != +TxType::LOGOUT && prev_tx_type != +TxType::VOTE) {
return err_state.DoS(10, false, REJECT_INVALID,
"bad-withdraw-prev-not-logout-or-vote");
}

std::vector<std::vector<unsigned char>> prev_solutions;
txnouttype prev_type_ret;
if (!Solver(prev_tx->vout[0].scriptPubKey, prev_type_ret, prev_solutions)) {
if (!Solver(prev_out_script, prev_type_ret, prev_solutions)) {
return err_state.DoS(10, false, REJECT_INVALID,
"bad-logout-script-not-solvable");
}
Expand All @@ -263,11 +293,6 @@ bool ContextualCheckWithdrawTx(const CTransaction &tx, CValidationState &err_sta
"bad-withdraw-invalid-state");
}

if (!prev_tx->IsLogout() && !prev_tx->IsVote()) {
return err_state.DoS(10, false, REJECT_INVALID,
"bad-withdraw-prev-not-logout-or-vote");
}

return true;
}

Expand Down Expand Up @@ -311,8 +336,8 @@ bool CheckVoteTx(const CTransaction &tx, CValidationState &err_state,
}

bool ContextualCheckVoteTx(const CTransaction &tx, CValidationState &err_state,
const Consensus::Params &consensus_params,
const FinalizationState &fin_state) {
const FinalizationState &fin_state,
const CCoinsView &view) {

Vote vote;
std::vector<unsigned char> vote_sig;
Expand All @@ -332,21 +357,21 @@ bool ContextualCheckVoteTx(const CTransaction &tx, CValidationState &err_state,
// check (potentially goes to disk) and there is a good chance that if the
// vote is not valid (i.e. outdated) then the function will return before
// reaching this point.
CTransactionRef prev_tx;
uint256 block_hash;
// We have to look into the tx database to find the prev tx, hence the
// use of fAllowSlow = true
if (!GetTransaction(tx.vin[0].prevout.hash, prev_tx, consensus_params,
block_hash, true)) {
return err_state.DoS(10, false, REJECT_INVALID, "bad-vote-no-prev-tx-found");

TxType prev_tx_type = TxType::REGULAR;
CScript prev_out_script;

if (!FindPrevOutData(tx.vin[0].prevout, view, &prev_tx_type, &prev_out_script)) {
return err_state.DoS(10, false, REJECT_INVALID,
"bad-vote-no-prev-tx-found");
}

if (!prev_tx->IsDeposit() && !prev_tx->IsVote() && !prev_tx->IsLogout()) {
if (prev_tx_type != +TxType::DEPOSIT && prev_tx_type != +TxType::VOTE && prev_tx_type != +TxType::LOGOUT) {
return err_state.DoS(10, false, REJECT_INVALID,
"bad-vote-prev-not-deposit-vote-or-logout");
}

if (prev_tx->vout[0].scriptPubKey != tx.vout[0].scriptPubKey) {
if (prev_out_script != tx.vout[0].scriptPubKey) {
return err_state.DoS(10, false, REJECT_INVALID,
"bad-vote-not-same-payvoteslash-script");
}
Expand Down Expand Up @@ -383,7 +408,6 @@ bool CheckSlashTx(const CTransaction &tx, CValidationState &err_state,
}

bool ContextualCheckSlashTx(const CTransaction &tx, CValidationState &err_state,
const Consensus::Params &consensus_params,
const FinalizationState &fin_state) {

Vote vote1;
Expand Down
19 changes: 10 additions & 9 deletions src/esperanza/checks.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,8 @@
#include <primitives/block.h>
#include <primitives/transaction.h>

class CCoinsView;

namespace esperanza {

class FinalizationState;
Expand All @@ -36,8 +38,8 @@ bool CheckFinalizerCommit(const CTransaction &tx, CValidationState &err_state);
//! \brief Generalized finalization transaction contextual check. Asserts on non-finalization transactions.
bool ContextualCheckFinalizerCommit(const CTransaction &tx,
CValidationState &err_state,
const Consensus::Params &params,
const FinalizationState &fin_state);
const FinalizationState &fin_state,
const CCoinsView &view);

bool CheckDepositTx(const CTransaction &tx, CValidationState &err_state,
uint160 *validator_address_out);
Expand All @@ -47,26 +49,25 @@ bool ContextualCheckDepositTx(const CTransaction &tx, CValidationState &err_stat
bool CheckVoteTx(const CTransaction &tx, CValidationState &err_state,
Vote *vote_out, std::vector<unsigned char> *vote_sig_out);
bool ContextualCheckVoteTx(const CTransaction &tx, CValidationState &err_state,
const Consensus::Params &consensus_params,
const FinalizationState &fin_state);
const FinalizationState &fin_state,
const CCoinsView &view);

bool CheckSlashTx(const CTransaction &tx, CValidationState &err_state,
Vote *vote1_out, Vote *vote2_out);
bool ContextualCheckSlashTx(const CTransaction &tx, CValidationState &err_state,
const Consensus::Params &consensus_params,
const FinalizationState &fin_state);

bool CheckLogoutTx(const CTransaction &tx, CValidationState &err_state,
uint160 *out_validator_address);
bool ContextualCheckLogoutTx(const CTransaction &tx, CValidationState &err_state,
const Consensus::Params &consensus_params,
const FinalizationState &fin_state);
const FinalizationState &fin_state,
const CCoinsView &view);

bool CheckWithdrawTx(const CTransaction &tx, CValidationState &err_state,
uint160 *out_validator_address);
bool ContextualCheckWithdrawTx(const CTransaction &tx, CValidationState &err_state,
const Consensus::Params &consensus_arams,
const FinalizationState &fin_state);
const FinalizationState &fin_state,
const CCoinsView &view);

bool CheckAdminTx(const CTransaction &tx, CValidationState &err_state,
std::vector<CPubKey> *keys_out);
Expand Down
Loading