Skip to content

Commit

Permalink
configurable tx routing horizon (#10251)
Browse files Browse the repository at this point in the history
Community reports that sometimes it takes minutes for a transaction to
be recorded on chain.
My guess is that it takes that much time to record a transaction on
chain, because it gets forwarded to nodes which either:
* fail to produce their chunks
* don't have time to process incoming tx RPCs before producing their
chunks
* the node producing a tx is out of sync and the set of validators is
already too late

All of these can be fixed by multicasting transactions further into the
future.

This PR is the simplest attempt to send transactions further into the
future.

Tested: Had a testnet node and submitted a tx that was supposed to be
forwarded to chunk producers of the next 100 blocks. In practice it gets
forwarded to 5 validators. Sending a message to a validator takes about
100µs for a trivial tx.
  • Loading branch information
nikurt authored Nov 28, 2023
1 parent cbd52ee commit 984d7f9
Show file tree
Hide file tree
Showing 7 changed files with 51 additions and 10 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@

* Add prometheus metrics for the internal state of the doomslug. [#9458](https://github.com/near/nearcore/pull/9458)
* Fix `EXPERIMENTAL_protocol_config` to apply overrides from `EpochConfig`. [#9692](https://github.com/near/nearcore/pull/9692)
* Add config option `tx_routing_height_horizon` to configure how many chunk producers are notified about the tx. [#10251](https://github.com/near/nearcore/pull/10251)

## 1.36.0

Expand Down
3 changes: 0 additions & 3 deletions chain/chain/src/chain.rs
Original file line number Diff line number Diff line change
Expand Up @@ -131,9 +131,6 @@ const NUM_PARENTS_TO_CHECK_FINALITY: usize = 20;
#[cfg(not(feature = "sandbox"))]
const ACCEPTABLE_TIME_DIFFERENCE: i64 = 12 * 10;

/// Over this block height delta in advance if we are not chunk producer - route tx to upcoming validators.
pub const TX_ROUTING_HEIGHT_HORIZON: BlockHeightDelta = 4;

/// Private constant for 1 NEAR (copy from near/config.rs) used for reporting.
const NEAR_BASE: Balance = 1_000_000_000_000_000_000_000_000;

Expand Down
10 changes: 5 additions & 5 deletions chain/client/src/client.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ use near_async::messaging::{CanSend, Sender};
use near_chain::chain::VerifyBlockHashAndSignatureResult;
use near_chain::chain::{
ApplyStatePartsRequest, BlockCatchUpRequest, BlockMissingChunks, BlocksCatchUpState,
OrphanMissingChunks, TX_ROUTING_HEIGHT_HORIZON,
OrphanMissingChunks,
};
use near_chain::flat_storage_creator::FlatStorageCreator;
use near_chain::resharding::StateSplitRequest;
Expand Down Expand Up @@ -2009,8 +2009,8 @@ impl Client {
let maybe_next_epoch_id = self.get_next_epoch_id_if_at_boundary(&head)?;

let mut validators = HashSet::new();
for horizon in
(2..=TX_ROUTING_HEIGHT_HORIZON).chain(vec![TX_ROUTING_HEIGHT_HORIZON * 2].into_iter())
for horizon in (2..=self.config.tx_routing_height_horizon)
.chain(vec![self.config.tx_routing_height_horizon * 2].into_iter())
{
let target_height = head.height + horizon - 1;
let validator =
Expand Down Expand Up @@ -2074,7 +2074,7 @@ impl Client {
+ self.config.epoch_length;

let epoch_boundary_possible =
head.height + TX_ROUTING_HEIGHT_HORIZON >= next_epoch_estimated_height;
head.height + self.config.tx_routing_height_horizon >= next_epoch_estimated_height;
if epoch_boundary_possible {
Ok(Some(self.epoch_manager.get_next_epoch_id_from_prev_block(&head.last_block_hash)?))
} else {
Expand Down Expand Up @@ -2231,7 +2231,7 @@ impl Client {
return Ok(false);
};

for i in 1..=TX_ROUTING_HEIGHT_HORIZON {
for i in 1..=self.config.tx_routing_height_horizon {
let chunk_producer =
self.epoch_manager.get_chunk_producer(&epoch_id, head.height + i, shard_id)?;
if &chunk_producer == account_id {
Expand Down
3 changes: 1 addition & 2 deletions chain/client/src/view_client.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@ use crate::{
};
use actix::{Actor, Addr, Handler, SyncArbiter, SyncContext};
use near_async::messaging::CanSend;
use near_chain::chain::TX_ROUTING_HEIGHT_HORIZON;
use near_chain::types::{RuntimeAdapter, Tip};
use near_chain::{
get_epoch_block_producers_view, Chain, ChainGenesis, ChainStoreAccess, DoomslugThresholdMode,
Expand Down Expand Up @@ -531,7 +530,7 @@ impl ViewClientActor {
.epoch_manager
.get_chunk_producer(
&head.epoch_id,
head.height + TX_ROUTING_HEIGHT_HORIZON - 1,
head.height + self.config.tx_routing_height_horizon - 1,
target_shard_id,
)
.map_err(|err| TxStatusError::ChainError(err.into()))?;
Expand Down
4 changes: 4 additions & 0 deletions core/chain-configs/src/client_config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -302,6 +302,9 @@ pub struct ClientConfig {
pub enable_multiline_logging: bool,
// Configuration for resharding.
pub state_split_config: StateSplitConfig,
/// If the node is not a chunk producer within that many blocks, then route
/// to upcoming chunk producers.
pub tx_routing_height_horizon: BlockHeightDelta,
}

impl ClientConfig {
Expand Down Expand Up @@ -377,6 +380,7 @@ impl ClientConfig {
transaction_pool_size_limit: None,
enable_multiline_logging: false,
state_split_config: StateSplitConfig::default(),
tx_routing_height_horizon: 4,
}
}
}
10 changes: 10 additions & 0 deletions nearcore/src/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -194,6 +194,10 @@ fn default_transaction_pool_size_limit() -> Option<u64> {
Some(100_000_000) // 100 MB.
}

fn default_tx_routing_height_horizon() -> BlockHeightDelta {
4
}

#[derive(serde::Serialize, serde::Deserialize, Clone, Debug)]
pub struct Consensus {
/// Minimum number of peers to start syncing.
Expand Down Expand Up @@ -341,7 +345,11 @@ pub struct Config {
/// Setting this value too low (<1MB) on the validator might lead to production of smaller
/// chunks and underutilizing the capacity of the network.
pub transaction_pool_size_limit: Option<u64>,
// Configuration for resharding.
pub state_split_config: StateSplitConfig,
/// If the node is not a chunk producer within that many blocks, then route
/// to upcoming chunk producers.
pub tx_routing_height_horizon: BlockHeightDelta,
}

fn is_false(value: &bool) -> bool {
Expand Down Expand Up @@ -383,6 +391,7 @@ impl Default for Config {
transaction_pool_size_limit: default_transaction_pool_size_limit(),
enable_multiline_logging: None,
state_split_config: StateSplitConfig::default(),
tx_routing_height_horizon: default_tx_routing_height_horizon(),
}
}
}
Expand Down Expand Up @@ -680,6 +689,7 @@ impl NearConfig {
transaction_pool_size_limit: config.transaction_pool_size_limit,
enable_multiline_logging: config.enable_multiline_logging.unwrap_or(true),
state_split_config: config.state_split_config,
tx_routing_height_horizon: config.tx_routing_height_horizon,
},
network_config: NetworkConfig::new(
config.network,
Expand Down
30 changes: 30 additions & 0 deletions nearcore/src/config_validate.rs
Original file line number Diff line number Diff line change
Expand Up @@ -147,6 +147,16 @@ impl<'a> ConfigValidator<'a> {
}
}
}

let tx_routing_height_horizon = self.config.tx_routing_height_horizon;
if tx_routing_height_horizon < 2 {
let error_message = format!("'config.tx_routing_height_horizon' needs to be at least 2, got {tx_routing_height_horizon}.");
self.validation_errors.push_config_semantics_error(error_message);
}
if tx_routing_height_horizon > 100 {
let error_message = format!("'config.tx_routing_height_horizon' can't be too high to avoid spamming the network. Keep it below 100. Got {tx_routing_height_horizon}.");
self.validation_errors.push_config_semantics_error(error_message);
}
}

fn result_with_full_error(&self) -> Result<(), ValidationError> {
Expand Down Expand Up @@ -220,4 +230,24 @@ mod test {
config.save_trie_changes = Some(false);
validate_config(&config).unwrap();
}

#[test]
#[should_panic(
expected = "\\nconfig.json semantic issue: 'config.tx_routing_height_horizon' needs to be at least 2, got 1."
)]
fn test_tx_routing_height_horizon_too_low() {
let mut config = Config::default();
config.tx_routing_height_horizon = 1;
validate_config(&config).unwrap();
}

#[test]
#[should_panic(
expected = "\\nconfig.json semantic issue: 'config.tx_routing_height_horizon' can't be too high to avoid spamming the network. Keep it below 100. Got 1000000000."
)]
fn test_tx_routing_height_horizon_too_high() {
let mut config = Config::default();
config.tx_routing_height_horizon = 1_000_000_000;
validate_config(&config).unwrap();
}
}

0 comments on commit 984d7f9

Please sign in to comment.