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 rest server support for wasm module, to query and create tx #7

Closed
4 tasks
ethanfrey opened this issue Jan 8, 2020 · 19 comments · Fixed by #45
Closed
4 tasks

Add rest server support for wasm module, to query and create tx #7

ethanfrey opened this issue Jan 8, 2020 · 19 comments · Fixed by #45
Assignees
Labels
API Fixes to the external client api

Comments

@ethanfrey
Copy link
Member

ethanfrey commented Jan 8, 2020

Summary

We only have CLI, and we will want to add support for a rest server. This would go into x/wasmd/client/rest and be exposed with the standard wasmcli rest server.

Problem Definition

We want to allow webapps to do everything we can do in the CLI. I have no clear design for the urls, so the first part is maybe to propose a set of endpoints. I want queries and ability to submit the 3 tx defined in this module. This will be the basis for a js api.

This will also probably involve refactoring the Querier a bit to allow better reuse between cli and rest.

Proposal

To be filled in (add your proposal of endpoints in the comments, please)


For Admin Use

  • Not duplicate issue
  • Appropriate labels applied
  • Appropriate contributors tagged
  • Contributor assigned/self-assigned
@ethanfrey ethanfrey mentioned this issue Jan 8, 2020
7 tasks
@ethanfrey ethanfrey added the API Fixes to the external client api label Jan 10, 2020
@sahith-narahari sahith-narahari self-assigned this Jan 12, 2020
@sahith-narahari
Copy link
Member

I feel endpoints should be grouped separately for code and contract. Here is the list of endpoints I think should be good. I wanted to make sure they don't deviate much from client.

Transactions:

POST /code/store
POST /contract/Instantiate
POST /contract/Execute

Query:

GET /code/list
GET /code/query
GET /contract/list
GET /contract/info
GET /contract/state

@alpe
Copy link
Member

alpe commented Jan 13, 2020

@sahith-narahari can you elaborate on the reason for the code - contract separation please?
IMHO their lifecycles are related. There can not be any contract instance without the code. Neither should we delete a code entry and keep an instance running for example.

I would propose the following endpoints with json body for non query operations:

Create Contract

POST /wasm/contract

Requires:

  • Sender
  • WASMByteCode

Returns:

  • Code: 201 - Created
  • Body: codeID - unique identifier
  • Header: Location: /wasm/contract/<codeID>

Create Instance

POST /wasm/contract/<codeID>

Requires:

  • Sender
  • InitMsg (or should be optional?)
  • InitFunds (or should be optional?)

Returns:

  • Code: 201 - Created
  • Body: ContractAddr - unique sdk address
  • Header: Location: /wasm/contract/<ContractAddr>

Execute contract

POST /wasm/contract/<ContractAddr>

Requires:

  • Sender
  • Msg
  • SentFunds (or should be optional?)

Returns

  • Status: 200 - OK
  • Body:
    • Data: contract result data
    • Log: contract result log
    • GasUsed

Future endpoints ?

DELETE /wasm/contract/<ContractAddr>

Requires:

  • Sender

Returns:

  • Code 200

@ethanfrey
Copy link
Member Author

ethanfrey commented Jan 13, 2020

Thank you both for your comments.

In general, the list looks correct for scope @sahith-narahari And I do agree (to 95%) with the detailed version from @alpe , thanks for spec'ing this more. It does look more RESTful,

The one issue with the second part is the confusion between /wasm/contract/<codeID> and /wasm/contract/<ContractAddr>. We cannot have two identical endpoints with different functions. I would make it:

  • POST /wasm/code -> create code
  • POST /wasm/code/<codeID> -> instantiate code (from one instance, returns contract instance)
  • POST /wasm/contract/<contractAddr> -> execute one contract

The DELETE is interesting, but very unlikely ever to be used (immutible contracts and all), however, there is a request for upgradable contracts, so we'll see...

@ethanfrey
Copy link
Member Author

Sticking with that pattern, I would make the queries more "RESTFUL":

  • GET /code/ -> list all code IDs
  • GET /code/<codeID> -> details of one code ID
  • GET /contract/ -> list all contracts addresses
  • GET /contract/<contractAddr> -> info on one contract
  • GET /contract/<contractAddr>/state -> internal state of one contract

I am unsure about code/contract vs codes/contracts, maybe the plural is more common in rest? But I do like consistency, not /contracts to list and /contract/<addr> for details.

And all cli flags we want to support should be turned into query args ?foo=bar style.

What do you think here?

@sahith-narahari
Copy link
Member

Thanks for the detailed explanation @alpe @ethanfrey. My main motto behind the separation was ListCode and ListContract will create an ambiguity with same endpoints.
I agree the better solution would be to separate only the query list APIs and maintain all the other endpoints as /wasm/contract consistently.

@sahith-narahari
Copy link
Member

Regarding contract/contracts I'm also in favor of using singulars to maintain consistency across various requests

@workshub
Copy link

workshub bot commented Jan 13, 2020

This issue is now published on WorksHub. If you would like to work on this issue you can
start work on the WorksHub Issue Details page.

@alpe
Copy link
Member

alpe commented Jan 13, 2020

Sorry about the ambiguity in the path of my proposal. I was doing some edits before submitting. It was like this before:

  • Instance
    POST /wasm/code/<codeID>/instance
  • Execute
    POST /wasm/code/<codeID>/instance/<ContractAddr>

This reflects the dependency between code and contract in a REST-ish style but for execution the caller would need to know or resolve codeID and contractAddr. That may not be very convenient.
@ethanfrey your proposal is a good compromise.

@workshub
Copy link

workshub bot commented Jan 13, 2020

@sahith-narahari started working on this issue via WorksHub.

@ethanfrey
Copy link
Member Author

Please note that #12 changes the contract state querier, so don't implement that part until #12 is merged and rebase on the new master

@sahith-narahari
Copy link
Member

Thanks @ethanfrey for the heads up, will ensure that.

@sahith-narahari
Copy link
Member

@ethanfrey I'm done with all functions except for contract state. I see in cli it was separated using sub-commands, how would you like me to go ahead with this in rest.

@ethanfrey
Copy link
Member Author

ethanfrey commented Jan 19, 2020

Hi @sahith-narahari can you make a pr with the current state. I would be happy to merge this while work continues, so I can start to test out on a live node.

As to the state query functions, I would us a separate path for each.

  • /contract/<contractAddr>/state - all
  • /contract/<contractAddr>/smart - smart query
  • /contract/<contractAddr>/raw - raw query

If you have better names, you can use those instead

@sahith-narahari
Copy link
Member

Thanks, will raise a PR with current state

@ethanfrey
Copy link
Member Author

ethanfrey commented Jan 20, 2020

I merged this PR.
Mainly missing two query functions (raw and smart queries on contract state).
And some tests on it would be nice - if nothing else to demo usage.
(But it seems there is no way to test this properly in cosmos-sdk???)

@sahith-narahari
Copy link
Member

Yes, I will the add remaining two query functions.

Even I wanted to test these, will try to find a way. Else I'll test them manually by deploying a contract using rest and query it

@ethanfrey
Copy link
Member Author

A manual test is better than no test at all. And good enough for now. If it works, the tests are there to keep us from breaking stuff in the future. Which we can do once we figure out a good approach (consulting cosmos-sdk core team on best practices)

alpe added a commit that referenced this issue Jan 21, 2021
* Update to ibc version of wasmvm

* Update test contracts

* Adjust code to updated wasmvm

* Update to tip of wasmvm PR 172

* Update test contracts and add stargate support to tests

* Upgrade to wasmvm 94043576bb71

Co-authored-by: Alex Peters <alpe@users.noreply.github.com>
giansalex pushed a commit to disperze/wasmd that referenced this issue Dec 17, 2021
@jsoneaday
Copy link

Is there an official doc for the rest paths? It's not clear which style got accepted

@alpe
Copy link
Member

alpe commented Jan 17, 2023

The REST integration was deprecated and is removed now. Instead there is a grpc-rest gateway endpoint. You can see the path defined in in the query proto file

loloicci pushed a commit to loloicci/wasmd that referenced this issue Apr 19, 2023
…osmWasm#7)

* featremove custom proto changes of genesis and types

* feat!: extract custom wasm logic for lbm-sdk

* feat: move custom cli of lbm-sdk to `wasmplus` client package

* feat: remove params for wasmplus. just use original params of x/wasm

* chore: remove unused customEncoding and customQuerier

* chore: apply a changes about separated ibc-go import path

* doc: add readme of `x/wasmplus`

* fix: lint error

* chore: add more unittest

* chore: add more unittest

* chore: add more unittest

* chore: add more unittest

* chore: update changelog

* chore: remove interchain-accounts

* chore: feedback reviews

* doc: update proto changes

* chore: apply review feedback
 - move all function of `wasmplus/keeper/keeper_extension.go` to `wasmplus/keeper/keeper.go`
 - remove no need files (keeper_extension.go, metrics.go)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
API Fixes to the external client api
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants