Skip to content
This repository has been archived by the owner on Nov 6, 2020. It is now read-only.

Propagate transactions for next 4 blocks. #9265

Merged
merged 1 commit into from
Aug 2, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
5 changes: 1 addition & 4 deletions ethcore/light/src/net/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -70,9 +70,6 @@ const PROPAGATE_TIMEOUT_INTERVAL: Duration = Duration::from_secs(5);
const RECALCULATE_COSTS_TIMEOUT: TimerToken = 3;
const RECALCULATE_COSTS_INTERVAL: Duration = Duration::from_secs(60 * 60);

/// Max number of transactions in a single packet.
const MAX_TRANSACTIONS_TO_PROPAGATE: usize = 64;

// minimum interval between updates.
const UPDATE_INTERVAL: Duration = Duration::from_millis(5000);

Expand Down Expand Up @@ -648,7 +645,7 @@ impl LightProtocol {
fn propagate_transactions(&self, io: &IoContext) {
if self.capabilities.read().tx_relay { return }

let ready_transactions = self.provider.ready_transactions(MAX_TRANSACTIONS_TO_PROPAGATE);
let ready_transactions = self.provider.transactions_to_propagate();
if ready_transactions.is_empty() { return }

trace!(target: "pip", "propagate transactions: {} ready", ready_transactions.len());
Expand Down
4 changes: 2 additions & 2 deletions ethcore/light/src/net/tests/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -171,8 +171,8 @@ impl Provider for TestProvider {
})
}

fn ready_transactions(&self, max_len: usize) -> Vec<PendingTransaction> {
self.0.client.ready_transactions(max_len)
fn transactions_to_propagate(&self) -> Vec<PendingTransaction> {
self.0.client.transactions_to_propagate()
}
}

