[AHM] referenda pallet #558

muharem · 2025-01-22T11:32:52Z

meant to be merged into dev-asset-hub-migration branch

referenda pallet migration

ggwpez · 2025-01-22T16:55:43Z

pallets/ah-migrator/src/referenda.rs

+
+		let referendum = if let ReferendumInfo::Ongoing(mut status) = referendum {
+			// TODO: map call and update preimage
+			// TODO: if mapping fails cancel referendum


I wonder if we have to set alarms in the scheduler? The scheduler normally checks if it went into "passing" or something.
Could be that it is not needed for big OpenGov refs since people vote on them often and that also sets the alarm. But it would probably cleaner to do.

Right. I keep it in mind. Not sure yet how we solve it, but for sure make sure it works correctly

muharem · 2025-01-28T08:54:17Z

pallets/ah-migrator/src/referenda.rs

+					i
+				} else {
+					defensive!("Call encoded length is too large for inline encoding");
+					// TODO: if we have such a case we would need to dispatch two call on behalf of


right now they actually all fit

You mean all ongoing referenda reference calls that - after translation - dont have more than 128 bytes? Huh, did not expect that.

yes, never panics here

muharem · 2025-01-28T08:59:47Z

system-parachains/common/src/pay.rs

+#[derive(
+	Encode, Decode, Eq, PartialEq, Clone, RuntimeDebug, scale_info::TypeInfo, MaxEncodedLen,
+)]
+pub enum VersionedLocatableAccount {


based on this RFC - paritytech/polkadot-sdk#4715

I would prefer these types to be located in the same repo as the runtimes itself. Bu that would mean we need to copy past them for westend, etc.

@ggwpez what you think? should I move it to sdk?

ggwpez · 2025-01-28T11:07:46Z

pallets/ah-migrator/src/referenda.rs

+/// The `RuntimeOrigin` is a type argument that needs to be mapped to AH `RuntimeOrigin`.
+/// Inline `proposal`s and the ones stored by `Preimage` pallet should also be mapped to get the
+/// final local `pallet_referenda::ReferendumInfoFor::<T, ()>`.
+pub type RcReferendumInfoOf<T, I> = ReferendumInfo<


Suggested change

pub type RcReferendumInfoOf<T, I> = ReferendumInfo<

// From https://github.com/polkadot-fellows/runtimes/blob/a560cf00324a8520cab2b1a9c5e4a36efea9dae9/pallets/ah-migrator/src/referenda.rs#L30-L39

pub type RcReferendumInfoOf<T, I> = ReferendumInfo<

I think its good to have the location as link here so we can easily compare with the original and check if something changed later on.

the link to the same type? the closes type is ReferendumInfoOf<T, I> in referenda pallet

🤦‍♂️ oh, yea thats what i meant

pallets/ah-migrator/src/referenda.rs

ggwpez · 2025-01-28T11:18:50Z

pallets/ah-migrator/src/referenda.rs

+					e
+				} else {
+					log::warn!("Failed to fetch preimage for referendum {}", id);
+					cancel_referendum(id, status);


Note: we have to cancel here because we cannot translate the preimage, right? So even though ongoing referenda without preimage are normally perfectly fine, in this case they are not because we can ensure that they are translated correctly?
Need to write this into some doc that people still try to post their preimages before the migration starts.

right if we cannot fetch and translate we cannot even keep the same hash, because it has not meaning on new chain.

ggwpez · 2025-01-28T11:23:11Z

pallets/ah-migrator/src/referenda.rs

+					i
+				} else {
+					defensive!("Call encoded length is too large for inline encoding");
+					// TODO: if we have such a case we would need to dispatch two call on behalf of


You mean all ongoing referenda reference calls that - after translation - dont have more than 128 bytes? Huh, did not expect that.

ggwpez · 2025-01-28T11:26:22Z

pallets/ah-migrator/src/referenda.rs

+					defensive!("Call encoded length is too large for inline encoding");
+					// TODO: if we have such a case we would need to dispatch two call on behalf of
+					// the original preimage submitter to release the funds for the new preimage
+					// deposit and dispatch the call to note a new preimage. or we provide a


As intermediate solution we could also just request the hash of the translated call, then we dont have to cancel the referenda and also dont have to make calls on behalf of the preimage submitter. The fellowship could then as cleanup step after the migration submit the translated preimage.
I think that should be good for now to not make it too complicated.

Otherwise we can probably also mess with the storage of the preimage pallet to overwrite the preimage with the new content and keep the deposit active.

I wont cancel them because they do not fit into inline. Just now no cases and I wanna move forward for now.
But right now, we can use Preimage::note, but that would request that preimage and for the referendums that wont pass we will have to cleanup/unrequest them. totally fine, but I leave it as is for now.

system-parachains/asset-hubs/asset-hub-polkadot/src/governance/mod.rs

system-parachains/asset-hubs/asset-hub-polkadot/src/governance/origins.rs

ggwpez · 2025-01-28T12:54:41Z

system-parachains/asset-hubs/asset-hub-polkadot/src/governance/tracks.rs

+
+//! Track configurations for governance.
+
+// TODO: RC_DAYS, RC_MINUTES should be RC_RC_DAYS, RC_MINUTES, etc.


Okay you marked it here 👍

system-parachains/asset-hubs/asset-hub-polkadot/src/governance/tracks.rs

system-parachains/asset-hubs/asset-hub-polkadot/src/lib.rs

ggwpez · 2025-01-28T16:08:23Z

system-parachains/asset-hubs/asset-hub-polkadot/src/migration.rs

+
+/// Relay Chain Utility Call obtained from cargo expand.
+///
+/// The variants that are not generally used in Governance are not included.


We have to pay attention that this does not change in the SDK.

right. we probably need to give some heads up to devs and ask them to let us know if they introducing any changes to migrating pallets

system-parachains/asset-hubs/asset-hub-polkadot/src/migration.rs

ggwpez · 2025-01-28T16:17:37Z

system-parachains/asset-hubs/asset-hub-polkadot/src/migration.rs

+				return Err(a)
+			},
+		};
+		Self::map(rc_call).map_err(|_| a)


