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

TX queue gas limit config and allow local transactions over the gas limit #2553

Merged
merged 2 commits into from
Oct 10, 2016

Conversation

arkpar
Copy link
Collaborator

@arkpar arkpar commented Oct 10, 2016

@arkpar arkpar added A0-pleasereview 🤓 Pull request needs code review. M4-core ⛓ Core client code / Rust. labels Oct 10, 2016
@coveralls
Copy link

Coverage Status

Coverage increased (+0.04%) to 86.408% when pulling dfe9916 on tx-queue-config into 271bcf4 on master.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.05%) to 86.419% when pulling c427b1f on tx-queue-config into 271bcf4 on master.

@@ -184,6 +184,10 @@ Sealing/Mining Options:
more than 32 characters. (default: {flag_extra_data:?})
--tx-queue-size LIMIT Maximum amount of transactions in the queue (waiting
to be included in next block) (default: {flag_tx_queue_size}).
--tx-queue-gas LIMIT Maximum amount of total gas for external transactions in
the queue. LIIMIT can be either an amont of gas or
Copy link

Choose a reason for hiding this comment

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

Typo amont -> amount

@tomusdrw
Copy link
Collaborator

Shouldn't we propagate limited number of transactions (for instance blockgas*2) in case of --tx-queue-gas off?

@arkpar
Copy link
Collaborator Author

arkpar commented Oct 10, 2016

What about --tx-queue-gas 100000000?
Also local transaction should be sent out regardless of the limit.
Better fix would be to track previously sent transactions for each peer.
This is left for a future PR.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.03%) to 86.414% when pulling 21a8afe on tx-queue-config into 64f6f83 on master.

@rphmeier
Copy link
Contributor

@arkpar We could track perviously-sent transactions for each peer (actually, it's considered Bad Form to send txs to the same peer more than once) but we have to be careful to limit the memory used while tracking this so the footprint of long-running nodes stays low.

@trapp
Copy link

trapp commented Oct 10, 2016

We could track perviously-sent transactions for each peer

If you do something like this please don't forget to whitelist the peers from the --reserved-peers list for those limits.

[ci:skip]
@gavofyork gavofyork added A8-looksgood 🦄 Pull request is reviewed well. and removed A0-pleasereview 🤓 Pull request needs code review. labels Oct 10, 2016
@gavofyork gavofyork merged commit 26d7712 into master Oct 10, 2016
@gavofyork gavofyork deleted the tx-queue-config branch October 10, 2016 21:04
arkpar added a commit that referenced this pull request Oct 12, 2016
…imit (#2553)

* Gas limit config; Allow local transactions over the limit

* Fix typo

[ci:skip]
arkpar added a commit that referenced this pull request Oct 12, 2016
* TX queue gas limit config and allow local transactions over the gas limit (#2553)

* Gas limit config; Allow local transactions over the limit

* Fix typo

[ci:skip]

* v1.3.7
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. M4-core ⛓ Core client code / Rust.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants