Avoid importing transactions with gas above 1.1*block_gas_limit to transaction queue #760

tomusdrw · 2016-03-17T17:32:59Z

No description provided.

gavofyork · 2016-03-17T22:29:52Z

chain::tests::should_add_transactions_to_queue failing. is this random?

tomusdrw · 2016-03-18T09:37:37Z

It's not random. This is sort of integration test that uses test_client. When I run tests locally I only run tests from crate that I'm modyfing (ethminer in that case) so it fails here.

This time the problem was that blocks generated by test_client have gas_limit set to 0. So no transactions could be imported.

Conflicts: miner/src/transaction_queue.rs

gavofyork · 2016-03-18T11:52:13Z

miner/src/transaction_queue.rs

 				minimal: self.minimal_gas_price,
 				got: tx.gas_price,
 			}));
 		}

+		if tx.gas > self.gas_limit {


this check should probably be moved be checked OpenBlock::push_transaction.

But there already is similar check (or executive will just fail with BlockGasLimitReached).
In this PR we want to avoid even storing such transactions in TransactionQueue (`self.gas_limit here is 1.1*best_block_gas_limit).

Avoid importing transactions with gas above 1.1*block_gas_limit to transaction queue

Ignoring transactions slightly above gas_limit

309af74

tomusdrw added the A0-pleasereview 🤓 Pull request needs code review. label Mar 17, 2016

Updating gas_limit in test_client generated blocks

7fb3656

Merge branch 'master' into tx_queue_gas_limit

58bb5e9

Conflicts: miner/src/transaction_queue.rs

gavofyork reviewed Mar 18, 2016
View reviewed changes

gavofyork added A5-grumble 🔥 Pull request has minor issues that must be addressed before merging. and removed A0-pleasereview 🤓 Pull request needs code review. labels Mar 18, 2016

tomusdrw changed the title ~~Ignoring transactions slightly above gas_limit~~ Avoid importing transactions with gas above gas_limit to transaction queue Mar 18, 2016

tomusdrw changed the title ~~Avoid importing transactions with gas above gas_limit to transaction queue~~ Avoid importing transactions with gas above 1.1*block_gas_limit to transaction queue Mar 18, 2016

tomusdrw added A0-pleasereview 🤓 Pull request needs code review. and removed A5-grumble 🔥 Pull request has minor issues that must be addressed before merging. labels Mar 18, 2016

gavofyork added a commit that referenced this pull request Mar 18, 2016

Merge pull request #760 from ethcore/tx_queue_gas_limit

2309e19

Avoid importing transactions with gas above 1.1*block_gas_limit to transaction queue

gavofyork merged commit 2309e19 into master Mar 18, 2016

gavofyork deleted the tx_queue_gas_limit branch March 18, 2016 17:05

arkpar added this to the 1.0 Parity milestone Mar 20, 2016

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Avoid importing transactions with gas above 1.1*block_gas_limit to transaction queue #760

Avoid importing transactions with gas above 1.1*block_gas_limit to transaction queue #760

tomusdrw commented Mar 17, 2016

gavofyork commented Mar 17, 2016

tomusdrw commented Mar 18, 2016

gavofyork Mar 18, 2016

tomusdrw Mar 18, 2016

Avoid importing transactions with gas above 1.1*block_gas_limit to transaction queue #760

Avoid importing transactions with gas above 1.1*block_gas_limit to transaction queue #760

Conversation

tomusdrw commented Mar 17, 2016

gavofyork commented Mar 17, 2016

tomusdrw commented Mar 18, 2016

gavofyork Mar 18, 2016

Choose a reason for hiding this comment

tomusdrw Mar 18, 2016

Choose a reason for hiding this comment