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 polkadot bulletin chain primitives #2542

Merged
merged 3 commits into from
Sep 8, 2023

Conversation

svyatonik
Copy link
Contributor

This is primitives for the Polkadot Bulletin Chain. It is going to be bridged with Polkadot BH. Some bridge details for bridge guys:

  • Polkadot Bulletin Chain tokens have no economic value, so we are going to have whitelisted set of relayers and check it in signed extension;
  • at Bridge Hub, we'll just enable fee refund for useful transactions. We don't want any rewards - will probably run internal (Parity?) relayer for that;
  • there'll be a separate Polkadot system parachain that will send messages to Bulletin Chain over bridge hub. So regular ExportMessage support. This also needs to be unpaid (?) at Polkadot.BH;
  • some messages will also be headed from Bulletin Chain to this new system parachain. We also need to make sure that everything is free here. We need to trust the Bulletin Chain validators then (otherwise it could then spam us with messages and halt the BH);
  • I have to add a separate branch for this bridge, because Bulletin Chain codebase is using the v1.0.0 branches and we need audited code there.

@zdave-parity @NachoPal In bridges we need to define chain primitives, because other (bridged) runtime need to know some facts about the chain it bridges with. In our case, Polkadot.BH will need to know primitives of Bulletin Chain and vice versa. This is also required for our offchain actors (relayers) - they should be able to craft valid transactions and make proper RPC calls. After we'll update subtree with this commit (ofc I'll need to backport it to polkadot-v.1.0.0-audited branch first), I'm gonna use typedefs from this crate in Bulletin Chain runtime to avoid code duplication.

@svyatonik svyatonik added A-chores Something that has to be done, as part of regular maintenance P-Runtime labels Sep 8, 2023
@svyatonik svyatonik merged commit 23bbfb3 into master Sep 8, 2023
15 checks passed
@svyatonik svyatonik deleted the add-polkadot-bulletin-chain-primitives branch September 8, 2023 10:07
svyatonik added a commit that referenced this pull request Sep 8, 2023
* add polkadot bulletin chain primitives

* also impl ChainWithMessages

* clippy
type Header = Header;

type AccountId = AccountId;
type Balance = Balance;

Choose a reason for hiding this comment

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

The Bulletin Chain does not have balances at all. It's not that the tokens are worthless, they really just don't exist. I don't know if the bridges stuff supports not having a balance type, but if it doesn't it's at least worth a comment here explaining why we have to provide one and why that isn't a problem (assuming it isn't).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That's a fair point. I don't think we will be using this type anywhere for something critical. I'll add a comment in next commits

use frame_system::limits;
use sp_runtime::{Perbill, StateVersion};

// This chain reuses most of Polkadot primitives.

Choose a reason for hiding this comment

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

Not sure it's a good idea to tie these things to Polkadot? If they are changed in Polkadot they're not going to automatically change in the Bulletin Chain...

Choose a reason for hiding this comment

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

Should just take the definitions from runtime/src/lib.rs in the Bulletin Chain repo?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not sure it's a good idea to tie these things to Polkadot? If they are changed in Polkadot they're not going to automatically change in the Bulletin Chain...

True, but if types will change in Polkadot, they won't automatically change in bridges bp-polkadot-core crate as well :) Because polkadot uses its own crates and our bp-polkadot-core is just a minimal knowledge of Polkadot-like chains that we need to implement bridges. We need to track the possible type changes and act accordingly.

In bridges there are two "base" chain types that we are able to handle currently - standalone (Polkadot-like) chains and parachains (Cumulus-based mostly). To me the Bulletin Chain looks more Polkadot-like, hence the usage of the bp_polkadot_core crate. I could just copy paste those types and constants here, but it'll just increase amount of code and won't change anything in practice :) But feel free to insist - it isn't a big deal

Should just take the definitions from runtime/src/lib.rs in the Bulletin Chain repo?

There are two huge issues with that:

  • we are in a different repo and if we'll start referring bulleting chain crates here, it'll cause circular references;
  • this crate will be used by polkadot bridge hub runtime and we don't want to embed (at least build) another runtime when building Polkadot BH.

Actually, with your chain it is easier and I'd like to "use typedefs from this crate in Bulletin Chain runtime to avoid code duplication" as the PR description states later, if you'll accept such PR. I.e. instead of having type BlockNumber = u32; in bulletin chain runtime, I'd like to have it here and then simply reuse in runtime. WDYT?

Copy link

@zdave-parity zdave-parity Sep 8, 2023

Choose a reason for hiding this comment

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

I meant cut the definitions out of the Bulletin Chain's runtime/src/lib.rs and paste them here. They're just typedefs of Substrate types AFAIK, shouldn't be any dependency issues.

I'm happy to have the Bulletin Chain then get the definitions from here to avoid duplication; we're going to be depending on the bridges stuff anyway.

svyatonik added a commit that referenced this pull request Sep 19, 2023
* polkadot-staging for v1.0.0

