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.

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

Update has_inputs_and_outputs() for new consensus rules #2398

Merged
merged 3 commits into from
Jun 28, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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: 7 additions & 0 deletions zebra-chain/src/transaction.rs
Original file line number Diff line number Diff line change
Expand Up @@ -483,4 +483,11 @@ impl Transaction {
.map(orchard::ShieldedData::nullifiers)
.flatten()
}

/// Access the [`orchard::Flags`] in this transaction, if there is any,
/// regardless of version.
pub fn orchard_flags(&self) -> Option<orchard::shielded_data::Flags> {
self.orchard_shielded_data()
.map(|orchard_shielded_data| orchard_shielded_data.flags)
}
}
26 changes: 20 additions & 6 deletions zebra-consensus/src/transaction/check.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,12 +13,14 @@ use crate::error::TransactionError;
/// Checks that the transaction has inputs and outputs.
///
/// For `Transaction::V4`:
/// * at least one of `tx_in_count`, `nSpendsSapling`, and `nJoinSplit` MUST be non-zero.
/// * at least one of `tx_out_count`, `nOutputsSapling`, and `nJoinSplit` MUST be non-zero.
/// * At least one of `tx_in_count`, `nSpendsSapling`, and `nJoinSplit` MUST be non-zero.
/// * At least one of `tx_out_count`, `nOutputsSapling`, and `nJoinSplit` MUST be non-zero.
///
/// For `Transaction::V5`:
/// * at least one of `tx_in_count`, `nSpendsSapling`, and `nActionsOrchard` MUST be non-zero.
/// * at least one of `tx_out_count`, `nOutputsSapling`, and `nActionsOrchard` MUST be non-zero.
/// * This condition must hold: `tx_in_count` > 0 or `nSpendsSapling` > 0 or
/// (`nActionsOrchard` > 0 and `enableSpendsOrchard` = 1)
/// * This condition must hold: `tx_out_count` > 0 or `nOutputsSapling` > 0 or
/// (`nActionsOrchard` > 0 and `enableOutputsOrchard` = 1)
///
/// This check counts both `Coinbase` and `PrevOut` transparent inputs.
///
Expand All @@ -30,10 +32,22 @@ pub fn has_inputs_and_outputs(tx: &Transaction) -> Result<(), TransactionError>
let n_spends_sapling = tx.sapling_spends_per_anchor().count();
let n_outputs_sapling = tx.sapling_outputs().count();
let n_actions_orchard = tx.orchard_actions().count();
let flags_orchard = tx.orchard_flags().unwrap_or_else(Flags::empty);

if tx_in_count + n_spends_sapling + n_joinsplit + n_actions_orchard == 0 {
// TODO: Improve the code to express the spec rules better #2410.
if tx_in_count
+ n_spends_sapling
+ n_joinsplit
+ (n_actions_orchard > 0 && flags_orchard.contains(Flags::ENABLE_SPENDS)) as usize
oxarbitrage marked this conversation as resolved.
Show resolved Hide resolved
== 0
{
Err(TransactionError::NoInputs)
} else if tx_out_count + n_outputs_sapling + n_joinsplit + n_actions_orchard == 0 {
} else if tx_out_count
+ n_outputs_sapling
+ n_joinsplit
+ (n_actions_orchard > 0 && flags_orchard.contains(Flags::ENABLE_OUTPUTS)) as usize
oxarbitrage marked this conversation as resolved.
Show resolved Hide resolved
== 0
{
Err(TransactionError::NoOutputs)
} else {
Ok(())
Expand Down
31 changes: 29 additions & 2 deletions zebra-consensus/src/transaction/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -86,8 +86,35 @@ fn fake_v5_transaction_with_orchard_actions_has_inputs_and_outputs() {
// guaranteed structurally by `orchard::ShieldedData`)
insert_fake_orchard_shielded_data(&mut transaction);

// If a transaction has at least one Orchard shielded action, it should be considered to have
// inputs and/or outputs
// The check will fail if the transaction has no flags
assert_eq!(
check::has_inputs_and_outputs(&transaction),
Err(TransactionError::NoInputs)
);

// If we add ENABLE_SPENDS flag it will pass the inputs check but fails with the outputs
// TODO: Avoid new calls to `insert_fake_orchard_shielded_data` for each check #2409.
let shielded_data = insert_fake_orchard_shielded_data(&mut transaction);
oxarbitrage marked this conversation as resolved.
Show resolved Hide resolved
shielded_data.flags = orchard::Flags::ENABLE_SPENDS;

assert_eq!(
check::has_inputs_and_outputs(&transaction),
Err(TransactionError::NoOutputs)
);

// If we add ENABLE_OUTPUTS flag it will pass the outputs check but fails with the inputs
let shielded_data = insert_fake_orchard_shielded_data(&mut transaction);
shielded_data.flags = orchard::Flags::ENABLE_OUTPUTS;

assert_eq!(
check::has_inputs_and_outputs(&transaction),
Err(TransactionError::NoInputs)
);

// Finally make it valid by adding both required flags
let shielded_data = insert_fake_orchard_shielded_data(&mut transaction);
shielded_data.flags = orchard::Flags::ENABLE_SPENDS | orchard::Flags::ENABLE_OUTPUTS;

assert!(check::has_inputs_and_outputs(&transaction).is_ok());
}

Expand Down