Flag for rejecting all outbound messages#982
Conversation
HCastano
left a comment
HCastano
left a comment
There was a problem hiding this comment.
I could also extract it to the bp-runtime-common && introduce a set of 'common' flags.
This could work, but I would wait until we implement this into some Polkadot based runtimes before splitting it out
| /// Rialto to Millau conversion rate. Initially we treat both tokens as equal. | ||
| pub storage RialtoToMillauConversionRate: FixedU128 = INITIAL_RIALTO_TO_MILLAU_CONVERSION_RATE; | ||
| /// If `true`, all Rialto -> Millau messages are rejected. | ||
| pub storage RejectOutboundMessages: bool = false; |
There was a problem hiding this comment.
Why not add this parameter directly to the pallet? This is not a bridge-specific parameter and as you mentioned it will need to be copy-pasted for every bridge instance.
We already have IsHalted in the pallet now. I think the two could either be merged in one Config object with two bool values or we could introduce enum OperatingMode { Full, Halted, Inbound }
The reason is that while the current solution reduces pallet-internal complexity it adds extra integration-level complexity and I feel that's already quite high.
There was a problem hiding this comment.
I'm considering this as 'our way' to ease upgrade process. Other may choose their own paths to solve that. Or, if they have non-upgradeable chains (like Rialto and Millau), they may just not use parameters like that. I would avoid adding any functionality that may need to be customized to the pallet itself - the pallet itself is not trivial && having potentially unused stuff there is undesirable.
There was a problem hiding this comment.
Actually - what's more important is that additional storage read. So since I don't want to make messages more expensive, I'll implement your option then
| if PalletOperatingMode::<I>::get() != OperatingMode::Normal { | ||
| Err(Error::<T, I>::Halted) | ||
| } else { | ||
| Ok(()) | ||
| } | ||
| } |
There was a problem hiding this comment.
| if PalletOperatingMode::<I>::get() != OperatingMode::Normal { | |
| Err(Error::<T, I>::Halted) | |
| } else { | |
| Ok(()) | |
| } | |
| } | |
| if PalletOperatingMode::<I>::get() == OperatingMode::Normal { | |
| Ok(()) | |
| } else { | |
| Err(Error::<T, I>::Halted) | |
| } | |
| } |
nit: But it matches the check that the function says it does
Co-authored-by: Hernando Castano <HCastano@users.noreply.github.com>
* add benchmarks for xcmp queue config data setters * add new benchmarks * cargo fmt * added newline * Additional weights for dmp queue for westmint * include new weights * Adding WeightInfo trait and friends * WeightInfo should be on xcmp rather than dmp pallet * cargo fmt * update scripts * mock weightinfo * cargo fmt * canvas kusama is substrate weight * weights from bm2 * expanding to other similar config functions * updated weights from bm2 * Revert "updated weights from bm2" This reverts commit b1702780982c278b44f572c2089b1d7ddc564d76. * Consolidation to one benchmark * reran weights * Update pallets/xcmp-queue/src/lib.rs Co-authored-by: Ignacio Palacios <ignacio.palacios.santos@gmail.com> * integrating review feedback * rerun weights * Add DispatchClass::Operational Co-authored-by: Squirrel <gilescope@gmail.com> Co-authored-by: Ignacio Palacios <ignacio.palacios.santos@gmail.com>
* flag for rejecting all outbound messages * update weights * Revert "update weights" This reverts commit 992b866. * Revert "flag for rejecting all outbound messages" This reverts commit d094964. * OperatingMode * Update modules/messages/src/lib.rs Co-authored-by: Hernando Castano <HCastano@users.noreply.github.com> * Poke CI * RustFmt Co-authored-by: Hernando Castano <HCastano@users.noreply.github.com> Co-authored-by: Hernando Castano <hernando@hcastano.com>
* flag for rejecting all outbound messages * update weights * Revert "update weights" This reverts commit 992b866. * Revert "flag for rejecting all outbound messages" This reverts commit d094964. * OperatingMode * Update modules/messages/src/lib.rs Co-authored-by: Hernando Castano <HCastano@users.noreply.github.com> * Poke CI * RustFmt Co-authored-by: Hernando Castano <HCastano@users.noreply.github.com> Co-authored-by: Hernando Castano <hernando@hcastano.com>
3 participants
closes #946 (not actually, because flag is runtime-specific and we'll need it for P<>K bridge; but we'll be mostly copying code from our testnets => closing now)
I could also extract it to the
bp-runtime-common&& introduce a set of 'common' flags. This would require either a way to merge two enums (something likedecl_outer_parameter!) or nested enums -enum CommonParameters { RejectOutboundMessages(bool), RuntimeSpecificParameters { enum RialtoSpecificParameter { MillauToRialtoConversionRate(FixedU128) } } }.