Skip to content

Conversation

@haerdib
Copy link
Contributor

@haerdib haerdib commented Dec 20, 2022

❗ Breaks submit_and_watch_extrinsic and submit_extrinsic interfaces:

  • extrinsic submission now expects an encoded extrinsic. Hex encoding is no longer allowed / required. The hex encoding is done in the functions themselves. This is needed to calculate the extrinsic hash.
  • The return value of submit_and_watch_extrinsic has been updated to ExtrinsicReport which includes the extrinsic hash, the (maybe) block hash, the transaction status and, possibly, the associated events. This has the following advantages:
    • Clearly distinguishes between the xt hash (returned by submit_extrinsic) and the block hash previously returned.
    • Allows to return all associated items of the extrinsic to the caller.

Preparatory step for #288.

@haerdib haerdib self-assigned this Dec 20, 2022
@haerdib haerdib changed the title SubmitAndWatch: Return report instead of block hash SubmitAndWatch: Return ExtrinsicReport instead of block hash Dec 20, 2022
@haerdib haerdib added E2-breaksapi F7-enhancement Enhances an already existing functionality labels Dec 20, 2022
@haerdib haerdib marked this pull request as ready for review December 20, 2022 11:52
Copy link
Contributor

@echevrier echevrier left a comment

Choose a reason for hiding this comment

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

LGTM

@haerdib haerdib requested review from clangenb and removed request for clangenb December 21, 2022 16:13
Copy link
Collaborator

@clangenb clangenb left a comment

Choose a reason for hiding this comment

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

Looks very nice and clean, I like it. I have just some questions. :)

@haerdib haerdib requested a review from clangenb December 22, 2022 09:43
Copy link
Collaborator

@clangenb clangenb left a comment

Choose a reason for hiding this comment

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

Very nice, I only have a comment left.

DispatchError::decode_from(event_details.field_bytes(), self.metadata());
return Err(Error::Dispatch(dispatch_error))
}
event_details.check_failed()?;
Copy link
Collaborator

Choose a reason for hiding this comment

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

I am uncertain if I agree with the naming. I propose a few here:

  • check_if_failed
  • xt_successful
  • xt_failed

What do you think?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I was thinking about the first one as well. Let me adapt.

@haerdib haerdib requested a review from clangenb December 22, 2022 16:00
Copy link
Collaborator

@clangenb clangenb left a comment

Choose a reason for hiding this comment

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

Thanks, looks good to me now!

@haerdib haerdib merged commit e5162ea into master Dec 23, 2022
@haerdib haerdib deleted the bh/return-report-instead-hash branch December 23, 2022 06:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

E2-breaksapi F7-enhancement Enhances an already existing functionality

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants