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

Improve Dispatch Errors #488

Closed
echevrier opened this issue Mar 29, 2023 · 3 comments · Fixed by #563
Closed

Improve Dispatch Errors #488

echevrier opened this issue Mar 29, 2023 · 3 comments · Fixed by #563
Assignees
Labels
F7-enhancement Enhances an already existing functionality Q9-needsdesign

Comments

@echevrier
Copy link
Contributor

For now we only decode the module variant and create a module error. All other errors are not decoded.
The Balances' calls throw now TokenError , which are not module errors and so not decoded.

subxt did it in a nice way, using scale-decode(https://github.com/paritytech/scale-decode) and scale-encode : paritytech/subxt#878
If we want to go this way we have to make these libs no-std compatible.

@haerdib
Copy link
Contributor

haerdib commented Mar 29, 2023

It seems scale value has been split into 3 crates: scale-value, scale-encode and scale-decode.

e.g. scale-value decode now uses the fairly similar function of scale-value:
https://github.com/paritytech/scale-decode/blob/main/scale-decode/src/visitor/decode.rs#L32-L61

new scale value:
https://github.com/paritytech/scale-value/blob/main/src/scale_impls/decode.rs#L26-L32

old scale value:

pub fn decode_value_as_type<Id: Into<TypeId>>(
	data: &mut &[u8],
	ty_id: Id,
	types: &PortableRegistry,
) -> Result<Value<TypeId>, DecodeError> {
	let ty_id = ty_id.into();
	let ty = types
		.resolve(ty_id.id())
		.ok_or_else(|| DecodeError::TypeIdNotFound(ty_id.id()))?;

	let value = match ty.type_def() {
		TypeDef::Composite(inner) =>
			decode_composite_value(data, inner, types).map(ValueDef::Composite),
		TypeDef::Sequence(inner) =>
			decode_sequence_value(data, inner, types).map(ValueDef::Composite),
		TypeDef::Array(inner) => decode_array_value(data, inner, types).map(ValueDef::Composite),
		TypeDef::Tuple(inner) => decode_tuple_value(data, inner, types).map(ValueDef::Composite),
		TypeDef::Variant(inner) => decode_variant_value(data, inner, types).map(ValueDef::Variant),
		TypeDef::Primitive(inner) => decode_primitive_value(data, inner).map(ValueDef::Primitive),
		TypeDef::Compact(inner) => decode_compact_value(data, inner, types),
		TypeDef::BitSequence(inner) =>
			decode_bit_sequence_value(data, inner, types).map(ValueDef::BitSequence),
	}?;

	Ok(Value { value, context: ty_id })
}

But using the visitor seems to allow zero-copy decoding, which is a huge benefit. But I'm not sure how easily that's convertable into no-std. We'll have to look into that. If that's not possible, I think we can continue using the old version (with maybe slight updates).

@haerdib haerdib added the F7-enhancement Enhances an already existing functionality label Apr 11, 2023
@echevrier
Copy link
Contributor Author

echevrier commented Apr 26, 2023

@echevrier
Copy link
Contributor Author

For now we add a quick fix without reworking all the encoding/decoding of substarte-api-client. This should be done in another step. See #564

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
F7-enhancement Enhances an already existing functionality Q9-needsdesign
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants