Sync upstream #174
Sync upstream #174
Conversation
Avoid duplicate function definitions for: - ensure_owner_or_root() - ensure_not_halted() - set_owner() - set_operating_mode() / set_operational()
Signed-off-by: Serban Iorga <serban@parity.io>
Define macro that generates tests for set_owner() and set_operating_mode() in order to avoid duplicate code. Signed-off-by: Serban Iorga <serban@parity.io>
Signed-off-by: Serban Iorga <serban@parity.io>
Add unit test in order to check that the submit_parachain_heads() call returns an error when the pallet is halted. Signed-off-by: Serban Iorga <serban@parity.io>
Signed-off-by: Serban Iorga <serban@parity.io>
This comment was marked as duplicate.
This comment was marked as duplicate.
|
Let's pay more attention to this. |
I recommend splitting the work into smaller ones before we begin reviewing this PR, because it is not so easy to review. |
|
| @@ -270,7 +271,7 @@ pub mod pallet { | |||
| /// Hash of the best finalized header. | |||
| #[pallet::storage] | |||
| pub type BestFinalized<T: Config<I>, I: 'static = ()> = | |||
| StorageValue<_, BridgedBlockHash<T, I>, ValueQuery>; | |||
| StorageValue<_, (BridgedBlockNumber<T, I>, BridgedBlockHash<T, I>), OptionQuery>; | |||
There was a problem hiding this comment.
The storage value type changed from BridgedBlockHash<T, I> to a tuple (BridgedBlockNumber<T, I>, BridgedBlockHash<T, I>). Need to dig deeper to see if it breaks the storage on the chain.
| #[pallet::storage] | ||
| pub(super) type IsHalted<T: Config<I>, I: 'static = ()> = StorageValue<_, bool, ValueQuery>; |
There was a problem hiding this comment.
This storage item is no longer used, needs a migration when upgrading the runtime.
| /// The goal of this extension is to avoid "mining" transactions that provide outdated bridged | ||
| /// headers and messages. Without that extension, even honest relayers may lose their funds if | ||
| /// there are multiple relays running and submitting the same information. | ||
| #[macro_export] |
There was a problem hiding this comment.
| @@ -17,25 +17,25 @@ | |||
| //! Storage keys of bridge GRANDPA pallet. | |||
|
|
|||
| /// Name of the `IsHalted` storage value. | |||
There was a problem hiding this comment.
The comment needs to be updated, No IsHalted anymore.
| // If this test fails, then something has been changed in module storage that is breaking | ||
| // compatibility with previous pallet. | ||
| let storage_key = best_finalized_hash_key("BridgeGrandpa").0; | ||
| let storage_key = best_finalized_key("BridgeGrandpa").0; | ||
| assert_eq!( |
There was a problem hiding this comment.
Seems to forget to update the implementation_match.rs
| @@ -705,12 +714,12 @@ pub mod pallet { | |||
| #[pallet::storage] | |||
| #[pallet::getter(fn operating_mode)] | |||
| pub type PalletOperatingMode<T: Config<I>, I: 'static = ()> = | |||
| StorageValue<_, OperatingMode, ValueQuery>; | |||
There was a problem hiding this comment.
Three storage types are updated, be careful.
|
I looked over all the changes besides para chain part. Left some feedback. @wuminzhe |
|
Suppressed by #206. But the reviews are still helpful. |
The Changes in runtime scope
https://github.com/paritytech/parity-bridges-common/commits/master?before=24e5a3747387178677370876fb78cf505d1127a6+35&path%5B%5D=modules
Sync Prs List