Skip to content

Commit

Permalink
Use proper meta input prevout (#1099)
Browse files Browse the repository at this point in the history
A coinbase transaction in both bitcoin as well as in unit-e includes a meta input which does not spend anything and is skipped in validation. It's scriptSig contains block height and snapshot hash.

The prevout of that input should be `0x00000` (which it is) and `0xFFFF` as index (currently uses 0). This is not correct and changed in this pull request. By using the default constructor of `COutPoint`, which in turn invokes `SetNull` (which sets the afore mentioned values), the right values are being filled in + delegated to the definition of `SetNull (as it should).

The problem otherwise: Theoretically it is allowed for a transaction to have the hash `0x00...00` (although this will lead to other problems in the wallet...), but it is super unlikely that any transaction would ever have 2^16 number of inputs. That both happen is astronomically super black hole merger unlikely.

Signed-off-by: Julian Fleischer <julian@thirdhash.com>
  • Loading branch information
scravy committed May 17, 2019
1 parent 1d0f478 commit 046062b
Show file tree
Hide file tree
Showing 6 changed files with 143 additions and 117 deletions.
2 changes: 1 addition & 1 deletion src/proposer/block_builder.cpp
Expand Up @@ -114,7 +114,7 @@ class BlockBuilderImpl : public BlockBuilder {
{
CScript script_sig = CScript() << CScriptNum::serialize(eligible_coin.target_height)
<< ToByteVector(snapshot_hash);
tx.vin.emplace_back(uint256(), 0, script_sig);
tx.vin.emplace_back(COutPoint(), script_sig);
}

// add stake
Expand Down
4 changes: 4 additions & 0 deletions src/staking/block_validator.cpp
Expand Up @@ -81,6 +81,10 @@ class BlockValidatorImpl : public AbstractBlockValidator {

const CScript &script_sig = in.scriptSig;

if (!in.prevout.IsNull()) {
return BlockValidationResult(Error::INVALID_META_INPUT_PREVOUT);
}

bool check;
opcodetype op;
std::vector<uint8_t> buf;
Expand Down
4 changes: 4 additions & 0 deletions src/staking/validation_error.cpp
Expand Up @@ -91,6 +91,10 @@ const ValidationError &GetValidationErrorFor(const staking::BlockValidationError
static ValidationError err("bad-blk-public-key");
return err;
}
case staking::BlockValidationError::INVALID_META_INPUT_PREVOUT: {
static ValidationError err("bad-cb-meta-input-prevout");
return err;
}
case staking::BlockValidationError::MISMATCHING_HEIGHT: {
static ValidationError err("bad-cb-height");
return err;
Expand Down
1 change: 1 addition & 0 deletions src/staking/validation_error.h
Expand Up @@ -30,6 +30,7 @@ BETTER_ENUM(
INVALID_BLOCK_HEIGHT,
INVALID_BLOCK_TIME,
INVALID_BLOCK_PUBLIC_KEY,
INVALID_META_INPUT_PREVOUT,
MERKLE_ROOT_MISMATCH,
MERKLE_ROOT_DUPLICATE_TRANSACTIONS,
MISMATCHING_HEIGHT,
Expand Down
23 changes: 20 additions & 3 deletions src/test/staking/block_validator_tests.cpp
Expand Up @@ -45,7 +45,7 @@ CTransactionRef MakeCoinbaseTransaction(const KeyFixture &key_fixture = MakeKeyF
// meta input: block height, snapshot hash, terminator
CScript script_sig = CScript() << CScriptNum::serialize(4711)
<< ToByteVector(uint256S("689dae90b6913ff34a64750dd537177afa58b3d012803a10793d74f1ebb88da9"));
tx.vin.emplace_back(uint256(), 0, script_sig);
tx.vin.emplace_back(COutPoint(), script_sig);
// stake
tx.vin.emplace_back(uint256(), 1);
tx.vin[1].scriptWitness.stack.emplace_back(); // signature, not checked
Expand Down Expand Up @@ -108,7 +108,7 @@ BOOST_AUTO_TEST_CASE(check_empty_block) {
const staking::BlockValidationResult validation_result = block_validator->CheckBlock(block, nullptr);

BOOST_CHECK(!validation_result);
BOOST_CHECK(validation_result.Is(Error::NO_TRANSACTIONS));
BOOST_CHECK_MESSAGE(validation_result.Is(Error::NO_TRANSACTIONS), validation_result.GetRejectionMessage());
}

BOOST_AUTO_TEST_CASE(check_first_transaction_not_a_coinbase_transaction) {
Expand All @@ -124,7 +124,24 @@ BOOST_AUTO_TEST_CASE(check_first_transaction_not_a_coinbase_transaction) {
const staking::BlockValidationResult validation_result = block_validator->CheckBlock(block, nullptr);

BOOST_CHECK(!validation_result);
BOOST_CHECK(validation_result.Is(Error::FIRST_TRANSACTION_NOT_A_COINBASE_TRANSACTION));
BOOST_CHECK_MESSAGE(validation_result.Is(Error::FIRST_TRANSACTION_NOT_A_COINBASE_TRANSACTION), validation_result.GetRejectionMessage());
}

BOOST_AUTO_TEST_CASE(check_coinbase_meta_input_malformed) {
const auto block_validator = staking::BlockValidator::New(b.get());

CBlock block;
{
CMutableTransaction tx;
tx.SetType(TxType::COINBASE);
tx.vin.emplace_back(uint256::zero, 0);
block.vtx.push_back(MakeTransactionRef(CTransaction(tx)));
}

const auto validation_result = block_validator->CheckBlock(block, nullptr);

BOOST_CHECK(!validation_result);
BOOST_CHECK_MESSAGE(validation_result.Is(Error::INVALID_META_INPUT_PREVOUT), validation_result.GetRejectionMessage());
}

BOOST_AUTO_TEST_CASE(check_coinbase_other_than_first) {
Expand Down

0 comments on commit 046062b

Please sign in to comment.