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

3rd party pallet support (e.g. ORML) #14

Open
xlc opened this issue May 22, 2024 · 34 comments
Open

3rd party pallet support (e.g. ORML) #14

xlc opened this issue May 22, 2024 · 34 comments
Assignees

Comments

@xlc
Copy link

xlc commented May 22, 2024

Is it possible to make psvm also supports other pallets like ORML? In that way it can identify the right version of ORML to use.
Happy to do whatever needed on ORML side to make this possible.

@patriciobcs
Copy link
Collaborator

In theory yes, it's definitely possible. I can see there is a Cargo.dev.toml in the root of the repo containing the versions. If this file is up to date (meaning that it contains the same version published for each crate as its crates.io release), then the only thing needed is to add the workflow of parsing this file, create the mapping and merge this mapping with the Polkadot SDK mapping.

I see there are branches in the repository with Polkadot SDK versions like polkadot-v1.9.0. Are this branches always up to date with the packages published in crates.io? All this branches contain an up to date Cargo.dev.toml file? Is the publishing process automatized? Are all the crates mentioned in Cargo.dev.toml published in crates.io for all the polkadot branches?

@xlc
Copy link
Author

xlc commented May 28, 2024

The Cargo.dev.toml is up to date. i.e. It is the file used for the publishing process.
I am using cargo release to do the publishing
One thing to keep in mind is that we don't update ORML for every polkadot-sdk release. e.g. there is no polkadot-v1.8.0 one and it can takes a few weeks delay before we update ORML to latest polkadot-sdk release.

All the crates in ORML have a same version number and all of them get released.

@patriciobcs
Copy link
Collaborator

In this case, I think is totally possible to integrate it. If ORML doesn't exists for an specific version, when trying to upgrade the versions of a project, the tool will just skip the ORML crates (potentially could be possible to add a warning in case a ORML crate is detected). Therefore, for now we could add a --orml flag to the CLI that if used, will fetch the versions from ORML's Cargo.dev.toml, parse them into a versions mapping and merge this mapping with the Polkadot SDK mapping before the mapping is applied to the project being upgraded.

@0xsouravm
Copy link
Contributor

@xlc So are we looking to overwrite our polkadot-sdk crates with the corresponding ORML crates for that version? Or are we trying to fetch the corresponding updated ORML crate version for the polkadot-sdk version we mentioned, given that we are using a few of the ORML crates?

@xlc
Copy link
Author

xlc commented Aug 2, 2024

The problem to solve is that for a chain that's using ORML, they can use psvm to manage polkadot-sdk version but still need to go through some hops to figure out what version of ORML to use. It is not too hard as all crates in ORML are sharing a single version number so there is only one number to deal with but still, that can be improved.

@0xsouravm
Copy link
Contributor

0xsouravm commented Aug 2, 2024

If for example the command is psvm --version 1.14.0 --orml.
The desired behaviour of the --orml flag is to update the versions of the normal polkadot-sdk crates to their corresponding versions and check if their are any ORML crates used and then update them based on the version in the ORML repository? Assuming there is a v1.14.0 in the ORML repository.

@xlc
Copy link
Author

xlc commented Aug 2, 2024

exactly

@0xsouravm
Copy link
Contributor

0xsouravm commented Aug 2, 2024

I wish to take up on this issue. Can you assign this to me @patriciobcs?

@shawntabrizi
Copy link
Member

I have assigned you @0xsouravm

@0xsouravm
Copy link
Contributor

0xsouravm commented Aug 3, 2024

@xlc Just to finalise the workflow:

  1. I have a dependency in the form of: orml-tokens = { git = "https://github.com/open-web3-stack/open-runtime-module-library/tokens", branch = "polkadot-v1.1.0", default-features = false } alongside other polkadot-sdk dependencies in my Cargo.toml

  2. I run psvm -v 1.10.0 --orml.

  3. All polkadot-sdk dependencies get updated to their corresponding v1.10.0 versions. And the orml-tokens dependency is transformed into: orml-tokens = { version = "0.10.0", default-features = false } because there is indeed a branch called polkadot-v1.10.0 in the orml repository

  4. If I had chosen a version like psvm -v 1.8.0 --orml, then all the polkadot-sdk dependencies would be updated to the mentioned version but not any orml dependencies because there is no polkadot-v1.8.0 branch in the orml repository.

NOTE: This is only supposed to work for /open-web3-stack/open-runtime-module-library i.e. the official repository of ORML, and not any repositories that have adapted and have their own versions of the ORML(Ex: bifrost-finance/open-runtime-module-library)

@xlc
Copy link
Author

xlc commented Aug 4, 2024

Yeah that will work. It will be great if you can define exactly what is expected by psvm and then I can ensure the ORML release process is compatiable.

Not exactly in scope but a nice to have thing is make this generalized so it can support other libraries as well as long as they are following the same release process.

@0xsouravm
Copy link
Contributor

0xsouravm commented Aug 5, 2024

My current methodology is:

  1. Fetch all versions in ORML that match with Polkadot-sdk (from v1.1.0 - v1.14.0)
  2. Check if the supplied version in the command is in the list of ORML versions.
  3. If it is then read the Cargo.dev.toml in the ORML repository for that version and fetch the participating crates under [workspace] members
  4. Fetch one of the above crates version and apply that to the local Cargo.toml since all ORML crates have the same version for a particular release branch.

The issue here is reading the Cargo.dev.toml and fetching the version by going though a crate mentioned in there. This is not a really robust approach and only applies to ORML.

If there were some file where the crates' names were mentioned alongside their versions (this would be redundant in case of ORML because every version would be same, but this is a more generic approach if we are planning to include support for other libraries) like a key-value pair for example, I could directly parse that file after STEP-2.

And the pre-requisite for this to work is those pallets in ORML should be released in crates.io for that particular version.

@xlc

@xlc
Copy link
Author

xlc commented Aug 5, 2024

ORML crates are published to crates.io so maybe it is possible to fetch the version mapping from there instead? If it is too slow or too much requests to make, maybe we can have a script that fetches the data, build a json and have it saved somewhere (this repo or ORML)

@0xsouravm
Copy link
Contributor

I think it might not be possible to fetch the correct version of the crate from crates.io corresponding to the release-branch version that is passed in the command. From the current usage of the crates.io API in the psvm, it simply fetches all the names of the crates that are owned by a particular user(Parity in this case).

@xlc
Copy link
Author

xlc commented Aug 5, 2024

I am not familiar with crates.io and cargo enough so this may not be feasible:

  • do normal update for polkadot-sdk
  • parse Cargo.lock or use whatever API from cargo to identify crates with multiple versions
  • filter the one from polkadot-sdk
  • identify what deps caused the multiple versions
  • use crates.io try to find new version of such deps that depends on the correct polkadot-sdk version
  • update it

@0xsouravm
Copy link
Contributor

0xsouravm commented Aug 5, 2024

I am not really sure if I understood your approach. Or maybe I understood the problem wrong.
Let's consider this as an example Cargo.toml file:

[dependencies]
codec = { package = "parity-scale-codec", version = "3.0.0", default-features = false, features = ["derive"] }
hex-literal = { version = "0.4.1", optional = true }

# Substrate
frame-benchmarking = { git = "https://github.com/.../polkadot-sdk", branch = "release-crates-io-v1.3.0", default-features = false }
frame-executive = { git = "https://github.com/.../polkadot-sdk", branch = "release-crates-io-v1.3.0", default-features = false }
frame-support = { git = "https://github.com/.../polkadot-sdk", branch = "release-crates-io-v1.3.0", default-features = false }

# ORML
orml-tokens = { git = "https://github.com/open-web3-stack/../tokens", branch = "polkadot-v1.1.0", default-features = false  }
orml-xcm = { git = "https://github.com/open-web3-stack/.../xcm", branch = "polkadot-v1.1.0", default-features = false  }

The command is: psvm --version 1.5.0 --orml

The output is:

[dependencies]
codec = { package = "parity-scale-codec", version = "3.0.0", default-features = false, features = ["derive"] }
hex-literal = { version = "0.4.1", optional = true }

# Substrate
frame-benchmarking = { version = "27.0.0", default-features = false }
frame-executive = { version = "27.0.0", default-features = false }
frame-support = { version = "27.0.0", default-features = false }

# ORML
orml-tokens = {version = "0.7.0", default-features = false  } # Assuming this is the correct version for polka_sdk-v1.5.0
orml-xcm = { version = "0.7.0", default-features = false  } # Assuming this is the correct version for polka_sdk-v1.5.0

This is my understanding: I will update the ORML crates with the correct version if I find them in the release branches in the ORML repository, else I will print a warning that ORML crates are found for which versions are unavailable.

@xlc
Copy link
Author

xlc commented Aug 5, 2024

Yes the expected output is correct. I am just proposing a more generic approach to address deps conflict that will work with any crates and does not require special branch strategy on ORML repo

@0xsouravm
Copy link
Contributor

Maybe we can work on the generic approach after this PR. I think the commands will run faster if we define a standard for psvm that other libraries should follow if they want their deps to be updated using the command. For now if you can mention in a comment the version of the crates being used in the Cargo.dev.toml, that would be really helpful and make the overall updating way faster than the method I am currently planning to employ for ORML Library.

Something like #crates_version=0.15.0 just somewhere in that toml file so I can parse it and extract the version. Else if you want to define a more standard Plan.toml like the Polkadot-sdk repository, that also works.

@xlc
Copy link
Author

xlc commented Aug 6, 2024

Yeah I can add something in Cargo.dev.toml for each release branch.
For Plan.toml, it seems to be generated by parity-publish which I don't know if it works with ORML. Should I give it a try or it is really just for parity crates?

@0xsouravm
Copy link
Contributor

0xsouravm commented Aug 6, 2024

I am not entirely sure if parity-publish will work for ORML because I think it takes some credentials in the crates.io API that is for parity alone. Maybe @shawntabrizi can clarify. Else we can have a similar tool for ORML that you can use instead of cargo release I think? But yeah for now I guess just adding something to the Cargo.dev.toml works.

@xlc
Copy link
Author

xlc commented Aug 6, 2024

can you open a PR to ORML to update the Cargo.dev.toml to add it? It is easier you do it to ensure it is exactly what you expecting.

@0xsouravm
Copy link
Contributor

Sure on it. I will be updating versions v1.1.0 - v1.14.0
Whichever is there in the ORML repo

@0xsouravm
Copy link
Contributor

On second thought I will have to raise 7-8 PRs for every applicable branch. Is it possible to add me as a collaborator or something so I can directly commit in the ORML repo? That many PRs for just 1 line of change each seems like an overkill.

@0xsouravm
Copy link
Contributor

I pushed the changes but CI/CD in some branches failed.

@xlc
Copy link
Author

xlc commented Aug 6, 2024

the errors are not critical. we don't need to republish the versions anyway

@0xsouravm
Copy link
Contributor

0xsouravm commented Aug 7, 2024

I finished this #23. cc @ggwpez and @xlc.

@0xsouravm
Copy link
Contributor

Can someone please review the above PR? @shawntabrizi @xlc @ggwpez

@ggwpez
Copy link
Member

ggwpez commented Aug 8, 2024

I will take a look tomorrow, thanks!

@0xsouravm
Copy link
Contributor

Sure, thanks!

@ggwpez
Copy link
Member

ggwpez commented Aug 9, 2024

Okay i put some comments.

I think for usability it would be nicer to have psvm --polkadot-sdk=1.4.0 --orml=1.3.0 (with backwards compatbility).

@0xsouravm
Copy link
Contributor

Perhaps that will result in conflicts because orml-1.3.0 is compatible with polkasdk-1.3.0 and orml-1.4.0 is compatible with polkasdk-1.4.0

That's why xlc suggested to have the same version and update if the corresponding release version is available in orml repo.

@ggwpez
Copy link
Member

ggwpez commented Aug 9, 2024

Something like #crates_version=0.15.0 just somewhere in that toml file so I can parse it and extract the version. Else if you want to define a more standard Plan.toml like the Polkadot-sdk repository, that also works.

There exists custom metadata that can be parsed by a TOML parser so that we dont need to rely on comments. Example.

That's why xlc suggested to have the same version and update if the corresponding release version is available in orml repo.

Okay i see. So it tries to find a the same version and errors if it cant find it? Yea then its fine 👍

@0xsouravm
Copy link
Contributor

I am updating this PR with the use of the toml parser library. I didn't know we could do that. I will update the OMRL branches with this metadata and use the toml library to fetch it in the code.

@0xsouravm
Copy link
Contributor

0xsouravm commented Aug 13, 2024

I made the changes:

  1. Used TOML parsing crate
  2. Added Rustdoc comments

Can you review @ggwpez ?

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

No branches or pull requests

5 participants