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

fix(light account response): update tx_queue on every account request/response #10545

Merged
merged 1 commit into from
Mar 31, 2019
Merged

fix(light account response): update tx_queue on every account request/response #10545

merged 1 commit into from
Mar 31, 2019

Conversation

niklasad1
Copy link
Collaborator

This PR changes so we update the transaction queue after every account request/response instead of having to wait every 10 minutes for the tx cull timer to expire.

It should be relatively cheap, requires to look a given key and loop through the transactions to find old ones and remove them!

However, should make the light client more responsive when more requests are performed

@niklasad1 niklasad1 changed the title fix(light account response): update tx_queue fix(light account response): update tx_queue on every account request/response Mar 28, 2019
@niklasad1 niklasad1 added A0-pleasereview 🤓 Pull request needs code review. B1-patch-beta 🕷🕷 M4-core ⛓ Core client code / Rust. B0-patch-stable 🕷 Pull request should also be back-ported to the stable branch. labels Mar 28, 2019
@niklasad1 niklasad1 added this to the 2.5 milestone Mar 28, 2019
ordian
ordian approved these changes Mar 29, 2019
View reviewed changes
Copy link
Collaborator

@ordian ordian 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. We keep the cull timer (in parity/light_helpers/queue_cull.rs), do we still need it?

@ordian ordian added A8-looksgood 🦄 Pull request is reviewed well. and removed A0-pleasereview 🤓 Pull request needs code review. labels Mar 29, 2019
@Tbaut Tbaut requested a review from cheme March 29, 2019 12:02
@niklasad1
Copy link
Collaborator Author

We keep the cull timer (in parity/light_helpers/queue_cull.rs), do we still need it?

It is possible that the account is never touched after sending a transaction for example then the queue will never be culled if we remove the cull_timer.

However, I'm working on get rid of the cull_timer completely but I'll take of it in another PR

@soc1c soc1c added A7-looksgoodcantmerge 🙄 Pull request is reviewed well, but cannot be merged due to conflicts. and removed A8-looksgood 🦄 Pull request is reviewed well. labels Mar 31, 2019
@soc1c
Copy link
Contributor

soc1c commented Mar 31, 2019

please rebase

@niklasad1 niklasad1 force-pushed the light-always-update-txqueue-after-account-request branch from a5d247d to 184b615 Compare March 31, 2019 09:20
@soc1c soc1c added A8-looksgood 🦄 Pull request is reviewed well. and removed A7-looksgoodcantmerge 🙄 Pull request is reviewed well, but cannot be merged due to conflicts. labels Mar 31, 2019
@soc1c soc1c merged commit 89f828b into master Mar 31, 2019
@soc1c soc1c deleted the light-always-update-txqueue-after-account-request branch March 31, 2019 09:54
This was referenced Mar 31, 2019
Merged
Merged
niklasad1 added a commit that referenced this pull request Apr 1, 2019
@niklasad1
fix(light account response): update tx_queue (#10545)
449a782
@niklasad1 niklasad1 mentioned this pull request Apr 1, 2019
Merged
8 tasks
soc1c pushed a commit that referenced this pull request Apr 1, 2019
@niklasad1 @soc1c
2.4.4 more beta backports (#10554)
8f6911a 
* 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
@ordian
Merge branch 'master' into ao-lmdb
60aa4d2 
* 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.
Reviewers

@ordian ordian ordian approved these changes

@cheme cheme Awaiting requested review from cheme

Assignees
No one assigned
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
Milestone
2.5
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@niklasad1 @soc1c @ordian