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.

Sign up for GitHub

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

Jump to bottom

Implement state handling for Retracted status #807

Merged
merged 12 commits into from
Oct 28, 2024
Merged

Implement state handling for Retracted status #807

merged 12 commits into from
Oct 28, 2024

Conversation

Niederb
Copy link
Contributor

@Niederb Niederb commented Oct 8, 2024 •
edited
Loading

  • Move Retracted enum value from UnexpectedTxStatus to XtStatus
  • Provide is_final method for TransactionStatus
    • Currently not used, but I think it makes sense to provide it
  • Return from watch method as soon as a final state is reached
  • Provide separate method to populate events for an ExtrinsicReport

@Niederb Niederb self-assigned this Oct 8, 2024
@Niederb
- Move Retractec from UnexpectedTxStatus to XtStatus
9db07c2 
- Provide is_final method for TransactionStatus
- Return from watch method as soon as a final state is reached
- Provide separate method to populate events for an ExtrinsicReport
@Niederb
Fix unittest
7ce954d
@Niederb
Remove unreachable handling of states with is_final() == True
b5e0b6d
@Niederb
Improve testing and error handling
6191cb8
@Niederb
Improve tests
833db41
@Niederb Niederb marked this pull request as ready for review October 9, 2024 06:43
@Niederb Niederb added E2-breaksapi F7-enhancement Enhances an already existing functionality labels Oct 9, 2024
@Niederb Niederb requested a review from haerdib October 9, 2024 07:24
@Niederb Niederb linked an issue Oct 9, 2024 that may be closed by this pull request
accept retracted XtStatus #759
Closed
@Niederb Niederb requested a review from haerdib October 24, 2024 11:51
haerdib
haerdib reviewed Oct 24, 2024
View reviewed changes
src/api/mod.rs Outdated
/// Returns an error if the events are missing or if one of the events indicates a problem.
pub fn status_based_on_events(&self, metadata: &Metadata) -> Result<()> {
if self.events.is_none() {
return Err(Error::Other("Report does not contain any events".into()))
Copy link
Contributor

Choose a reason for hiding this comment

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

Would it make sense adding an extra Error Type like NoEvent (or something alike) instead of using Other?

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 added two error types for event related errors

src/api/rpc_api/author.rs Show resolved Hide resolved
@Niederb Niederb requested a review from haerdib October 28, 2024 08:12
haerdib
haerdib approved these changes Oct 28, 2024
View reviewed changes
Copy link
Contributor

@haerdib haerdib left a comment

Choose a reason for hiding this comment

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

LGTM, thank you very much for your patience! Only one change suggestion left, but you can go on without it as well.

src/api/rpc_api/author.rs Outdated Show resolved Hide resolved
@Niederb @haerdib
Improve documentation
ae65f6c 
Co-authored-by: Bigna Härdi <Bigna.Haerdi@scs.ch>
@Niederb Niederb merged commit d915763 into master Oct 28, 2024
61 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@haerdib haerdib haerdib approved these changes

Assignees

@Niederb Niederb

Labels
E2-breaksapi F7-enhancement Enhances an already existing functionality
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

accept retracted XtStatus
2 participants
@Niederb @haerdib