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

addition of spacewalk pallets to pendulum #401

Merged
merged 7 commits into from
Jan 26, 2024

Conversation

gianfra-t
Copy link
Contributor

@gianfra-t gianfra-t commented Jan 25, 2024

@pendulum-chain/product: This PR adds changes to the node client code that require a redeployment of the collator nodes to take effect.
Please ensure that the collator nodes are redeployed after this PR is merged. This is because of the new RPC methods that Spacewalk brings (and genesis config).

Closes #400 .

Changes

This PR is for the addition of all the relevant Spacewalk pallets into the Pendulum runtime. It mostly adopt the same configuration used in Amplitude, except for:

Extra notes

  • Although we must add entries into the genesis config for the new pallets, this values are taken from Amplitude as-is. In any case this values will not be used on the live chain.
  • On storage versions: After addition, the new pallets will have a storage version of 0 or default. This is fine since none of them have ever performed migrations.

@gianfra-t gianfra-t linked an issue Jan 25, 2024 that may be closed by this pull request
@gianfra-t gianfra-t requested a review from a team January 25, 2024 14:43
@gianfra-t
Copy link
Contributor Author

@ebma should we test the functionality of the newly added pallets in something like zombienet/chopsticks? Or is it okay since we already know they work in Amplitude? WDYT

Copy link
Member

@ebma ebma left a comment

Choose a reason for hiding this comment

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

Looks good to me overall. CI failed though. Maybe it's fixed by my suggested code changes, not sure.

runtime/pendulum/Cargo.toml Show resolved Hide resolved
node/src/chain_spec.rs Outdated Show resolved Hide resolved
@ebma
Copy link
Member

ebma commented Jan 25, 2024

should we test the functionality of the newly added pallets in something like zombienet/chopsticks? Or is it okay since we already know they work in Amplitude? WDYT

I think we should test it but let's rather test the whole runtime when we are about to release. Meaning, I would rather test the runtime once we have the PR to bump spec and transaction versions.

@gianfra-t
Copy link
Contributor Author

gianfra-t commented Jan 25, 2024

let's rather test the whole runtime when we are about to release.

Makes sense!

Copy link
Member

@ebma ebma left a comment

Choose a reason for hiding this comment

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

Seems like formatting .toml files with rustfmt is an open issue that still has some developments going on. Not sure if it's worth to look for other tooling here in the meantime or to deal with it manually for the time being.

The changes look good to me overall and the CI passed so I think this is ready to merge. (Ideally after the indentation is streamlined but that's just nitpicking.)

runtime/pendulum/Cargo.toml Outdated Show resolved Hide resolved
@gianfra-t
Copy link
Contributor Author

Sorry @ebma, I think you need to approve again. I fixed indentations as much as I could and was waiting for the CI.

Copy link
Member

@ebma ebma left a comment

Choose a reason for hiding this comment

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

Thanks for fixing it 🙏

@gianfra-t gianfra-t merged commit 67e39a0 into main Jan 26, 2024
2 checks passed
@gianfra-t gianfra-t deleted the 400-implement-spacewalk-on-pendulum branch January 26, 2024 14:55
@bogdanS98 bogdanS98 changed the title addition of spwacewalk pallets to pendulum addition of spacewalk pallets to pendulum Feb 7, 2024
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.

Implement Spacewalk on Pendulum
2 participants