-
Notifications
You must be signed in to change notification settings - Fork 2.7k
Reject too large transactions #558
Conversation
polkadot/transaction-pool/src/lib.rs
Outdated
|
|
||
| /// Maximal size of a single encoded extrinsic. | ||
| /// | ||
| /// See also substrate-consensus::MAX_TRANSACTIONS_SIZE |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hm was unable to find this!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good catch, I meant polkadot-consensus.
| 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)); |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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"
There was a problem hiding this comment.
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 : )
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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
* Set version as V0.9.10 * Change telemetery chain name * fix rpc trading_pairs overflow * fix rpc quotations overflow * Use account instead of authority_id in xsession initializetion Closes paritytech#558 * Update substrate * Update genesis * Add tty password * Modified team & council public key * Update concil as 5 * Update genesis btc-header * Update team and council account * update substrate update substrate to 96986d4
Move rejection from consensus to pool verifier, so that large transactions are not propagated around.