From 74cab50d4f743043c51f81fd07c8f9983a17992d Mon Sep 17 00:00:00 2001 From: jjy Date: Fri, 7 Mar 2025 16:54:01 +0800 Subject: [PATCH 1/2] fix: compute signing message from tx inputs and outputs instead of using tx_hash --- contracts/commitment-lock/src/main.rs | 25 +++++++++++-- contracts/funding-lock/src/main.rs | 25 ++++++++++++- tests/src/tests.rs | 54 +++++++++++++++++++-------- 3 files changed, 82 insertions(+), 22 deletions(-) diff --git a/contracts/commitment-lock/src/main.rs b/contracts/commitment-lock/src/main.rs index f7057c7..e37cec5 100644 --- a/contracts/commitment-lock/src/main.rs +++ b/contracts/commitment-lock/src/main.rs @@ -4,7 +4,7 @@ #[cfg(test)] extern crate alloc; -use ckb_hash::blake2b_256; +use ckb_hash::{blake2b_256, new_blake2b}; #[cfg(not(test))] use ckb_std::default_alloc; #[cfg(not(test))] @@ -19,7 +19,7 @@ use ckb_std::{ error::SysError, high_level::{ exec_cell, load_cell, load_cell_capacity, load_cell_data, load_cell_lock, load_cell_type, - load_input_since, load_script, load_tx_hash, load_witness, QueryIter, + load_input_out_point, load_input_since, load_script, load_witness, QueryIter, }, since::{EpochNumberWithFraction, LockValue, Since}, }; @@ -193,7 +193,7 @@ fn auth() -> Result<(), Error> { ]; exec_cell(&AUTH_CODE_HASH, ScriptHashType::Data1, &args).map_err(|_| Error::AuthError)?; - return Ok(()); + Ok(()) } else if unlock_type == 0xFE { // non-pending HTLC unlock process @@ -250,7 +250,24 @@ fn auth() -> Result<(), Error> { } let raw_since_value = load_input_since(0, Source::GroupInput)?; - let message = load_tx_hash()?; + let message = { + let mut hasher = new_blake2b(); + // hash all input out points + QueryIter::new(load_input_out_point, Source::Input).for_each(|out_point| { + hasher.update(out_point.as_slice()); + }); + // hash all outputs with data + QueryIter::new(load_cell, Source::Output) + .zip(QueryIter::new(load_cell_data, Source::Output)) + .for_each(|(cell, data)| { + hasher.update(cell.as_slice()); + hasher.update((data.len() as u32).to_le_bytes().as_ref()); + hasher.update(data.as_slice()); + }); + let mut hash_result = [0u8; 32]; + hasher.finalize(&mut hash_result); + hash_result + }; let mut signature = [0u8; 65]; let mut pubkey_hash = [0u8; 20]; diff --git a/contracts/funding-lock/src/main.rs b/contracts/funding-lock/src/main.rs index d3a1373..56d473f 100644 --- a/contracts/funding-lock/src/main.rs +++ b/contracts/funding-lock/src/main.rs @@ -4,6 +4,7 @@ #[cfg(test)] extern crate alloc; +use ckb_hash::new_blake2b; #[cfg(not(test))] use ckb_std::default_alloc; #[cfg(not(test))] @@ -16,7 +17,10 @@ use ckb_std::{ ckb_constants::Source, ckb_types::{bytes::Bytes, core::ScriptHashType, prelude::*}, error::SysError, - high_level::{exec_cell, load_input_since, load_script, load_tx_hash, load_witness}, + high_level::{ + exec_cell, load_cell, load_cell_data, load_input_out_point, load_input_since, load_script, + load_witness, QueryIter, + }, }; use hex::encode; @@ -76,7 +80,24 @@ fn auth() -> Result<(), Error> { return Err(Error::EmptyWitnessArgsError); } - let message = load_tx_hash()?; + let message = { + let mut hasher = new_blake2b(); + // hash all input out points + QueryIter::new(load_input_out_point, Source::Input).for_each(|out_point| { + hasher.update(out_point.as_slice()); + }); + // hash all outputs with data + QueryIter::new(load_cell, Source::Output) + .zip(QueryIter::new(load_cell_data, Source::Output)) + .for_each(|(cell, data)| { + hasher.update(cell.as_slice()); + hasher.update((data.len() as u32).to_le_bytes().as_ref()); + hasher.update(data.as_slice()); + }); + let mut hash_result = [0u8; 32]; + hasher.finalize(&mut hash_result); + hash_result + }; let mut pubkey_hash = [0u8; 20]; let script = load_script()?; diff --git a/tests/src/tests.rs b/tests/src/tests.rs index 548ead6..4e24548 100644 --- a/tests/src/tests.rs +++ b/tests/src/tests.rs @@ -5,8 +5,10 @@ use ckb_std::since::{EpochNumberWithFraction, Since}; use ckb_testtool::{ builtin::ALWAYS_SUCCESS, ckb_crypto::secp::Generator, - ckb_hash::blake2b_256, - ckb_types::{bytes::Bytes, core::TransactionBuilder, packed::*, prelude::*}, + ckb_hash::{blake2b_256, new_blake2b}, + ckb_types::{ + bytes::Bytes, core::TransactionBuilder, core::TransactionView, packed::*, prelude::*, + }, context::Context, }; use musig2::{ @@ -95,6 +97,26 @@ fn multisig( aggregated_signature_1.to_bytes().to_vec() } +/// Compute the message of a transaction +/// We prefer computing the message this way rather than using the transaction hash. +/// This ensures the signature remains valid even if the script code is updated. +fn compute_tx_message(tx: &TransactionView) -> [u8; 32] { + let mut hasher = new_blake2b(); + // all input out points + for input in tx.inputs() { + hasher.update(input.previous_output().as_slice()); + } + // all outputs with data + for (output, data) in tx.outputs_with_data_iter() { + hasher.update(output.as_slice()); + hasher.update((data.len() as u32).to_le_bytes().as_ref()); + hasher.update(&data); + } + let mut hash_result = [0u8; 32]; + hasher.finalize(&mut hash_result); + hash_result +} + #[test] fn test_funding_lock() { // deploy contract @@ -159,7 +181,7 @@ fn test_funding_lock() { .build(); // sign and add witness - let message: [u8; 32] = tx.hash().as_slice().try_into().unwrap(); + let message: [u8; 32] = compute_tx_message(&tx); let signature = multisig(sec_key_1, sec_key_2, key_agg_ctx, message); let witness = [ @@ -480,7 +502,7 @@ fn test_commitment_lock_with_two_pending_htlcs() { .capacity((1000 * BYTE_SHANNONS - payment_amount1 as u64).pack()) .lock(new_lock_script.clone()) .build()]; - let outputs_data = vec![Bytes::new()]; + let outputs_data = [Bytes::new()]; let tx = TransactionBuilder::default() .cell_deps(cell_deps.clone()) .inputs(inputs) @@ -489,7 +511,7 @@ fn test_commitment_lock_with_two_pending_htlcs() { .build(); // sign with remote_htlc_pubkey - let message: [u8; 32] = tx.hash().as_slice().try_into().unwrap(); + let message: [u8; 32] = compute_tx_message(&tx); let signature = remote_htlc_key1 .0 @@ -558,7 +580,7 @@ fn test_commitment_lock_with_two_pending_htlcs() { .capacity((1000 * BYTE_SHANNONS - payment_amount1 as u64).pack()) .lock(new_lock_script.clone()) .build()]; - let outputs_data = vec![Bytes::new()]; + let outputs_data = [Bytes::new()]; let tx = TransactionBuilder::default() .cell_deps(cell_deps.clone()) .inputs(inputs) @@ -567,7 +589,7 @@ fn test_commitment_lock_with_two_pending_htlcs() { .build(); // sign with local_htlc_pubkey - let message: [u8; 32] = tx.hash().as_slice().try_into().unwrap(); + let message: [u8; 32] = compute_tx_message(&tx); let signature = local_htlc_key1 .0 @@ -605,7 +627,7 @@ fn test_commitment_lock_with_two_pending_htlcs() { .build(); // sign with local_htlc_pubkey - let message: [u8; 32] = tx.hash().as_slice().try_into().unwrap(); + let message: [u8; 32] = compute_tx_message(&tx); let signature = local_htlc_key1 .0 @@ -656,7 +678,7 @@ fn test_commitment_lock_with_two_pending_htlcs() { .capacity((1000 * BYTE_SHANNONS - payment_amount2 as u64).pack()) .lock(new_lock_script.clone()) .build()]; - let outputs_data = vec![Bytes::new()]; + let outputs_data = [Bytes::new()]; let tx = TransactionBuilder::default() .cell_deps(cell_deps.clone()) .inputs(inputs) @@ -665,7 +687,7 @@ fn test_commitment_lock_with_two_pending_htlcs() { .build(); // sign with remote_htlc_pubkey - let message: [u8; 32] = tx.hash().as_slice().try_into().unwrap(); + let message: [u8; 32] = compute_tx_message(&tx); let signature = remote_htlc_key2 .0 @@ -705,7 +727,7 @@ fn test_commitment_lock_with_two_pending_htlcs() { .capacity((1000 * BYTE_SHANNONS - payment_amount2 as u64).pack()) .lock(new_lock_script.clone()) .build()]; - let outputs_data = vec![Bytes::new()]; + let outputs_data = [Bytes::new()]; let tx = TransactionBuilder::default() .cell_deps(cell_deps) .inputs(inputs) @@ -714,7 +736,7 @@ fn test_commitment_lock_with_two_pending_htlcs() { .build(); // sign with local_htlc_pubkey - let message: [u8; 32] = tx.hash().as_slice().try_into().unwrap(); + let message: [u8; 32] = compute_tx_message(&tx); let signature = local_htlc_key2 .0 @@ -931,7 +953,7 @@ fn test_commitment_lock_with_two_pending_htlcs_and_sudt() { .build(); // sign with remote_htlc_pubkey - let message: [u8; 32] = tx.hash().as_slice().try_into().unwrap(); + let message: [u8; 32] = compute_tx_message(&tx); let signature = remote_htlc_key1 .0 @@ -981,7 +1003,7 @@ fn test_commitment_lock_with_two_pending_htlcs_and_sudt() { .build(); // sign with local_htlc_pubkey - let message: [u8; 32] = tx.hash().as_slice().try_into().unwrap(); + let message: [u8; 32] = compute_tx_message(&tx); let signature = local_htlc_key1 .0 @@ -1047,7 +1069,7 @@ fn test_commitment_lock_with_two_pending_htlcs_and_sudt() { .build(); // sign with remote_htlc_pubkey - let message: [u8; 32] = tx.hash().as_slice().try_into().unwrap(); + let message: [u8; 32] = compute_tx_message(&tx); let signature = remote_htlc_key2 .0 @@ -1100,7 +1122,7 @@ fn test_commitment_lock_with_two_pending_htlcs_and_sudt() { .build(); // sign with local_htlc_pubkey - let message: [u8; 32] = tx.hash().as_slice().try_into().unwrap(); + let message: [u8; 32] = compute_tx_message(&tx); let signature = local_htlc_key2 .0 From ba2c0e3d80c26a5b986977ec05ee06942e4682b7 Mon Sep 17 00:00:00 2001 From: jjy Date: Tue, 18 Mar 2025 09:26:03 +0100 Subject: [PATCH 2/2] fix: empty cell_deps before compute signing message --- contracts/commitment-lock/src/main.rs | 26 ++++++++---------------- contracts/funding-lock/src/main.rs | 29 ++++++++------------------- tests/src/tests.rs | 23 ++++++++------------- 3 files changed, 24 insertions(+), 54 deletions(-) diff --git a/contracts/commitment-lock/src/main.rs b/contracts/commitment-lock/src/main.rs index e37cec5..be9fe3e 100644 --- a/contracts/commitment-lock/src/main.rs +++ b/contracts/commitment-lock/src/main.rs @@ -4,7 +4,7 @@ #[cfg(test)] extern crate alloc; -use ckb_hash::{blake2b_256, new_blake2b}; +use ckb_hash::blake2b_256; #[cfg(not(test))] use ckb_std::default_alloc; #[cfg(not(test))] @@ -19,7 +19,7 @@ use ckb_std::{ error::SysError, high_level::{ exec_cell, load_cell, load_cell_capacity, load_cell_data, load_cell_lock, load_cell_type, - load_input_out_point, load_input_since, load_script, load_witness, QueryIter, + load_input_since, load_script, load_transaction, load_witness, QueryIter, }, since::{EpochNumberWithFraction, LockValue, Since}, }; @@ -251,22 +251,12 @@ fn auth() -> Result<(), Error> { let raw_since_value = load_input_since(0, Source::GroupInput)?; let message = { - let mut hasher = new_blake2b(); - // hash all input out points - QueryIter::new(load_input_out_point, Source::Input).for_each(|out_point| { - hasher.update(out_point.as_slice()); - }); - // hash all outputs with data - QueryIter::new(load_cell, Source::Output) - .zip(QueryIter::new(load_cell_data, Source::Output)) - .for_each(|(cell, data)| { - hasher.update(cell.as_slice()); - hasher.update((data.len() as u32).to_le_bytes().as_ref()); - hasher.update(data.as_slice()); - }); - let mut hash_result = [0u8; 32]; - hasher.finalize(&mut hash_result); - hash_result + let tx = load_transaction()? + .raw() + .as_builder() + .cell_deps(Default::default()) + .build(); + blake2b_256(tx.as_slice()) }; let mut signature = [0u8; 65]; let mut pubkey_hash = [0u8; 20]; diff --git a/contracts/funding-lock/src/main.rs b/contracts/funding-lock/src/main.rs index 56d473f..719ee9a 100644 --- a/contracts/funding-lock/src/main.rs +++ b/contracts/funding-lock/src/main.rs @@ -4,7 +4,7 @@ #[cfg(test)] extern crate alloc; -use ckb_hash::new_blake2b; +use ckb_hash::blake2b_256; #[cfg(not(test))] use ckb_std::default_alloc; #[cfg(not(test))] @@ -17,10 +17,7 @@ use ckb_std::{ ckb_constants::Source, ckb_types::{bytes::Bytes, core::ScriptHashType, prelude::*}, error::SysError, - high_level::{ - exec_cell, load_cell, load_cell_data, load_input_out_point, load_input_since, load_script, - load_witness, QueryIter, - }, + high_level::{exec_cell, load_input_since, load_script, load_transaction, load_witness}, }; use hex::encode; @@ -81,22 +78,12 @@ fn auth() -> Result<(), Error> { } let message = { - let mut hasher = new_blake2b(); - // hash all input out points - QueryIter::new(load_input_out_point, Source::Input).for_each(|out_point| { - hasher.update(out_point.as_slice()); - }); - // hash all outputs with data - QueryIter::new(load_cell, Source::Output) - .zip(QueryIter::new(load_cell_data, Source::Output)) - .for_each(|(cell, data)| { - hasher.update(cell.as_slice()); - hasher.update((data.len() as u32).to_le_bytes().as_ref()); - hasher.update(data.as_slice()); - }); - let mut hash_result = [0u8; 32]; - hasher.finalize(&mut hash_result); - hash_result + let tx = load_transaction()? + .raw() + .as_builder() + .cell_deps(Default::default()) + .build(); + blake2b_256(tx.as_slice()) }; let mut pubkey_hash = [0u8; 20]; diff --git a/tests/src/tests.rs b/tests/src/tests.rs index 4e24548..4130be7 100644 --- a/tests/src/tests.rs +++ b/tests/src/tests.rs @@ -5,7 +5,7 @@ use ckb_std::since::{EpochNumberWithFraction, Since}; use ckb_testtool::{ builtin::ALWAYS_SUCCESS, ckb_crypto::secp::Generator, - ckb_hash::{blake2b_256, new_blake2b}, + ckb_hash::blake2b_256, ckb_types::{ bytes::Bytes, core::TransactionBuilder, core::TransactionView, packed::*, prelude::*, }, @@ -101,20 +101,13 @@ fn multisig( /// We prefer computing the message this way rather than using the transaction hash. /// This ensures the signature remains valid even if the script code is updated. fn compute_tx_message(tx: &TransactionView) -> [u8; 32] { - let mut hasher = new_blake2b(); - // all input out points - for input in tx.inputs() { - hasher.update(input.previous_output().as_slice()); - } - // all outputs with data - for (output, data) in tx.outputs_with_data_iter() { - hasher.update(output.as_slice()); - hasher.update((data.len() as u32).to_le_bytes().as_ref()); - hasher.update(&data); - } - let mut hash_result = [0u8; 32]; - hasher.finalize(&mut hash_result); - hash_result + let tx = tx + .data() + .raw() + .as_builder() + .cell_deps(Default::default()) + .build(); + blake2b_256(tx.as_slice()) } #[test]