opened
ethanfrey
username_0
<!-- < < < < < < < < < < < < < < < < < < < < < < < < < < < < < < < < < ☺ 
v                            ✰  Thanks for opening an issue! ✰    
v    Before smashing the submit button please review the template.
v    Word of caution: poorly thought-out proposals may be rejected 
v                     without deliberation 
☺ > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > >  -->

## 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

<!-- Why do we need this feature? 
What problems may be addressed by introducing this feature?
What benefits does gaia stand to gain by including this feature?
Are there any disadvantages of including this feature? -->

## Proposal

<!-- Detailed description of requirements of implementation -->

____

#### For Admin Use

- [ ] Not duplicate issue
- [ ] Appropriate labels applied
- [ ] Appropriate contributors tagged
- [ ] Contributor assigned/self-assigned
Add rest server support for wasm module, to query and create tx
issue
created
sahith-narahari
username_1
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*
comment
created
alpe
username_2
@username_1 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.

## 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
comment
created
ethanfrey
username_0
Thank you both for your comments.

In general, the list looks correct for scope @username_1  And I do agree (to 95%) with the detailed version from @username_2 , 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...
comment
created
ethanfrey
username_0
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?
comment
created
sahith-narahari
username_1
Thanks for the detailed explanation @username_2 @username_0. 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.
comment
created
sahith-narahari
username_1
Regarding `contract/contracts` I'm also in favor of using singulars to maintain consistency across various requests
comment
created
alpe
username_2
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. 
@username_0 your proposal is a good compromise.
comment
created
ethanfrey
username_0
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
comment
created
sahith-narahari
username_1
Thanks @username_0 for the heads up, will ensure that.
comment
created
sahith-narahari
username_1
@username_0 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.
comment
created
ethanfrey
username_0
Hi @username_1 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
comment
created
sahith-narahari
username_1
Thanks, will raise a PR with current state
comment
created
ethanfrey
username_0
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.
comment
created
sahith-narahari
username_1
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
comment
created
ethanfrey
username_0
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)
comment
closed
ethanfrey
username_0
issue