From 1430d4af49a114e6f9f610cebf5606b995ee879d Mon Sep 17 00:00:00 2001 From: Ivan Shumkov Date: Fri, 6 Oct 2023 12:15:58 +0700 Subject: [PATCH 1/6] chore: add a breaking changes checkbox to the PR template (#1455) --- .github/PULL_REQUEST_TEMPLATE.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/.github/PULL_REQUEST_TEMPLATE.md b/.github/PULL_REQUEST_TEMPLATE.md index 1d8af7bf58..cda6858ce4 100644 --- a/.github/PULL_REQUEST_TEMPLATE.md +++ b/.github/PULL_REQUEST_TEMPLATE.md @@ -26,7 +26,8 @@ - [ ] I have performed a self-review of my own code - [ ] I have commented my code, particularly in hard-to-understand areas - [ ] I have added or updated relevant unit/integration/functional/e2e tests -- [ ] I have made corresponding changes to the documentation +- [ ] I have added "!" to the title and described breaking changes in the corresponding section if my code contains any +- [ ] I have made corresponding changes to the documentation if needed **For repository code-owners and collaborators only** - [ ] I have assigned this pull request to a milestone From 6063b9159ed820b9af099b13caee78853ac0f76d Mon Sep 17 00:00:00 2001 From: Ivan Shumkov Date: Fri, 6 Oct 2023 14:27:09 +0700 Subject: [PATCH 2/6] chore(drive): improve quorum info update logs (#1444) --- .../update_quorum_info/v0/mod.rs | 120 +++++++++--------- .../state/v0/fetch_documents.rs | 2 - packages/rs-drive-abci/src/rpc/core.rs | 36 ++++-- .../tests/strategy_tests/execution.rs | 28 ++-- 4 files changed, 103 insertions(+), 83 deletions(-) diff --git a/packages/rs-drive-abci/src/execution/platform_events/core_based_updates/update_quorum_info/v0/mod.rs b/packages/rs-drive-abci/src/execution/platform_events/core_based_updates/update_quorum_info/v0/mod.rs index 07a3058b46..7c5d1a1ed1 100644 --- a/packages/rs-drive-abci/src/execution/platform_events/core_based_updates/update_quorum_info/v0/mod.rs +++ b/packages/rs-drive-abci/src/execution/platform_events/core_based_updates/update_quorum_info/v0/mod.rs @@ -9,8 +9,8 @@ use crate::platform_types::validator_set::ValidatorSet; use crate::rpc::core::CoreRPCLike; use dpp::dashcore::QuorumHash; -use std::cmp::Ordering; use std::collections::BTreeMap; +use tracing::Level; impl Platform where @@ -33,51 +33,58 @@ where core_block_height: u32, start_from_scratch: bool, ) -> Result<(), Error> { - if !start_from_scratch && core_block_height == block_platform_state.core_height() { + let _span = tracing::span!(Level::TRACE, "update_quorum_info", core_block_height).entered(); + + if start_from_scratch { + tracing::debug!("update quorum info from scratch up to {core_block_height}"); + } else if core_block_height != block_platform_state.core_height() { tracing::debug!( - method = "update_quorum_info_v0", - "no update quorum at height {}", + previous_core_block_height = block_platform_state.core_height(), + "update quorum info from {} to {}", + block_platform_state.core_height(), core_block_height ); + } else { + tracing::debug!("quorum info at height {core_block_height} already updated"); + return Ok(()); // no need to do anything } - tracing::debug!( - method = "update_quorum_info_v0", - "update of quorums for height {}", - core_block_height - ); - let quorum_list = self + + let all_quorums_by_type = self .core_rpc - .get_quorum_listextended(Some(core_block_height))?; - let quorum_info = quorum_list - .quorums_by_type - .get(&self.config.quorum_type()) - .ok_or(Error::Execution(ExecutionError::DashCoreBadResponseError( - format!( - "expected quorums of type {}, but did not receive any from Dash Core", - self.config.quorum_type - ), - )))?; - - tracing::debug!( - method = "update_quorum_info_v0", - "old {:?}", - block_platform_state.validator_sets() - ); - - tracing::debug!( - method = "update_quorum_info_v0", - "new quorum_info {:?}", - quorum_info - ); + .get_quorum_listextended_by_type(Some(core_block_height))?; + + let validator_quorums_list = + all_quorums_by_type + .get(&self.config.quorum_type()) + .ok_or(Error::Execution(ExecutionError::DashCoreBadResponseError( + format!( + "expected quorums of type {}, but did not receive any from Dash Core", + self.config.quorum_type + ), + )))?; // Remove validator_sets entries that are no longer valid for the core block height block_platform_state .validator_sets_mut() - .retain(|key, _| quorum_info.contains_key(key)); + .retain(|quorum_hash, _| { + let has_quorum = validator_quorums_list.contains_key::(quorum_hash); - // Fetch quorum info results and their keys from the RPC - let mut quorum_infos = quorum_info + if has_quorum { + tracing::trace!( + ?quorum_hash, + quorum_type = ?self.config.quorum_type(), + "remove validator set {} with quorum type {}", + quorum_hash, + self.config.quorum_type() + ) + } + + has_quorum + }); + + // Fetch quorum info and their keys from the RPC for new quorums + let mut quorum_infos = validator_quorums_list .iter() .filter(|(key, _)| { !block_platform_state @@ -88,6 +95,7 @@ where let quorum_info_result = self.core_rpc .get_quorum_info(self.config.quorum_type(), key, None)?; + Ok((*key, quorum_info_result)) }) .collect::, Error>>()?; @@ -102,27 +110,39 @@ where } }); - // Map to quorums - let new_quorums = quorum_infos + // Map to validator sets + let new_validator_sets = quorum_infos .into_iter() - .map(|(key, info_result)| { + .map(|(quorum_hash, info_result)| { let validator_set = ValidatorSet::V0(ValidatorSetV0::try_from_quorum_info_result( info_result, block_platform_state, )?); - Ok((key, validator_set)) + + tracing::trace!( + ?validator_set, + ?quorum_hash, + quorum_type = ?self.config.quorum_type(), + "add new validator set {} with quorum type {}", + quorum_hash, + self.config.quorum_type() + ); + + Ok((quorum_hash, validator_set)) }) .collect::, Error>>()?; + // Add new validator_sets entries block_platform_state .validator_sets_mut() - .extend(new_quorums.into_iter()); + .extend(new_validator_sets); + // Sort all validator sets into deterministic order by core block height of creation block_platform_state .validator_sets_mut() .sort_by(|_, quorum_a, _, quorum_b| { let primary_comparison = quorum_b.core_height().cmp(&quorum_a.core_height()); - if primary_comparison == Ordering::Equal { + if primary_comparison == std::cmp::Ordering::Equal { quorum_b .quorum_hash() .cmp(quorum_a.quorum_hash()) @@ -132,23 +152,7 @@ where } }); - tracing::debug!( - method = "update_quorum_info_v0", - "new {:?}", - block_platform_state.validator_sets() - ); - - let quorums_by_type = quorum_list - .quorums_by_type - .into_iter() - .map(|(quorum_type, quorum_list)| { - let sorted_quorum_list = quorum_list.into_iter().collect(); - - (quorum_type, sorted_quorum_list) - }) - .collect(); - - block_platform_state.set_quorums_extended_info(quorums_by_type); + block_platform_state.set_quorums_extended_info(all_quorums_by_type); Ok(()) } diff --git a/packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/documents_batch/state/v0/fetch_documents.rs b/packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/documents_batch/state/v0/fetch_documents.rs index 75f5abfe02..1b55ac6a67 100644 --- a/packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/documents_batch/state/v0/fetch_documents.rs +++ b/packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/documents_batch/state/v0/fetch_documents.rs @@ -95,8 +95,6 @@ pub(crate) fn fetch_documents_for_transitions_knowing_contract_id_and_document_t )); }; - let contract_fetch_info = contract_fetch_info; - let Some(document_type) = contract_fetch_info .contract .document_type_optional_for_name(document_type_name) diff --git a/packages/rs-drive-abci/src/rpc/core.rs b/packages/rs-drive-abci/src/rpc/core.rs index a24a667462..d1b74005e1 100644 --- a/packages/rs-drive-abci/src/rpc/core.rs +++ b/packages/rs-drive-abci/src/rpc/core.rs @@ -9,12 +9,12 @@ use dashcore_rpc::json::GetTransactionResult; use dashcore_rpc::{Auth, Client, Error, RpcApi}; use dpp::dashcore::{hashes::Hash, InstantLock}; use serde_json::Value; -use std::collections::HashMap; +use std::collections::BTreeMap; use std::time::Duration; use tenderdash_abci::proto::types::CoreChainLock; /// Information returned by QuorumListExtended -pub type QuorumListExtendedInfo = HashMap; +pub type QuorumListExtendedInfo = BTreeMap; /// Core height must be of type u32 (Platform heights are u64) pub type CoreHeight = u32; @@ -51,13 +51,13 @@ pub trait CoreRPCLike { /// Get chain tips fn get_chain_tips(&self) -> Result; - /// Get list of quorums at a given height. + /// Get list of quorums by type at a given height. /// /// See - fn get_quorum_listextended( + fn get_quorum_listextended_by_type( &self, height: Option, - ) -> Result; + ) -> Result, Error>; /// Get quorum information. /// @@ -224,11 +224,24 @@ impl CoreRPCLike for DefaultCoreRPC { retry!(self.inner.get_chain_tips()) } - fn get_quorum_listextended( + fn get_quorum_listextended_by_type( &self, height: Option, - ) -> Result { - retry!(self.inner.get_quorum_listextended(height)) + ) -> Result, Error> { + let all_quorums_list = get_quorum_listextended(&self.inner, height)?; + + // Sort in deterministic order + let sorted_quorums_by_type = all_quorums_list + .quorums_by_type + .into_iter() + .map(|(quorum_type, quorum_list)| { + let sorted_quorum_list: BTreeMap<_, _> = quorum_list.into_iter().collect(); + + (quorum_type, sorted_quorum_list) + }) + .collect(); + + Ok(sorted_quorums_by_type) } fn get_quorum_info( @@ -310,3 +323,10 @@ impl CoreRPCLike for DefaultCoreRPC { retry!(self.inner.mnsync_status()) } } + +fn get_quorum_listextended( + inner: &Client, + height: Option, +) -> Result { + retry!(inner.get_quorum_listextended(height)) +} diff --git a/packages/rs-drive-abci/tests/strategy_tests/execution.rs b/packages/rs-drive-abci/tests/strategy_tests/execution.rs index 8ac9bf446a..96d313147e 100644 --- a/packages/rs-drive-abci/tests/strategy_tests/execution.rs +++ b/packages/rs-drive-abci/tests/strategy_tests/execution.rs @@ -260,15 +260,13 @@ pub(crate) fn run_chain_for_strategy( platform .core_rpc - .expect_get_quorum_listextended() + .expect_get_quorum_listextended_by_type() .returning(move |core_height: Option| { if !strategy.rotate_quorums { - Ok(dashcore_rpc::dashcore_rpc_json::ExtendedQuorumListResult { - quorums_by_type: HashMap::from([( - QuorumType::Llmq100_67, - quorums_details.clone().into_iter().collect(), - )]), - }) + Ok(BTreeMap::from([( + QuorumType::Llmq100_67, + quorums_details.clone().into_iter().collect(), + )])) } else { let core_height = core_height.expect("expected a core height"); // if we rotate quorums we shouldn't give back the same ones every time @@ -296,9 +294,7 @@ pub(crate) fn run_chain_for_strategy( .collect() }; - Ok(dashcore_rpc::dashcore_rpc_json::ExtendedQuorumListResult { - quorums_by_type: HashMap::from([(QuorumType::Llmq100_67, quorums)]), - }) + Ok(BTreeMap::from([(QuorumType::Llmq100_67, quorums)])) } }); @@ -312,7 +308,7 @@ pub(crate) fn run_chain_for_strategy( .expect_get_quorum_info() .returning(move |_, quorum_hash: &QuorumHash, _| { Ok(quorums_info - .get(quorum_hash) + .get::(quorum_hash) .unwrap_or_else(|| panic!("expected to get quorum {}", hex::encode(quorum_hash))) .clone()) }); @@ -525,7 +521,7 @@ pub(crate) fn start_chain_for_strategy( .expect("expected quorums to be initialized"); let current_quorum_with_test_info = quorums - .get(¤t_quorum_hash) + .get::(¤t_quorum_hash) .expect("expected a quorum to be found"); // init chain @@ -635,7 +631,8 @@ pub(crate) fn continue_chain_for_strategy( let mut total_withdrawals = vec![]; - let mut current_quorum_with_test_info = quorums.get(¤t_quorum_hash).unwrap(); + let mut current_quorum_with_test_info = + quorums.get::(¤t_quorum_hash).unwrap(); let mut validator_set_updates = BTreeMap::new(); @@ -662,7 +659,8 @@ pub(crate) fn continue_chain_for_strategy( epoch: Epoch::new(epoch_info.current_epoch_index).unwrap(), }; if current_quorum_with_test_info.quorum_hash != current_quorum_hash { - current_quorum_with_test_info = quorums.get(¤t_quorum_hash).unwrap(); + current_quorum_with_test_info = + quorums.get::(¤t_quorum_hash).unwrap(); } let proposer = current_quorum_with_test_info @@ -686,7 +684,7 @@ pub(crate) fn continue_chain_for_strategy( next_protocol_version, change_block_height, } = proposer_versions - .get(&proposer.pro_tx_hash) + .get::(&proposer.pro_tx_hash) .expect("expected to have version"); if &block_height >= change_block_height { *next_protocol_version From 9a6fca879eba9222dc0a410bd8895f376eec648d Mon Sep 17 00:00:00 2001 From: Ivan Shumkov Date: Fri, 6 Oct 2023 14:53:00 +0700 Subject: [PATCH 3/6] chore(drive): log grovedb operations (#1446) --- packages/rs-drive/Cargo.toml | 1 + .../v0/mod.rs | 30 ++++++++++++++++ .../grove_operations/grove_clear/v0/mod.rs | 34 +++++++++++++++++-- 3 files changed, 63 insertions(+), 2 deletions(-) diff --git a/packages/rs-drive/Cargo.toml b/packages/rs-drive/Cargo.toml index c923db94d8..fbc4013db5 100644 --- a/packages/rs-drive/Cargo.toml +++ b/packages/rs-drive/Cargo.toml @@ -91,4 +91,5 @@ full = [ "rust_decimal_macros", "lazy_static", ] +grovedb_operations_logging = [] verify = ["grovedb/verify", "grovedb-costs"] diff --git a/packages/rs-drive/src/drive/grove_operations/grove_apply_batch_with_add_costs/v0/mod.rs b/packages/rs-drive/src/drive/grove_operations/grove_apply_batch_with_add_costs/v0/mod.rs index 0763f56fe8..d1c696c920 100644 --- a/packages/rs-drive/src/drive/grove_operations/grove_apply_batch_with_add_costs/v0/mod.rs +++ b/packages/rs-drive/src/drive/grove_operations/grove_apply_batch_with_add_costs/v0/mod.rs @@ -11,6 +11,7 @@ use grovedb::batch::{BatchApplyOptions, GroveDbOp}; use grovedb::TransactionArg; use grovedb_costs::storage_cost::removal::StorageRemovedBytes::BasicStorageRemoval; use grovedb_costs::storage_cost::transition::OperationStorageTransitionType; +use tracing::Level; impl Drive { /// Applies the given groveDB operations batch and gets and passes the costs to `push_drive_operation_result`. @@ -42,6 +43,15 @@ impl Drive { } } + // Clone ops only if we log them + #[cfg(feature = "grovedb_operations_logging")] + let maybe_ops_for_logs = if tracing::event_enabled!(target: "grovedb_operations", Level::TRACE) + { + Some(ops.clone()) + } else { + None + }; + let cost_context = self.grove.apply_batch_with_element_flags_update( ops.operations, Some(BatchApplyOptions { @@ -140,6 +150,26 @@ impl Drive { }, transaction, ); + + #[cfg(feature = "grovedb_operations_logging")] + if tracing::event_enabled!(target: "grovedb_operations", Level::TRACE) + && cost_context.value.is_ok() + { + let root_hash = self + .grove + .root_hash(transaction) + .unwrap() + .map_err(Error::GroveDB)?; + + tracing::trace!( + target = "grovedb_operations", + ops = ?maybe_ops_for_logs.unwrap(), + root_hash = ?root_hash, + is_transactional = transaction.is_some(), + "grovedb batch applied", + ); + } + push_drive_operation_result(cost_context, drive_operations) } } diff --git a/packages/rs-drive/src/drive/grove_operations/grove_clear/v0/mod.rs b/packages/rs-drive/src/drive/grove_operations/grove_clear/v0/mod.rs index 9777b56744..1b41f82bf6 100644 --- a/packages/rs-drive/src/drive/grove_operations/grove_clear/v0/mod.rs +++ b/packages/rs-drive/src/drive/grove_operations/grove_clear/v0/mod.rs @@ -3,6 +3,7 @@ use crate::error::Error; use grovedb::operations::delete::ClearOptions; use grovedb::TransactionArg; use grovedb_path::SubtreePath; +use tracing::Level; impl Drive { /// Pushes the `OperationCost` of deleting an element in groveDB to `drive_operations`. @@ -16,10 +17,39 @@ impl Drive { allow_deleting_subtrees: false, trying_to_clear_with_subtrees_returns_error: false, }; + + #[cfg(feature = "grovedb_operations_logging")] + let maybe_path_for_logs = if tracing::event_enabled!(target: "grovedb_operations", Level::TRACE) + { + Some(path.clone()) + } else { + None + }; + // we will always return true if there is no error when we don't check for subtrees - self.grove + let result = self + .grove .clear_subtree(path, Some(options), transaction) .map_err(Error::GroveDB) - .map(|_| ()) + .map(|_| ()); + + #[cfg(feature = "grovedb_operations_logging")] + if tracing::event_enabled!(target: "grovedb_operations", Level::TRACE) && result.is_ok() { + let root_hash = self + .grove + .root_hash(transaction) + .unwrap() + .map_err(Error::GroveDB)?; + + tracing::trace!( + target = "grovedb_operations", + path = ?maybe_path_for_logs.unwrap().to_vec(), + root_hash = ?root_hash, + is_transactional = transaction.is_some(), + "grovedb clear", + ); + } + + result } } From c985509e0133100bd9903ce0d6d8e8bedf56b2d7 Mon Sep 17 00:00:00 2001 From: Ivan Shumkov Date: Sat, 7 Oct 2023 08:27:35 +0700 Subject: [PATCH 4/6] fix(drive)!: document type doesn't match array value --- .../rs-dpp/schema/document/document-base.json | 37 ------------- .../schema/document/documentExtended.json | 55 ------------------- .../document/v0/document-meta.json | 21 +++++++ 3 files changed, 21 insertions(+), 92 deletions(-) delete mode 100644 packages/rs-dpp/schema/document/document-base.json delete mode 100644 packages/rs-dpp/schema/document/documentExtended.json diff --git a/packages/rs-dpp/schema/document/document-base.json b/packages/rs-dpp/schema/document/document-base.json deleted file mode 100644 index 6ddb9aa33c..0000000000 --- a/packages/rs-dpp/schema/document/document-base.json +++ /dev/null @@ -1,37 +0,0 @@ -{ - "$schema": "https://json-schema.org/draft/2020-12/schema", - "type": "object", - "properties": { - "$id": { - "type": "array", - "byteArray": true, - "minItems": 32, - "maxItems": 32, - "contentMediaType": "application/x.dash.dpp.identifier" - }, - "$ownerId": { - "type": "array", - "byteArray": true, - "minItems": 32, - "maxItems": 32, - "contentMediaType": "application/x.dash.dpp.identifier" - }, - "$revision": { - "type": "integer", - "minimum": 1 - }, - "$createdAt": { - "type": "integer", - "minimum": 0 - }, - "$updatedAt": { - "type": "integer", - "minimum": 0 - } - }, - "required": [ - "$id", - "$ownerId" - ], - "additionalProperties": false -} \ No newline at end of file diff --git a/packages/rs-dpp/schema/document/documentExtended.json b/packages/rs-dpp/schema/document/documentExtended.json deleted file mode 100644 index acff4cd2cf..0000000000 --- a/packages/rs-dpp/schema/document/documentExtended.json +++ /dev/null @@ -1,55 +0,0 @@ -{ - "$schema": "https://json-schema.org/draft/2020-12/schema", - "type": "object", - "properties": { - "$protocolVersion": { - "type": "integer", - "$comment": "Maximum is the latest protocol version" - }, - "$id": { - "type": "array", - "byteArray": true, - "minItems": 32, - "maxItems": 32, - "contentMediaType": "application/x.dash.dpp.identifier" - }, - "$type": { - "type": "string" - }, - "$revision": { - "type": "integer", - "minimum": 1 - }, - "$dataContractId": { - "type": "array", - "byteArray": true, - "minItems": 32, - "maxItems": 32, - "contentMediaType": "application/x.dash.dpp.identifier" - }, - "$ownerId": { - "type": "array", - "byteArray": true, - "minItems": 32, - "maxItems": 32, - "contentMediaType": "application/x.dash.dpp.identifier" - }, - "$createdAt": { - "type": "integer", - "minimum": 0 - }, - "$updatedAt": { - "type": "integer", - "minimum": 0 - } - }, - "required": [ - "$protocolVersion", - "$id", - "$type", - "$revision", - "$dataContractId", - "$ownerId" - ], - "additionalProperties": false -} \ No newline at end of file diff --git a/packages/rs-dpp/schema/meta_schemas/document/v0/document-meta.json b/packages/rs-dpp/schema/meta_schemas/document/v0/document-meta.json index 35d20a33c4..524316ba74 100644 --- a/packages/rs-dpp/schema/meta_schemas/document/v0/document-meta.json +++ b/packages/rs-dpp/schema/meta_schemas/document/v0/document-meta.json @@ -281,6 +281,27 @@ } }, "allOf": [ + { + "$comment": "allow only byte arrays", + "if": { + "properties": { + "type": { + "const": "array" + } + }, + "required": [ + "type" + ] + }, + "then": { + "properties": { + "byteArray": true + }, + "required": [ + "byteArray" + ] + } + }, { "$comment": "array must contain items", "if": { From 3e166a2f928b644a7441ff626f7a2b6b5678c031 Mon Sep 17 00:00:00 2001 From: Ivan Shumkov Date: Sat, 7 Oct 2023 08:31:00 +0700 Subject: [PATCH 5/6] revert: v0.25 changes --- .github/PULL_REQUEST_TEMPLATE.md | 3 +- .../update_quorum_info/v0/mod.rs | 120 +++++++++--------- packages/rs-drive-abci/src/rpc/core.rs | 36 ++---- .../tests/strategy_tests/execution.rs | 28 ++-- packages/rs-drive/Cargo.toml | 1 - .../v0/mod.rs | 30 ----- .../grove_operations/grove_clear/v0/mod.rs | 34 +---- 7 files changed, 84 insertions(+), 168 deletions(-) diff --git a/.github/PULL_REQUEST_TEMPLATE.md b/.github/PULL_REQUEST_TEMPLATE.md index cda6858ce4..1d8af7bf58 100644 --- a/.github/PULL_REQUEST_TEMPLATE.md +++ b/.github/PULL_REQUEST_TEMPLATE.md @@ -26,8 +26,7 @@ - [ ] I have performed a self-review of my own code - [ ] I have commented my code, particularly in hard-to-understand areas - [ ] I have added or updated relevant unit/integration/functional/e2e tests -- [ ] I have added "!" to the title and described breaking changes in the corresponding section if my code contains any -- [ ] I have made corresponding changes to the documentation if needed +- [ ] I have made corresponding changes to the documentation **For repository code-owners and collaborators only** - [ ] I have assigned this pull request to a milestone diff --git a/packages/rs-drive-abci/src/execution/platform_events/core_based_updates/update_quorum_info/v0/mod.rs b/packages/rs-drive-abci/src/execution/platform_events/core_based_updates/update_quorum_info/v0/mod.rs index 7c5d1a1ed1..07a3058b46 100644 --- a/packages/rs-drive-abci/src/execution/platform_events/core_based_updates/update_quorum_info/v0/mod.rs +++ b/packages/rs-drive-abci/src/execution/platform_events/core_based_updates/update_quorum_info/v0/mod.rs @@ -9,8 +9,8 @@ use crate::platform_types::validator_set::ValidatorSet; use crate::rpc::core::CoreRPCLike; use dpp::dashcore::QuorumHash; +use std::cmp::Ordering; use std::collections::BTreeMap; -use tracing::Level; impl Platform where @@ -33,58 +33,51 @@ where core_block_height: u32, start_from_scratch: bool, ) -> Result<(), Error> { - let _span = tracing::span!(Level::TRACE, "update_quorum_info", core_block_height).entered(); - - if start_from_scratch { - tracing::debug!("update quorum info from scratch up to {core_block_height}"); - } else if core_block_height != block_platform_state.core_height() { + if !start_from_scratch && core_block_height == block_platform_state.core_height() { tracing::debug!( - previous_core_block_height = block_platform_state.core_height(), - "update quorum info from {} to {}", - block_platform_state.core_height(), + method = "update_quorum_info_v0", + "no update quorum at height {}", core_block_height ); - } else { - tracing::debug!("quorum info at height {core_block_height} already updated"); - return Ok(()); // no need to do anything } - - let all_quorums_by_type = self + tracing::debug!( + method = "update_quorum_info_v0", + "update of quorums for height {}", + core_block_height + ); + let quorum_list = self .core_rpc - .get_quorum_listextended_by_type(Some(core_block_height))?; - - let validator_quorums_list = - all_quorums_by_type - .get(&self.config.quorum_type()) - .ok_or(Error::Execution(ExecutionError::DashCoreBadResponseError( - format!( - "expected quorums of type {}, but did not receive any from Dash Core", - self.config.quorum_type - ), - )))?; + .get_quorum_listextended(Some(core_block_height))?; + let quorum_info = quorum_list + .quorums_by_type + .get(&self.config.quorum_type()) + .ok_or(Error::Execution(ExecutionError::DashCoreBadResponseError( + format!( + "expected quorums of type {}, but did not receive any from Dash Core", + self.config.quorum_type + ), + )))?; + + tracing::debug!( + method = "update_quorum_info_v0", + "old {:?}", + block_platform_state.validator_sets() + ); + + tracing::debug!( + method = "update_quorum_info_v0", + "new quorum_info {:?}", + quorum_info + ); // Remove validator_sets entries that are no longer valid for the core block height block_platform_state .validator_sets_mut() - .retain(|quorum_hash, _| { - let has_quorum = validator_quorums_list.contains_key::(quorum_hash); + .retain(|key, _| quorum_info.contains_key(key)); - if has_quorum { - tracing::trace!( - ?quorum_hash, - quorum_type = ?self.config.quorum_type(), - "remove validator set {} with quorum type {}", - quorum_hash, - self.config.quorum_type() - ) - } - - has_quorum - }); - - // Fetch quorum info and their keys from the RPC for new quorums - let mut quorum_infos = validator_quorums_list + // Fetch quorum info results and their keys from the RPC + let mut quorum_infos = quorum_info .iter() .filter(|(key, _)| { !block_platform_state @@ -95,7 +88,6 @@ where let quorum_info_result = self.core_rpc .get_quorum_info(self.config.quorum_type(), key, None)?; - Ok((*key, quorum_info_result)) }) .collect::, Error>>()?; @@ -110,39 +102,27 @@ where } }); - // Map to validator sets - let new_validator_sets = quorum_infos + // Map to quorums + let new_quorums = quorum_infos .into_iter() - .map(|(quorum_hash, info_result)| { + .map(|(key, info_result)| { let validator_set = ValidatorSet::V0(ValidatorSetV0::try_from_quorum_info_result( info_result, block_platform_state, )?); - - tracing::trace!( - ?validator_set, - ?quorum_hash, - quorum_type = ?self.config.quorum_type(), - "add new validator set {} with quorum type {}", - quorum_hash, - self.config.quorum_type() - ); - - Ok((quorum_hash, validator_set)) + Ok((key, validator_set)) }) .collect::, Error>>()?; - // Add new validator_sets entries block_platform_state .validator_sets_mut() - .extend(new_validator_sets); + .extend(new_quorums.into_iter()); - // Sort all validator sets into deterministic order by core block height of creation block_platform_state .validator_sets_mut() .sort_by(|_, quorum_a, _, quorum_b| { let primary_comparison = quorum_b.core_height().cmp(&quorum_a.core_height()); - if primary_comparison == std::cmp::Ordering::Equal { + if primary_comparison == Ordering::Equal { quorum_b .quorum_hash() .cmp(quorum_a.quorum_hash()) @@ -152,7 +132,23 @@ where } }); - block_platform_state.set_quorums_extended_info(all_quorums_by_type); + tracing::debug!( + method = "update_quorum_info_v0", + "new {:?}", + block_platform_state.validator_sets() + ); + + let quorums_by_type = quorum_list + .quorums_by_type + .into_iter() + .map(|(quorum_type, quorum_list)| { + let sorted_quorum_list = quorum_list.into_iter().collect(); + + (quorum_type, sorted_quorum_list) + }) + .collect(); + + block_platform_state.set_quorums_extended_info(quorums_by_type); Ok(()) } diff --git a/packages/rs-drive-abci/src/rpc/core.rs b/packages/rs-drive-abci/src/rpc/core.rs index d1b74005e1..a24a667462 100644 --- a/packages/rs-drive-abci/src/rpc/core.rs +++ b/packages/rs-drive-abci/src/rpc/core.rs @@ -9,12 +9,12 @@ use dashcore_rpc::json::GetTransactionResult; use dashcore_rpc::{Auth, Client, Error, RpcApi}; use dpp::dashcore::{hashes::Hash, InstantLock}; use serde_json::Value; -use std::collections::BTreeMap; +use std::collections::HashMap; use std::time::Duration; use tenderdash_abci::proto::types::CoreChainLock; /// Information returned by QuorumListExtended -pub type QuorumListExtendedInfo = BTreeMap; +pub type QuorumListExtendedInfo = HashMap; /// Core height must be of type u32 (Platform heights are u64) pub type CoreHeight = u32; @@ -51,13 +51,13 @@ pub trait CoreRPCLike { /// Get chain tips fn get_chain_tips(&self) -> Result; - /// Get list of quorums by type at a given height. + /// Get list of quorums at a given height. /// /// See - fn get_quorum_listextended_by_type( + fn get_quorum_listextended( &self, height: Option, - ) -> Result, Error>; + ) -> Result; /// Get quorum information. /// @@ -224,24 +224,11 @@ impl CoreRPCLike for DefaultCoreRPC { retry!(self.inner.get_chain_tips()) } - fn get_quorum_listextended_by_type( + fn get_quorum_listextended( &self, height: Option, - ) -> Result, Error> { - let all_quorums_list = get_quorum_listextended(&self.inner, height)?; - - // Sort in deterministic order - let sorted_quorums_by_type = all_quorums_list - .quorums_by_type - .into_iter() - .map(|(quorum_type, quorum_list)| { - let sorted_quorum_list: BTreeMap<_, _> = quorum_list.into_iter().collect(); - - (quorum_type, sorted_quorum_list) - }) - .collect(); - - Ok(sorted_quorums_by_type) + ) -> Result { + retry!(self.inner.get_quorum_listextended(height)) } fn get_quorum_info( @@ -323,10 +310,3 @@ impl CoreRPCLike for DefaultCoreRPC { retry!(self.inner.mnsync_status()) } } - -fn get_quorum_listextended( - inner: &Client, - height: Option, -) -> Result { - retry!(inner.get_quorum_listextended(height)) -} diff --git a/packages/rs-drive-abci/tests/strategy_tests/execution.rs b/packages/rs-drive-abci/tests/strategy_tests/execution.rs index 96d313147e..8ac9bf446a 100644 --- a/packages/rs-drive-abci/tests/strategy_tests/execution.rs +++ b/packages/rs-drive-abci/tests/strategy_tests/execution.rs @@ -260,13 +260,15 @@ pub(crate) fn run_chain_for_strategy( platform .core_rpc - .expect_get_quorum_listextended_by_type() + .expect_get_quorum_listextended() .returning(move |core_height: Option| { if !strategy.rotate_quorums { - Ok(BTreeMap::from([( - QuorumType::Llmq100_67, - quorums_details.clone().into_iter().collect(), - )])) + Ok(dashcore_rpc::dashcore_rpc_json::ExtendedQuorumListResult { + quorums_by_type: HashMap::from([( + QuorumType::Llmq100_67, + quorums_details.clone().into_iter().collect(), + )]), + }) } else { let core_height = core_height.expect("expected a core height"); // if we rotate quorums we shouldn't give back the same ones every time @@ -294,7 +296,9 @@ pub(crate) fn run_chain_for_strategy( .collect() }; - Ok(BTreeMap::from([(QuorumType::Llmq100_67, quorums)])) + Ok(dashcore_rpc::dashcore_rpc_json::ExtendedQuorumListResult { + quorums_by_type: HashMap::from([(QuorumType::Llmq100_67, quorums)]), + }) } }); @@ -308,7 +312,7 @@ pub(crate) fn run_chain_for_strategy( .expect_get_quorum_info() .returning(move |_, quorum_hash: &QuorumHash, _| { Ok(quorums_info - .get::(quorum_hash) + .get(quorum_hash) .unwrap_or_else(|| panic!("expected to get quorum {}", hex::encode(quorum_hash))) .clone()) }); @@ -521,7 +525,7 @@ pub(crate) fn start_chain_for_strategy( .expect("expected quorums to be initialized"); let current_quorum_with_test_info = quorums - .get::(¤t_quorum_hash) + .get(¤t_quorum_hash) .expect("expected a quorum to be found"); // init chain @@ -631,8 +635,7 @@ pub(crate) fn continue_chain_for_strategy( let mut total_withdrawals = vec![]; - let mut current_quorum_with_test_info = - quorums.get::(¤t_quorum_hash).unwrap(); + let mut current_quorum_with_test_info = quorums.get(¤t_quorum_hash).unwrap(); let mut validator_set_updates = BTreeMap::new(); @@ -659,8 +662,7 @@ pub(crate) fn continue_chain_for_strategy( epoch: Epoch::new(epoch_info.current_epoch_index).unwrap(), }; if current_quorum_with_test_info.quorum_hash != current_quorum_hash { - current_quorum_with_test_info = - quorums.get::(¤t_quorum_hash).unwrap(); + current_quorum_with_test_info = quorums.get(¤t_quorum_hash).unwrap(); } let proposer = current_quorum_with_test_info @@ -684,7 +686,7 @@ pub(crate) fn continue_chain_for_strategy( next_protocol_version, change_block_height, } = proposer_versions - .get::(&proposer.pro_tx_hash) + .get(&proposer.pro_tx_hash) .expect("expected to have version"); if &block_height >= change_block_height { *next_protocol_version diff --git a/packages/rs-drive/Cargo.toml b/packages/rs-drive/Cargo.toml index fbc4013db5..c923db94d8 100644 --- a/packages/rs-drive/Cargo.toml +++ b/packages/rs-drive/Cargo.toml @@ -91,5 +91,4 @@ full = [ "rust_decimal_macros", "lazy_static", ] -grovedb_operations_logging = [] verify = ["grovedb/verify", "grovedb-costs"] diff --git a/packages/rs-drive/src/drive/grove_operations/grove_apply_batch_with_add_costs/v0/mod.rs b/packages/rs-drive/src/drive/grove_operations/grove_apply_batch_with_add_costs/v0/mod.rs index d1c696c920..0763f56fe8 100644 --- a/packages/rs-drive/src/drive/grove_operations/grove_apply_batch_with_add_costs/v0/mod.rs +++ b/packages/rs-drive/src/drive/grove_operations/grove_apply_batch_with_add_costs/v0/mod.rs @@ -11,7 +11,6 @@ use grovedb::batch::{BatchApplyOptions, GroveDbOp}; use grovedb::TransactionArg; use grovedb_costs::storage_cost::removal::StorageRemovedBytes::BasicStorageRemoval; use grovedb_costs::storage_cost::transition::OperationStorageTransitionType; -use tracing::Level; impl Drive { /// Applies the given groveDB operations batch and gets and passes the costs to `push_drive_operation_result`. @@ -43,15 +42,6 @@ impl Drive { } } - // Clone ops only if we log them - #[cfg(feature = "grovedb_operations_logging")] - let maybe_ops_for_logs = if tracing::event_enabled!(target: "grovedb_operations", Level::TRACE) - { - Some(ops.clone()) - } else { - None - }; - let cost_context = self.grove.apply_batch_with_element_flags_update( ops.operations, Some(BatchApplyOptions { @@ -150,26 +140,6 @@ impl Drive { }, transaction, ); - - #[cfg(feature = "grovedb_operations_logging")] - if tracing::event_enabled!(target: "grovedb_operations", Level::TRACE) - && cost_context.value.is_ok() - { - let root_hash = self - .grove - .root_hash(transaction) - .unwrap() - .map_err(Error::GroveDB)?; - - tracing::trace!( - target = "grovedb_operations", - ops = ?maybe_ops_for_logs.unwrap(), - root_hash = ?root_hash, - is_transactional = transaction.is_some(), - "grovedb batch applied", - ); - } - push_drive_operation_result(cost_context, drive_operations) } } diff --git a/packages/rs-drive/src/drive/grove_operations/grove_clear/v0/mod.rs b/packages/rs-drive/src/drive/grove_operations/grove_clear/v0/mod.rs index 1b41f82bf6..9777b56744 100644 --- a/packages/rs-drive/src/drive/grove_operations/grove_clear/v0/mod.rs +++ b/packages/rs-drive/src/drive/grove_operations/grove_clear/v0/mod.rs @@ -3,7 +3,6 @@ use crate::error::Error; use grovedb::operations::delete::ClearOptions; use grovedb::TransactionArg; use grovedb_path::SubtreePath; -use tracing::Level; impl Drive { /// Pushes the `OperationCost` of deleting an element in groveDB to `drive_operations`. @@ -17,39 +16,10 @@ impl Drive { allow_deleting_subtrees: false, trying_to_clear_with_subtrees_returns_error: false, }; - - #[cfg(feature = "grovedb_operations_logging")] - let maybe_path_for_logs = if tracing::event_enabled!(target: "grovedb_operations", Level::TRACE) - { - Some(path.clone()) - } else { - None - }; - // we will always return true if there is no error when we don't check for subtrees - let result = self - .grove + self.grove .clear_subtree(path, Some(options), transaction) .map_err(Error::GroveDB) - .map(|_| ()); - - #[cfg(feature = "grovedb_operations_logging")] - if tracing::event_enabled!(target: "grovedb_operations", Level::TRACE) && result.is_ok() { - let root_hash = self - .grove - .root_hash(transaction) - .unwrap() - .map_err(Error::GroveDB)?; - - tracing::trace!( - target = "grovedb_operations", - path = ?maybe_path_for_logs.unwrap().to_vec(), - root_hash = ?root_hash, - is_transactional = transaction.is_some(), - "grovedb clear", - ); - } - - result + .map(|_| ()) } } From 23449db3fced3394b3975d94aa23344921a0d58c Mon Sep 17 00:00:00 2001 From: Ivan Shumkov Date: Sat, 7 Oct 2023 08:51:22 +0700 Subject: [PATCH 6/6] tests: remove array field from fixture --- packages/rs-dpp/src/tests/fixtures/get_data_contract.rs | 6 ------ packages/rs-drive/tests/deterministic_root_hash.rs | 2 +- 2 files changed, 1 insertion(+), 7 deletions(-) diff --git a/packages/rs-dpp/src/tests/fixtures/get_data_contract.rs b/packages/rs-dpp/src/tests/fixtures/get_data_contract.rs index 4f10cfb544..6cd424bd04 100644 --- a/packages/rs-dpp/src/tests/fixtures/get_data_contract.rs +++ b/packages/rs-dpp/src/tests/fixtures/get_data_contract.rs @@ -132,12 +132,6 @@ pub fn get_data_contract_fixture( "name": { "type": "string" }, - "array": { - "type": "array", - "items": { - "type": "number" - } - } }, "additionalProperties": false }, diff --git a/packages/rs-drive/tests/deterministic_root_hash.rs b/packages/rs-drive/tests/deterministic_root_hash.rs index e221b6affc..a08b920631 100644 --- a/packages/rs-drive/tests/deterministic_root_hash.rs +++ b/packages/rs-drive/tests/deterministic_root_hash.rs @@ -450,7 +450,7 @@ fn test_root_hash_with_batches(drive: &Drive, db_transaction: &Transaction) { .unwrap() .expect("should return app hash"); - let expected_app_hash = "e2337ce8f1edc0d0c27c7efb8050ce2ca92c7431e5b43686f84e9f8e96a28fd9"; + let expected_app_hash = "1fecc17a75bf6e3c149adf78086f54a85a240d0741fd356204f7817ccb72c32d"; assert_eq!(hex::encode(app_hash), expected_app_hash); }