rpc: /block_results fix docs + write test + restructure response

BREAKING

Example response:

```json
{
  "jsonrpc": "2.0",
  "id": "",
  "result": {
    "height": "2109",
    "txs_results": null,
    "validator_updates": null,
    "consensus_param_updates": null
  }
}
```

Old result consisted of ABCIResponses struct and height. Exposing
internal ABCI structures (which we store in state package) in RPC seems
bad to me for the following reasons:

1) high risk of breaking the API when somebody changes internal structs
(HAPPENED HERE!)
2) RPC is aware of ABCI, which I'm not sure we want

CHANGELOG_PENDING.md

@@ -10,6 +10,19 @@ program](https://hackerone.com/tendermint).
 ### BREAKING CHANGES:
 
 * CLI/RPC/Config
+- [rpc] `/block_results` response format updated (see RPC docs for details)
+  ```
+  {
+    "jsonrpc": "2.0",
+    "id": "",
+    "result": {
+      "height": "2109",
+      "txs_results": null,
+      "validator_updates": null,
+      "consensus_param_updates": null
+    }
+  }
+  ```
 
 * Apps
 

consensus/replay_test.go

@@ -532,7 +532,7 @@ func TestMockProxyApp(t *testing.T) {
 		abciResWithEmptyDeliverTx.DeliverTx = make([]*abci.ResponseDeliverTx, 0)
 		abciResWithEmptyDeliverTx.DeliverTx = append(abciResWithEmptyDeliverTx.DeliverTx, &abci.ResponseDeliverTx{})
 
-		// called when saveABCIResponses:
+		// called when SaveABCIResponses:
 		bytes := cdc.MustMarshalBinaryBare(abciResWithEmptyDeliverTx)
 		loadedAbciRes := new(sm.ABCIResponses)
 

rpc/client/rpc_test.go

@@ -209,9 +209,9 @@ func TestAppCalls(t *testing.T) {
 		blockResults, err := c.BlockResults(&txh)
 		require.Nil(err, "%d: %+v", i, err)
 		assert.Equal(txh, blockResults.Height)
-		if assert.Equal(1, len(blockResults.Results.DeliverTx)) {
+		if assert.Equal(1, len(blockResults.TxsResults)) {
 			// check success code
-			assert.EqualValues(0, blockResults.Results.DeliverTx[0].Code)
+			assert.EqualValues(0, blockResults.TxsResults[0].Code)
 		}
 
 		// check blockchain info, now that we know there is info

rpc/core/blocks.go

@@ -364,25 +364,36 @@ func Commit(ctx *rpctypes.Context, heightPtr *int64) (*ctypes.ResultCommit, erro
 //   "jsonrpc": "2.0",
 //   "id": "",
 //   "result": {
-//     "height": "39",
+//     "height": "437",
-//     "results": {
+//     "txs_results": [
-//       "deliver_tx": [
+//       {
-//         {
+//         "gas_wanted": 1,
-//           "tags": [
+//         "gas_used": 1,
-//             {
+//         "tags": [
-//               "key": "YXBwLmNyZWF0b3I=",
+//           {
-//               "value": "Q29zbW9zaGkgTmV0b3dva28="
+//             "key": "YXBwLmNyZWF0b3I=",
-//             }
+//             "value": "Q29zbW9zaGkgTmV0b3dva28="
-//           ]
+//           },
-//         }
+//         ]
-//       ],
-//       "end_block": {
-//         "validator_updates": null
 //       },
-//       "begin_block": {}
+//       {
-//     }
+//         "code": 1,
-//   }
+//         "codespace": "ibc",
-// }
+//         "log": "not enough gas",
+//         "gas_wanted": 1,
+//         "gas_used": 2,
+//         "tags": [
+//           {
+//             "key": "YXBwLmNyZWF0b3I=",
+//             "value": "Q29zbW9zaGkgTmV0b3dva28="
+//           },
+//         ]
+//       },
+//    ],
+//    "validator_updates": null,
+//    "consensus_param_updates": null,
+//  }
+//}
 // ```
 func BlockResults(ctx *rpctypes.Context, heightPtr *int64) (*ctypes.ResultBlockResults, error) {
 	storeHeight := blockStore.Height()
@@ -396,11 +407,12 @@ func BlockResults(ctx *rpctypes.Context, heightPtr *int64) (*ctypes.ResultBlockR
 		return nil, err
 	}
 
-	res := &ctypes.ResultBlockResults{
+	return &ctypes.ResultBlockResults{
-		Height:  height,
+		Height:                height,
-		Results: results,
+		TxsResults:            results.DeliverTx,
-	}
+		ValidatorUpdates:      results.EndBlock.ValidatorUpdates,
-	return res, nil
+		ConsensusParamUpdates: results.EndBlock.ConsensusParamUpdates,
+	}, nil
 }
 
 func getHeight(currentHeight int64, heightPtr *int64) (int64, error) {

rpc/core/blocks_test.go

@@ -4,7 +4,15 @@ import (
 	"fmt"
 	"testing"
 
+	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
+
+	abci "github.com/tendermint/tendermint/abci/types"
+	dbm "github.com/tendermint/tendermint/libs/db"
+	ctypes "github.com/tendermint/tendermint/rpc/core/types"
+	rpctypes "github.com/tendermint/tendermint/rpc/lib/types"
+	sm "github.com/tendermint/tendermint/state"
+	"github.com/tendermint/tendermint/types"
 )
 
 func TestBlockchainInfo(t *testing.T) {
@@ -54,5 +62,59 @@ func TestBlockchainInfo(t *testing.T) {
 			require.Equal(t, 1+max-min, c.resultLength, caseString)
 		}
 	}
-
+}
+
+func TestBlockResults(t *testing.T) {
+	results := &sm.ABCIResponses{
+		DeliverTx: []*abci.ResponseDeliverTx{
+			{Code: 0, Data: []byte{0x01}, Log: "ok"},
+			{Code: 0, Data: []byte{0x02}, Log: "ok"},
+			{Code: 1, Log: "not ok"},
+		},
+		EndBlock:   &abci.ResponseEndBlock{},
+		BeginBlock: &abci.ResponseBeginBlock{},
+	}
+	stateDB = dbm.NewMemDB()
+	sm.SaveABCIResponses(stateDB, 100, results)
+
+	blockStore = mockBlockStore{height: 100}
+
+	testCases := []struct {
+		height  int64
+		wantErr bool
+		wantRes *ctypes.ResultBlockResults
+	}{
+		{-1, true, nil},
+		{0, true, nil},
+		{101, true, nil},
+		{100, false, &ctypes.ResultBlockResults{
+			Height:                100,
+			TxsResults:            results.DeliverTx,
+			ValidatorUpdates:      results.EndBlock.ValidatorUpdates,
+			ConsensusParamUpdates: results.EndBlock.ConsensusParamUpdates,
+		}},
+	}
+
+	for _, tc := range testCases {
+		res, err := BlockResults(&rpctypes.Context{}, &tc.height)
+		if tc.wantErr {
+			assert.Error(t, err)
+		} else {
+			assert.NoError(t, err)
+			assert.Equal(t, tc.wantRes, res)
+		}
+	}
+}
+
+type mockBlockStore struct {
+	height int64
+}
+
+func (store mockBlockStore) Height() int64                               { return store.height }
+func (mockBlockStore) LoadBlockMeta(height int64) *types.BlockMeta       { return nil }
+func (mockBlockStore) LoadBlock(height int64) *types.Block               { return nil }
+func (mockBlockStore) LoadBlockPart(height int64, index int) *types.Part { return nil }
+func (mockBlockStore) LoadBlockCommit(height int64) *types.Commit        { return nil }
+func (mockBlockStore) LoadSeenCommit(height int64) *types.Commit         { return nil }
+func (mockBlockStore) SaveBlock(block *types.Block, blockParts *types.PartSet, seenCommit *types.Commit) {
 }

rpc/core/types/responses.go

@@ -9,7 +9,6 @@ import (
 	cmn "github.com/tendermint/tendermint/libs/common"
 
 	"github.com/tendermint/tendermint/p2p"
-	"github.com/tendermint/tendermint/state"
 	"github.com/tendermint/tendermint/types"
 )
 
@@ -38,8 +37,10 @@ type ResultCommit struct {
 
 // ABCI results from a block
 type ResultBlockResults struct {
-	Height  int64                `json:"height"`
+	Height                int64                     `json:"height"`
-	Results *state.ABCIResponses `json:"results"`
+	TxsResults            []*abci.ResponseDeliverTx `json:"txs_results"`
+	ValidatorUpdates      []abci.ValidatorUpdate    `json:"validator_updates"`
+	ConsensusParamUpdates *abci.ConsensusParams     `json:"consensus_param_updates"`
 }
 
 // NewResultCommit is a helper to initialize the ResultCommit with

state/execution.go

@@ -131,7 +131,7 @@ func (blockExec *BlockExecutor) ApplyBlock(state State, blockID types.BlockID, b
 	fail.Fail() // XXX
 
 	// Save the results before we commit.
-	saveABCIResponses(blockExec.db, block.Height, abciResponses)
+	SaveABCIResponses(blockExec.db, block.Height, abciResponses)
 
 	fail.Fail() // XXX
 

state/state_test.go

@@ -165,7 +165,11 @@ func TestABCIResponsesSaveLoad2(t *testing.T) {
 			DeliverTx: tc.added,
 			EndBlock:  &abci.ResponseEndBlock{},
 		}
+<<<<<<< HEAD
 		sm.SaveABCIResponses(stateDB, h, responses)
+=======
+		SaveABCIResponses(stateDB, h, responses)
+>>>>>>> rpc: /block_results fix docs + write test + restructure response
 	}
 
 	// Query all before, should return expected value.

state/store.go

@@ -166,7 +166,7 @@ func LoadABCIResponses(db dbm.DB, height int64) (*ABCIResponses, error) {
 // SaveABCIResponses persists the ABCIResponses to the database.
 // This is useful in case we crash after app.Commit and before s.Save().
 // Responses are indexed by height so they can also be loaded later to produce Merkle proofs.
-func saveABCIResponses(db dbm.DB, height int64, abciResponses *ABCIResponses) {
+func SaveABCIResponses(db dbm.DB, height int64, abciResponses *ABCIResponses) {
 	db.SetSync(calcABCIResponsesKey(height), abciResponses.Bytes())
 }