diff --git a/rpc/README.md b/rpc/README.md index a42bd48d05..10125a38d0 100644 --- a/rpc/README.md +++ b/rpc/README.md @@ -1654,7 +1654,7 @@ Response "result": { "block_version": "0x0", "cellbase_maturity": "0x10000000000", - "dao_type_hash": null, + "dao_type_hash": "0x0000000000000000000000000000000000000000000000000000000000000000", "epoch_duration_target": "0x3840", "genesis_hash": "0x7978ec7ce5b507cfb52e149e36b1a23f6062ed150503c85bbf825da3599095ed", "hardfork_features": [ @@ -5703,7 +5703,7 @@ Consensus defines various parameters that influence chain consensus * `genesis_hash`: [`H256`](#type-h256) - The genesis block hash -* `dao_type_hash`: [`H256`](#type-h256) `|` `null` - The dao type hash +* `dao_type_hash`: [`H256`](#type-h256) - The dao type hash * `secp256k1_blake160_sighash_all_type_hash`: [`H256`](#type-h256) `|` `null` - The secp256k1_blake160_sighash_all_type_hash diff --git a/rpc/src/module/chain.rs b/rpc/src/module/chain.rs index 36e0f4238f..6a013be67c 100644 --- a/rpc/src/module/chain.rs +++ b/rpc/src/module/chain.rs @@ -1336,7 +1336,7 @@ pub trait ChainRpc { /// "result": { /// "block_version": "0x0", /// "cellbase_maturity": "0x10000000000", - /// "dao_type_hash": null, + /// "dao_type_hash": "0x0000000000000000000000000000000000000000000000000000000000000000", /// "epoch_duration_target": "0x3840", /// "genesis_hash": "0x7978ec7ce5b507cfb52e149e36b1a23f6062ed150503c85bbf825da3599095ed", /// "hardfork_features": [ diff --git a/rpc/src/module/pool.rs b/rpc/src/module/pool.rs index 1fe3e45de6..8f30f9b571 100644 --- a/rpc/src/module/pool.rs +++ b/rpc/src/module/pool.rs @@ -616,9 +616,7 @@ impl<'a> WellKnownScriptsOnlyValidator<'a> { Some(script) => { if !script.is_hash_type_type() { Err(DefaultOutputsValidatorError::HashType) - } else if script.code_hash() - != self.consensus.dao_type_hash().expect("No dao system cell") - { + } else if script.code_hash() != self.consensus.dao_type_hash() { Err(DefaultOutputsValidatorError::CodeHash) } else if output.lock().args().len() == BLAKE160_LEN + SINCE_LEN { // https://github.com/nervosnetwork/ckb/wiki/Common-Gotchas#nervos-dao diff --git a/rpc/src/tests/module/pool.rs b/rpc/src/tests/module/pool.rs index dfdef67552..5d9ba19834 100644 --- a/rpc/src/tests/module/pool.rs +++ b/rpc/src/tests/module/pool.rs @@ -37,7 +37,7 @@ fn test_default_outputs_validator() { // invalid code hash let tx = build_tx( - &consensus.dao_type_hash().unwrap(), + &consensus.dao_type_hash(), core::ScriptHashType::Type, vec![1; 20], ); @@ -76,7 +76,7 @@ fn test_default_outputs_validator() { let lock_type_hash = consensus .secp256k1_blake160_multisig_all_type_hash() .unwrap(); - let type_type_hash = consensus.dao_type_hash().unwrap(); + let type_type_hash = consensus.dao_type_hash(); // valid output lock let tx = build_tx_with_type( &lock_type_hash, diff --git a/spec/src/consensus.rs b/spec/src/consensus.rs index 461225a3d9..0f059d1dfb 100644 --- a/spec/src/consensus.rs +++ b/spec/src/consensus.rs @@ -276,7 +276,7 @@ impl ConsensusBuilder { median_time_block_count: MEDIAN_TIME_BLOCK_COUNT, max_block_cycles: MAX_BLOCK_CYCLES, max_block_bytes: MAX_BLOCK_BYTES, - dao_type_hash: None, + dao_type_hash: Byte32::default(), secp256k1_blake160_sighash_all_type_hash: None, secp256k1_blake160_multisig_all_type_hash: None, genesis_epoch_ext, @@ -347,7 +347,7 @@ impl ConsensusBuilder { "genesis block must contain the witness for cellbase" ); - self.inner.dao_type_hash = self.get_type_hash(OUTPUT_INDEX_DAO); + self.inner.dao_type_hash = self.get_type_hash(OUTPUT_INDEX_DAO).unwrap_or_default(); self.inner.secp256k1_blake160_sighash_all_type_hash = self.get_type_hash(OUTPUT_INDEX_SECP256K1_BLAKE160_SIGHASH_ALL); self.inner.secp256k1_blake160_multisig_all_type_hash = @@ -514,7 +514,7 @@ pub struct Consensus { /// The dao type hash /// /// [nervos-dao](https://github.com/nervosnetwork/rfcs/blob/master/rfcs/0024-ckb-genesis-script-list/0024-ckb-genesis-script-list.md#nervos-dao) - pub dao_type_hash: Option, + pub dao_type_hash: Byte32, /// The secp256k1_blake160_sighash_all_type_hash /// /// [SECP256K1/blake160](https://github.com/nervosnetwork/rfcs/blob/master/rfcs/0024-ckb-genesis-script-list/0024-ckb-genesis-script-list.md#secp256k1blake160) @@ -626,7 +626,7 @@ impl Consensus { /// The dao type hash /// /// [nervos-dao](https://github.com/nervosnetwork/rfcs/blob/master/rfcs/0024-ckb-genesis-script-list/0024-ckb-genesis-script-list.md#nervos-dao) - pub fn dao_type_hash(&self) -> Option { + pub fn dao_type_hash(&self) -> Byte32 { self.dao_type_hash.clone() } @@ -1111,7 +1111,7 @@ impl From for ckb_jsonrpc_types::Consensus { Self { id: consensus.id, genesis_hash: consensus.genesis_hash.unpack(), - dao_type_hash: consensus.dao_type_hash.map(|h| h.unpack()), + dao_type_hash: consensus.dao_type_hash.unpack(), secp256k1_blake160_sighash_all_type_hash: consensus .secp256k1_blake160_sighash_all_type_hash .map(|h| h.unpack()), diff --git a/test/src/specs/dao/dao_user.rs b/test/src/specs/dao/dao_user.rs index 8b13ecfd09..aa729e8476 100644 --- a/test/src/specs/dao/dao_user.rs +++ b/test/src/specs/dao/dao_user.rs @@ -187,7 +187,7 @@ impl<'a> DAOUser<'a> { pub fn dao_type_script(&self) -> Script { Script::new_builder() - .code_hash(self.node.consensus().dao_type_hash().unwrap()) + .code_hash(self.node.consensus().dao_type_hash()) .hash_type(ScriptHashType::Type.into()) .build() } diff --git a/test/src/specs/dao/dao_verifier.rs b/test/src/specs/dao/dao_verifier.rs index feb1b6b0ee..b2b1fab86a 100644 --- a/test/src/specs/dao/dao_verifier.rs +++ b/test/src/specs/dao/dao_verifier.rs @@ -257,7 +257,7 @@ impl DAOVerifier { return false; } - let dao_type_hash = self.consensus.dao_type_hash().unwrap(); + let dao_type_hash = self.consensus.dao_type_hash(); self.get_output(out_point) .type_() .to_opt() diff --git a/test/src/specs/tx_pool/replace.rs b/test/src/specs/tx_pool/replace.rs index 90b972a640..0e37877801 100644 --- a/test/src/specs/tx_pool/replace.rs +++ b/test/src/specs/tx_pool/replace.rs @@ -1,4 +1,4 @@ -use crate::{Node, Spec}; +use crate::{utils::wait_until, Node, Spec}; use ckb_jsonrpc_types::Status; use ckb_logger::info; use ckb_types::{ @@ -465,18 +465,21 @@ impl Spec for RbfRejectReplaceProposed { assert!(ret.is_ok()); } - node0.mine_with_blocking(|template| template.proposals.len() != max_count); + let proposed = node0.mine_with_blocking(|template| template.proposals.len() != max_count); let ret = node0.rpc_client().get_transaction(txs[2].hash()); assert!( matches!(ret.tx_status.status, Status::Pending), "tx1 should be pending" ); - node0.mine(1); - let ret = node0.rpc_client().get_transaction(txs[2].hash()); - assert!( - matches!(ret.tx_status.status, Status::Proposed), - "tx1 should be proposed" - ); + + node0.mine_with_blocking(|template| template.number.value() != (proposed + 1)); + + let rpc_client0 = node0.rpc_client(); + let ret = wait_until(20, || { + let res = rpc_client0.get_transaction(txs[2].hash()); + res.tx_status.status == Status::Proposed + }); + assert!(ret, "tx1 should be proposed"); let clone_tx = txs[2].clone(); // Set tx2 fee to a higher value @@ -498,6 +501,26 @@ impl Spec for RbfRejectReplaceProposed { .unwrap() .to_string() .contains("all conflict Txs should be in Pending status")); + + // when tx1 was confirmed, tx2 should be rejected + let window_count = node0.consensus().tx_proposal_window().closest(); + node0.mine(window_count); + let ret = wait_until(20, || { + let res = rpc_client0.get_transaction(txs[2].hash()); + res.tx_status.status == Status::Committed + }); + assert!(ret, "tx1 should be committed"); + let res = node0 + .rpc_client() + .send_transaction_result(tx2.data().into()); + assert!(res.is_err(), "tx2 should be rejected"); + + // resolve tx2 failed with `unknown` when resolve inputs used by tx1 + assert!(res + .err() + .unwrap() + .to_string() + .contains("TransactionFailedToResolve: Resolve failed Unknown")); } fn modify_app_config(&self, config: &mut ckb_app_config::CKBAppConfig) { diff --git a/util/dao/src/lib.rs b/util/dao/src/lib.rs index 069329681f..ebb550871c 100644 --- a/util/dao/src/lib.rs +++ b/util/dao/src/lib.rs @@ -221,8 +221,7 @@ impl<'a, DL: CellDataProvider + EpochProvider + HeaderProvider> DaoCalculator<'a let is_dao_type_script = |type_script: Script| { Into::::into(type_script.hash_type()) == Into::::into(ScriptHashType::Type) - && type_script.code_hash() - == self.consensus.dao_type_hash().expect("No dao system cell") + && type_script.code_hash() == self.consensus.dao_type_hash() }; let is_withdrawing_input = |cell_meta: &CellMeta| match self.data_loader.load_cell_data(cell_meta) { diff --git a/util/jsonrpc-types/src/blockchain.rs b/util/jsonrpc-types/src/blockchain.rs index 07bc13435e..d444f0f559 100644 --- a/util/jsonrpc-types/src/blockchain.rs +++ b/util/jsonrpc-types/src/blockchain.rs @@ -1339,7 +1339,7 @@ pub struct Consensus { /// The genesis block hash pub genesis_hash: H256, /// The dao type hash - pub dao_type_hash: Option, + pub dao_type_hash: H256, /// The secp256k1_blake160_sighash_all_type_hash pub secp256k1_blake160_sighash_all_type_hash: Option, /// The secp256k1_blake160_multisig_all_type_hash diff --git a/util/light-client-protocol-server/src/components/get_blocks_proof.rs b/util/light-client-protocol-server/src/components/get_blocks_proof.rs index f071177dd8..dd8e0ae174 100644 --- a/util/light-client-protocol-server/src/components/get_blocks_proof.rs +++ b/util/light-client-protocol-server/src/components/get_blocks_proof.rs @@ -40,14 +40,15 @@ impl<'a> GetBlocksProofProcess<'a> { let snapshot = self.protocol.shared.snapshot(); - let last_hash = self.message.last_hash().to_entity(); - let last_block = if let Some(block) = snapshot.get_block(&last_hash) { - block - } else { + let last_block_hash = self.message.last_hash().to_entity(); + if !snapshot.is_main_chain(&last_block_hash) { return self .protocol .reply_tip_state::(self.peer, self.nc); - }; + } + let last_block = snapshot + .get_block(&last_block_hash) + .expect("block should be in store"); let block_hashes: Vec<_> = self .message @@ -59,42 +60,30 @@ impl<'a> GetBlocksProofProcess<'a> { let mut uniq = HashSet::new(); if !block_hashes .iter() - .chain([last_hash.clone()].iter()) + .chain([last_block_hash].iter()) .all(|hash| uniq.insert(hash)) { return StatusCode::MalformedProtocolMessage .with_context("duplicate block hash exists"); } - let (positions, block_headers, missing_blocks) = block_hashes + let (found, missing): (Vec<_>, Vec<_>) = block_hashes .into_iter() - .map(|block_hash| { - snapshot - .get_block_header(&block_hash) - .map(|header| header.number()) - .filter(|number| *number != last_block.number()) - .and_then(|number| snapshot.get_ancestor(&last_hash, number)) - .filter(|header| header.hash() == block_hash) - .ok_or(block_hash) - }) - .fold( - (Vec::new(), Vec::new(), Vec::new()), - |(mut positions, mut block_headers, mut missing_blocks), result| { - match result { - Ok(header) => { - positions.push(leaf_index_to_pos(header.number())); - block_headers.push(header); - } - Err(block_hash) => { - missing_blocks.push(block_hash); - } - } - (positions, block_headers, missing_blocks) - }, - ); + .partition(|block_hash| snapshot.is_main_chain(block_hash)); + + let mut positions = Vec::with_capacity(found.len()); + let mut block_headers = Vec::with_capacity(found.len()); + + for block_hash in found { + let header = snapshot + .get_block_header(&block_hash) + .expect("header should be in store"); + positions.push(leaf_index_to_pos(header.number())); + block_headers.push(header.data()); + } - let proved_items = block_headers.into_iter().map(|view| view.data()).pack(); - let missing_items = missing_blocks.pack(); + let proved_items = block_headers.pack(); + let missing_items = missing.pack(); self.protocol.reply_proof::( self.peer, diff --git a/util/light-client-protocol-server/src/components/get_last_state_proof.rs b/util/light-client-protocol-server/src/components/get_last_state_proof.rs index b2352236d9..1129eea868 100644 --- a/util/light-client-protocol-server/src/components/get_last_state_proof.rs +++ b/util/light-client-protocol-server/src/components/get_last_state_proof.rs @@ -204,18 +204,14 @@ impl<'a> GetLastStateProofProcess<'a> { let snapshot = self.protocol.shared.snapshot(); let last_block_hash = self.message.last_hash().to_entity(); - let last_block = if !snapshot.is_main_chain(&last_block_hash) { + if !snapshot.is_main_chain(&last_block_hash) { return self .protocol .reply_tip_state::(self.peer, self.nc); - } else if let Some(block) = snapshot.get_block(&last_block_hash) { - block - } else { - let errmsg = format!( - "the block is in the main chain but not found, its hash is {last_block_hash:#x}" - ); - return StatusCode::InternalError.with_context(errmsg); - }; + } + let last_block = snapshot + .get_block(&last_block_hash) + .expect("block should be in store"); let start_block_hash = self.message.start_hash().to_entity(); let start_block_number: BlockNumber = self.message.start_number().unpack(); diff --git a/util/light-client-protocol-server/src/components/get_transactions_proof.rs b/util/light-client-protocol-server/src/components/get_transactions_proof.rs index f8b22c7a7a..4b77884cfe 100644 --- a/util/light-client-protocol-server/src/components/get_transactions_proof.rs +++ b/util/light-client-protocol-server/src/components/get_transactions_proof.rs @@ -40,107 +40,83 @@ impl<'a> GetTransactionsProofProcess<'a> { let snapshot = self.protocol.shared.snapshot(); - let last_hash = self.message.last_hash().to_entity(); - let last_block = if let Some(block) = snapshot.get_block(&last_hash) { - block - } else { + let last_block_hash = self.message.last_hash().to_entity(); + if !snapshot.is_main_chain(&last_block_hash) { return self .protocol .reply_tip_state::(self.peer, self.nc); - }; + } + let last_block = snapshot + .get_block(&last_block_hash) + .expect("block should be in store"); - let (txs_in_blocks, missing_txs) = self + let (found, missing): (Vec<_>, Vec<_>) = self .message .tx_hashes() .to_entity() .into_iter() - .map(|tx_hash| { - let tx_with_info = snapshot.get_transaction_with_info(&tx_hash); - (tx_hash, tx_with_info) - }) - .fold( - (HashMap::new(), Vec::new()), - |(mut found, mut missing_txs), (tx_hash, tx_with_info)| { - if let Some((tx, tx_info)) = tx_with_info { - found - .entry(tx_info.block_hash) - .or_insert_with(Vec::new) - .push((tx, tx_info.index)); - } else { - missing_txs.push(tx_hash); - } - (found, missing_txs) - }, - ); - - let (positions, filtered_blocks, missing_txs) = txs_in_blocks - .into_iter() - .map(|(block_hash, txs_and_tx_indices)| { + .partition(|tx_hash| { snapshot - .get_block_header(&block_hash) - .map(|header| header.number()) - .filter(|number| *number != last_block.number()) - .and_then(|number| snapshot.get_ancestor(&last_hash, number)) - .filter(|header| header.hash() == block_hash) - .and_then(|_| snapshot.get_block(&block_hash)) - .map(|block| (block, txs_and_tx_indices.clone())) - .ok_or_else(|| { - txs_and_tx_indices - .into_iter() - .map(|(tx, _)| tx.hash()) - .collect::>() - }) - }) - .fold( - (Vec::new(), Vec::new(), missing_txs), - |(mut positions, mut filtered_blocks, mut missing_txs), result| { - match result { - Ok((block, txs_and_tx_indices)) => { - let merkle_proof = CBMT::build_merkle_proof( - &block - .transactions() - .iter() - .map(|tx| tx.hash()) - .collect::>(), - &txs_and_tx_indices - .iter() - .map(|(_, index)| *index as u32) - .collect::>(), - ) - .expect("build proof with verified inputs should be OK"); - - let txs: Vec<_> = txs_and_tx_indices - .into_iter() - .map(|(tx, _)| tx.data()) - .collect(); - - let filtered_block = packed::FilteredBlock::new_builder() - .header(block.header().data()) - .witnesses_root(block.calc_witnesses_root()) - .transactions(txs.pack()) - .proof( - packed::MerkleProof::new_builder() - .indices(merkle_proof.indices().to_owned().pack()) - .lemmas(merkle_proof.lemmas().to_owned().pack()) - .build(), - ) - .build(); - - positions.push(leaf_index_to_pos(block.number())); - filtered_blocks.push(filtered_block); - } - Err(tx_hashes) => { - missing_txs.extend(tx_hashes); - } - } - (positions, filtered_blocks, missing_txs) - }, - ); + .get_transaction_info(tx_hash) + .map(|tx_info| snapshot.is_main_chain(&tx_info.block_hash)) + .unwrap_or_default() + }); + + let mut txs_in_blocks = HashMap::new(); + for tx_hash in found { + let (tx, tx_info) = snapshot + .get_transaction_with_info(&tx_hash) + .expect("tx exists"); + txs_in_blocks + .entry(tx_info.block_hash) + .or_insert_with(Vec::new) + .push((tx, tx_info.index)); + } + + let mut positions = Vec::with_capacity(txs_in_blocks.len()); + let mut filtered_blocks = Vec::with_capacity(txs_in_blocks.len()); + for (block_hash, txs_and_tx_indices) in txs_in_blocks.into_iter() { + let block = snapshot + .get_block(&block_hash) + .expect("block should be in store"); + let merkle_proof = CBMT::build_merkle_proof( + &block + .transactions() + .iter() + .map(|tx| tx.hash()) + .collect::>(), + &txs_and_tx_indices + .iter() + .map(|(_, index)| *index as u32) + .collect::>(), + ) + .expect("build proof with verified inputs should be OK"); + + let txs: Vec<_> = txs_and_tx_indices + .into_iter() + .map(|(tx, _)| tx.data()) + .collect(); + + let filtered_block = packed::FilteredBlock::new_builder() + .header(block.header().data()) + .witnesses_root(block.calc_witnesses_root()) + .transactions(txs.pack()) + .proof( + packed::MerkleProof::new_builder() + .indices(merkle_proof.indices().to_owned().pack()) + .lemmas(merkle_proof.lemmas().to_owned().pack()) + .build(), + ) + .build(); + + positions.push(leaf_index_to_pos(block.number())); + filtered_blocks.push(filtered_block); + } let proved_items = packed::FilteredBlockVec::new_builder() .set(filtered_blocks) .build(); - let missing_items = missing_txs.pack(); + let missing_items = missing.pack(); self.protocol.reply_proof::( self.peer, diff --git a/verification/src/tests/transaction_verifier.rs b/verification/src/tests/transaction_verifier.rs index 4dd123e90b..16f05fe1fe 100644 --- a/verification/src/tests/transaction_verifier.rs +++ b/verification/src/tests/transaction_verifier.rs @@ -104,7 +104,7 @@ pub fn test_capacity_outofbound() { resolved_dep_groups: vec![], }); let dao_type_hash = build_genesis_type_id_script(OUTPUT_INDEX_DAO).calc_script_hash(); - let verifier = CapacityVerifier::new(rtx, Some(dao_type_hash)); + let verifier = CapacityVerifier::new(rtx, dao_type_hash); assert_error_eq!( verifier.verify().unwrap_err(), @@ -136,7 +136,7 @@ pub fn test_skip_dao_capacity_check() { resolved_inputs: vec![], resolved_dep_groups: vec![], }); - let verifier = CapacityVerifier::new(rtx, Some(dao_type_script.calc_script_hash())); + let verifier = CapacityVerifier::new(rtx, dao_type_script.calc_script_hash()); assert!(verifier.verify().is_ok()); } @@ -329,7 +329,7 @@ pub fn test_capacity_invalid() { resolved_dep_groups: vec![], }); let dao_type_hash = build_genesis_type_id_script(OUTPUT_INDEX_DAO).calc_script_hash(); - let verifier = CapacityVerifier::new(rtx, Some(dao_type_hash)); + let verifier = CapacityVerifier::new(rtx, dao_type_hash); assert_error_eq!( verifier.verify().unwrap_err(), @@ -808,7 +808,7 @@ fn build_consensus_with_dao_limiting_block(block_number: u64) -> (Arc // the dao script. For simplicity, we are hacking consensus here with // a dao_type_hash value, a proper way should be creating a proper genesis // block here, but we will leave it till we really need it. - consensus.dao_type_hash = Some(dao_script.calc_script_hash()); + consensus.dao_type_hash = dao_script.calc_script_hash(); let dao_type_script = Script::new_builder() .code_hash(dao_script.calc_script_hash()) diff --git a/verification/src/transaction_verifier.rs b/verification/src/transaction_verifier.rs index 11214f583f..b9ec653022 100644 --- a/verification/src/transaction_verifier.rs +++ b/verification/src/transaction_verifier.rs @@ -523,16 +523,12 @@ impl<'a> DuplicateDepsVerifier<'a> { /// Perform inputs and outputs `capacity` field related verification pub struct CapacityVerifier { resolved_transaction: Arc, - // It's Option because special genesis block do not have dao system cell - dao_type_hash: Option, + dao_type_hash: Byte32, } impl CapacityVerifier { /// Create a new `CapacityVerifier` - pub fn new( - resolved_transaction: Arc, - dao_type_hash: Option, - ) -> Self { + pub fn new(resolved_transaction: Arc, dao_type_hash: Byte32) -> Self { CapacityVerifier { resolved_transaction, dao_type_hash, @@ -584,12 +580,7 @@ impl CapacityVerifier { self.resolved_transaction .resolved_inputs .iter() - .any(|cell_meta| { - cell_uses_dao_type_script( - &cell_meta.cell_output, - self.dao_type_hash.as_ref().expect("No dao system cell"), - ) - }) + .any(|cell_meta| cell_uses_dao_type_script(&cell_meta.cell_output, &self.dao_type_hash)) } } @@ -990,17 +981,14 @@ impl DaoScriptSizeVerifier
{ } } - fn dao_type_hash(&self) -> Option { + fn dao_type_hash(&self) -> Byte32 { self.consensus.dao_type_hash() } /// Verifies that for all Nervos DAO transactions, withdrawing cells must use lock scripts /// of the same size as corresponding deposit cells pub fn verify(&self) -> Result<(), Error> { - if self.dao_type_hash().is_none() { - return Ok(()); - } - let dao_type_hash = self.dao_type_hash().unwrap(); + let dao_type_hash = self.dao_type_hash(); for (i, (input_meta, cell_output)) in self .resolved_transaction .resolved_inputs