Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

scale-type-resolver integration #1460

Merged
merged 13 commits into from
Mar 13, 2024

Conversation

tadeohepperle
Copy link
Contributor

This PR updates subxt to the latest versions of scale-value, scale-decode, scale-encode and scale-typgen which rely on the scale-type-resolver crate James wrote.

@@ -65,10 +65,6 @@ pub struct Payload<CallData> {
validation_hash: Option<[u8; 32]>,
}

/// A boxed transaction payload.
// Dev Note: Arc used to enable easy cloning (given that we can't have dyn Clone).
pub type BoxedPayload = Payload<Arc<dyn EncodeAsFields + Send + Sync + 'static>>;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Note: This was removed, because EncodeAsFields is not object safe anymore and in the public element room for subxt noone seemed to care about it not being supported anymore.

@tadeohepperle tadeohepperle marked this pull request as ready for review March 1, 2024 08:23
@tadeohepperle tadeohepperle requested a review from a team as a code owner March 1, 2024 08:23
) -> Result<Self::Value<'scale, 'info>, Self::Error> {
use scale_decode::error::{Error, ErrorKind};

if value.path().ident().as_deref() != Some("WrapperKeepOpaque") {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Nit: can we add a TODO here like:

// TODO: When `scale-type-resolver` provides struct names, check that this struct name is `WrapperKeepOpaque`

Or alterantely link an issue that says the same :)

Copy link
Collaborator

@jsdw jsdw left a comment

Choose a reason for hiding this comment

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

Looks great to me, good job getting this done!

let scale_info::TypeDef::Composite(_) = &ty.type_def else {
return Err(Error::new(ErrorKind::WrongShape {
let visitor = visitor::new(out, |_, _| {
// Check that the target shape lines up: any other shape but composite it wrong.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
// Check that the target shape lines up: any other shape but composite it wrong.
// Check that the target shape lines up: any other shape but the composite is wrong.

@tadeohepperle tadeohepperle merged commit 8bdd276 into master Mar 13, 2024
13 checks passed
@tadeohepperle tadeohepperle deleted the tadeohepperle/scale-type-resolver-integration branch March 13, 2024 13:48
@jsdw jsdw mentioned this pull request Mar 21, 2024
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants