remove chain specific macros and one metadata file for all chains #387
Conversation
niklasad1
changed the title
src/epm_dynamic.rs
Outdated
| fn to_scale_value<T: scale_info::TypeInfo + 'static + Encode>(val: T) -> Result<Value, Error> { | ||
| let (ty_id, types) = make_type::<T>(); | ||
|
|
||
| let bytes = val.encode(); | ||
|
|
||
| decode_as_type(&mut bytes.as_ref(), ty_id, &types) | ||
| .map(|v| v.remove_context()) | ||
| .map_err(|e| { | ||
| Error::DynamicTransaction(format!( | ||
| "Failed to decode {}: {:?}", | ||
| std::any::type_name::<T>(), | ||
| e | ||
| )) | ||
| }) | ||
| } |
There was a problem hiding this comment.
Ooh, I see, this is a clever way to take some type implementing TypeInfo + Encode into a scale_value::Value! It's a shame to have to encode it to bytes and then decode from the bytes, but offhand I can't think of a better/more reliable way (sortof just a limitation of those traits and their expectations).
You can go from a T: Serialize straight into a scale_value::Value, which would be more efficient (no intermediate step), but it also goes through the serde data model. Might be worth trying if you wanted faster and fewer lines of code, but the downside is that the serde and scale impls don't necessarily need to agree with eachother (eg H256 serializes to a string, which would turn into a scale_value primitive String rather than a sequence of bytes, and the string then wouldn't encode to bytes as would be needed to encode it properly).
There was a problem hiding this comment.
yeah but these types doesn't implement serde maybe I can convince @kianenigma to add deserialize/serialize on these types :)
There was a problem hiding this comment.
Makes sense! Aside from using Serde, or some other similar machinery, I think what you've done is the best possible approach anyways :)
src/epm_dynamic.rs
Outdated
| fn encode_scale_value<T: TypeInfo + 'static>(val: &Value) -> Result<Vec<u8>, Error> { | ||
| let (ty_id, types) = make_type::<T>(); | ||
|
|
||
| let mut bytes = Vec::new(); | ||
|
|
||
| encode_as_type(val, ty_id, &types, &mut bytes).map_err(|e| { | ||
| Error::DynamicTransaction(format!( | ||
| "Failed to encode {}: {:?}", | ||
| std::any::type_name::<T>(), | ||
| e | ||
| )) | ||
| })?; | ||
| Ok(bytes) | ||
| } |
There was a problem hiding this comment.
If you wanted to pass this to some call (api.tx().submit..) as a Value, this would use the runtime metadata to encode properly I'd hope. But I'm guessing you ran into a problem or couldn't do that so I'm intrigued :)
There was a problem hiding this comment.
I had to use this because I wanted to read a storage value and then codec::Decode::decode -> SignedSubmission see fn signed_submission_at above.
The reason is that the epm pallet types doesn't implement serde, not sure if there is a more clever way to do it :)
src/helpers.rs
Outdated
| static_types::MaxWeight::set(max_weight); | ||
| static_types::MaxLength::set(max_length); | ||
| static_types::MaxVotesPerVoter::set(max_votes_per_voter); | ||
| pub async fn snapshot<T: MinerConfig + Send + Sync + 'static>( |
There was a problem hiding this comment.
I am generally inclined to ask you to document all public functions in here. Some are clear, some are not. And you and I both know this codebase fairly well, which means we're biased.
There was a problem hiding this comment.
I tried to write some docs for these and move the epm related stuff to the epm module instead.
src/helpers.rs
Outdated
| .await? | ||
| .unwrap_or_default(); | ||
|
|
||
| let voters: Vec<_> = voters |
There was a problem hiding this comment.
I recall I once looked at why we need this hacky code and it seemed neccessary, I want to revisit it now again.
There was a problem hiding this comment.
it's to fetch to the snapshot data as we use Miner::mine_solution_with_snapshot added some docs.
Builds on top of #357
Requires polkadot v0.9.30 because of
Weight v2 proof size