Skip to content
This repository was archived by the owner on Nov 15, 2023. It is now read-only.

Conversation

tomusdrw
Copy link
Contributor

If the pool occupies the queue for more than 5 minutes:

  1. Remove it from the pool
  2. Ban for next 30 minutes

@tomusdrw tomusdrw added A0-please_review Pull request needs code review. M4-core labels Aug 23, 2018
@tomusdrw tomusdrw requested a review from arkpar August 23, 2018 14:01

/// Removes timed bans.
pub fn clear_timeouts(&self, now: &Instant) {
let to_remove = {
Copy link
Member

Choose a reason for hiding this comment

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

Why is this in {}s?


/// Removes timed bans.
pub fn clear_timeouts(&self, now: &Instant) {
let to_remove = {
Copy link
Member

Choose a reason for hiding this comment

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

Why the {}s?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We were collecting the keys to remove using only the .read() lock. I changed the code though to acquire the lock only once.

fn rotator() -> PoolRotator {
PoolRotator {
ban_time: Duration::from_millis(10),
..Default::default()
Copy link
Member

Choose a reason for hiding this comment

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

Nice :-)

Copy link
Contributor Author

@tomusdrw tomusdrw left a comment

Choose a reason for hiding this comment

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

Updated and fixed the borked merged (that resulted in duplicated code).


/// Removes timed bans.
pub fn clear_timeouts(&self, now: &Instant) {
let to_remove = {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

We were collecting the keys to remove using only the .read() lock. I changed the code though to acquire the lock only once.

@gavofyork gavofyork added A8-looksgood and removed A0-please_review Pull request needs code review. labels Aug 24, 2018
@gavofyork gavofyork merged commit 452726a into v0.2 Aug 24, 2018
@tomusdrw tomusdrw deleted the td-poolbanning branch August 24, 2018 14:15
tomusdrw added a commit that referenced this pull request Aug 24, 2018
* Allow replacing transactions.

* Clear old transactions and ban them temporarily.

* Move to a separate module and add some tests.

* Add bound to banned transactions.

* Remove unnecessary block and double PoolRotator.
rphmeier pushed a commit that referenced this pull request Aug 24, 2018
* Allow replacing transactions.

* Clear old transactions and ban them temporarily.

* Move to a separate module and add some tests.

* Add bound to banned transactions.

* Remove unnecessary block and double PoolRotator.
dvdplm added a commit that referenced this pull request Aug 27, 2018
…and-rlpcodec

* master:
  Contract signatures checking (#478)
  extrinsic-pool: use retain() (#613)
  rename Polkadot to Substrate in the license header via following four commands (#614)
  typo fixes (#608)
  RPC: Block number to block hash (#584)
  Minor fixes for nightly 2018-08-18 (#600)
  Time-based transaction banning (#594) (#602)
  cargo --force to allow CI to build. (#599)
  Fix logging (#587)
  Fix runtime version cache (#586)
liuchengxu pushed a commit to chainx-org/substrate that referenced this pull request Aug 23, 2021
* Fix xaccounts test build error

* Modified xasset module mock

* Update test

* xassets module test ok

* Update substrate
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants