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

Always Propose when empty_block_timeout is reached and remove redundant proposal conditions #1706

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Always Propose when empty_block_timeout is reached and remove redundant proposal conditions #1706

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
04a660c
feat: remove has_txn and exponential timeout check from block proposa…
86667 Oct 25, 2024
c5ceac6
feat: propose blocks as soon as empty_block_timeout is reached
86667 Oct 25, 2024
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
1 change: 0 additions & 1 deletion z2/src/docgen.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -382,7 +382,6 @@ pub fn get_implemented_jsonrpc_methods() -> Result<HashMap<ApiMethod, PageStatus
scilla_stdlib_dir: scilla_stdlib_dir_default(),
scilla_ext_libs_path: scilla_ext_libs_path_default(),
main_shard_id: None,
minimum_time_left_for_empty_block: minimum_time_left_for_empty_block_default(),
scilla_address: scilla_address_default(),
blocks_per_epoch: 3600,
epochs_per_checkpoint: 24,
Expand Down
4 changes: 1 addition & 3 deletions z2/src/setup.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,7 @@ use zilliqa::{
allowed_timestamp_skew_default, block_request_batch_size_default,
block_request_limit_default, consensus_timeout_default, disable_rpc_default,
empty_block_timeout_default, eth_chain_id_default, failed_request_sleep_duration_default,
local_address_default, max_blocks_in_flight_default,
minimum_time_left_for_empty_block_default, scilla_address_default,
local_address_default, max_blocks_in_flight_default, scilla_address_default,
scilla_ext_libs_path_default, scilla_stdlib_dir_default, state_rpc_limit_default,
total_native_token_supply_default, Amount, ConsensusConfig,
},
Expand Down Expand Up @@ -512,7 +511,6 @@ impl Setup {
scilla_address: scilla_address_default(),
scilla_stdlib_dir: scilla_stdlib_dir_default(),
scilla_ext_libs_path: scilla_ext_libs_path_default(),
minimum_time_left_for_empty_block: minimum_time_left_for_empty_block_default(),
main_shard_id: None,
local_address: local_address_default(),
consensus_timeout: consensus_timeout_default(),
Expand Down
4 changes: 0 additions & 4 deletions zilliqa/src/cfg.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -240,10 +240,6 @@ pub struct ConsensusConfig {
/// Minimum time to wait for consensus to propose new block if there are no transactions. This therefore acts also as the minimum block time.
#[serde(default = "empty_block_timeout_default")]
pub empty_block_timeout: Duration,
/// Minimum remaining time before end of round in which Proposer has the opportunity to broadcast empty block proposal.
/// If there is less time than this value left in a round then the view will likely move on before a proposal has time to be finalised.
#[serde(default = "minimum_time_left_for_empty_block_default")]
pub minimum_time_left_for_empty_block: Duration,
/// Address of the Scilla server. Defaults to "http://localhost:3000".
#[serde(default = "scilla_address_default")]
pub scilla_address: String,
Expand Down
70 changes: 18 additions & 52 deletions zilliqa/src/consensus.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -425,29 +425,16 @@ impl Consensus {
return Ok(None);
}

let (
time_since_last_view_change,
exponential_backoff_timeout,
minimum_time_left_for_empty_block,
) = self.get_consensus_timeout_params();
let (time_since_last_view_change, exponential_backoff_timeout, empty_block_timeout) =
self.get_consensus_timeout_params();

trace!(
"timeout reached create_next_block_on_timeout: {}",
self.create_next_block_on_timeout
);
if self.create_next_block_on_timeout {
let empty_block_timeout_ms =
self.config.consensus.empty_block_timeout.as_millis() as u64;

let has_txns_for_next_block = self.transaction_pool.has_txn_ready();

// Check if enough time elapsed or there's something in mempool or we don't have enough
// time but let's try at least until new view can happen
if time_since_last_view_change > empty_block_timeout_ms
|| has_txns_for_next_block
|| (time_since_last_view_change + minimum_time_left_for_empty_block
>= exponential_backoff_timeout)
{
// Check if enough time elapsed to propse block
if time_since_last_view_change > empty_block_timeout {
if let Ok(Some((block, transactions))) = self.propose_new_block() {
self.create_next_block_on_timeout = false;
return Ok(Some((
Expand All @@ -457,15 +444,14 @@ impl Consensus {
};
} else {
self.reset_timeout.send(Duration::from_millis(
empty_block_timeout_ms - time_since_last_view_change + 1,
empty_block_timeout - time_since_last_view_change + 1,
))?;
return Ok(None);
}
}

// Now consider whether we want to timeout - the timeout duration doubles every time, so it
// Should eventually have all nodes on the same view

if time_since_last_view_change < exponential_backoff_timeout {
trace!(
"Not proceeding with view change. Current view: {} - time since last: {}, timeout requires: {}",
Expand Down Expand Up @@ -532,23 +518,18 @@ impl Consensus {
// i.e. to get `consensus_timeout_ms * 2^0` we have to subtract 2 from `view_difference`
let exponential_backoff_timeout =
consensus_timeout_ms * 2u64.pow((view_difference as u32).saturating_sub(2));

let minimum_time_left_for_empty_block = self
.config
.consensus
.minimum_time_left_for_empty_block
.as_millis() as u64;
let empty_block_timeout = self.config.consensus.empty_block_timeout.as_millis() as u64;

trace!(
time_since_last_view_change,
exponential_backoff_timeout,
minimum_time_left_for_empty_block,
empty_block_timeout
);

(
time_since_last_view_change,
exponential_backoff_timeout,
minimum_time_left_for_empty_block,
empty_block_timeout,
)
}

Expand Down Expand Up @@ -1265,15 +1246,10 @@ impl Consensus {
// Assemble new block with whatever is in the mempool
while let Some(tx) = self.transaction_pool.best_transaction() {
// First - check if we have time left to process txns and give enough time for block propagation
let (
time_since_last_view_change,
exponential_backoff_timeout,
minimum_time_left_for_empty_block,
) = self.get_consensus_timeout_params();
let (time_since_last_view_change, _, empty_block_timeout) =
self.get_consensus_timeout_params();

if time_since_last_view_change + minimum_time_left_for_empty_block
>= exponential_backoff_timeout
{
if time_since_last_view_change > empty_block_timeout {
// don't have time, reinsert txn.
self.transaction_pool.insert_ready_transaction(tx);
break;
Expand Down Expand Up @@ -1367,15 +1343,10 @@ impl Consensus {
/// Either propose now or set timeout to allow for txs to come in.
fn ready_for_block_proposal(&mut self) -> Result<Option<(Block, Vec<VerifiedTransaction>)>> {
// Check if there's enough time to wait on a timeout and then propagate an empty block in the network before other participants trigger NewView
let (
time_since_last_view_change,
exponential_backoff_timeout,
minimum_time_left_for_empty_block,
) = self.get_consensus_timeout_params();
let (time_since_last_view_change, _, empty_block_timeout) =
self.get_consensus_timeout_params();

if time_since_last_view_change + minimum_time_left_for_empty_block
>= exponential_backoff_timeout
{
if time_since_last_view_change > empty_block_timeout {
return self.propose_new_block();
}

Expand Down Expand Up @@ -1445,16 +1416,11 @@ impl Consensus {
let pending = self.transaction_pool.pending_hashes();

for hash in pending.into_iter() {
// First - check for time
let (
time_since_last_view_change,
exponential_backoff_timeout,
minimum_time_left_for_empty_block,
) = self.get_consensus_timeout_params();
// First - check if there is time
let (time_since_last_view_change, _, empty_block_timeout) =
self.get_consensus_timeout_params();

if time_since_last_view_change + minimum_time_left_for_empty_block
>= exponential_backoff_timeout
{
if time_since_last_view_change > empty_block_timeout {
break;
}

Expand Down
15 changes: 0 additions & 15 deletions zilliqa/src/pool.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -340,21 +340,6 @@ impl TransactionPool {
self.hash_to_index.clear();
std::mem::take(&mut self.transactions).into_values()
}

/// Check the ready transactions in arbitrary order, for one that is Ready
pub fn has_txn_ready(&self) -> bool {
for ReadyItem { tx_index, .. } in self.ready.iter() {
// A transaction might have been ready, but it might have gotten popped
// or the sender's nonce might have increased, making it invalid. In this case,
// we will have a stale reference would still exist in the heap.
let Some(_) = self.transactions.get(tx_index) else {
continue;
};

return true;
}
false
}
}

#[cfg(test)]
Expand Down
11 changes: 3 additions & 8 deletions zilliqa/tests/it/main.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -67,10 +67,9 @@ use zilliqa::{
allowed_timestamp_skew_default, block_request_batch_size_default,
block_request_limit_default, disable_rpc_default, eth_chain_id_default,
failed_request_sleep_duration_default, json_rpc_port_default, local_address_default,
max_blocks_in_flight_default, minimum_time_left_for_empty_block_default,
scilla_address_default, scilla_ext_libs_path_default, scilla_stdlib_dir_default,
state_rpc_limit_default, total_native_token_supply_default, Amount, Checkpoint,
ConsensusConfig, NodeConfig,
max_blocks_in_flight_default, scilla_address_default, scilla_ext_libs_path_default,
scilla_stdlib_dir_default, state_rpc_limit_default, total_native_token_supply_default,
Amount, Checkpoint, ConsensusConfig, NodeConfig,
},
crypto::{NodePublicKey, SecretKey, TransactionPublicKey},
db,
Expand Down Expand Up @@ -314,7 +313,6 @@ impl Network {
genesis_deposits: genesis_deposits.clone(),
is_main: send_to_parent.is_none(),
consensus_timeout: Duration::from_secs(5),
minimum_time_left_for_empty_block: minimum_time_left_for_empty_block_default(),
// Give a genesis account 1 billion ZIL.
genesis_accounts: Self::genesis_accounts(&genesis_key),
empty_block_timeout: Duration::from_millis(25),
Expand Down Expand Up @@ -445,7 +443,6 @@ impl Network {
eth_block_gas_limit: EvmGas(84000000),
gas_price: 4_761_904_800_000u128.into(),
main_shard_id: None,
minimum_time_left_for_empty_block: minimum_time_left_for_empty_block_default(),
scilla_address: scilla_address_default(),
blocks_per_epoch: 10,
epochs_per_checkpoint: 1,
Expand Down Expand Up @@ -551,8 +548,6 @@ impl Network {
gas_price: 4_761_904_800_000u128.into(),
local_address: local_address_default(),
main_shard_id: None,
minimum_time_left_for_empty_block:
minimum_time_left_for_empty_block_default(),
scilla_address: scilla_address_default(),
blocks_per_epoch: 10,
epochs_per_checkpoint: 1,
Expand Down
6 changes: 2 additions & 4 deletions zilliqa/tests/it/persistence.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,9 +12,8 @@ use zilliqa::{
allowed_timestamp_skew_default, block_request_batch_size_default,
block_request_limit_default, consensus_timeout_default, eth_chain_id_default,
failed_request_sleep_duration_default, json_rpc_port_default, max_blocks_in_flight_default,
minimum_time_left_for_empty_block_default, scilla_address_default,
scilla_ext_libs_path_default, scilla_stdlib_dir_default, state_rpc_limit_default,
total_native_token_supply_default, Checkpoint,
scilla_address_default, scilla_ext_libs_path_default, scilla_stdlib_dir_default,
state_rpc_limit_default, total_native_token_supply_default, Checkpoint,
},
crypto::{Hash, SecretKey},
transaction::EvmGas,
Expand Down Expand Up @@ -104,7 +103,6 @@ async fn block_and_tx_data_persistence(mut network: Network) {
consensus_timeout: consensus_timeout_default(),
genesis_deposits: Vec::new(),
main_shard_id: None,
minimum_time_left_for_empty_block: minimum_time_left_for_empty_block_default(),
scilla_address: scilla_address_default(),
blocks_per_epoch: 10,
epochs_per_checkpoint: 1,
Expand Down
Loading