* Add polkadot bulletin chain primitives (#2542)

* add polkadot bulletin chain primitives

* also impl ChainWithMessages

* clippy

* instead of requiring sp_std::vec::Vec import when using runtime API generation macro, let's use full type path directly in macro (#2551)

* Polkadot Bulletin Chain client (#2552)

* relay-polkadot-bulletin-client

* generate Polkadot Bulletin Chain Runtime

* Add relays that will be used in Polkadot Bulletin <> Polkadot.BH bridge (#2556)

* added Polkadot.BH <> Polkadot Bulletin chain relays

* uncommented ED stuff

* complex PolkadotBulletin <> Polkadot.BH relay

* removed TODO

* spelling

* prepare refund extension infra to add refund extension for messages from standalone chain (#2558)

* prepare refund extension infra to add refund extension for messages from standalone chain

* spelling

* apply adapter to fix compilation

* clippy

* added POLKADOT_BULLETIN_CHAIN_ID constant

* RefundBridgedGrandpaMessages to refund transaction costs for messages coming to/from bridged standalone/relay chain (#2566)

* RefundBridgedGrandpaMessages to refund transaction costs for messages coming to/from bridged standalone/relay chain

* clippy

* fix compilation

* fix codec dependency (#2567)

* Support message relay limits override (#2570)

* support message relay limits overrides for bridges

* spelling

* export EXTRA_STORAGE_PROOF_SIZE for Polkadot Bulletin (#2572)
serban300 pushed a commit to serban300/parity-bridges-common that referenced this pull request Mar 27, 2024
* polkadot-staging for v1.0.0

* Add polkadot bulletin chain primitives (paritytech#2542)

* add polkadot bulletin chain primitives

* also impl ChainWithMessages

* clippy

* instead of requiring sp_std::vec::Vec import when using runtime API generation macro, let's use full type path directly in macro (paritytech#2551)

* Polkadot Bulletin Chain client (paritytech#2552)

* relay-polkadot-bulletin-client

* generate Polkadot Bulletin Chain Runtime

* Add relays that will be used in Polkadot Bulletin <> Polkadot.BH bridge (paritytech#2556)

* added Polkadot.BH <> Polkadot Bulletin chain relays

* uncommented ED stuff

* complex PolkadotBulletin <> Polkadot.BH relay

* removed TODO

* spelling

* prepare refund extension infra to add refund extension for messages from standalone chain (paritytech#2558)

* prepare refund extension infra to add refund extension for messages from standalone chain

* spelling

* apply adapter to fix compilation

* clippy

* added POLKADOT_BULLETIN_CHAIN_ID constant

* RefundBridgedGrandpaMessages to refund transaction costs for messages coming to/from bridged standalone/relay chain (paritytech#2566)

* RefundBridgedGrandpaMessages to refund transaction costs for messages coming to/from bridged standalone/relay chain

* clippy

* fix compilation

* fix codec dependency (paritytech#2567)

* Support message relay limits override (paritytech#2570)

* support message relay limits overrides for bridges

* spelling

* export EXTRA_STORAGE_PROOF_SIZE for Polkadot Bulletin (paritytech#2572)
serban300 pushed a commit to serban300/parity-bridges-common that referenced this pull request Apr 8, 2024
* polkadot-staging for v1.0.0

* Add polkadot bulletin chain primitives (paritytech#2542)

* add polkadot bulletin chain primitives

* also impl ChainWithMessages

* clippy

* instead of requiring sp_std::vec::Vec import when using runtime API generation macro, let's use full type path directly in macro (paritytech#2551)

* Polkadot Bulletin Chain client (paritytech#2552)

* relay-polkadot-bulletin-client

* generate Polkadot Bulletin Chain Runtime

* Add relays that will be used in Polkadot Bulletin <> Polkadot.BH bridge (paritytech#2556)

* added Polkadot.BH <> Polkadot Bulletin chain relays

* uncommented ED stuff

* complex PolkadotBulletin <> Polkadot.BH relay

* removed TODO

* spelling

* prepare refund extension infra to add refund extension for messages from standalone chain (paritytech#2558)

* prepare refund extension infra to add refund extension for messages from standalone chain

* spelling

* apply adapter to fix compilation

* clippy

* added POLKADOT_BULLETIN_CHAIN_ID constant

* RefundBridgedGrandpaMessages to refund transaction costs for messages coming to/from bridged standalone/relay chain (paritytech#2566)

* RefundBridgedGrandpaMessages to refund transaction costs for messages coming to/from bridged standalone/relay chain

* clippy

* fix compilation

* fix codec dependency (paritytech#2567)

* Support message relay limits override (paritytech#2570)

* support message relay limits overrides for bridges

* spelling

* export EXTRA_STORAGE_PROOF_SIZE for Polkadot Bulletin (paritytech#2572)
bkontur pushed a commit that referenced this pull request May 7, 2024
* add polkadot bulletin chain primitives

* also impl ChainWithMessages

* clippy
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-chores Something that has to be done, as part of regular maintenance P-Runtime
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants