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

tx-pool: check transaction readiness before replacing #10526

Merged
merged 19 commits into from
Apr 1, 2019

Conversation

ascjones
Copy link
Contributor

@ascjones ascjones commented Mar 26, 2019

Depends on paritytech/parity-common#116.

When the pool becomes full, before replacing an existing transaction with a new one: check whether a Future transaction is not replacing a Ready one.

@ascjones ascjones requested a review from tomusdrw March 26, 2019 12:30
@ascjones ascjones added A0-pleasereview 🤓 Pull request needs code review. M4-core ⛓ Core client code / Rust. A1-onice 🌨 Pull request is reviewed well, but should not yet be merged. labels Mar 26, 2019
@ascjones
Copy link
Contributor Author

Waiting for paritytech/parity-common#116 to be merged and transaction-pool 2.0 to be released

@ascjones ascjones added A8-looksgood 🦄 Pull request is reviewed well. and removed A0-pleasereview 🤓 Pull request needs code review. A8-looksgood 🦄 Pull request is reviewed well. labels Mar 26, 2019
@soc1c soc1c added this to the 2.5 milestone Mar 26, 2019
@soc1c soc1c added B1-patch-beta 🕷🕷 B0-patch-stable 🕷 Pull request should also be back-ported to the stable branch. labels Mar 27, 2019
@ascjones ascjones added A8-looksgood 🦄 Pull request is reviewed well. and removed A1-onice 🌨 Pull request is reviewed well, but should not yet be merged. labels Mar 28, 2019
Copy link
Collaborator

@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.

Looks good, minor optimization possible

miner/src/pool/replace.rs Outdated Show resolved Hide resolved
This was referenced Mar 31, 2019
@soc1c soc1c added A7-looksgoodtestsfail 🤖 Pull request is reviewed well, but cannot be merged due to tests failing. and removed A8-looksgood 🦄 Pull request is reviewed well. labels Mar 31, 2019
@soc1c
Copy link
Contributor

soc1c commented Mar 31, 2019

This seemingly broke a test, I tried 3x with same test getting stuck:

test tests::private::send_private_transaction ... test tests::private::send_private_transaction has been running for over 60 seconds
ERROR: Job failed: execution took longer than 3h10m0s seconds

@ascjones ascjones added A8-looksgood 🦄 Pull request is reviewed well. and removed A7-looksgoodtestsfail 🤖 Pull request is reviewed well, but cannot be merged due to tests failing. labels Apr 1, 2019
@soc1c soc1c merged commit d9673b0 into master Apr 1, 2019
@soc1c soc1c deleted the aj-tx-pool-should-replace branch April 1, 2019 08:49
@niklasad1 niklasad1 mentioned this pull request Apr 1, 2019
8 tasks
ascjones added a commit that referenced this pull request Apr 1, 2019
* Update to vanilla tx pool error

* Prevent a non ready tx replacing a ready tx

* Make tests compile

* Test ready tx not replaced by future tx

* Transaction indirection

* Use StateReadiness to calculate Ready in `should_replace`

* Test existing txs from same sender are used to compute Readiness

* private-tx: Wire up ShouldReplace

* Revert "Use StateReadiness to calculate Ready in `should_replace`"

This reverts commit af9e69c

* Make replace generic so it works with private-tx

* Rename Replace and add missing docs

* ShouldReplace no longer mutable

* tx-pool: update to transaction-pool 2.0 from crates.io

* tx-pool: generic error type alias

* Exit early for first unmatching nonce

* Fix private-tx test, use existing write lock

* Use read lock for pool scoring
soc1c pushed a commit that referenced this pull request Apr 1, 2019
* fix(rpc-types): replace uint and hash with `ethereum_types v0.4` (#10217)

* fix(rpc-types): remove uint and hash wrappers

* fix(tests)

* fix(cleanup)

* grumbles(rpc-api): revert `verify_signature`

* revert change of `U64` -> `u64`

* fix(cleanup after bad merge)

* chore(bump ethereum-types)

* fix(bad merge)

* feat(tests ethereum-types): add tests

* chore(update `ethereum-types` to 0.4.2)

* feat(tests for h256)

* chore(rpc): remove `ethbloom` import

Use re-export from `ethereum-types` instead

* fix(bad merge): remove `DefaultAccount` type

* doc(add TODO with issue link)

* chore(bump ethereum-types) (#10396)

Fixes a de-serialization bug in `ethereum-tyes`

* fix(light eth_gasPrice): ask network if not in cache (#10535)

* fix(light eth_gasPrice): ask N/W if not in cache

* fix(bad rebase)

* fix(light account response): update `tx_queue` (#10545)

* fix(bump dependencies) (#10540)

* cargo update -p log:0.4.5

* cargo update -p regex:1.0.5

* cargo update -p parking_lot

* cargo update -p serde_derive

* cargo update -p serde_json

* cargo update -p serde

* cargo update -p lazy_static

* cargo update -p num_cpus

* cargo update -p toml

# Conflicts:
#	Cargo.lock

* tx-pool: check transaction readiness before replacing (#10526)

* Update to vanilla tx pool error

* Prevent a non ready tx replacing a ready tx

* Make tests compile

* Test ready tx not replaced by future tx

* Transaction indirection

* Use StateReadiness to calculate Ready in `should_replace`

* Test existing txs from same sender are used to compute Readiness

* private-tx: Wire up ShouldReplace

* Revert "Use StateReadiness to calculate Ready in `should_replace`"

This reverts commit af9e69c

* Make replace generic so it works with private-tx

* Rename Replace and add missing docs

* ShouldReplace no longer mutable

* tx-pool: update to transaction-pool 2.0 from crates.io

* tx-pool: generic error type alias

* Exit early for first unmatching nonce

* Fix private-tx test, use existing write lock

* Use read lock for pool scoring

* fix #10390 (#10391)

* private-tx: replace error_chain (#10510)

* Update to vanilla tx pool error

* private-tx: remove error-chain, implement Error, derive Display

* private-tx: replace ErrorKind and bail!

* private-tx: add missing From impls and other compiler errors

* private-tx: use original tx-pool error

* Don't be silly cargo
ordian added a commit that referenced this pull request Apr 5, 2019
* master:
  fix(light cull): poll light cull instead of timer (#10559)
  Update Issue Template to direct security issue to email (#10562)
  RPC: Implements eth_subscribe("syncing") (#10311)
  Explicitly enable or disable Stratum in config file (Issue 9785) (#10521)
  version: bump master to 2.6 (#10560)
  tx-pool: check transaction readiness before replacing (#10526)
  fix(light account response): update `tx_queue` (#10545)
  Update light client harcoded headers (#10547)
  fix(light eth_gasPrice): ask network if not in cache (#10535)
  Implement caching for service transactions checker (#10088)
  build android with cache, win fixes (#10546)
  clique: make state backfill time measurement more accurate (#10551)
  updated lru-cache to 0.1.2 (#10542)
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
A8-looksgood 🦄 Pull request is reviewed well. B0-patch-stable 🕷 Pull request should also be back-ported to the stable branch. M4-core ⛓ Core client code / Rust.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants