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

Add kusama and polkadot SP constants #115

Merged
merged 17 commits into from
Jan 4, 2024

Conversation

seadanda
Copy link
Contributor

@seadanda seadanda commented Dec 8, 2023

This PR brings the Kusama and Polkadot constants for System Parachains into the control of the Fellowship.

Following up on paritytech/polkadot-sdk#2620 and paired with paritytech/polkadot-sdk#2666.

Types are still used from the parachains-common crate, but constants are moved into a new crate system-parachains-constants in this repo.

The re-exports pub use parachains_common as common in both asset hubs and collectives have been removed. This is a breaking change for anybody using them in this way, but these should be directly imported from parachains-common. This should be noted on the changelog.

@seadanda seadanda marked this pull request as ready for review December 12, 2023 16:11
@seadanda seadanda changed the title Add kusama and polkadot SP constants (WIP) Add kusama and polkadot SP constants Dec 13, 2023
system-parachains/constants/Cargo.toml Outdated Show resolved Hide resolved
system-parachains/constants/src/kusama.rs Outdated Show resolved Hide resolved
system-parachains/constants/src/kusama.rs Show resolved Hide resolved
system-parachains/constants/src/lib.rs Outdated Show resolved Hide resolved
@ggwpez
Copy link
Member

ggwpez commented Jan 3, 2024

@Bullrich the bot is not picking up the approvals here.

@Bullrich
Copy link
Contributor

Bullrich commented Jan 3, 2024

@Bullrich the bot is not picking up the approvals here.

Has something changed in the identity pallet? Some users who had an account now are null.

For example, when fetching @joepetrowski's account:

Fetching identity of '15DCWHQknBjc5YPFoVj8Pn2KoqrqYywJJ95BYNYJ4Fj3NLqz', rank: 2
Identity is null. Skipping

This used to work before and I haven't modified any of this code, so I first need to be aware of any changes so I can keep the code up to date.

@Bullrich
Copy link
Contributor

Bullrich commented Jan 3, 2024

Seems that the problem is caused by subidentities. I have an issue for it: paritytech/review-bot#107

Will put it as top priority and start working on it right now 🦾

@bkchr bkchr enabled auto-merge (squash) January 4, 2024 12:19
@bkchr bkchr merged commit 923f6c1 into polkadot-fellows:main Jan 4, 2024
13 checks passed
@seadanda seadanda deleted the donal-mv-sp-common-fellowship branch January 4, 2024 16:12
@joepetrowski joepetrowski mentioned this pull request Jan 24, 2024
fellowship-merge-bot bot pushed a commit that referenced this pull request Jan 25, 2024
Since #115, chains do
not use their local `constants.rs` values, so
#131 did not have the
desired effect.

Would suggest a special Asset Hub 1.1.1 release to get this deployed as
soon as possible since a lot of applications were planning on it.
ggwpez pushed a commit that referenced this pull request Jan 25, 2024
Since #115, chains do
not use their local `constants.rs` values, so
#131 did not have the
desired effect.

Would suggest a special Asset Hub 1.1.1 release to get this deployed as
soon as possible since a lot of applications were planning on it.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants