Allow specifying local accounts via CLI #9960

insipx · 2018-11-24T06:50:05Z

Add tx-queue-locals CLI option
ethcore: modify miner to check options vec before importing transaction

Resolves #9634

parity-cla-bot · 2018-11-24T06:50:08Z

It looks like @insipx hasn't signed our Contributor License Agreement, yet.

The purpose of a CLA is to ensure that the guardian of a project's outputs has the necessary ownership or grants of rights over all contributions to allow them to distribute under the chosen licence.
Wikipedia

You can read and sign our full Contributor License Agreement at the following URL: https://cla.parity.io

Once you've signed, please reply to this thread with [clabot:check] to prove it.

Many thanks,

Parity Technologies CLA Bot

insipx · 2018-11-24T06:53:09Z

[clabot:check]

parity-cla-bot · 2018-11-24T06:53:11Z

It looks like @insipx signed our Contributor License Agreement. 👍

Many thanks,

Parity Technologies CLA Bot

sorpaas

LGTM!

tomusdrw

Test would be nice

tomusdrw · 2018-11-25T17:48:48Z

ethcore/src/miner/miner.rs

 		let sender = pending.sender();
 		let treat_as_local = trusted
 			|| !self.options.tx_queue_no_unfamiliar_locals
-			|| self.accounts.as_ref().map(|accts| accts.has_account(sender)).unwrap_or(false);
+			|| self.accounts.as_ref().map(|accts| accts.has_account(sender)).unwrap_or(false)
+			|| self.options.tx_queue_locals.iter().any(|addr| *addr == sender);


Most likely the set should be considered even if tx_queue_no_unfamiliar_locals is enabled

Looks like that's the current behavior.

ethcore/src/miner/miner.rs

5chdn · 2018-12-04T10:38:03Z

👍 needs a 2nd review

dvdplm

Looks good overall. Minor nits and maybe improved test.

ethcore/src/miner/miner.rs

parity/cli/mod.rs

5chdn · 2018-12-14T13:54:10Z

Are you still working on this @insipx?

insipx · 2018-12-14T21:47:52Z

Are you still working on this @insipx?

Yep,
I am catching up on work this week, just finished my last final 👍

5chdn · 2018-12-28T13:16:07Z

@dvdplm could you have a look at this again?

tomusdrw · 2018-12-31T12:49:42Z

@insipx could you rebase?

* Add `tx-queue-locals` CLI option * ethcore: modify miner to check options vec before importing transaction * modify tests (ethcore/parity) Resolves openethereum#9634

Co-Authored-By: insipx <aplaza@liquidthink.net>

…nsi-local-priority-txs

absent

5chdn · 2019-01-07T08:33:50Z

Restarted CI

…local-priority-txs

5chdn · 2019-01-28T10:02:00Z

@tomusdrw could you look at this again?

insipx force-pushed the insi-local-priority-txs branch from d758b77 to 6aba887 Compare November 24, 2018 22:11

sorpaas approved these changes Nov 25, 2018

View reviewed changes

sorpaas added A8-looksgood 🦄 Pull request is reviewed well. M4-core ⛓ Core client code / Rust. B7-releasenotes 📜 Changes should be mentioned in the release notes of the next minor version release. labels Nov 25, 2018

tomusdrw reviewed Nov 25, 2018

View reviewed changes

5chdn added this to the 2.3 milestone Nov 25, 2018

sorpaas reviewed Nov 26, 2018

View reviewed changes

ethcore/src/miner/miner.rs Outdated Show resolved Hide resolved

sorpaas added A5-grumble 🔥 Pull request has minor issues that must be addressed before merging. and removed A8-looksgood 🦄 Pull request is reviewed well. labels Nov 26, 2018

5chdn added A0-pleasereview 🤓 Pull request needs code review. and removed A5-grumble 🔥 Pull request has minor issues that must be addressed before merging. labels Dec 3, 2018

sorpaas approved these changes Dec 3, 2018

View reviewed changes

sorpaas added A8-looksgood 🦄 Pull request is reviewed well. and removed A0-pleasereview 🤓 Pull request needs code review. labels Dec 3, 2018

dvdplm previously requested changes Dec 4, 2018

View reviewed changes

ethcore/src/miner/miner.rs Outdated Show resolved Hide resolved

ethcore/src/miner/miner.rs Outdated Show resolved Hide resolved

ethcore/src/miner/miner.rs Outdated Show resolved Hide resolved

parity/cli/mod.rs Outdated Show resolved Hide resolved

5chdn added A5-grumble 🔥 Pull request has minor issues that must be addressed before merging. and removed A8-looksgood 🦄 Pull request is reviewed well. labels Dec 5, 2018

5chdn added the A3-stale 🍃 Pull request did not receive any updates in a long time. No review needed at this stage. Close it. label Dec 14, 2018

insipx force-pushed the insi-local-priority-txs branch 3 times, most recently from d32e7fa to f37089b Compare December 23, 2018 20:08

5chdn added the A0-pleasereview 🤓 Pull request needs code review. label Dec 28, 2018

5chdn removed A3-stale 🍃 Pull request did not receive any updates in a long time. No review needed at this stage. Close it. A5-grumble 🔥 Pull request has minor issues that must be addressed before merging. labels Dec 28, 2018

Andrew Plaza and others added 10 commits December 31, 2018 12:05

Allow specifying local accounts via CLI

530002d

* Add `tx-queue-locals` CLI option * ethcore: modify miner to check options vec before importing transaction * modify tests (ethcore/parity) Resolves openethereum#9634

fix formatting

f320f89

fixes: Make prefer HashSet over Vec<>, add test, comment formatting

93e0383

Update ethcore/src/miner/miner.rs

7080a68

Co-Authored-By: insipx <aplaza@liquidthink.net>

Fix comments and add helper for set->vec conversion

77e7422

remove blank line from use statement

4c09991

fix helper test

01e1f4b

formatting

b161b3e

fix test to pass on nightly

9ae2bfa

revert test fix for nightly

a7680c6

insipx force-pushed the insi-local-priority-txs branch from f37089b to a7680c6 Compare December 31, 2018 17:07

Merge branch 'master' of github.com:paritytech/parity-ethereum into i…

aabc5a2

…nsi-local-priority-txs

5chdn added A8-looksgood 🦄 Pull request is reviewed well. and removed A0-pleasereview 🤓 Pull request needs code review. labels Jan 7, 2019

Merge branch 'master' of github.com:insipx/parity-ethereum into insi-…

3cbc1e1

…local-priority-txs

5chdn modified the milestones: 2.3, 2.4 Jan 10, 2019

tomusdrw approved these changes Jan 28, 2019

View reviewed changes

5chdn merged commit 50f5ccc into openethereum:master Jan 28, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Allow specifying local accounts via CLI #9960

Allow specifying local accounts via CLI #9960

insipx commented Nov 24, 2018

parity-cla-bot commented Nov 24, 2018

insipx commented Nov 24, 2018

parity-cla-bot commented Nov 24, 2018

sorpaas left a comment

tomusdrw left a comment

tomusdrw Nov 25, 2018

sorpaas Nov 26, 2018

5chdn commented Dec 4, 2018

dvdplm left a comment

5chdn commented Dec 14, 2018

insipx commented Dec 14, 2018

5chdn commented Dec 28, 2018

tomusdrw commented Dec 31, 2018

5chdn commented Jan 7, 2019

5chdn commented Jan 28, 2019

Allow specifying local accounts via CLI #9960

Allow specifying local accounts via CLI #9960

Conversation

insipx commented Nov 24, 2018

parity-cla-bot commented Nov 24, 2018

insipx commented Nov 24, 2018

parity-cla-bot commented Nov 24, 2018

sorpaas left a comment

Choose a reason for hiding this comment

tomusdrw left a comment

Choose a reason for hiding this comment

tomusdrw Nov 25, 2018

Choose a reason for hiding this comment

sorpaas Nov 26, 2018

Choose a reason for hiding this comment

5chdn commented Dec 4, 2018

dvdplm left a comment

Choose a reason for hiding this comment

5chdn commented Dec 14, 2018

insipx commented Dec 14, 2018

5chdn commented Dec 28, 2018

tomusdrw commented Dec 31, 2018

5chdn commented Jan 7, 2019

5chdn commented Jan 28, 2019