Fixes required for Cumulus #485

bkchr · 2019-10-18T11:14:27Z

No description provided.

gui1117

looks good to me

gui1117 · 2019-10-18T12:19:08Z

service/src/lib.rs

 	let is_collator = config.custom.collating_for.is_some();
+	let is_authority = config.roles.is_authority() && !is_collator;


I don't know if this breaks some assumption in the code though

What kind of assumptions? As collator we are currently a full node and this line expresses this.

ok I should have put this comment on the if is_collator then return.

maybe there is more logic wrong there, like network_gossip register two times is this correct ?

polkadot/service/src/lib.rs

Lines 208 to 211 in 4a983a8

let gossip_validator = network_gossip::register_validator(

service.network(),

(is_known, client.clone()),

);

polkadot/service/src/lib.rs

Line 280 in 4a983a8

network_gossip::register_non_authority_validator(service.network());

This is the stuff I need :) Otherwise the node always disconnects, because of unknown gossip messages.

I still don't understand why in case of a collator we register two times: one time as validator and another time as non_authority_validator

This is not only done for collators, also for full nodes.
I think this probably can be moved into the if.

@andresilva confirmed that this is a bug, as you highlighted. We will open another pr to fix this.

not sure about if this break some network configure assumption

kianenigma

LGTM!

* add authority discovery module (#444) * grandpa: set justification period to 512 blocks (#445) * lock (#446) * Kusama CC2 spec (#449) * update readme for cc2 (#453) * docs: add security policy (#450) * docs: add security policy * Update SECURITY.md Co-Authored-By: joe petrowski <25483142+joepetrowski@users.noreply.github.com> * Enable `substrate-session` `std` feature and remove unused traits (#456) * update (#457) * Update latest substrate master (#462) * update substrate to latest polkadot-master * fix test runtime * Fix compilation in wasm (#465) Also fix the weird file structure by making `wasm_executor.rs` -> `wasm_executor/mod.rs`. * Make `ParachainContext` take self as mutable (#458) This replicates changes to `Environment`/`Proposer` that are used internally at Cumulus. * wasm_executor: fix wasm signature checker (#471) Signed-off-by: yjhmelody <465402634@qq.com> * Changes for substrate#3699 (#466) * change upstream and branch * Fix build * remove doc renamings * Fix tests * . * Revert changing fork and branch * Update Cargo.toml * Update parachains.rs * update branch name in Cargo.lock (#473) * Introduce Parathreads (runtime) (#341) * Rest of parathread draft implementation, parachain permissioning. * Update Substrate * Update Substrate again * Integrate weight/fee stuff. * Council * Build fixes * More fixes * Minor additions * fix some small errors * Revert "fix some small errors" This reverts commit 4fb52c8. * Merge fix. * do_swap -> on_swap * Update depdendency to polkadot-master * Fix more merge problems * Some patching of errors * Fix storage closure * Actually fix storage. It builds! * Tests run... but not successfully. * Add `run_to_block` to get parachains active to start * More `run_to_block` * Fix build * Queue up changes to threads * Move registration test * Fix regsiter/deregister test * Retry queue. * Minor refactor * Refactor to avoid heavy storage items * Make tests pass * remove para on deregister, add events * Remove println * Fix register/deregister parathread test * fix merge * Parathread can be activated test * Test auction * Add `Debtors` storage item I considered putting the debtor information in `ParaInfo`, but it did not make sense to me since this information only applies to parathreads, not `paras` in general. * remove comment code * Some new tests * Fixes for removing threads when scheduled. Tests. * Test progression of threads. * Test that reschedule queuing works properly. * Make test slightly more interesting * whitespace * Swap works properly. * Update locks * Build * Rename can_swap * Add test for funds to be correctly returned after a swap Swap does not seem to have logic which correctly swaps the debtor account to the new parathread. * Make tests consistant * Add check that `PendingSwap` is cleaned up * Update runtime/src/parachains.rs Co-Authored-By: Robert Habermeier <rphmeier@gmail.com> * Update runtime/src/registrar.rs Co-Authored-By: Robert Habermeier <rphmeier@gmail.com> * Some fixes/suggestions from review * Docs * Apply suggestions from code review Co-Authored-By: Robert Habermeier <rphmeier@gmail.com> Co-Authored-By: Shawn Tabrizi <shawntabrizi@gmail.com> * Update network/src/gossip.rs Co-Authored-By: Robert Habermeier <rphmeier@gmail.com> * Rename OnSwap * Add missing `]` * Rejig ordering semantics, making everything a bit slower but correct. * Some Fixes to Parathread Compile (#470) * Some Fixes * Fix queue_upward_messages * Change back to const * Build fixes * Fix tests * Update to latest Substrate master (#472) * Update to latest Substrate master * Fix * Fix compilation * fix var name for post_upward_message (#474) Signed-off-by: yjhmelody <465402634@qq.com> * change versioning and tagging of releases (#476) * change versioning and tagging of releases * Make `ParaId` constructible from a const context (#483) * Pass `client` and `task_executor` to `BuildParachainContext` (#484) * Pass `client` and `task_executor` to `BuildParachainContext` * Update `Cargo.lock` * integrate minor weight/fee changes (#482) * Update cargo files * Make it build again. * Fix build * revert cargo file * New lockfile * Bump. * Update to latest Substrate master (#486) * Fixes required for Cumulus (#485) * Collator node need to register all gossip validators as well * Make sure that parachain authorities are only written once at genesis * Fix test * fix dockerfile build - upgrade image base from ubuntu 16 to 18 (#423) * Only register one gossip validator for full nodes (#487) * ci: fix publishing of ci builds (#488) * Support `account_nextIndex` RPC. (#460) * Use node-rpc extensions to support account_nextIndex. * Remove todo. * Update lock. * Use new srml_system_rpc crate. * Update to substrate=master * Update lockfile. * Update to polkadot-master. * Apply suggestions from code review Co-Authored-By: Bastian Köcher <bkchr@users.noreply.github.com> * Update to latest substrate master (#491) * update to latest substrate master * Fix compilation * Switch elections to Phragmen, enable them in PoA (#492) * Switch elections to Phragmen, enable them in PoA * Remove superfluous code. * Build fixes * Update to substrate master * Build fixes * Add warning * Disable authority discovery for now * Remove commented code * Fix warning * service: cleanup task spawning (#495) * Update to latest substrate polkadot-master (#496) * update to latest substrate polkadot-master * Bump to 0.6.3 * service: don't use the grandpa observer (#494) (#498) * service: don't use the grandpa observer * service: remove unnecessary boxing * service: fix indentation * service: remove unnecessary on_exit tasks spawned with `spawn_task`/`spawn_essential_task` are already guarded by `on_exit`. * Update service/src/lib.rs Co-Authored-By: Gavin Wood <gavin@parity.io>

* Copy-Pasta owner and freezing code from `message-lane` * Halt pallet if bridge hasn't been initialized * Make owner optional in `message-lane` pallet * Add `is_halted` to `InitializationData` * Fix initialization tests * Only allow pallet to be initialized once * Add some logging around halting and ownership changes * Remove `target` in debugging calls

) * update substrate/polkadot with construct_runtime changes * fix update * Fixes * More fixes * fix test, but might be wrong fix Co-authored-by: Bastian Köcher <info@kchr.de>

