Skip to content
This repository was archived by the owner on Nov 15, 2023. It is now read-only.
Merged
Changes from 1 commit
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
Next Next commit
Fix is_valid condition when removing transactions from the pool.
  • Loading branch information
tomusdrw committed Aug 13, 2018
commit b622f09182785137ea35cb53c6fee7f6cdc069d7
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