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

impl_tanssi_pallets_config! macro for container chains #416

Merged
merged 8 commits into from
Feb 20, 2024

Conversation

nanocryk
Copy link
Contributor

Provide a new impl_tanssi_pallets_config! macro that implements Config from pallet_author_inherent, pallet_timestamp and pallet_cc_authorities_noting with the proper parameters to work with Tanssi.

Customizable parameters are set by impl impl_tanssi_pallets_config::Config.

The macro can't check at compile time that the pallets have been listed in the construct_runtime! macro, however it will generate a test for that.

@nanocryk nanocryk added B7-runtimenoteworthy Changes should be noted in any runtime-upgrade release notes not-breaking Does not need to be mentioned in breaking changes labels Feb 15, 2024
Copy link
Contributor

github-actions bot commented Feb 15, 2024

Coverage Report

(master)

@@                           Coverage Diff                            @@
##           master   jeremy-construct-tanssi-runtime-macro     +/-   ##
========================================================================
  Coverage   77.37%                                  77.37%   0.00%     
+ Files         109                                     110      +1     
+ Lines       28595                                   28620     +25     
========================================================================
+ Hits        22123                                   22144     +21     
+ Misses       6472                                    6476      +4     
Files Changed Coverage
/container-chains/templates/frontier/runtime/src/lib.rs 54.95% (+0.06%) 🔼
/container-chains/templates/simple/runtime/src/lib.rs 64.64% (+0.07%) 🔼

Coverage generated Mon Feb 19 15:40:35 UTC 2024

Copy link
Contributor

@girazoki girazoki left a comment

Choose a reason for hiding this comment

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

Love this! Maybe we should put the impl-tanssi-pallets-config in dancekit though, what do you think @nanocryk? Ideally dancekit should contain the tools to implement a runtime. But we can do it in a separate PR

@@ -917,6 +889,15 @@ impl pallet_tx_pause::Config for Runtime {
type WeightInfo = pallet_tx_pause::weights::SubstrateWeight<Runtime>;
}

impl tp_impl_tanssi_pallets_config::Config for Runtime {
const SLOT_DURATION: u64 = SLOT_DURATION;
Copy link
Contributor

Choose a reason for hiding this comment

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

@girazoki is this configurable? Won't container chains break if you set this to something other than 12?

Copy link
Contributor

Choose a reason for hiding this comment

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

I think if they missconfigure it, they wont produce blocks. So yeah maybe its better if we set it ourselves. @nanocryk can you do that?

Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe let's keep this until we change it to 6 seconds, the transition will be easier if this is configurable

Copy link
Contributor

Choose a reason for hiding this comment

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

"easier"

Copy link
Contributor

Choose a reason for hiding this comment

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

😭

@girazoki
Copy link
Contributor

@nanocryk can you merge master back? I think we can merge this after.

@nanocryk nanocryk merged commit 3b61b58 into master Feb 20, 2024
31 checks passed
@nanocryk nanocryk deleted the jeremy-construct-tanssi-runtime-macro branch February 20, 2024 13:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
B7-runtimenoteworthy Changes should be noted in any runtime-upgrade release notes not-breaking Does not need to be mentioned in breaking changes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants