Skip to content

Commit

Permalink
Relative locktime bug fix (#289)
Browse files Browse the repository at this point in the history
  • Loading branch information
@maciejka
maciejka authored Nov 4, 2024
1 parent 02a1304 commit ca0ae49
Show file tree
Hide file tree
Showing 4 changed files with 41 additions and 30 deletions.
2 changes: 1 addition & 1 deletion packages/consensus/src/types/chain_state.cairo
View file
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ pub impl BlockValidatorImpl of BlockValidator {
TransactionData::MerkleRoot(root) => root,
TransactionData::Transactions(txs) => {
let (total_fees, txid_root, wtxid_root) = compute_and_validate_tx_data(
txs, block_height, median_time_past, ref utxo_set
txs, block_height, block.header.time, median_time_past, ref utxo_set
)?;
validate_coinbase(txs[0], total_fees, block_height, wtxid_root)?;
if execute_script {
Expand Down
10 changes: 8 additions & 2 deletions packages/consensus/src/validation/block.cairo
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,11 @@ pub fn validate_block_weight(weight: usize) -> Result<(), ByteArray> {
/// - wTXID commitment (only for blocks after Segwit upgrade, otherwise return zero hash)
/// - Block weight
pub fn compute_and_validate_tx_data(
txs: Span<Transaction>, block_height: u32, median_time_past: u32, ref utxo_set: UtxoSet
txs: Span<Transaction>,
block_height: u32,
block_time: u32,
median_time_past: u32,
ref utxo_set: UtxoSet
) -> Result<(u64, Digest, Digest), ByteArray> {
let mut txids: Array<Digest> = array![];
let mut wtxids: Array<Digest> = array![];
Expand Down Expand Up @@ -92,7 +96,9 @@ pub fn compute_and_validate_tx_data(
is_coinbase = false;
} else {
let fee =
match validate_transaction(tx, block_height, median_time_past, txid, ref utxo_set) {
match validate_transaction(
tx, block_height, block_time, median_time_past, txid, ref utxo_set
) {
Result::Ok(fee) => fee,
Result::Err(err) => {
inner_result = Result::Err(err);
Expand Down
4 changes: 2 additions & 2 deletions packages/consensus/src/validation/locktime.cairo
View file
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ pub fn validate_relative_locktime(
if absolute_lock_time > median_time_past {
return Result::Err(
format!(
"Relative time-based lock time is not respected: current MTP {}, outpoint MTP {}, lock time {} seconds",
"Relative time-based lock time is not respected: current MTP: {}, outpoint MTP: {}, lock time: {} seconds",
median_time_past,
*input.previous_output.median_time_past,
lock_time
Expand All @@ -94,7 +94,7 @@ pub fn validate_relative_locktime(
if absolute_lock_time > block_height {
return Result::Err(
format!(
"Relative block-based lock time is not respected: current height {}, outpoint height {}, lock time {} blocks",
"Relative block-based lock time is not respected: current height: {}, outpoint height: {}, lock time: {} blocks",
block_height,
*input.previous_output.block_height,
value
Expand Down
55 changes: 30 additions & 25 deletions packages/consensus/src/validation/transaction.cairo
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,12 @@ const MAX_SCRIPT_SIZE: u32 = 10000;
///
/// This does not include script checks and outpoint inclusion verification.
pub fn validate_transaction(
tx: @Transaction, block_height: u32, median_time_past: u32, txid: Digest, ref utxo_set: UtxoSet
tx: @Transaction,
block_height: u32,
block_time: u32,
median_time_past: u32,
txid: Digest,
ref utxo_set: UtxoSet
) -> Result<u64, ByteArray> {
if (*tx.inputs).is_empty() {
return Result::Err("transaction inputs are empty");
Expand Down Expand Up @@ -48,7 +53,7 @@ pub fn validate_transaction(
}
}

if !is_input_final(*input.sequence) {
if *tx.version >= 2 && !is_input_final(*input.sequence) {
inner_result = validate_relative_locktime(input, block_height, median_time_past);
if inner_result.is_err() {
break;
Expand All @@ -65,7 +70,7 @@ pub fn validate_transaction(

if !is_tx_final {
// If at least one input is not final
validate_absolute_locktime(*tx.lock_time, block_height, median_time_past)?;
validate_absolute_locktime(*tx.lock_time, block_height, block_time)?;
}

// Validate and process transaction outputs
Expand Down Expand Up @@ -180,11 +185,11 @@ mod tests {
let txid = double_sha256_byte_array(tx_bytes_legacy);
let mut utxo_set: UtxoSet = Default::default();

assert!(validate_transaction(@tx, 0, 0, txid, ref utxo_set).is_err());
assert!(validate_transaction(@tx, 0, 0, 0, txid, ref utxo_set).is_err());

utxo_set = Default::default();

let fee = validate_transaction(@tx, 101, 0, txid, ref utxo_set).unwrap();
let fee = validate_transaction(@tx, 101, 0, 0, txid, ref utxo_set).unwrap();
assert_eq!(fee, 10);
}

Expand All @@ -209,7 +214,7 @@ mod tests {
let txid = double_sha256_byte_array(tx_bytes_legacy);
let mut utxo_set: UtxoSet = Default::default();

let result = validate_transaction(@tx, 0, 0, txid, ref utxo_set);
let result = validate_transaction(@tx, 0, 0, 0, txid, ref utxo_set);
assert!(result.is_err());
// assert_eq!(result.unwrap_err(), "transaction inputs are empty");
}
Expand Down Expand Up @@ -245,15 +250,15 @@ mod tests {
let txid = double_sha256_byte_array(tx_bytes_legacy);
let mut utxo_set: UtxoSet = Default::default();

let result = validate_transaction(@tx, 0, 0, txid, ref utxo_set);
let result = validate_transaction(@tx, 0, 0, 0, txid, ref utxo_set);
assert!(result.is_err());
// assert_eq!(result.unwrap_err(), "transaction outputs are empty");
}

#[test]
fn test_absolute_locktime_block_height() {
let tx = Transaction {
version: 1,
version: 2,
is_segwit: false,
inputs: array![
TxIn {
Expand Down Expand Up @@ -289,7 +294,7 @@ mod tests {
let mut utxo_set: UtxoSet = Default::default();

// Transaction should be invalid when current block height is less than locktime
let result = validate_transaction(@tx, 500000, 0, txid, ref utxo_set);
let result = validate_transaction(@tx, 500000, 0, 0, txid, ref utxo_set);
assert!(result.is_err());
assert_eq!(
result.unwrap_err().into(),
Expand All @@ -300,14 +305,14 @@ mod tests {

// Transaction should be valid when current block height is equal to or greater than
// locktime
let result = validate_transaction(@tx, 500001, 0, txid, ref utxo_set);
let result = validate_transaction(@tx, 500001, 0, 0, txid, ref utxo_set);
assert!(result.is_ok());
}

#[test]
fn test_absolute_locktime_block_time() {
let tx = Transaction {
version: 1,
version: 2,
is_segwit: false,
inputs: array![
TxIn {
Expand Down Expand Up @@ -343,7 +348,7 @@ mod tests {
let mut utxo_set: UtxoSet = Default::default();

// Transaction should be invalid when current block time is not greater than locktime
let result = validate_transaction(@tx, 0, 1600000000, txid, ref utxo_set);
let result = validate_transaction(@tx, 0, 1600000000, 1600000000, txid, ref utxo_set);
assert!(result.is_err());
assert_eq!(
result.unwrap_err().into(),
Expand All @@ -353,7 +358,7 @@ mod tests {
utxo_set = Default::default();

// Transaction should be valid when current block time is equal to or greater than locktime
let result = validate_transaction(@tx, 0, 1600000001, txid, ref utxo_set);
let result = validate_transaction(@tx, 0, 1600000001, 1600000001, txid, ref utxo_set);
assert!(result.is_ok());
}

Expand Down Expand Up @@ -396,13 +401,13 @@ mod tests {
let mut utxo_set: UtxoSet = Default::default();

// Transaction should still valid when current block time is not greater than locktime
let result = validate_transaction(@tx, 0, 1600000000, txid, ref utxo_set);
let result = validate_transaction(@tx, 0, 1600000000, 1600000000, txid, ref utxo_set);
assert!(result.is_ok());

utxo_set = Default::default();

// Transaction should be valid when current block time is greater than locktime
let result = validate_transaction(@tx, 0, 1600000001, txid, ref utxo_set);
let result = validate_transaction(@tx, 0, 1600000001, 1600000001, txid, ref utxo_set);
assert!(result.is_ok());
}

Expand Down Expand Up @@ -445,13 +450,13 @@ mod tests {
let mut utxo_set: UtxoSet = Default::default();

// Transaction should still valid when current block time is not greater than locktime
let result = validate_transaction(@tx, 500000, 0, txid, ref utxo_set);
let result = validate_transaction(@tx, 500000, 0, 0, txid, ref utxo_set);
assert!(result.is_ok());

utxo_set = Default::default();

// Transaction should be valid when current block time is greater than locktime
let result = validate_transaction(@tx, 500001, 0, txid, ref utxo_set);
let result = validate_transaction(@tx, 500001, 0, 0, txid, ref utxo_set);
assert!(result.is_ok());
}

Expand Down Expand Up @@ -495,7 +500,7 @@ mod tests {
let txid = double_sha256_byte_array(tx_bytes_legacy);
let mut utxo_set: UtxoSet = Default::default();

validate_transaction(@tx, block_height, 0, txid, ref utxo_set).unwrap_err();
validate_transaction(@tx, block_height, 0, 0, txid, ref utxo_set).unwrap_err();
}

#[test]
Expand Down Expand Up @@ -538,7 +543,7 @@ mod tests {
let txid = double_sha256_byte_array(tx_bytes_legacy);
let mut utxo_set: UtxoSet = Default::default();

validate_transaction(@tx, block_height, 0, txid, ref utxo_set).unwrap();
validate_transaction(@tx, block_height, 0, 0, txid, ref utxo_set).unwrap();
}

#[test]
Expand Down Expand Up @@ -582,7 +587,7 @@ mod tests {
let txid = double_sha256_byte_array(tx_bytes_legacy);
let mut utxo_set: UtxoSet = Default::default();

validate_transaction(@tx, block_height, 0, txid, ref utxo_set).unwrap();
validate_transaction(@tx, block_height, 0, 0, txid, ref utxo_set).unwrap();
}

#[test]
Expand Down Expand Up @@ -630,7 +635,7 @@ mod tests {
cache.insert(outpoint_hash, TX_OUTPUT_STATUS_UNSPENT);
let mut utxo_set = UtxoSet { cache, ..Default::default() };

validate_transaction(@tx, block_height, 0, txid, ref utxo_set).unwrap();
validate_transaction(@tx, block_height, 0, 0, txid, ref utxo_set).unwrap();
}

#[test]
Expand Down Expand Up @@ -677,7 +682,7 @@ mod tests {
cache.insert(outpoint_hash, TX_OUTPUT_STATUS_UNSPENT);
let mut utxo_set = UtxoSet { cache, ..Default::default() };

validate_transaction(@tx, block_height, 0, txid, ref utxo_set).unwrap();
validate_transaction(@tx, block_height, 0, 0, txid, ref utxo_set).unwrap();
}

#[test]
Expand Down Expand Up @@ -732,7 +737,7 @@ mod tests {
cache.insert(outpoint_hash, TX_OUTPUT_STATUS_UNSPENT);
let mut utxo_set = UtxoSet { cache, ..Default::default() };

let result = validate_transaction(@tx, block_height, 0, txid, ref utxo_set);
let result = validate_transaction(@tx, block_height, 0, 0, txid, ref utxo_set);
assert!(result.is_err());
assert_eq!(result.unwrap_err(), "The output has already been added");
}
Expand Down Expand Up @@ -796,7 +801,7 @@ mod tests {
cache.insert(outpoint_hash, TX_OUTPUT_STATUS_UNSPENT);
let mut utxo_set = UtxoSet { cache, ..Default::default() };

let result = validate_transaction(@tx, block_height, 0, txid, ref utxo_set);
let result = validate_transaction(@tx, block_height, 0, 0, txid, ref utxo_set);
assert!(result.is_err());
assert_eq!(result.unwrap_err(), "The output has already been spent");
}
Expand Down Expand Up @@ -856,7 +861,7 @@ mod tests {
let txid = double_sha256_byte_array(tx_bytes_legacy);
let mut utxo_set: UtxoSet = Default::default();

let result = validate_transaction(@tx, block_height, 0, txid, ref utxo_set);
let result = validate_transaction(@tx, block_height, 0, 0, txid, ref utxo_set);
assert!(result.is_err());
assert_eq!(result.unwrap_err(), "The output has already been spent");
}
Expand Down

0 comments on commit ca0ae49

Please sign in to comment.