{"action":"created","issue":{"url":"https://api.github.com/repos/tendermint/abci/issues/58","repository_url":"https://api.github.com/repos/tendermint/abci","labels_url":"https://api.github.com/repos/tendermint/abci/issues/58/labels{/name}","comments_url":"https://api.github.com/repos/tendermint/abci/issues/58/comments","events_url":"https://api.github.com/repos/tendermint/abci/issues/58/events","html_url":"https://github.com/tendermint/abci/pull/58","id":202724141,"number":58,"title":"First commit to demonstrate change","user":{"login":"jaekwon","id":53785,"avatar_url":"https://avatars.githubusercontent.com/u/53785?v=3","gravatar_id":"","url":"https://api.github.com/users/jaekwon","html_url":"https://github.com/jaekwon","followers_url":"https://api.github.com/users/jaekwon/followers","following_url":"https://api.github.com/users/jaekwon/following{/other_user}","gists_url":"https://api.github.com/users/jaekwon/gists{/gist_id}","starred_url":"https://api.github.com/users/jaekwon/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/jaekwon/subscriptions","organizations_url":"https://api.github.com/users/jaekwon/orgs","repos_url":"https://api.github.com/users/jaekwon/repos","events_url":"https://api.github.com/users/jaekwon/events{/privacy}","received_events_url":"https://api.github.com/users/jaekwon/received_events","type":"User","site_admin":false},"labels":[],"state":"open","locked":false,"assignee":null,"assignees":[],"milestone":null,"comments":0,"created_at":"2017-01-24T04:48:47Z","updated_at":"2017-01-24T12:09:57Z","closed_at":null,"pull_request":{"url":"https://api.github.com/repos/tendermint/abci/pulls/58","html_url":"https://github.com/tendermint/abci/pull/58","diff_url":"https://github.com/tendermint/abci/pull/58.diff","patch_url":"https://github.com/tendermint/abci/pull/58.patch"},"body":""},"comment":{"url":"https://api.github.com/repos/tendermint/abci/issues/comments/274787007","html_url":"https://github.com/tendermint/abci/pull/58#issuecomment-274787007","issue_url":"https://api.github.com/repos/tendermint/abci/issues/58","id":274787007,"user":{"login":"ethanfrey","id":5689864,"avatar_url":"https://avatars.githubusercontent.com/u/5689864?v=3","gravatar_id":"","url":"https://api.github.com/users/ethanfrey","html_url":"https://github.com/ethanfrey","followers_url":"https://api.github.com/users/ethanfrey/followers","following_url":"https://api.github.com/users/ethanfrey/following{/other_user}","gists_url":"https://api.github.com/users/ethanfrey/gists{/gist_id}","starred_url":"https://api.github.com/users/ethanfrey/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/ethanfrey/subscriptions","organizations_url":"https://api.github.com/users/ethanfrey/orgs","repos_url":"https://api.github.com/users/ethanfrey/repos","events_url":"https://api.github.com/users/ethanfrey/events{/privacy}","received_events_url":"https://api.github.com/users/ethanfrey/received_events","type":"User","site_admin":false},"created_at":"2017-01-24T12:09:57Z","updated_at":"2017-01-24T12:09:57Z","body":"Hey Jae, in general, I agree with the changes.  It makes both queries and proofs much richer structures. And seems much more extensible as a general interface.\r\n\r\nHowever, I have some questions, which may just be resolved with a bit more documentation.\r\n\r\n1. **QueryRequest** what are the semantics of both `Path` and `Query` and how do they play together?  Guessing from a discussion we had, `Path` informs the application how to interpret the `Query` bytes.  `ByKey`, `ByName`, etc... I assume these are app-specific?  But it would be nice to have one reserved keyword for `ByKey`, more info in part 2. A bit more explanation would be very helpful, so developers of abci apps know how to properly organize the apps.\r\n\r\n2. **QueryResponse** Nice to add `Height` even for queries.  But what are the semantics of the `Proof` and `Data` Bytes?  If I make a query with a path other than `ByKey`, then does `Data` contain both the key and the value?  Or just the value? How are they packed together?? \r\n  * In the `Proof` case, one can only provide proofs based on queries for the primary key.  I can provide a proof for the state or absence of one key in the tree without problems.  However, it is impossible to provide a proof that a given name is not in the tree (or is only in there the two times I return, not a third).  Thus, one could only safely set the `Proof` flag on the query if the `Path` is `ByKey`? Or should one return proofs for the 1 or more results?\r\n  * Also, what is the proof now?  It contains a list of nodes/hashes from the AppHash (root) down to the specified key/value pair.  That is clear.  However, we then need to hash the key/value pair to verify it matches the hash for the last node in this path.  Do we embed the key/value pair in the proof as `IAVLProof` does now? Or do we abstract the proof to just the path, and then return Key and Value in the query, leaving it to the app to hash them together and verify they match the proof?  This seems the best way to avoid redundant data, but involves changing a bit of the data structures and logic (not too bad).\r\n\r\nHere is my proposal to address these points, please take what pieces seem relevant to you into this PR:\r\n\r\n#### Query\r\n  * __Arguments__:\r\n    * `Query ([]byte)`: The query request bytes\r\n    * `Path (string)`: Path of request\r\n      * This is like the prefix bytes to the query and determines how to interpret it\r\n      * All abci apps *must* interpret `/` as a query by key on the store\r\n      * Apps *may* allow queries over specific types like `/accounts/` or `/votes/`\r\n      * Apps *may* allow queries over fields of the data like `/accounts/?name=` to search all accounts with a name that matches the query bytes\r\n    * `Height (uint64)`: The block height for which you want the query (default=0 returns data for the latest committed block)\r\n    * `Prove (bool)`: Return Merkle proof with response, returns error if the path is not by key (eg. `/accounts/?name=`)\r\n  * __Returns__:\r\n    * `Code (uint32)`: Response code\t\t\r\n    * `Key ([]byte)`: The key of the matching data (useful for proofs especially)\r\n    * `Value ([]byte)`: The value of the matching data\r\n    * `Log (string)`: Debug or error message\r\n    * `Height (uint64)`: The block height from which data was derived\r\n    * `Proof ([]byte)`: Proof for the data, if requested.  This is a list of node hashes from the root (AppHash) down to the specified node.  The last element is hash(key+value), which can be used to verify the data in `Key` and `Value` is correct.\t  \r\n\r\nThis adds a lot more semantics that `[]byte` to the return value and we must make it clear to developers what information should be returned where, so clients can be written with the expectation of a consistent interface between one app and another. \r\n\r\nThe one question I have this this is how it would handle returning multiple values (if I search by name for example).  It doesn't really fit in this schema.  Or the last one either. "}}
{"actor":{"display_login":"ethanfrey"}}