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

fix failures request in contract manager #663

Merged
merged 8 commits into from
Sep 4, 2024
Merged

Conversation

quasisamurai
Copy link
Contributor

@quasisamurai quasisamurai commented Aug 8, 2024

TASK

Related PRs

@quasisamurai quasisamurai changed the title intro new request & modify old one fix failures request in contract manager Aug 9, 2024
@quasisamurai quasisamurai marked this pull request as ready for review August 9, 2024 09:59
joldie777
joldie777 previously approved these changes Aug 13, 2024
}

// QueryFailureRequest is request type for the Query/Failures RPC method.
message QueryFailureRequest {
Copy link
Contributor

Choose a reason for hiding this comment

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

pagination field seems to be not needed logically (and not used) for QueryFailureRequest.
Please remove it

@@ -18,7 +18,7 @@ service Query {
}

// Queries a Failure by contract address and failure ID.
rpc AddressFailure(QueryFailuresRequest) returns (QueryFailuresResponse) {
rpc AddressFailure(QueryFailureRequest) returns (QueryFailuresResponse) {
Copy link
Contributor

Choose a reason for hiding this comment

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

How about making QueryFailureResponse struct without pagination? It seems strange that we return it in array even tho it's always one failure. And pagination always null

@NeverHappened
Copy link
Contributor

Also - please attach PR to neutronjs here

@pr0n00gler pr0n00gler marked this pull request as draft August 20, 2024 14:35
@quasisamurai quasisamurai marked this pull request as ready for review August 29, 2024 16:21
@quasisamurai
Copy link
Contributor Author

@pr0n00gler pr0n00gler merged commit 5b6f7c5 into main Sep 4, 2024
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants