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 EventReport and failed xt check to examples #615

Merged
merged 13 commits into from
Jul 24, 2023
Merged

Conversation

haerdib
Copy link
Contributor

@haerdib haerdib commented Jul 11, 2023

The following has been changed:

Examples

  • All examples:
    • Updated file description to use DefaultRuntimeConfig instead of Polkadot Config and added a little more text.
    • Where used: changed function submit_and_watch_extrinsic_until_without_event to submit_and_watch_extrinsic_until. Reasoning: submit_and_watch_extrinsic_until_without_events does not check if the extrinsic has failed, which might lead to confusion. So I'd suggest to use submit_and_watch_extrinsic_until instead, which checks for failure if possible, and if not (due to watching not long enough), it acts the very same as submit_and_watch_extrinsic_until_without_events.
  • renamed example event_callback to subscribe_events. I believe that's more descriptive.
  • Example event_error_details
    • renamed to check_extrinsic_events
    • added an extrinsic submission (that should succeed) to the example, which shows all the parameters that ExtrinsicReport contains with an descriptive println message. It also checks the returned events.

Tests

  • Updated author_tests to check for:
    • Returned TransactionStatus
    • That events are None or Some as one would expected. I therefore also changed from submit_and_watch_extrinsic_until_without_events to submit_and_watch_extrinsic_until. To still test submit_and_watch_extrinsic_until_without_events I added two new tests further below.

closes #591

@haerdib haerdib self-assigned this Jul 11, 2023
@haerdib haerdib added Z3-example Add or fix an example E1-breaksnothing labels Jul 11, 2023
@haerdib haerdib marked this pull request as ready for review July 12, 2023 15:05
@haerdib haerdib requested a review from echevrier July 12, 2023 15:05
@haerdib haerdib changed the title Improve examples Add EventReport check to example and check for failed xt Jul 12, 2023
@haerdib haerdib changed the title Add EventReport check to example and check for failed xt Add EventReport and failed xt check to examples Jul 12, 2023
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.

Really clean. But I'm still missing the 'little' differences between submit_and_watch_extrinsic_until_without_event and submit_and_watch_extrinsic_until.

  • submit_and_watch_extrinsic_until_without_event doesn't thrown an error when the extrinsic has failed onchain.
  • how can we get the triggered events when the extrinsic has failed?

let balance_of_bob = api.get_account_data(&bob).unwrap().unwrap_or_default().free;
println!("[+] Bob's Free Balance is {balance_of_bob}\n");

// First we want to see the events of a failed extrinsic.
Copy link
Contributor

Choose a reason for hiding this comment

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

How do you want to do it?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good Input - let me do something about that

Copy link
Contributor Author

@haerdib haerdib Jul 14, 2023

Choose a reason for hiding this comment

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

See #617: I think that should be done in a different PR, as it affects the API. What do you think?

Regarding your point of

submit_and_watch_extrinsic_until_without_event doesn't thrown an error when the extrinsic has failed onchain.
What do you propose for this? I think the comments are quite telling already.. so I removed the without_events from the examples. It should not be used if one does not know what one is doing.

Copy link
Contributor

Choose a reason for hiding this comment

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

#617 makes sense. Adding the report to the Error gives access to all the information we have collected.

Copy link
Contributor

Choose a reason for hiding this comment

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

submit_and_watch_extrinsic_until_without_event doesn't thrown an error when the extrinsic has failed onchain.
What do you propose for this? I think the comments are quite telling already.. so I removed the without_events from the examples. It should not be used if one does not know what one is doing.

Not showing any examples is just a way of forcing the user not to use it. Is that really what we want?
I'm not sure the comments are clear enough. Because we're talking about an error when an extrinsic has failed on the chain, and not an event, the user may think that submit_and_watch_extrinsic_until_without_event informs her when an extrinsic fails on the chain. Without studying the code, they may miss this point.
Maybe add an example check_extrinsic_without_events similar to check_extrinsic_events? We could do it in another issue. Maybe I'm too picky, so it's up to you to decide.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Created a follow up issue in #627

@haerdib haerdib marked this pull request as draft July 13, 2023 14:42
@haerdib haerdib marked this pull request as ready for review July 14, 2023 18:00
@haerdib haerdib requested a review from echevrier July 14, 2023 18:00
@haerdib haerdib merged commit f25124a into master Jul 24, 2023
37 checks passed
@haerdib haerdib deleted the bh/improve-examples branch July 24, 2023 13:14
@haerdib haerdib added the F7-enhancement Enhances an already existing functionality label Aug 7, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
E1-breaksnothing F7-enhancement Enhances an already existing functionality Z3-example Add or fix an example
Projects
None yet
Development

Successfully merging this pull request may close these issues.

improve examples to check for extrinsic-success and report errors and events triggered by the extrinsic
2 participants