zdave-parity / polkadot-bulletin-chain

MIT No Attribution
0 stars 3 forks source link

Add bridges subtree #18

Closed svyatonik closed 1 year ago

svyatonik commented 1 year ago

To add a bridges subtree is the only integration option we support now:

Before this PR, we have had two main branches in our repo:

This codebase uses v1.0.0.0 git references, so I had to add another branch to be able to use it here.

Once subtree is added, I'll add all required bridge pallets to the runtime + will add a relay code to our codebase. Once it is done, we could start working on local tests (zombienet based) for this bridge.

svyatonik commented 1 year ago

cc @zdave @NachoPal

zdave-parity commented 1 year ago

How would I update this subtree? Could you stick something in docs?

svyatonik commented 1 year ago

How would I update this subtree? Could you stick something in docs?

I've added this document

svyatonik commented 1 year ago

Actually let me check @NachoPal suggestion to reuse bridges subtree from Cumulus repo :) Don't know if it'll work

zdave-parity commented 1 year ago

Actually let me check @NachoPal suggestion to reuse bridges subtree from Cumulus repo :) Don't know if it'll work

This subtree seems preferable to me?

NachoPal commented 1 year ago

Actually let me check @NachoPal suggestion to reuse bridges subtree from Cumulus repo :) Don't know if it'll work

This subtree seems preferable to me?

Why is it necessary to copy the whole subtree instead of importing what's needed from polkadot-sdk?

zdave-parity commented 1 year ago

Well it seems that using git subtree is the recommended way to pull the bridges stuff into your project. Yes, it seems that Cumulus has pulled a copy in that it might be possible to reference, but adding a dependency on Cumulus just for this does not seem wise to me.

NachoPal commented 1 year ago

What do you think @svyatonik? What will happen for the next release? what option is easier to maintain?

svyatonik commented 1 year ago

I see you stick now to the polkadot-v1.0.0. If we'll keep doing that, then subtree is the option. If we'll migrate to polkadot-sdk references, then probably referencing polkadot-sdk/bridges is better. However I'm still not sure if this is possible.

So please decide - whether we need to migrate to polkadot-sdk or leave 1.0.0 refs. This should answer the other question.

importing what's needed

@NachoPal Actually this isn't the whole brdges repo in this subtree - you can see there's a lot of other crates there. We are only adding what we need. But this "what we need" is for bridge hub parachains, so it references some Cumulus stuff. We could go further and start removing everything cumulus specific from this subtree, but it'd require some more efforts. So if we start with subtree, let's start with what we already have and afterwards we'll remove extra stuff that is not required here (we need to alter or add a script similar to this one)

NachoPal commented 1 year ago

Ok, let's use subtree

NachoPal commented 1 year ago

So please decide - whether we need to migrate to polkadot-sdk or leave 1.0.0 refs. This should answer the other question.

@svyatonik Actually, we should reference to polkadot-sdk, but the question here is what will happen with bridges stuff when polkadot-sdk crates are released. Then we should ref the crates, and not any polkadot-sdk branch. We'll have to stick to polkadot-sdk release branches until bridges is audited?

svyatonik commented 1 year ago

I think in this case we could just change the polkadot-v.1.0.0-audited branch to reference the Substrate/Polkadot crates from crates.io. And keep using the subtree. We are not planning to publish bridges crates from polkadot-sdk in near future (until we'll resolve issues we have with our testnets + migrate to polkadot-sdk + deprecate the parity-bridges-common repo at all).

svyatonik commented 1 year ago

Hey @NachoPal @zdave-parity . Let's use subtree for now - we may reconsider it later. If you don't have any other questions/suggestions for this PR, let's maybe merge it? I have more follow-up PRs depending on that (still will need some time to polish the code).

zdave-parity commented 1 year ago

Thought I had approved this before, sorry...