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

New query API plugin to more easily make GET/POST requests to readonly actions #2112

Open
aaroncox opened this issue Jan 19, 2024 · 8 comments

Comments

@aaroncox
Copy link
Contributor

In order to facilitate easier access to eosio::read_only actions, I'd like to propose either a new plugin or new API endpoint which can specifically be used to access these actions.

The goal here is to allow developers to use readonly actions in their contracts to essentially build custom API endpoints for their contract to use in applications. This is possible now, but requires assembling a transaction, faking the tapos information, removing the authorizations, and pushing it similar to a normal transaction.

This API endpoint could live as its own optionally enabled plugin:

/v1/query/<contract>/<action>

Or enabled by default as part of the chain_api plugin:

/v1/chain/query/<contract>/<action>

This endpoint will accept the action data required by read only action as either a GET:

/v1/query/<contract>/<action>?account=foo&bar=baz

or as a POST request:

curl localhost:8888/v1/query/<contract>/<action> -d '{"account": "foo", "bar": "baz"}'

The endpoint will then take the contract, action, and submitted action data, and route it through a similar kind of flow that v1/chain/send_read_only_transaction would use. It however won't return a processed transaction (with all the traces, etc), it will instead just return a JSON response containing the return_value_data and/or return_value_hex_data (or a JSON array of them? I'm not sure if readonly actions allow inlines that could result in more than return value).

One additional nice-to-haves might also include a parameter on the request to specify whether the response is encoded/decoded, similar to how get_table_rows works. The requesting client can specify whether they'd like the server to decode the data or if the client itself will decode it. If this is too much, just always return the encoded value and force the client to decode it using the ABI (Wharf can handle this).

Additional thoughts and improvements on the idea are welcome. Performing readonly transactions right now is complicated. Luckily things like Wharf abstract this complexity away from developers, but it certainly would be nice to be able to just use a URL directly with fetch to retrieve this data.

@nsjames
Copy link
Contributor

nsjames commented Jan 19, 2024

Something we were toying around with the passthrough was returning things besides json.

In the case of that pass through you had to define it on the return value, but you could just as easily define it in the headers on the request

Though it does have a security impact on integrators it extends functionality significantly as you could even return HTML directly from nodeos

@ericpassmore
Copy link
Contributor

Suggest that content type be control via the Accept header and not via a parameter.
https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Accept

  • Accept: application/json : json
  • Accept: */* : binary
  • Accept: application/* : binary

The read-only api doesn't change state, and per HTTP convention only GET requests should be supported. No POST PUT or PATCH requests.

@ericpassmore
Copy link
Contributor

ericpassmore commented Jan 19, 2024

In addition suggestion returning a simple checksum of the content as an ETag value in the header. This provides a way for clients to check if the content has changed. A HEAD request would return these values and this would be beneficial in skipping full GET requests for large payloads.

Also consider if the if-match or if-none-match header requests should be supported https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/If-None-Match

@ericpassmore
Copy link
Contributor

Something we were toying around with the passthrough was returning things besides json.

In the case of that pass through you had to define it on the return value, but you could just as easily define it in the headers on the request

Though it does have a security impact on integrators it extends functionality significantly as you could even return HTML directly from nodeos

Escape return values to minimize security issues with embedded HTML or JS.

@aaroncox
Copy link
Contributor Author

The read-only api doesn't change state, and per HTTP convention only GET requests should be supported. No POST PUT or PATCH requests.

The only reason I suggested still supporting POST is because essentially every other API is POST (and the majority of them also do not modify state).

I agree with this idea, as well as someday changing all the other endpoints - its just a matter of "when" we we cut over.

@ericpassmore
Copy link
Contributor

The read-only api doesn't change state, and per HTTP convention only GET requests should be supported. No POST PUT or PATCH requests.

The only reason I suggested still supporting POST is because essentially every other API is POST (and the majority of them also do not modify state).

Yes many POST APIs that don't change data 😢, slightly off return codes 😢 returning 201 when it should be 200, and it 🐛 bugs me.

I agree with this idea, as well as someday changing all the other endpoints - its just a matter of "when" we we cut over.

For my clarity if we didn't change the other APIs, should we restrict the query API to GET requests, or should keep GET and POST methods in a new query plugin for consistency? Open to your advice.

@matthewdarwin
Copy link

fix everything properly, but have 1 release where the "new proper way" and the "old broken way" are both supported, where the "old broken way" can be turned off (or is off by default). It will take some education to get everyone to move.

Publishing a document saying what is the right way to do things will be a good starting point and then expanding that into a roadmap of plans.

@matthewdarwin
Copy link

Or maybe better there is a "shim" proxy server to emulate the old API and route things to the new APIs. People can run the shim or not.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Todo
Development

No branches or pull requests

5 participants