Skip to content
This repository was archived by the owner on Nov 15, 2023. It is now read-only.
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
6 changes: 0 additions & 6 deletions polkadot/consensus/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -770,12 +770,6 @@ impl<C> CreateProposal<C> where C: PolkadotApi {
let result = self.transaction_pool.cull_and_get_pending(BlockId::hash(self.parent_hash), |pending_iterator| {
let mut pending_size = 0;
for pending in pending_iterator {
// skip and cull transactions which are too large.
if pending.encoded_size() > MAX_TRANSACTIONS_SIZE {
unqueue_invalid.push(pending.hash().clone());
continue
}

if pending_size + pending.encoded_size() >= MAX_TRANSACTIONS_SIZE { break }

match block_builder.push_extrinsic(pending.primitive_extrinsic()) {
Expand Down
5 changes: 5 additions & 0 deletions polkadot/transaction-pool/src/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,11 @@ error_chain! {
description("Unrecognised address in extrinsic"),
display("Unrecognised address in extrinsic: {}", who),
}
/// Extrinsic too large
TooLarge(got: usize, max: usize) {
description("Extrinsic too large"),
display("Extrinsic is too large ({} > {})", got, max),
}
}
}

Expand Down
13 changes: 11 additions & 2 deletions polkadot/transaction-pool/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,11 @@ use substrate_runtime_primitives::traits::{Bounded, Checkable, Hash as HashT, Bl
pub use extrinsic_pool::txpool::{Options, Status, LightStatus, VerifiedTransaction as VerifiedTransactionOps};
pub use error::{Error, ErrorKind, Result};

/// Maximal size of a single encoded extrinsic.
///
/// See also polkadot-consensus::MAX_TRANSACTIONS_SIZE
const MAX_TRANSACTION_SIZE: usize = 4 * 1024 * 1024;

/// Type alias for convenience.
pub type CheckedExtrinsic = <UncheckedExtrinsic as Checkable<fn(Address) -> std::result::Result<AccountId, &'static str>>>::Checked;

Expand Down Expand Up @@ -279,14 +284,18 @@ impl<'a, A> txpool::Verifier<UncheckedExtrinsic> for Verifier<'a, A> where
type Error = Error;

fn verify_transaction(&self, uxt: UncheckedExtrinsic) -> Result<Self::VerifiedTransaction> {

if !uxt.is_signed() {
bail!(ErrorKind::IsInherent(uxt))
}

let encoded = uxt.encode();
let (encoded_size, hash) = (encoded.len(), BlakeTwo256::hash(&encoded));
let encoded_size = encoded.len();

if encoded_size > MAX_TRANSACTION_SIZE {
bail!(ErrorKind::TooLarge(encoded_size, MAX_TRANSACTION_SIZE));
}

let hash = BlakeTwo256::hash(&encoded);
debug!(target: "transaction-pool", "Transaction submitted: {}", ::substrate_primitives::hexdisplay::HexDisplay::from(&encoded));

let inner = match uxt.clone().check_with(|a| self.lookup(a)) {
Expand Down
2 changes: 1 addition & 1 deletion substrate/extrinsic-pool/src/pool.rs
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,7 @@ impl<Hash, VEx, S, E> Pool<Hash, VEx, S, E> where
let mut pool = self.pool.write();
let mut results = Vec::with_capacity(hashes.len());
for hash in hashes {
results.push(pool.remove(hash, is_valid));
results.push(pool.remove(hash, !is_valid));
Copy link
Contributor

@pepyakin pepyakin Aug 13, 2018

Choose a reason for hiding this comment

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

I'm not sure I follow this one. Why is it inverted here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The transaction-pool API expects an inverted bool (is_invalid) - that was a bug that was causing the transactions to be logged as "canceled" instead of "invalid"

Copy link
Contributor

@pepyakin pepyakin Aug 13, 2018

Choose a reason for hiding this comment

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

Oh, maybe we should rename it then, since it's presently named is_valid? This is actually where my confusion comes from : )

https://github.com/paritytech/polkadot/blob/e89b8aa7e4ceb2b8f1348e027d61ebb9c820eec4/polkadot/transaction-pool/src/lib.rs#L389

Copy link
Contributor Author

Choose a reason for hiding this comment

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

So the composition looks like this:

polkadot-transaction-pool -> substrate-extrinsic-pool -> [parity-]transaction-pool

The code in polkadot transaction pool is fine it uses is_valid and passes it further to extrinsics pool. Extrinsics pool uses the external transaction-pool (take from ethereum/parity) and that one is using is_invalid (https://docs.rs/transaction-pool/1.12.1/transaction_pool/struct.Pool.html#method.remove) hence the inversion here.

Copy link
Contributor

Choose a reason for hiding this comment

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

I can see it now! I mistakingly assumed that transaction_pool as txpool refers to polkadot-transaction-pool

}
results
}
Expand Down