bkchr added 2 commits October 18, 2019 12:51

Collator node need to register all gossip validators as well

05875eb

Make sure that parachain authorities are only written once at genesis

67a22a2

bkchr added the A0-please_review Pull request needs code review. label Oct 18, 2019

gui1117 previously approved these changes Oct 18, 2019

View reviewed changes

gui1117 reviewed Oct 18, 2019

View reviewed changes

gui1117 requested review from gui1117 and removed request for gui1117 October 18, 2019 12:19

Fix test

7827524

kianenigma approved these changes Oct 21, 2019

View reviewed changes

bkchr merged commit b5cdfbe into master Oct 21, 2019

bkchr deleted the bkchr-cumulus-fixes branch October 21, 2019 08:10

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fixes required for Cumulus #485

Fixes required for Cumulus #485

bkchr commented Oct 18, 2019

gui1117 left a comment

gui1117 Oct 18, 2019

bkchr Oct 18, 2019

gui1117 Oct 18, 2019

bkchr Oct 19, 2019

gui1117 Oct 21, 2019

bkchr Oct 21, 2019

bkchr Oct 21, 2019

kianenigma left a comment

		let is_collator = config.custom.collating_for.is_some();
		let is_authority = config.roles.is_authority() && !is_collator;

	let gossip_validator = network_gossip::register_validator(
	service.network(),
	(is_known, client.clone()),
	);

Fixes required for Cumulus #485

Fixes required for Cumulus #485

Conversation

bkchr commented Oct 18, 2019

gui1117 left a comment

Choose a reason for hiding this comment

gui1117 Oct 18, 2019

Choose a reason for hiding this comment

bkchr Oct 18, 2019

Choose a reason for hiding this comment

gui1117 Oct 18, 2019

Choose a reason for hiding this comment

bkchr Oct 19, 2019

Choose a reason for hiding this comment

gui1117 Oct 21, 2019

Choose a reason for hiding this comment

bkchr Oct 21, 2019

Choose a reason for hiding this comment

bkchr Oct 21, 2019

Choose a reason for hiding this comment

kianenigma left a comment

Choose a reason for hiding this comment