Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove pallet::getter usage from bridges/modules #7120

Open
wants to merge 10 commits into
base: master
Choose a base branch
from

Conversation

muraca
Copy link
Contributor

@muraca muraca commented Jan 12, 2025

Description

Part of #3326

As per title, the pallet:getter usage has been removed from:

  • pallet-bridge-grandpa
  • pallet-bridge-messages
  • pallet-bridge-relayers

polkadot address: 12poSUQPtcF1HUPQGY3zZu2P8emuW9YnsPduA4XG3oCEfJVp

@muraca muraca marked this pull request as ready for review January 12, 2025 16:49
@paritytech-review-bot paritytech-review-bot bot requested a review from a team January 12, 2025 16:50
@@ -916,7 +917,7 @@ impl_runtime_apis! {

impl bp_westend::WestendFinalityApi<Block> for Runtime {
fn best_finalized() -> Option<HeaderId<bp_westend::Hash, bp_westend::BlockNumber>> {
BridgeWestendGrandpa::best_finalized()
pallet_bridge_grandpa::BestFinalized::<Runtime, Instance3>::get()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Huh that is a bit unfortunate that we have to pull in the Instances... first time that I see a downside of this migration.
For these cases we could still write a manual getter though.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For these cases we could still write a manual getter though.

Yes, I agree. It would also be less error-prone to avoid accidentally using an invalid InstanceXYZ, and it would significantly reduce the changes in this PR. There would only be a small internal change to the pallet.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll update the PR with a manual getter implementation and revert the changes on the runtimes

prdoc/pr_7120.prdoc Outdated Show resolved Hide resolved
@ggwpez ggwpez added the T2-pallets This PR/Issue is related to a particular pallet. label Jan 14, 2025
@ggwpez
Copy link
Member

ggwpez commented Jan 14, 2025

/tip small

Copy link

@ggwpez A referendum for a small (20 DOT) tip was successfully submitted for @muraca (12poSUQPtcF1HUPQGY3zZu2P8emuW9YnsPduA4XG3oCEfJVp on polkadot).

Referendum number: 1385.
tip

Copy link

The referendum has appeared on Polkassembly.

@@ -916,7 +917,7 @@ impl_runtime_apis! {

impl bp_westend::WestendFinalityApi<Block> for Runtime {
fn best_finalized() -> Option<HeaderId<bp_westend::Hash, bp_westend::BlockNumber>> {
BridgeWestendGrandpa::best_finalized()
pallet_bridge_grandpa::BestFinalized::<Runtime, Instance3>::get()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
pallet_bridge_grandpa::BestFinalized::<Runtime, Instance3>::get()
pallet_bridge_grandpa::BestFinalized::<Runtime, bridge_common_config::BridgeGrandpaWestendInstance>::get()

@@ -970,7 +971,7 @@ impl_runtime_apis! {

impl bp_polkadot_bulletin::PolkadotBulletinFinalityApi<Block> for Runtime {
fn best_finalized() -> Option<bp_runtime::HeaderId<bp_polkadot_bulletin::Hash, bp_polkadot_bulletin::BlockNumber>> {
BridgePolkadotBulletinGrandpa::best_finalized()
pallet_bridge_grandpa::BestFinalized::<Runtime, Instance4>::get()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
pallet_bridge_grandpa::BestFinalized::<Runtime, Instance4>::get()
pallet_bridge_grandpa::BestFinalized::<Runtime, bridge_common_config::BridgeGrandpaRococoBulletinInstance>::get()

@@ -1343,7 +1344,7 @@ impl_runtime_apis! {
let bench_lane_id = <Self as BridgeMessagesConfig<bridge_to_westend_config::WithBridgeHubWestendMessagesInstance>>::bench_lane_id();
use bp_runtime::Chain;
let bridged_chain_id =<Self as pallet_bridge_messages::Config<bridge_to_westend_config::WithBridgeHubWestendMessagesInstance>>::BridgedChain::ID;
pallet_bridge_relayers::Pallet::<Runtime>::relayer_reward(
pallet_bridge_relayers::RelayerRewards::<Runtime>::get(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
pallet_bridge_relayers::RelayerRewards::<Runtime>::get(
pallet_bridge_relayers::RelayerRewards::<Runtime, bridge_common_config::RelayersForLegacyLaneIdsMessagesInstance>::get(

@@ -63,6 +63,7 @@ use frame_support::{
construct_runtime, derive_impl,
dispatch::DispatchClass,
genesis_builder_helper::{build_state, get_preset},
instances::Instance1,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
instances::Instance1,

@@ -849,7 +850,7 @@ impl_runtime_apis! {

impl bp_rococo::RococoFinalityApi<Block> for Runtime {
fn best_finalized() -> Option<HeaderId<bp_rococo::Hash, bp_rococo::BlockNumber>> {
BridgeRococoGrandpa::best_finalized()
pallet_bridge_grandpa::BestFinalized::<Runtime, Instance1>::get()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
pallet_bridge_grandpa::BestFinalized::<Runtime, Instance1>::get()
pallet_bridge_grandpa::BestFinalized::<Runtime, bridge_to_rococo_config::BridgeGrandpaRococoInstance>::get()

@@ -62,6 +62,7 @@ use frame_support::{
construct_runtime, derive_impl,
dispatch::DispatchClass,
genesis_builder_helper::{build_state, get_preset},
instances::{Instance3, Instance4},
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
instances::{Instance3, Instance4},

@@ -1224,7 +1225,7 @@ impl_runtime_apis! {
let bench_lane_id = <Self as BridgeMessagesConfig<bridge_to_rococo_config::WithBridgeHubRococoMessagesInstance>>::bench_lane_id();
use bp_runtime::Chain;
let bridged_chain_id =<Self as pallet_bridge_messages::Config<bridge_to_rococo_config::WithBridgeHubRococoMessagesInstance>>::BridgedChain::ID;
pallet_bridge_relayers::Pallet::<Runtime>::relayer_reward(
pallet_bridge_relayers::RelayerRewards::<Runtime>::get(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
pallet_bridge_relayers::RelayerRewards::<Runtime>::get(
pallet_bridge_relayers::RelayerRewards::<Runtime, bridge_common_config::RelayersForLegacyLaneIdsMessagesInstance>::get(

@@ -916,7 +917,7 @@ impl_runtime_apis! {

impl bp_westend::WestendFinalityApi<Block> for Runtime {
fn best_finalized() -> Option<HeaderId<bp_westend::Hash, bp_westend::BlockNumber>> {
BridgeWestendGrandpa::best_finalized()
pallet_bridge_grandpa::BestFinalized::<Runtime, Instance3>::get()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For these cases we could still write a manual getter though.

Yes, I agree. It would also be less error-prone to avoid accidentally using an invalid InstanceXYZ, and it would significantly reduce the changes in this PR. There would only be a small internal change to the pallet.

@paritytech-review-bot paritytech-review-bot bot requested a review from a team January 15, 2025 14:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
T2-pallets This PR/Issue is related to a particular pallet.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants