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

Use defaultConfig for pallet_contracts #1817

Merged
merged 6 commits into from
Apr 12, 2024
Merged

Conversation

pgherveou
Copy link
Contributor

No description provided.

@pgherveou pgherveou added R0-silent Changes should not be mentioned in any release notes T1-FRAME This PR/Issue is related to core FRAME, the framework. labels Oct 9, 2023
@pgherveou pgherveou marked this pull request as ready for review October 9, 2023 07:54
@pgherveou pgherveou requested a review from athei as a code owner October 9, 2023 07:54
@pgherveou pgherveou requested review from a team and gupnik October 9, 2023 07:54
substrate/frame/contracts/src/lib.rs Show resolved Hide resolved
bkchr pushed a commit that referenced this pull request Apr 10, 2024
* functions to benchmark messages pallet with linked to parachain

* unused imports

* fmt
@gupnik
Copy link
Contributor

gupnik commented Apr 11, 2024

@pgherveou Are we good to proceed here? Any blockers?

@pgherveou
Copy link
Contributor Author

@pgherveou Are we good to proceed here? Any blockers?

Kind of forgot about it, will bring it up to date :)

@pgherveou
Copy link
Contributor Author

just merge conflicts, will re-go through the different types to pick sensible defaults

@paritytech-cicd-pr
Copy link

The CI pipeline was cancelled due to failure one of the required jobs.
Job name: cargo-clippy
Logs: https://gitlab.parity.io/parity/mirrors/polkadot-sdk/-/jobs/5887491

@pgherveou
Copy link
Contributor Author

cc @gupnik how does that look now?

pub trait Config: frame_system::Config {
/// The time implementation used to supply timestamps to contracts through `seal_now`.
#[pallet::no_default]
Copy link
Contributor

Choose a reason for hiding this comment

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

Not possible to provide a default here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah you would most likely want to override it, if you do anything that use that type in your tests, but I can provide an implementation that is unimplented!

type ChainExtension: chain_extension::ChainExtension<Self> + Default;

/// Cost schedule and limits.
#[pallet::constant]
#[pallet::no_default]
Copy link
Contributor

Choose a reason for hiding this comment

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

Tried with no_default_bounds?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@@ -326,6 +339,7 @@ pub mod pallet {
///
/// This setting along with [`MaxCodeLen`](#associatedtype.MaxCodeLen) directly affects
/// memory usage of your runtime.
#[pallet::no_default]
Copy link
Contributor

Choose a reason for hiding this comment

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

Same?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

same, the trait is generic over T

substrate/frame/contracts/src/tests.rs Show resolved Hide resolved
@pgherveou pgherveou enabled auto-merge April 12, 2024 07:00
@pgherveou pgherveou added this pull request to the merge queue Apr 12, 2024
Merged via the queue into master with commit 033484c Apr 12, 2024
132 of 136 checks passed
@pgherveou pgherveou deleted the pg/contract_default_trait branch April 12, 2024 14:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
R0-silent Changes should not be mentioned in any release notes T1-FRAME This PR/Issue is related to core FRAME, the framework.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants