Skip to content

Refactor: Validation: Make parts of IsValidPeginWitness templated #370

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

Merged
instagibbs merged 2 commits into from
Jun 26, 2018
Merged

Refactor: Validation: Make parts of IsValidPeginWitness templated #370

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
2be9cae
Refactor: Validation: Make parts of IsValidPeginWitness templated
jtimon Jun 12, 2018
a20986b
2WP: Refactor out GetAmountFromParentChainPegin
jtimon Jun 12, 2018
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
126 changes: 83 additions & 43 deletions src/validation.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2359,6 +2359,78 @@ CScript calculate_contract(const CScript& federationRedeemScript, const CScript&
return scriptDestination;
}

bool GetAmountFromParentChainPegin(CAmount& amount, const Sidechain::Bitcoin::CTransaction& txBTC, unsigned int nOut)
{
amount = txBTC.vout[nOut].nValue;
return true;
Copy link
Contributor

@instagibbs instagibbs Jun 13, 2018

Choose a reason for hiding this comment

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

just return the value? It seem this can't fail.

Copy link
Contributor Author

@jtimon jtimon Jun 14, 2018

Choose a reason for hiding this comment

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

The point is to make the interface like 23af084#diff-24efdb00bfbe56b140fb006b562cc70bR2369 which can fail.

}

template<typename T>
Copy link
Contributor

@jhfrontz jhfrontz Jun 18, 2018

Choose a reason for hiding this comment

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

I'm curious about the use of a template here (vs., say, an inline static function). This isn't a header file and there only appears to be one use of this template/function -- is this a project coding convention or is there some performance/maintenance gain?

Copy link
Contributor Author

@jtimon jtimon Jun 18, 2018

Choose a reason for hiding this comment

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

No, it is just in preparation for #362

Copy link

@mword mword Jun 22, 2018

Choose a reason for hiding this comment

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

I'm not the biggest fan of adding code for future use into a pull request, it could sit there making no sense for a long time.

Copy link
Contributor Author

@jtimon jtimon Jun 26, 2018

Choose a reason for hiding this comment

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

Yeah, I did it only because @instagibbs asked. Perhaps I should have done 2 commits but not 2 PRs

static bool GetBlockAndTxFromMerkleBlock(uint256& block_hash, uint256& tx_hash, T& merkle_block, const std::vector<unsigned char>& merkle_block_raw)
{
try {
std::vector<uint256> tx_hashes;
std::vector<unsigned int> tx_indices;
CDataStream merkle_block_stream(merkle_block_raw, SER_NETWORK, PROTOCOL_VERSION);
merkle_block_stream >> merkle_block;
block_hash = merkle_block.header.GetHash();

if (!merkle_block_stream.empty()) {
return false;
}
if (merkle_block.txn.ExtractMatches(tx_hashes, tx_indices) != merkle_block.header.hashMerkleRoot || tx_hashes.size() != 1) {
return false;
}
tx_hash = tx_hashes[0];
} catch (std::exception& e) {
// Invalid encoding of merkle block
return false;
}
return true;
}

template<typename T>
static bool CheckPeginTx(const std::vector<unsigned char>& tx_data, T& pegtx, const COutPoint& prevout, const CAmount claim_amount, const CScript& claim_script)
{
try {
CDataStream pegtx_stream(tx_data, SER_NETWORK, PROTOCOL_VERSION);
pegtx_stream >> pegtx;
if (!pegtx_stream.empty()) {
return false;
}
} catch (std::exception& e) {
// Invalid encoding of transaction
return false;
}

// Check that transaction matches txid
if (pegtx->GetHash() != prevout.hash) {
return false;
}

if (prevout.n >= pegtx->vout.size()) {
return false;
}
CAmount amount = 0;
if (!GetAmountFromParentChainPegin(amount, *pegtx, prevout.n)) {
return false;
}
// Check the transaction nout/value matches
if (claim_amount != amount) {
return false;
}

// Check that the witness program matches the p2ch on the p2sh-p2wsh transaction output
CScript tweaked_fedpegscript = calculate_contract(Params().GetConsensus().fedpegScript, claim_script);
CScript witness_output(GetScriptForWitness(tweaked_fedpegscript));
CScript expected_script(CScript() << OP_HASH160 << ToByteVector(CScriptID(witness_output)) << OP_EQUAL);
if (pegtx->vout[prevout.n].scriptPubKey != expected_script) {
return false;
}

return true;
}

