Reject blocks without justification which don't have the best number #105

cecton · 2020-05-27T13:48:21Z

Related to #7

Forked at: 8f02e23 Parent branch: origin/master

This reverts commit ee60e12.

JoshOrndorff · 2020-05-27T15:22:05Z

network/src/lib.rs

 		header: &B::Header,
 		mut data: &[u8],
 	) -> Result<Validation, Box<dyn std::error::Error + Send>> {
 		// If no data is provided the announce is valid.


Comment needs updating

fixed but the whole code changed

Forked at: 8f02e23 Parent branch: origin/master

bkchr · 2020-05-28T08:21:01Z

network/src/lib.rs

+			let best_number = self.parachain_client.info().best_number;
+			let block_number: <<B as BlockT>::Header as HeaderT>::Number = *header.number();
+
+			if !(block_number == best_number + One::one() || block_number == best_number) {


Okay, I thought again about this. And I made a mistake.

We should extract the last parachain header that was stored on the relay chain. We get the number of this Parachain header an make sure that:
block_number == parachain_header_number + 1

This way we make sure that we support forks on the same height

If this condition holds, we need to make sure that there is a justification.

Forked at: 8f02e23 Parent branch: origin/master

bkchr · 2020-05-29T11:00:50Z

network/src/lib.rs

 use std::{marker::PhantomData, sync::Arc};
 use parking_lot::Mutex;

+/// The head data of the parachain, stored in the relay chain.


Please move that into primitives and consolidate with the one in collator

bkchr · 2020-05-29T11:03:43Z

network/src/lib.rs

 		if data.is_empty() {
+			// Check if block is one higher than best
+			let runtime_api_block_id = BlockId::Hash(polkadot_info.best_hash);
+			let block_number: <<B as BlockT>::Header as HeaderT>::Number = *header.number();


Suggested change

let block_number: <<B as BlockT>::Header as HeaderT>::Number = *header.number();

let block_number = header.number().clone();

network/src/lib.rs

bkchr · 2020-05-29T11:08:00Z

network/src/lib.rs

+				.map_err(|e| Box::new(ClientError::Msg(format!("{:?}", e))) as Box<_>)?;
+			let known_best_number = *parent_head.header.number();
+
+			if !(block_number < known_best_number) {


Suggested change

if !(block_number < known_best_number) {

let res = if block_number >= known_best_number {

Negations always makes stuff much more complicated.

bkchr · 2020-05-29T11:17:12Z

network/src/lib.rs

+		let polkadot_info = self.polkadot_client.info();
+
 		if data.is_empty() {
+			// Check if block is one higher than best


You need a justification if this is a new best block.

like this? 2f9131c

bkchr · 2020-05-29T11:18:24Z

network/src/lib.rs

+				return Ok(Validation::Failure);
+			}
+
 			return Ok(Validation::Success);


Suggested change

return Ok(Validation::Failure);

}

return Ok(Validation::Success);

Validation::Failure

} else {

Validation::Success

};

return Ok(res);

eww no, the rest of the function follows the pattern:

if <condition> { return ... // short-circuit }

Yeah, but here a else makes sense.

Co-authored-by: Bastian Köcher <[email protected]>

Forked at: 8f02e23 Parent branch: origin/master

primitives/src/lib.rs

network/src/lib.rs

Co-authored-by: Bastian Köcher <[email protected]>

commit 1eec2cad1f92dece71a8813de18f3adaa3a3307f Author: Albrecht <[email protected]> Date: Wed May 27 09:45:54 2020 +0200 kilt runtime in cumulus commit 05a83e8 Author: Bastian Köcher <[email protected]> Date: Fri Jun 19 13:15:56 2020 +0200 Take storage root from parent header (paritytech#123) commit 62c22b8 Author: Bastian Köcher <[email protected]> Date: Thu Jun 18 12:10:20 2020 +0200 Implement basic upward & downward messages (paritytech#118) * Start by replacing branch names and set `DownwardMessage` * Add the upward-message crate * Add Kusama & Polkadot * More work on getting the upward messages working * Fix build * Begin to integrate it into the test Parachain * Update * Make everything compile again * Switch to westend and print parachain account on startup * Use MultiSignature etc * Fix validate block * Some downward messages work * Update git reference * More downward messages integration * Update test runtime for downward messages * Enable downward message handler and withdraw send tokens * Add some docs * Begin to implement simple XCMP * More work * Fixes and make parachain id configurable * Make parachain ID be part of the genesis * Finishing the XCMP message demo * Update and fixes tests * Update branch commit b082e7f Author: Joshy Orndorff <[email protected]> Date: Thu Jun 18 03:54:47 2020 -0400 Allow custom polkadot chainspec. (paritytech#122) commit 8b1fd65 Author: Cecile Tonglet <[email protected]> Date: Mon Jun 15 12:40:15 2020 +0200 Update polkadot & substrate (paritytech#112) commit 41376ce Author: Cecile Tonglet <[email protected]> Date: Thu Jun 11 12:39:20 2020 +0200 Prefix logs of parachain and relaychain differently + remove light client of relay chain (paritytech#109) commit b3603d1 Author: Bastian Köcher <[email protected]> Date: Fri Jun 5 16:57:02 2020 +0200 Update to latest Substrate & Polkadot (paritytech#107) * Update to latest Substrate & Polkadot * Replace --unsafe-rpc-export with --unsafe-rpc-external * Add --rpc-methods=Unsafe Documented in substrate 24486f52929e9e518eeccbc6ad6da70e9e5bdf8a * typos * more typo * fixed rpc expose * Disable the integration test Co-authored-by: Cecile Tonglet <[email protected]> commit 4c6b959 Author: Cecile Tonglet <[email protected]> Date: Wed Jun 3 17:51:49 2020 +0200 Enable mdns polkadot (paritytech#106) Fixes paritytech#57 commit 1afdba7 Author: Cecile Tonglet <[email protected]> Date: Wed Jun 3 14:15:52 2020 +0200 Reject blocks without justification which don't have the best number (paritytech#105) * Initial commit Forked at: 8f02e23 Parent branch: origin/master * Reject blocks without justification * Revert "Reject blocks without justification" This reverts commit ee60e12. * WIP Forked at: 8f02e23 Parent branch: origin/master * WIP Forked at: 8f02e23 Parent branch: origin/master * WIP Forked at: 8f02e23 Parent branch: origin/master * CLEANUP Forked at: 8f02e23 Parent branch: origin/master * WIP Forked at: 8f02e23 Parent branch: origin/master * WIP Forked at: 8f02e23 Parent branch: origin/master * CLEANUP Forked at: 8f02e23 Parent branch: origin/master * WIP Forked at: 8f02e23 Parent branch: origin/master * WIP Forked at: 8f02e23 Parent branch: origin/master * Move HeadData to primitives * Update network/src/lib.rs Co-authored-by: Bastian Köcher <[email protected]> * Update network/src/lib.rs Co-authored-by: Bastian Köcher <[email protected]> * CLEANUP Forked at: 8f02e23 Parent branch: origin/master * fix * CLEANUP Forked at: 8f02e23 Parent branch: origin/master * messages * for the greater good * Update primitives/src/lib.rs Co-authored-by: Bastian Köcher <[email protected]> * Update network/src/lib.rs Co-authored-by: Bastian Köcher <[email protected]> * Update network/src/lib.rs Co-authored-by: Bastian Köcher <[email protected]> * Update network/src/lib.rs Co-authored-by: Bastian Köcher <[email protected]> Co-authored-by: Bastian Köcher <[email protected]> commit 8f02e23 Author: Cecile Tonglet <[email protected]> Date: Wed May 27 14:43:45 2020 +0200 Ensure relay chain parent is the relay chain head (paritytech#103)

* bump encointer-pallets * benchmark encointer pallets * update weight of purge_community_ceremony Co-authored-by: Christian Langenbacher <[email protected]>

cecton added 2 commits May 27, 2020 15:40

Initial commit

ae3ce09

Forked at: 8f02e23 Parent branch: origin/master

Reject blocks without justification

ee60e12

cecton requested a review from bkchr May 27, 2020 13:48

cecton self-assigned this May 27, 2020

Revert "Reject blocks without justification"

5bb6082

This reverts commit ee60e12.

JoshOrndorff reviewed May 27, 2020

View reviewed changes

cecton added 2 commits May 27, 2020 18:32

WIP

bf53b31

Forked at: 8f02e23 Parent branch: origin/master

WIP

86b8403

Forked at: 8f02e23 Parent branch: origin/master

cecton changed the title ~~Reject blocks without justification~~ Reject blocks without justification which don't have the best number May 28, 2020

cecton added 2 commits May 28, 2020 07:21

WIP

2cac26a

Forked at: 8f02e23 Parent branch: origin/master

CLEANUP

c2a063a

Forked at: 8f02e23 Parent branch: origin/master

cecton requested a review from JoshOrndorff May 28, 2020 05:23

bkchr suggested changes May 28, 2020

View reviewed changes

cecton added 5 commits May 28, 2020 12:34

WIP

36fc71a

Forked at: 8f02e23 Parent branch: origin/master

WIP

a86bfdd

Forked at: 8f02e23 Parent branch: origin/master

CLEANUP

452586e

Forked at: 8f02e23 Parent branch: origin/master

WIP

d92acd6

Forked at: 8f02e23 Parent branch: origin/master

WIP

615e949

Forked at: 8f02e23 Parent branch: origin/master

bkchr reviewed May 29, 2020

View reviewed changes

cecton and others added 7 commits May 29, 2020 13:28

Move HeadData to primitives

3a4a6ba

Update network/src/lib.rs

36ecce0

Co-authored-by: Bastian Köcher <[email protected]>

Update network/src/lib.rs

4b022b1

Co-authored-by: Bastian Köcher <[email protected]>

CLEANUP

8bccf15

Forked at: 8f02e23 Parent branch: origin/master

fix

65c571b

CLEANUP

3b3bf98

Forked at: 8f02e23 Parent branch: origin/master

messages

2f9131c

cecton force-pushed the cecton-forbid-block-without-justification branch from aeb7216 to 2f9131c Compare May 29, 2020 11:41

cecton requested a review from bkchr May 29, 2020 11:42

for the greater good

a5bc4bd

bkchr approved these changes Jun 3, 2020

View reviewed changes

primitives/src/lib.rs Outdated Show resolved Hide resolved

network/src/lib.rs Outdated Show resolved Hide resolved

network/src/lib.rs Outdated Show resolved Hide resolved

network/src/lib.rs Outdated Show resolved Hide resolved

cecton and others added 4 commits June 3, 2020 13:38

Update primitives/src/lib.rs

eb6103b

Co-authored-by: Bastian Köcher <[email protected]>

Update network/src/lib.rs

b33c2ba

Co-authored-by: Bastian Köcher <[email protected]>

Update network/src/lib.rs

1394c70

Co-authored-by: Bastian Köcher <[email protected]>

Update network/src/lib.rs

64a3302

Co-authored-by: Bastian Köcher <[email protected]>

bkchr approved these changes Jun 3, 2020

View reviewed changes

bkchr merged commit 1afdba7 into master Jun 3, 2020

bkchr deleted the cecton-forbid-block-without-justification branch June 3, 2020 12:15

	let block_number: <<B as BlockT>::Header as HeaderT>::Number = *header.number();
	let block_number = header.number().clone();

	if !(block_number < known_best_number) {
	let res = if block_number >= known_best_number {

Reject blocks without justification which don't have the best number #105

Reject blocks without justification which don't have the best number #105

Uh oh!

Conversation

cecton commented May 27, 2020

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants