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

Never drop local transactions from different senders. #9002

Merged
merged 2 commits into from
Jul 3, 2018

Conversation

tomusdrw
Copy link
Collaborator

#8981

In case we submit multiple transactions from the same sender and max_per_sender is reached the transaction we attempt to insert should fail with an error, although for multiple senders, one local sender will never push out local transactions from another sender - those will be accepted over the limit.

Warning!

Careful with #8820, exposing sendRawTransaction without running with the flag introduced in the other issue can lead to OOM.

@tomusdrw tomusdrw added A0-pleasereview 🤓 Pull request needs code review. B0-patchthis M4-core ⛓ Core client code / Rust. B7-releasenotes 📜 Changes should be mentioned in the release notes of the next minor version release. labels Jun 28, 2018
@5chdn 5chdn added this to the 1.12 milestone Jun 28, 2018
Copy link
Collaborator

@dvdplm dvdplm left a comment

Choose a reason for hiding this comment

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

lgtm

@5chdn 5chdn mentioned this pull request Jun 30, 2018
12 tasks
@sorpaas
Copy link
Collaborator

sorpaas commented Jul 2, 2018

My main concern is just #8820. Should we enable flag tx_queue_no_unfamiliar_locals by default to protect against this?

@tomusdrw
Copy link
Collaborator Author

tomusdrw commented Jul 2, 2018

@sorpaas we can, sure.
Although the JSON-RPC interface is not exposed by default anyway, so if someone does expose it they need to be aware that the node is vulnerable to all sorts of attacks, this one being one of them.\

That said I'm ok with enabling the flag by default, or maybe rather enabling the flag if the interface is not localhost.

@5chdn 5chdn requested a review from sorpaas July 2, 2018 13:02
@5chdn 5chdn added A8-looksgood 🦄 Pull request is reviewed well. and removed A0-pleasereview 🤓 Pull request needs code review. labels Jul 2, 2018
@5chdn 5chdn merged commit 00e61a9 into master Jul 3, 2018
@5chdn 5chdn deleted the td-dont-drop-local branch July 3, 2018 09:37
5chdn added a commit that referenced this pull request Jul 9, 2018
* parity-version: bump beta to 1.11.6

* scripts: remove md5 checksums (#8884)

* Add support for --chain tobalaba

* Convert indents to tabs :)

* Fixes for misbehavior reporting in AuthorityRound (#8998)

* aura: only report after checking for repeated skipped primaries

* aura: refactor duplicate code for getting epoch validator set

* aura: verify_external: report on validator set contract instance

* aura: use correct validator set epoch number when reporting

* aura: use epoch set when verifying blocks

* aura: report skipped primaries when generating seal

* aura: handle immediate transitions

* aura: don't report skipped steps from genesis to first block

* aura: fix reporting test

* aura: refactor duplicate code to handle immediate_transitions

* aura: let reporting fail on verify_block_basic

* aura: add comment about possible failure of reporting

* Only return error log for rustls (#9025)

* Transaction Pool improvements (#8470)

* Don't use ethereum_types in transaction pool.

* Hide internal insertion_id.

* Fix tests.

* Review grumbles.

* Improve should_replace on NonceAndGasPrice (#8980)

* Additional tests for NonceAndGasPrice::should_replace.

* Fix should_replace in the distinct sender case.

* Use natural priority ordering to simplify should_replace.

* Minimal effective gas price in the queue (#8934)

* Minimal effective gas price.

* Fix naming, add test

* Fix minimal entry score and add test.

* Fix worst_transaction.

* Remove effective gas price threshold.

* Don't leak gas_price decisions out of Scoring.

* Never drop local transactions from different senders. (#9002)

* Recently rejected cache for transaction queue (#9005)

* Store recently rejected transactions.

* Don't cache AlreadyImported rejections.

* Make the size of transaction verification queue dependent on pool size.

* Add a test for recently rejected.

* Fix logging for recently rejected.

* Make rejection cache smaller.

* obsolete test removed

* obsolete test removed

* Construct cache with_capacity.

* Optimize pending transactions filter (#9026)

* rpc: return unordered transactions in pending transactions filter

* ethcore: use LruCache for nonce cache

Only clear the nonce cache when a block is retracted

* Revert "ethcore: use LruCache for nonce cache"

This reverts commit b382c19.

* Use only cached nonces when computing pending hashes.

* Give filters their own locks, so that they don't block one another.

* Fix pending transaction count if not sealing.

* Clear cache only when block is enacted.

* Fix RPC tests.

* Address review comments.

* A last bunch of txqueue performance optimizations (#9024)

* Clear cache only when block is enacted.

* Add tracing for cull.

* Cull split.

* Cull after creating pending block.

* Add constant, remove sync::read tracing.

* Reset debug.

* Remove excessive tracing.

* Use struct for NonceCache.

* Fix build

* Remove warnings.

* Fix build again.

* miner: add missing macro use for trace_time

* ci: remove md5 merge leftovers
ordian added a commit to ordian/parity that referenced this pull request Jul 9, 2018
…rp_sync_on_light_client

* 'master' of https://github.com/paritytech/parity:
  Never drop local transactions from different senders. (openethereum#9002)
  Precise HTTP or WebSockets for JSON-RPC options (openethereum#9027)
  Recently rejected cache for transaction queue (openethereum#9005)
  Make HashDB generic (openethereum#8739)
  Only return error log for rustls (openethereum#9025)
  Update Changelogs for 1.10.8 and 1.11.5 (openethereum#9012)
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. B7-releasenotes 📜 Changes should be mentioned in the release notes of the next minor version release. M4-core ⛓ Core client code / Rust.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants