Add POSDAO transition and malice report queue. #11245

afck · 2019-11-06T09:29:13Z

This adds a posdaoTransition parameter to enable:

Calling the emitInitiateChange function of the validator set contract.
The POSDAO network needs one of the validators to call this, so that a log event is added to a block when the validator set changes.
A malice report queue.
Misbehaving validators can be reported to the validator set contract, by making a transaction with a call to reportMalicious. However, transactions may fail to get included and may even become obsolete, if some other transaction was made with their nonce. The queue keeps track of malice reports, and retries them with updated nonces until they succeed.
Setting the gas price for malice reports to zero: Validator nodes shouldn't need to have positive balance in their signing accounts.

This is based on poanetwork#151, poanetwork#129, and poanetwork@424c593.

It shares some code with #10946.

parity-cla-bot · 2019-11-06T09:29:17Z

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

Many thanks,

Parity Technologies CLA Bot

afck · 2019-11-06T09:49:29Z

Those test failures seem to be problems with the CI setup again.

dvdplm · 2020-01-13T14:30:44Z

ethcore/client-traits/src/lib.rs

@@ -450,6 +450,12 @@ impl TransactionRequest {
 		self
 	}

+	/// Sets a gas price, or clears it. If this is `None`, a sensible default is used.
+	pub fn opt_gas_price(mut self, gas_price: Option<U256>) -> TransactionRequest {


Suggested change

pub fn opt_gas_price(mut self, gas_price: Option<U256>) -> TransactionRequest {

pub fn set_gas_price(mut self, gas_price: Option<U256>) -> TransactionRequest {

ethcore/engines/authority-round/src/lib.rs

dvdplm · 2020-01-13T14:41:04Z

ethcore/engines/authority-round/src/lib.rs

+
+	fn run_posdao(&self, block: &ExecutedBlock, nonce: Option<U256>) -> Result<Vec<SignedTransaction>, Error> {
+		// Skip the rest of the function unless there has been a transition to POSDAO AuRa.
+		if self.posdao_transition.map_or(true, |block_num| block.header.number() < block_num) {


Suggested change

if self.posdao_transition.map_or(true, |block_num| block.header.number() < block_num) {

if self.posdao_transition.map_or(true, |posdao_block| block.header.number() < posdao_block) {

ethcore/engines/authority-round/src/lib.rs

ethcore/engines/validator-set/src/safe_contract.rs

ethcore/engines/validator-set/src/contract.rs

ordian

I'll try to leave a more meaning review in the coming days.

ordian · 2020-01-10T15:35:07Z

ethcore/client-traits/src/lib.rs

@@ -450,6 +450,12 @@ impl TransactionRequest {
 		self
 	}

+	/// Sets a gas price, or clears it. If this is `None`, a sensible default is used.
+	pub fn opt_gas_price(mut self, gas_price: Option<U256>) -> TransactionRequest {


Do we need both gas_price and opt_gas_price? I would just change the gas_price method.

ordian · 2020-01-10T15:43:40Z

ethcore/engines/authority-round/src/lib.rs

+		})?;
+		let full_client = client.as_full_client().ok_or_else(|| {
+                    EngineError::FailedSystemCall("Failed to upgrade to BlockchainClient.".to_string())
+                })?;


ethcore/engines/validator-set/src/contract.rs

afck · 2020-01-14T14:41:32Z

Sorry for the indentation confusion! (Trying with vim-IndentConsistencyCop now.)
I tried to address all comments so far.

afck · 2020-01-14T15:28:18Z

I can't reproduce those test failures locally.

dvdplm · 2020-01-14T20:13:01Z

I can't reproduce those test failures locally.

It's a flaky test, apologies. Restarted the job.

varasev · 2020-01-21T08:53:44Z

Hi all - It seems all suggestions have been addressed. Are there any other proposals we could take into account here? Or this PR could be approved?

dvdplm

lgtm

ethcore/engines/validator-set/src/safe_contract.rs

ordian

I must admit that I don't fully understand the logic here, but don't want to block merging the PR either. Would appreciate though if you could walk me through the code in a call or a meetup.

Left a few suggestions.

ethcore/engines/authority-round/src/lib.rs

ethcore/engines/validator-set/src/safe_contract.rs

ethcore/engines/validator-set/src/contract.rs

afck · 2020-01-28T09:50:10Z

I addressed the comments, rebased, and fixed the bug @varasev found: The nonce was not incremented between the randomness and the POSDAO calls after the refactoring (moving the transaction generation into separate methods).
Let's run posdao-test-setup as well before merging, to make sure.

Happy to discuss this PR in a call (see Discord)!

varasev · 2020-01-28T10:09:30Z

Let's run posdao-test-setup as well before merging, to make sure.

Yes, I will retest that

varasev · 2020-01-28T11:10:53Z

The nonce was not incremented between the randomness and the POSDAO calls after the refactoring (moving the transaction generation into separate methods). Let's run posdao-test-setup as well before merging, to make sure.

That works 👌

afck · 2020-01-28T12:44:11Z

Works for me as well. 👍

afck · 2020-01-29T10:14:57Z

We made the changes we discussed yesterday. Please take a look at the two new commits.

ordian

LGTM, thanks!

varasev · 2020-01-29T10:28:55Z

ethcore/engines/validator-set/src/contract.rs

@@ -165,13 +165,17 @@ impl ValidatorSet for ValidatorContract {
 	fn report_malicious(&self, address: &Address, _set_block: BlockNumber, block: BlockNumber, proof: Bytes) {
 		if let Err(s) = self.do_report_malicious(address, block, proof) {
 			warn!(target: "engine", "Validator {} could not be reported ({}) on block {}", address, s, block);
+		} else {
+			info!(target: "engine", "Reporting malicious validator {} on block {}", address, block);


Seems we already have the warning message inside the do_report_malicious function.

varasev · 2020-01-29T10:29:20Z

ethcore/engines/validator-set/src/contract.rs

 		}
 	}

 	fn report_benign(&self, address: &Address, _set_block: BlockNumber, block: BlockNumber) {
 		trace!(target: "engine", "validator set recording benign misbehaviour at block #{} by {:#x}", block, address);
 		if let Err(s) = self.do_report_benign(address, block) {
 			warn!(target: "engine", "Validator {} could not be reported ({}) on block {}", address, s, block);
+		} else {
+			info!(target: "engine", "Reporting validator {} on block {} (benign fault)", address, block);


Seems we already have the warning message inside the do_report_benign function.

afck · 2020-01-29T10:39:22Z

You're right, @varasev, we completely missed that yesterday! I reverted that part of the change.

varasev · 2020-01-29T11:31:38Z

@dvdplm Will it be correct to contribute to Parity docs through https://github.com/paritytech/wiki repository after our changes are released? Or there are other places we should add the docs to?

dvdplm · 2020-01-29T13:12:46Z

Will it be correct to contribute to Parity docs through https://github.com/paritytech/wiki repository after our changes are released? Or there are other places we should add the docs to?

That is a great question. I don't think we have a comprehensive plan for the client docs once we migrate away from the Parity org, so until we do I'd say the parity wiki is the least worst option?

ethcore/res/validator_contract.sol

Co-Authored-By: David <dvdplm@gmail.com>

varasev · 2020-01-29T13:22:22Z

I'd say the parity wiki is the least worst option?

Ok, thanks

…pstream * master: Add POSDAO transition and malice report queue. (#11245) update master/nightly version: v2.8.0 (#11419) ethcore/res: remove morden testnet (#11392) fix: export hardcoded sync format (#11416) update hardcoded headers: mainnet and ropsten (#11414) AuthorityEngine: Minor cleanups. (#11408) Update POA bootnodes (#11411) Add EtherCore support (#11402) verification: fix race same block + misc (#11400) Update ProgPoW to 0.9.3 (#11407) update classic testnet bootnodes (#11398) dependencies: bump `derive_more v0.99` (#11405) engine error: remove faulty/unused `From` (#11404) Switching to stable-track (#11377) ethcore/res: fix ethereum classic chainspec blake2_f activation block num (#11391) Update copyright notice 2020 (#11386)

* Add POSDAO transition; call emitInitiateChange. * Retry failed malice reports. * Make malice reports with zero gas price. * Address review comments. * Extract ReportQueue from ValidatorSafeContract. * Add shouldValidatorReport to validator set contract. * Rename queue_report to enqueue_report * Increment nonce between randomness and POSDAO transactions. * Refactor the test ValidatorSet contract * Address review comments, docs * Update ethcore/res/validator_contract.sol Co-Authored-By: David <dvdplm@gmail.com> * Update ethcore/res/validator_contract.sol Co-Authored-By: David <dvdplm@gmail.com> Co-authored-by: varasev <33550681+varasev@users.noreply.github.com> Co-authored-by: David <dvdplm@gmail.com>

dvdplm mentioned this pull request Nov 18, 2019

Add randomness contract support to AuthorityRound. #10946

Merged

afck force-pushed the poanetwork-posdao-transition branch from 762b8cd to 95ada81 Compare December 17, 2019 15:29

varasev mentioned this pull request Jan 6, 2020

Error(Engine(NotProposer(Mismatch and Block(InvalidGasLimit(OutOfBounds poanetwork/parity-ethereum#203

Open

ordian self-requested a review January 10, 2020 15:31

afck force-pushed the poanetwork-posdao-transition branch from 95ada81 to 733d1bb Compare January 13, 2020 11:32

dvdplm reviewed Jan 13, 2020

View reviewed changes

ordian reviewed Jan 13, 2020

View reviewed changes

afck force-pushed the poanetwork-posdao-transition branch from 733d1bb to dbdd9c5 Compare January 14, 2020 14:40

dvdplm approved these changes Jan 24, 2020

View reviewed changes

ethcore/engines/validator-set/src/safe_contract.rs Outdated Show resolved Hide resolved

ordian approved these changes Jan 27, 2020

View reviewed changes

ethcore/engines/authority-round/src/lib.rs Outdated Show resolved Hide resolved

ethcore/engines/validator-set/src/safe_contract.rs Outdated Show resolved Hide resolved

ethcore/engines/validator-set/src/contract.rs Outdated Show resolved Hide resolved

afck and others added 8 commits January 28, 2020 09:46

Add POSDAO transition; call emitInitiateChange.

76a7fa8

Retry failed malice reports.

6ba71fb

Make malice reports with zero gas price.

0d1ae52

Address review comments.

674fa99

Extract ReportQueue from ValidatorSafeContract.

a634fa7

Add shouldValidatorReport to validator set contract.

31c0414

Rename queue_report to enqueue_report

afc661a

Increment nonce between randomness and POSDAO transactions.

07a392c

afck force-pushed the poanetwork-posdao-transition branch from f7c6325 to 07a392c Compare January 28, 2020 09:47

Refactor the test ValidatorSet contract

3b52afa

ordian approved these changes Jan 29, 2020

View reviewed changes

varasev reviewed Jan 29, 2020

View reviewed changes

Address review comments, docs

5443f2d

afck force-pushed the poanetwork-posdao-transition branch from cc755bf to 5443f2d Compare January 29, 2020 10:39

dvdplm reviewed Jan 29, 2020

View reviewed changes

ethcore/res/validator_contract.sol Outdated Show resolved Hide resolved

dvdplm approved these changes Jan 29, 2020

View reviewed changes

ethcore/res/validator_contract.sol Outdated Show resolved Hide resolved

varasev and others added 2 commits January 29, 2020 16:19

Update ethcore/res/validator_contract.sol

f60e55f

Co-Authored-By: David <dvdplm@gmail.com>

Update ethcore/res/validator_contract.sol

ca0a596

Co-Authored-By: David <dvdplm@gmail.com>

ordian merged commit bf44f02 into openethereum:master Jan 29, 2020

afck deleted the poanetwork-posdao-transition branch January 29, 2020 14:21

LukaszRozmej mentioned this pull request Feb 7, 2020

Add POSDAO transition and malice report queue. NethermindEth/nethermind#1417

Closed

rakita mentioned this pull request Dec 7, 2020

Backport xDAI support openethereum/openethereum#118

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add POSDAO transition and malice report queue. #11245

Add POSDAO transition and malice report queue. #11245

afck commented Nov 6, 2019 •

edited

Loading

parity-cla-bot commented Nov 6, 2019

afck commented Nov 6, 2019

dvdplm Jan 13, 2020

dvdplm Jan 13, 2020

ordian left a comment

ordian Jan 10, 2020

ordian Jan 10, 2020

afck commented Jan 14, 2020

afck commented Jan 14, 2020

dvdplm commented Jan 14, 2020

varasev commented Jan 21, 2020

dvdplm left a comment

ordian left a comment •

edited

Loading

afck commented Jan 28, 2020

varasev commented Jan 28, 2020

varasev commented Jan 28, 2020

afck commented Jan 28, 2020

afck commented Jan 29, 2020

ordian left a comment

varasev Jan 29, 2020

varasev Jan 29, 2020

afck commented Jan 29, 2020

varasev commented Jan 29, 2020

dvdplm commented Jan 29, 2020

varasev commented Jan 29, 2020

	pub fn opt_gas_price(mut self, gas_price: Option<U256>) -> TransactionRequest {
	pub fn set_gas_price(mut self, gas_price: Option<U256>) -> TransactionRequest {

	if self.posdao_transition.map_or(true, \|block_num\| block.header.number() < block_num) {
	if self.posdao_transition.map_or(true, \|posdao_block\| block.header.number() < posdao_block) {

Add POSDAO transition and malice report queue. #11245

Add POSDAO transition and malice report queue. #11245

Conversation

afck commented Nov 6, 2019 • edited Loading

parity-cla-bot commented Nov 6, 2019

afck commented Nov 6, 2019

dvdplm Jan 13, 2020

Choose a reason for hiding this comment

dvdplm Jan 13, 2020

Choose a reason for hiding this comment

ordian left a comment

Choose a reason for hiding this comment

ordian Jan 10, 2020

Choose a reason for hiding this comment

ordian Jan 10, 2020

Choose a reason for hiding this comment

afck commented Jan 14, 2020

afck commented Jan 14, 2020

dvdplm commented Jan 14, 2020

varasev commented Jan 21, 2020

dvdplm left a comment

Choose a reason for hiding this comment

ordian left a comment • edited Loading

Choose a reason for hiding this comment

afck commented Jan 28, 2020

varasev commented Jan 28, 2020

varasev commented Jan 28, 2020

afck commented Jan 28, 2020

afck commented Jan 29, 2020

ordian left a comment

Choose a reason for hiding this comment

varasev Jan 29, 2020

Choose a reason for hiding this comment

varasev Jan 29, 2020

Choose a reason for hiding this comment

afck commented Jan 29, 2020

varasev commented Jan 29, 2020

dvdplm commented Jan 29, 2020

varasev commented Jan 29, 2020

afck commented Nov 6, 2019 •

edited

Loading

ordian left a comment •

edited

Loading