bool IsValidPeginWitness(const CScriptWitness& pegin_witness, const COutPoint& prevout, bool check_depth) {

// Format on stack is as follows:
Expand Down Expand Up @@ -2409,45 +2481,26 @@ bool IsValidPeginWitness(const CScriptWitness& pegin_witness, const COutPoint& p
return false;
}

// Get serialized transaction
Sidechain::Bitcoin::CTransactionRef pegtx;
try {
CDataStream pegtx_stream(stack[4], SER_NETWORK, PROTOCOL_VERSION);
pegtx_stream >> pegtx;
if (!pegtx_stream.empty()) {
return false;
}
} catch (std::exception& e) {
// Invalid encoding of transaction
return false;
}
uint256 block_hash;
uint256 tx_hash;

// Get txout proof
Sidechain::Bitcoin::CMerkleBlock merkle_block;
std::vector<uint256> txHashes;
std::vector<unsigned int> txIndices;

try {
CDataStream merkleBlockStream(stack[5], SER_NETWORK, PROTOCOL_VERSION);
merkleBlockStream >> merkle_block;
if (!merkleBlockStream.empty() || !CheckBitcoinProof(merkle_block.header.GetHash(), merkle_block.header.nBits)) {
return false;
}
if (merkle_block.txn.ExtractMatches(txHashes, txIndices) != merkle_block.header.hashMerkleRoot || txHashes.size() != 1) {
return false;
}
} catch (std::exception& e) {
// Invalid encoding of merkle block
if (!GetBlockAndTxFromMerkleBlock(block_hash, tx_hash, merkle_block, stack[5])) {
return false;
}
if (!CheckBitcoinProof(block_hash, merkle_block.header.nBits)) {
return false;
}

// Check that transaction matches txid
if (pegtx->GetHash() != prevout.hash) {
// Get serialized transaction
Sidechain::Bitcoin::CTransactionRef pegtx;
if (!CheckPeginTx(stack[4], pegtx, prevout, value, claim_script)) {
return false;
}

// Check that the merkle proof corresponds to the txid
if (prevout.hash != txHashes[0]) {
if (prevout.hash != tx_hash) {
return false;
}

Expand All @@ -2461,22 +2514,9 @@ bool IsValidPeginWitness(const CScriptWitness& pegin_witness, const COutPoint& p
return false;
}

// Check the transaction nout/value matches
if (prevout.n >= pegtx->vout.size() || value != pegtx->vout[prevout.n].nValue) {
return false;
}

// Check that the witness program matches the p2ch on the p2sh-p2wsh transaction output
CScript tweaked_fedpegscript = calculate_contract(Params().GetConsensus().fedpegScript, claim_script);
CScript witness_output(GetScriptForWitness(tweaked_fedpegscript));
CScript expected_script(CScript() << OP_HASH160 << ToByteVector(CScriptID(witness_output)) << OP_EQUAL);
if (pegtx->vout[prevout.n].scriptPubKey != expected_script) {
return false;
}

// Finally, validate peg-in via rpc call
if (check_depth && GetBoolArg("-validatepegin", DEFAULT_VALIDATE_PEGIN)) {
return IsConfirmedBitcoinBlock(merkle_block.header.GetHash(), Params().GetConsensus().pegin_min_depth);
return IsConfirmedBitcoinBlock(block_hash, Params().GetConsensus().pegin_min_depth);
}
return true;
}
Expand Down
5 changes: 5 additions & 0 deletions src/validation.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,10 @@
#include <boost/unordered_map.hpp>
#include <boost/filesystem/path.hpp>

namespace Sidechain { namespace Bitcoin {
class CTransaction;
}}

class CBlockIndex;
class CBlockTreeDB;
class CBloomFilter;
Expand Down Expand Up @@ -262,6 +266,7 @@ void UnloadBlockIndex();
void ThreadScriptCheck();
/** Check if bitcoind connection via RPC is correctly working*/
bool BitcoindRPCCheck(bool init);
bool GetAmountFromParentChainPegin(CAmount& amount, const Sidechain::Bitcoin::CTransaction& txBTC, unsigned int nOut);
/** Checks pegin witness for validity */
bool IsValidPeginWitness(const CScriptWitness& pegin_witness, const COutPoint& prevout, bool check_depth = true);
/** Extracts an output from pegin witness for evaluation as a normal output */
Expand Down