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 the ability to declare multiple impl blocks #6795

Draft
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

pkhry
Copy link
Contributor

@pkhry pkhry commented Dec 9, 2024

Description

Adds the ability to split single impl_runtime_apis into multiple modules through sp_api::impl_runtime_apis_ext.

Example block would look like:

impl_runtime_apis! {
    impl trait Example<Block> for Runtime {
             fn example() {}
    }
    // `use` makes metadata from and runtime_versions visible to the main impl block.
    use other::*;
}

#[sp_api::impl_runtime_apis_ext]
mod other {
        use super::*; 
     	impl super::Example2<Block> for Runtime {
		fn same_name() -> String {
			"example".to_string()
		}
	}
}

also see: #3067

Review Notes

The main difference between impl_runtime_apis and impl_runtime_apis_ext is the fact that the latter doesn't generate base runtime structures and doesn't track the imports to aggregate them inside the metadata and runtime_api_versions.

TODOs:

  • RUNTIME_API_VERSION
    was a const before, i've moved it to a standalone function inside the main impl block.
    The question is should it be tied to the runtime via trait e.g RuntimeApiVersion and if yes where should it go?(sp_api or sp_version)
    I've tried to figure out a way to use a const but given that all of the const declarations contain data inside a std::borrow::Cow and and there's no way to get value out in const expression and i've failed to do so, if someone has any idea how to get around this i would really appreciate some input.
    		pub fn runtime_api_versions() -> #c::ApisVec {
    		let api = #c::vec::Vec::from([RUNTIME_API_VERSIONS.into_owned(), #(#modules::RUNTIME_API_VERSIONS.into_owned()),*]).concat();
    		api.into()
    	}

@pkhry
Copy link
Contributor Author

pkhry commented Dec 11, 2024

/cmd fmt

@pkhry pkhry added I5-enhancement An additional feature request. T1-FRAME This PR/Issue is related to core FRAME, the framework. D1-medium Can be fixed by a coder with good Rust knowledge but little knowledge of the codebase. labels Dec 11, 2024
Copy link

Command "fmt" has started 🚀 See logs here

Copy link

Command "fmt" has finished ✅ See logs here

@paritytech-workflow-stopper
Copy link

All GitHub workflows were cancelled due to failure one of the required jobs.
Failed workflow url: https://github.com/paritytech/polkadot-sdk/actions/runs/12272999891
Failed job name: run-frame-omni-bencher

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
D1-medium Can be fixed by a coder with good Rust knowledge but little knowledge of the codebase. I5-enhancement An additional feature request. 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.

1 participant