Simplify service requests #3581
Conversation
examples/crowd-funding/README.md
Outdated
| applicationId: "$APP_ID_1" | ||
| ) } | ||
| ``` | ||
|
|
||
| If you enter the `applications` query again, both entries will appear in the second wallet as |
There was a problem hiding this comment.
Mmm this looks outdated. I'll make another pass on the README files before landing the PR.
|
|
||
| trace!("Query requested a new block with operations: {operations:?}"); |
There was a problem hiding this comment.
Maybe there should be a way for the user to tell the client that they do not want to create a new block, regardless of what the application returns.
There was a problem hiding this comment.
Yes we're certainly not going to auto-sign block proposals like this in the future but instead properly integrate with wallets.
| let response = async_graphql::Response::new(hash.to_value()); | ||
| Ok(serde_json::to_vec(&response)?) |
There was a problem hiding this comment.
I'm not sure if this is really an improvement - sure the type is Vec<u8> but it is a serialized JSON. If it can't be anything else I think the previous type was better b/c it more precisely defined how to handle the data. Right now callers have to read the docs, previously it was in the type.
There was a problem hiding this comment.
Actually, wouldn't it be more consistent to not return the block hash here, and instead return the response from the QueryOutcome, just like in line 903 above?
There was a problem hiding this comment.
Yes see the summary
Motivation
After #3217, we do not need to introspect service queries to applications.
Proposal
Note the assumption on JSON outputs could be relaxed if we stopped returning the hash of the certified block and return the query response instead. However, this is a breaking change so this should be a separate PR.
Test Plan
CI