Expand Down
14 changes: 6 additions & 8 deletions ethcore/light/src/provider.rs
Original file line number Diff line number Diff line change
Expand Up @@ -128,7 +128,7 @@ pub trait Provider: Send + Sync {
fn header_proof(&self, req: request::CompleteHeaderProofRequest) -> Option<request::HeaderProofResponse>;

/// Provide pending transactions.
fn ready_transactions(&self, max_len: usize) -> Vec<PendingTransaction>;
fn transactions_to_propagate(&self) -> Vec<PendingTransaction>;

/// Provide a proof-of-execution for the given transaction proof request.
/// Returns a vector of all state items necessary to execute the transaction.
Expand Down Expand Up @@ -283,8 +283,8 @@ impl<T: ProvingBlockChainClient + ?Sized> Provider for T {
.map(|(_, proof)| ::request::ExecutionResponse { items: proof })
}

fn ready_transactions(&self, max_len: usize) -> Vec<PendingTransaction> {
BlockChainClient::ready_transactions(self, max_len)
fn transactions_to_propagate(&self) -> Vec<PendingTransaction> {
BlockChainClient::transactions_to_propagate(self)
.into_iter()
.map(|tx| tx.pending().clone())
.collect()
Expand Down Expand Up @@ -370,12 +370,10 @@ impl<L: AsLightClient + Send + Sync> Provider for LightProvider<L> {
None
}

fn ready_transactions(&self, max_len: usize) -> Vec<PendingTransaction> {
fn transactions_to_propagate(&self) -> Vec<PendingTransaction> {
let chain_info = self.chain_info();
let mut transactions = self.txqueue.read()
.ready_transactions(chain_info.best_block_number, chain_info.best_block_timestamp);
transactions.truncate(max_len);
transactions
self.txqueue.read()
.ready_transactions(chain_info.best_block_number, chain_info.best_block_timestamp)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is uncapped right? It's probably not a problem for the light client but I think it should be bounded.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, but like clients store only their own transactions anyway, they don't relay anything at all.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Makes sense, since they'll only propagate local transactions shouldn't be an issue.

}
}

Expand Down
21 changes: 20 additions & 1 deletion ethcore/src/client/client.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
// along with Parity. If not, see <http://www.gnu.org/licenses/>.

use std::collections::{HashSet, BTreeMap, VecDeque};
use std::cmp;
use std::fmt;
use std::str::FromStr;
use std::sync::atomic::{AtomicUsize, AtomicBool, Ordering as AtomicOrdering};
Expand Down Expand Up @@ -1944,7 +1945,25 @@ impl BlockChainClient for Client {
(*self.build_last_hashes(&self.chain.read().best_block_hash())).clone()
}

fn ready_transactions(&self, max_len: usize) -> Vec<Arc<VerifiedTransaction>> {
fn transactions_to_propagate(&self) -> Vec<Arc<VerifiedTransaction>> {
const PROPAGATE_FOR_BLOCKS: u32 = 4;
const MIN_TX_TO_PROPAGATE: usize = 256;

let block_gas_limit = *self.best_block_header().gas_limit();
let min_tx_gas: U256 = self.latest_schedule().tx_gas.into();

let max_len = if min_tx_gas.is_zero() {
usize::max_value()
} else {
cmp::max(
MIN_TX_TO_PROPAGATE,
cmp::min(
(block_gas_limit / min_tx_gas) * PROPAGATE_FOR_BLOCKS,
// never more than usize
usize::max_value().into()
).as_u64() as usize
)
};
self.importer.miner.ready_transactions(self, max_len, ::miner::PendingOrdering::Priority)
}

Expand Down
4 changes: 2 additions & 2 deletions ethcore/src/client/test_client.rs
Original file line number Diff line number Diff line change
Expand Up @@ -805,8 +805,8 @@ impl BlockChainClient for TestBlockChainClient {
self.traces.read().clone()
}

fn ready_transactions(&self, max_len: usize) -> Vec<Arc<VerifiedTransaction>> {
self.miner.ready_transactions(self, max_len, miner::PendingOrdering::Priority)
fn transactions_to_propagate(&self) -> Vec<Arc<VerifiedTransaction>> {
self.miner.ready_transactions(self, 4096, miner::PendingOrdering::Priority)
}

fn signing_chain_id(&self) -> Option<u64> { None }
Expand Down
4 changes: 2 additions & 2 deletions ethcore/src/client/traits.rs
Original file line number Diff line number Diff line change
Expand Up @@ -320,8 +320,8 @@ pub trait BlockChainClient : Sync + Send + AccountData + BlockChain + CallContra
/// Get last hashes starting from best block.
fn last_hashes(&self) -> LastHashes;

/// List all transactions that are allowed into the next block.
fn ready_transactions(&self, max_len: usize) -> Vec<Arc<VerifiedTransaction>>;
/// List all ready transactions that should be propagated to other peers.
fn transactions_to_propagate(&self) -> Vec<Arc<VerifiedTransaction>>;

/// Sorted list of transaction gas prices from at least last sample_size blocks.
fn gas_price_corpus(&self, sample_size: usize) -> ::stats::Corpus<U256> {
Expand Down
4 changes: 2 additions & 2 deletions ethcore/src/tests/client.rs
Original file line number Diff line number Diff line change
Expand Up @@ -334,11 +334,11 @@ fn does_not_propagate_delayed_transactions() {

client.miner().import_own_transaction(&*client, tx0).unwrap();
client.miner().import_own_transaction(&*client, tx1).unwrap();
assert_eq!(0, client.ready_transactions(10).len());
assert_eq!(0, client.transactions_to_propagate().len());
assert_eq!(0, client.miner().ready_transactions(&*client, 10, PendingOrdering::Priority).len());
push_blocks_to_client(&client, 53, 2, 2);
client.flush_queue();
assert_eq!(2, client.ready_transactions(10).len());
assert_eq!(2, client.transactions_to_propagate().len());
assert_eq!(2, client.miner().ready_transactions(&*client, 10, PendingOrdering::Priority).len());
}

Expand Down
6 changes: 0 additions & 6 deletions ethcore/sync/src/chain/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -149,12 +149,6 @@ const MAX_NEW_HASHES: usize = 64;
const MAX_NEW_BLOCK_AGE: BlockNumber = 20;
// maximal packet size with transactions (cannot be greater than 16MB - protocol limitation).
const MAX_TRANSACTION_PACKET_SIZE: usize = 8 * 1024 * 1024;
// Maximal number of transactions queried from miner to propagate.
// This set is used to diff with transactions known by the peer and
// we will send a difference of length up to `MAX_TRANSACTIONS_TO_PROPAGATE`.
const MAX_TRANSACTIONS_TO_QUERY: usize = 4096;
// Maximal number of transactions in sent in single packet.
const MAX_TRANSACTIONS_TO_PROPAGATE: usize = 64;
// Min number of blocks to be behind for a snapshot sync
const SNAPSHOT_RESTORE_THRESHOLD: BlockNumber = 30000;
const SNAPSHOT_MIN_PEERS: usize = 3;
Expand Down
7 changes: 2 additions & 5 deletions ethcore/sync/src/chain/propagator.rs
Original file line number Diff line number Diff line change
Expand Up @@ -29,11 +29,9 @@ use transaction::SignedTransaction;
use super::{
random,
ChainSync,
MAX_TRANSACTION_PACKET_SIZE,
MAX_PEER_LAG_PROPAGATION,
MAX_PEERS_PROPAGATION,
MAX_TRANSACTION_PACKET_SIZE,
MAX_TRANSACTIONS_TO_PROPAGATE,
MAX_TRANSACTIONS_TO_QUERY,
MIN_PEERS_PROPAGATION,
CONSENSUS_DATA_PACKET,
NEW_BLOCK_HASHES_PACKET,
Expand Down Expand Up @@ -121,7 +119,7 @@ impl SyncPropagator {
return 0;
}

let transactions = io.chain().ready_transactions(MAX_TRANSACTIONS_TO_QUERY);
let transactions = io.chain().transactions_to_propagate();
if transactions.is_empty() {
return 0;
}
Expand Down Expand Up @@ -184,7 +182,6 @@ impl SyncPropagator {

// Get hashes of all transactions to send to this peer
let to_send = all_transactions_hashes.difference(&peer_info.last_sent_transactions)
.take(MAX_TRANSACTIONS_TO_PROPAGATE)
.cloned()
.collect::<HashSet<_>>();
if to_send.is_empty() {
Expand Down