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.

Sign up for GitHub

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

Jump to bottom

chore(drive): improve quorum info update logs #1444

Merged
merged 5 commits into from
Oct 6, 2023
Merged

chore(drive): improve quorum info update logs #1444

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
9631089
chore(drive): deterministic new quorum info log
shumkov Sep 29, 2023
a4e9663
chore(drive): improve logging
shumkov Oct 1, 2023
522f61c
feat: improve logging fields
shumkov Oct 6, 2023
cf537bc
feat: ensure deterministic order in Core RPC module
shumkov Oct 6, 2023
d6a038b
style: fix clippy error
shumkov Oct 6, 2023
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
120 changes: 62 additions & 58 deletions ...-drive-abci/src/execution/platform_events/core_based_updates/update_quorum_info/v0/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,8 @@
use crate::rpc::core::CoreRPCLike;

use dpp::dashcore::QuorumHash;
use std::cmp::Ordering;
use std::collections::BTreeMap;

Check warning on line 12 in packages/rs-drive-abci/src/execution/platform_events/core_based_updates/update_quorum_info/v0/mod.rs

View workflow job for this annotation

GitHub Actions / clippy

unused import: `std::collections::BTreeMap` 

warning: unused import: `std::collections::BTreeMap`
  --> packages/rs-drive-abci/src/execution/platform_events/core_based_updates/update_quorum_info/v0/mod.rs:12:5
   |
12 | use std::collections::BTreeMap;
   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^
use tracing::Level;

impl<C> Platform<C>
where
Expand All @@ -33,51 +33,58 @@
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!(
shumkov marked this conversation as resolved.
Show resolved Hide resolved
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::<QuorumHash>(quorum_hash);
shumkov marked this conversation as resolved.
Show resolved Hide resolved

// 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
Expand All @@ -88,6 +95,7 @@
let quorum_info_result =
self.core_rpc
.get_quorum_info(self.config.quorum_type(), key, None)?;

Ok((*key, quorum_info_result))
})
.collect::<Result<Vec<_>, Error>>()?;
Expand All @@ -102,27 +110,39 @@
}
});

// 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::<Result<Vec<_>, 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 {
shumkov marked this conversation as resolved.
Show resolved Hide resolved
quorum_b
.quorum_hash()
.cmp(quorum_a.quorum_hash())
Expand All @@ -132,23 +152,7 @@
}
});

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(())
}
Expand Down
2 changes: 0 additions & 2 deletions ...validation/state_transition/state_transitions/documents_batch/state/v0/fetch_documents.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
36 changes: 28 additions & 8 deletions packages/rs-drive-abci/src/rpc/core.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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<QuorumHash, ExtendedQuorumDetails>;
pub type QuorumListExtendedInfo = BTreeMap<QuorumHash, ExtendedQuorumDetails>;

/// Core height must be of type u32 (Platform heights are u64)
pub type CoreHeight = u32;
Expand Down Expand Up @@ -51,13 +51,13 @@ pub trait CoreRPCLike {
/// Get chain tips
fn get_chain_tips(&self) -> Result<GetChainTipsResult, Error>;

/// Get list of quorums at a given height.
/// Get list of quorums by type at a given height.
///
/// See <https://dashcore.readme.io/v19.0.0/docs/core-api-ref-remote-procedure-calls-evo#quorum-listextended>
fn get_quorum_listextended(
fn get_quorum_listextended_by_type(
&self,
height: Option<CoreHeight>,
) -> Result<ExtendedQuorumListResult, Error>;
) -> Result<BTreeMap<QuorumType, QuorumListExtendedInfo>, Error>;

/// Get quorum information.
///
Expand Down Expand Up @@ -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<CoreHeight>,
) -> Result<ExtendedQuorumListResult, Error> {
retry!(self.inner.get_quorum_listextended(height))
) -> Result<BTreeMap<QuorumType, QuorumListExtendedInfo>, 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(
Expand Down Expand Up @@ -310,3 +323,10 @@ impl CoreRPCLike for DefaultCoreRPC {
retry!(self.inner.mnsync_status())
}
}

fn get_quorum_listextended(
inner: &Client,
height: Option<CoreHeight>,
) -> Result<ExtendedQuorumListResult, Error> {
retry!(inner.get_quorum_listextended(height))
}
28 changes: 13 additions & 15 deletions packages/rs-drive-abci/tests/strategy_tests/execution.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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<u32>| {
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
Expand Down Expand Up @@ -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)]))
}
});

Expand All @@ -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::<QuorumHash>(quorum_hash)
shumkov marked this conversation as resolved.
Show resolved Hide resolved
.unwrap_or_else(|| panic!("expected to get quorum {}", hex::encode(quorum_hash)))
.clone())
});
Expand Down Expand Up @@ -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(&current_quorum_hash)
.get::<QuorumHash>(&current_quorum_hash)
shumkov marked this conversation as resolved.
Show resolved Hide resolved
.expect("expected a quorum to be found");

// init chain
Expand Down Expand Up @@ -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(&current_quorum_hash).unwrap();
let mut current_quorum_with_test_info =
quorums.get::<QuorumHash>(&current_quorum_hash).unwrap();
shumkov marked this conversation as resolved.
Show resolved Hide resolved

let mut validator_set_updates = BTreeMap::new();

Expand All @@ -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(&current_quorum_hash).unwrap();
current_quorum_with_test_info =
quorums.get::<QuorumHash>(&current_quorum_hash).unwrap();
shumkov marked this conversation as resolved.
Show resolved Hide resolved
}

let proposer = current_quorum_with_test_info
Expand All @@ -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::<ProTxHash>(&proposer.pro_tx_hash)
shumkov marked this conversation as resolved.
Show resolved Hide resolved
.expect("expected to have version");
if &block_height >= change_block_height {
*next_protocol_version
Expand Down
Loading