Do you know what calls are in the referenda pallet that we do not translate? Should also be in the user impact section

not yet. my plan still to have everything translated. I think I will know later if we have anything we cannot translate

system-parachains/asset-hubs/asset-hub-polkadot/src/migration.rs

ggwpez · 2025-01-28T16:24:11Z

system-parachains/asset-hubs/asset-hub-polkadot/src/migration.rs

+			},
+			RcRuntimeCall::Treasury(inner_call) => {
+				let call =
+					inner_call.using_encoded(|mut e| Decode::decode(&mut e)).map_err(|err| {


Sorry what is this doing? Why do we not have to call map?

here we know that RC inner call layout is basically the same with AH inner call, it encodes to the same sequence of bytes, and instead of manually mapping it, we can just encode RC treasury call and decode it to AH treasury call (true for all treasury calls for example except treasury spend call for which we have an arm above).

Ah okay, since this pallet's calls do not have anything to translate in them 👍

ggwpez · 2025-01-28T17:02:59Z

system-parachains/common/src/pay.rs

+
+/// Pay on the local chain with `fungibles` implementation if the beneficiary and the asset are both
+/// local.
+pub struct LocalPay<F, A, C>(core::marker::PhantomData<(F, A, C)>);


Suggested change

pub struct LocalPay<F, A, C>(core::marker::PhantomData<(F, A, C)>);

// From https://github.com/polkadot-fellows/runtimes/blob/a560cf00324a8520cab2b1a9c5e4a36efea9dae9/system-parachains/common/src/pay.rs#L42-L114

pub struct LocalPay<F, A, C>(core::marker::PhantomData<(F, A, C)>);

This is a new type (=

ggwpez · 2025-01-28T17:03:53Z

Cargo.toml

Did you add the new crate to the members as well?

…harem-ahm-referenda

Signed-off-by: Oliver Tale-Yazdi <[email protected]>

Co-authored-by: Oliver Tale-Yazdi <[email protected]>

muharem · 2025-01-29T10:22:46Z

ah, I have two very different branches, local and remote

…rem-ahm-referenda

ggwpez · 2025-01-29T10:34:03Z

Oh damn, i wrote you that i rebased your branch 🙃 you can also force push and rebase again otherwise

muharem · 2025-01-29T10:38:35Z

Oh damn, i wrote you that i rebased your branch 🙃 you can also force push and rebase again otherwise

I think solved. Can you check my replies and lets merge it

muharem · 2025-01-29T10:41:17Z

Oh damn, i wrote you that i rebased your branch 🙃 you can also force push and rebase again otherwise

Yes. my fault. I also accepted your code proposals from review after merging dev branch locally and introducing inner referenda stage (=

muharem added 3 commits January 22, 2025 19:26

referenda pallet

f3777c9

uncomment

01c291b

fix

8d38766

ggwpez reviewed Jan 22, 2025

View reviewed changes

ggwpez mentioned this pull request Jan 23, 2025

Asset Hub Migration: Pallet List ☂ #555

Open

64 tasks

onging referenda migration

a560cf0

muharem commented Jan 28, 2025

View reviewed changes

ggwpez reviewed Jan 28, 2025

View reviewed changes

system-parachains/asset-hubs/asset-hub-polkadot/src/migration.rs Outdated Show resolved Hide resolved

ggwpez reviewed Jan 28, 2025

View reviewed changes

system-parachains/asset-hubs/asset-hub-polkadot/src/migration.rs Outdated Show resolved Hide resolved

ggwpez reviewed Jan 28, 2025

View reviewed changes

ggwpez approved these changes Jan 28, 2025

View reviewed changes

ggwpez mentioned this pull request Jan 28, 2025

[AHM] Misc Issues paritytech/polkadot-sdk#7369

Open

23 tasks

ggwpez and others added 5 commits January 28, 2025 19:11

Merge remote-tracking branch 'origin/dev-asset-hub-migration' into mu…

e5ddb88

…harem-ahm-referenda

merge fixup

f00271f

Signed-off-by: Oliver Tale-Yazdi <[email protected]>

Merge branch 'dev-asset-hub-migration' into muharem-ahm-referenda

954a19b

fixes

495d7aa

Apply suggestions from code review

45d9e85

Co-authored-by: Oliver Tale-Yazdi <[email protected]>

Merge remote-tracking branch 'origin/muharem-ahm-referenda' into muha…

7d00c7e

…rem-ahm-referenda

fixes

90c386e

ggwpez approved these changes Jan 29, 2025

View reviewed changes

ggwpez marked this pull request as ready for review January 29, 2025 11:03

ggwpez merged commit 3dbad2b into polkadot-fellows:dev-asset-hub-migration Jan 29, 2025
33 of 51 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[AHM] referenda pallet #558

[AHM] referenda pallet #558

muharem commented Jan 22, 2025

ggwpez Jan 22, 2025 •

edited

Loading

muharem Jan 23, 2025

muharem Jan 28, 2025

ggwpez Jan 28, 2025

muharem Jan 29, 2025

muharem Jan 28, 2025

muharem Jan 29, 2025

ggwpez Jan 28, 2025

muharem Jan 29, 2025

ggwpez Jan 29, 2025

ggwpez Jan 28, 2025

muharem Jan 29, 2025

ggwpez Jan 28, 2025

ggwpez Jan 28, 2025

muharem Jan 29, 2025

ggwpez Jan 28, 2025

ggwpez Jan 28, 2025

muharem Jan 29, 2025

ggwpez Jan 28, 2025 •

edited

Loading

muharem Jan 29, 2025

ggwpez Jan 28, 2025

muharem Jan 29, 2025

ggwpez Jan 29, 2025

ggwpez Jan 28, 2025

muharem Jan 29, 2025

ggwpez Jan 28, 2025

muharem commented Jan 29, 2025 •

edited

Loading

ggwpez commented Jan 29, 2025

muharem commented Jan 29, 2025

muharem commented Jan 29, 2025 •

edited

Loading

	pub type RcReferendumInfoOf<T, I> = ReferendumInfo<
	// From https://github.com/polkadot-fellows/runtimes/blob/a560cf00324a8520cab2b1a9c5e4a36efea9dae9/pallets/ah-migrator/src/referenda.rs#L30-L39
	pub type RcReferendumInfoOf<T, I> = ReferendumInfo<


		//! Track configurations for governance.

		// TODO: RC_DAYS, RC_MINUTES should be RC_RC_DAYS, RC_MINUTES, etc.

	pub struct LocalPay<F, A, C>(core::marker::PhantomData<(F, A, C)>);
	// From https://github.com/polkadot-fellows/runtimes/blob/a560cf00324a8520cab2b1a9c5e4a36efea9dae9/system-parachains/common/src/pay.rs#L42-L114
	pub struct LocalPay<F, A, C>(core::marker::PhantomData<(F, A, C)>);

[AHM] referenda pallet #558

[AHM] referenda pallet #558

Conversation

muharem commented Jan 22, 2025

ggwpez Jan 22, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ggwpez Jan 28, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

muharem commented Jan 29, 2025 • edited Loading

ggwpez commented Jan 29, 2025

muharem commented Jan 29, 2025

muharem commented Jan 29, 2025 • edited Loading

ggwpez Jan 22, 2025 •

edited

Loading

ggwpez Jan 28, 2025 •

edited

Loading

muharem commented Jan 29, 2025 •

edited

Loading

muharem commented Jan 29, 2025 •

edited

Loading