From 59da313bc0a0c86ab68ed6e489d9b74bb6d816bb Mon Sep 17 00:00:00 2001 From: Anton Kaliaev Date: Thu, 14 Nov 2019 13:34:35 +0400 Subject: [PATCH] rpc: /block_results fix docs + write test + restructure response (#3615) BREAKING Example response: ```json { "jsonrpc": "2.0", "id": "", "result": { "height": "2109", "txs_results": null, "begin_block_events": null, "end_block_events": 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 | 17 ++- consensus/replay.go | 2 +- consensus/replay_test.go | 8 +- go.sum | 1 + rpc/client/rpc_test.go | 4 +- rpc/core/blocks.go | 13 ++- rpc/core/blocks_test.go | 65 ++++++++++- rpc/core/types/responses.go | 9 +- rpc/swagger/swagger.yaml | 221 +++++++++++++++++++++++++++--------- state/execution.go | 8 +- state/export_test.go | 6 - state/state_test.go | 6 +- state/store.go | 13 ++- 13 files changed, 285 insertions(+), 88 deletions(-) diff --git a/CHANGELOG_PENDING.md b/CHANGELOG_PENDING.md index 037b90c33..37413640c 100644 --- a/CHANGELOG_PENDING.md +++ b/CHANGELOG_PENDING.md @@ -11,6 +11,21 @@ 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, + "begin_block_events": null, + "end_block_events": null, + "validator_updates": null, + "consensus_param_updates": null + } + } + ``` - Apps @@ -27,7 +42,7 @@ program](https://hackerone.com/tendermint). - [libs/pubsub] [\#4070](https://github.com/tendermint/tendermint/pull/4070) No longer panic in `Query#(Matches|Conditions)` preferring to return an error instead. - [libs/pubsub] [\#4070](https://github.com/tendermint/tendermint/pull/4070) Strip out non-numeric characters when attempting to match numeric values. - [p2p] [\#3991](https://github.com/tendermint/tendermint/issues/3991) Log "has been established or dialed" as debug log instead of Error for connected peers (@whunmr) -- [rpc] [\#4077](https://github.com/tendermint/tendermint/pull/4077) Added support for `EXISTS` clause to the Websocket query interface. +- [rpc] [\#4077](https://github.com/tendermint/tendermint/pull/4077) Added support for `EXISTS` clause to the Websocket query interface. - [privval] Add `SignerDialerEndpointRetryWaitInterval` option (@cosmostuba) - [crypto] Add `RegisterKeyType` to amino to allow external key types registration (@austinabell) diff --git a/consensus/replay.go b/consensus/replay.go index e5f28aa03..a4c0aef19 100644 --- a/consensus/replay.go +++ b/consensus/replay.go @@ -530,7 +530,7 @@ type mockProxyApp struct { } func (mock *mockProxyApp) DeliverTx(req abci.RequestDeliverTx) abci.ResponseDeliverTx { - r := mock.abciResponses.DeliverTx[mock.txCount] + r := mock.abciResponses.DeliverTxs[mock.txCount] mock.txCount++ if r == nil { //it could be nil because of amino unMarshall, it will cause an empty ResponseDeliverTx to become nil return abci.ResponseDeliverTx{} diff --git a/consensus/replay_test.go b/consensus/replay_test.go index b3ebf7340..6a804c307 100644 --- a/consensus/replay_test.go +++ b/consensus/replay_test.go @@ -548,8 +548,8 @@ func TestMockProxyApp(t *testing.T) { assert.NotPanics(t, func() { abciResWithEmptyDeliverTx := new(sm.ABCIResponses) - abciResWithEmptyDeliverTx.DeliverTx = make([]*abci.ResponseDeliverTx, 0) - abciResWithEmptyDeliverTx.DeliverTx = append(abciResWithEmptyDeliverTx.DeliverTx, &abci.ResponseDeliverTx{}) + abciResWithEmptyDeliverTx.DeliverTxs = make([]*abci.ResponseDeliverTx, 0) + abciResWithEmptyDeliverTx.DeliverTxs = append(abciResWithEmptyDeliverTx.DeliverTxs, &abci.ResponseDeliverTx{}) // called when saveABCIResponses: bytes := cdc.MustMarshalBinaryBare(abciResWithEmptyDeliverTx) @@ -562,7 +562,7 @@ func TestMockProxyApp(t *testing.T) { mock := newMockProxyApp([]byte("mock_hash"), loadedAbciRes) abciRes := new(sm.ABCIResponses) - abciRes.DeliverTx = make([]*abci.ResponseDeliverTx, len(loadedAbciRes.DeliverTx)) + abciRes.DeliverTxs = make([]*abci.ResponseDeliverTx, len(loadedAbciRes.DeliverTxs)) // Execute transactions and get hash. proxyCb := func(req *abci.Request, res *abci.Response) { if r, ok := res.Value.(*abci.Response_DeliverTx); ok { @@ -576,7 +576,7 @@ func TestMockProxyApp(t *testing.T) { logger.Debug("Invalid tx", "code", txRes.Code, "log", txRes.Log) invalidTxs++ } - abciRes.DeliverTx[txIndex] = txRes + abciRes.DeliverTxs[txIndex] = txRes txIndex++ } } diff --git a/go.sum b/go.sum index 83f1073cb..9dbce32ff 100644 --- a/go.sum +++ b/go.sum @@ -191,6 +191,7 @@ github.com/syndtr/goleveldb v1.0.1-0.20190318030020-c3a204f8e965 h1:1oFLiOyVl+W7 github.com/syndtr/goleveldb v1.0.1-0.20190318030020-c3a204f8e965/go.mod h1:9OrXJhf154huy1nPWmuSrkgjPUtUNhA+Zmy+6AESzuA= github.com/tendermint/go-amino v0.14.1 h1:o2WudxNfdLNBwMyl2dqOJxiro5rfrEaU0Ugs6offJMk= github.com/tendermint/go-amino v0.14.1/go.mod h1:i/UKE5Uocn+argJJBb12qTZsCDBcAYMbR92AaJVmKso= +github.com/tendermint/tm-cmn v0.2.0 h1:7t7apCmQATegPtv75NLrKoQ3Mjo4jWcGjsedy43ANgE= github.com/tendermint/tm-db v0.2.0 h1:rJxgdqn6fIiVJZy4zLpY1qVlyD0TU6vhkT4kEf71TQQ= github.com/tendermint/tm-db v0.2.0/go.mod h1:0cPKWu2Mou3IlxecH+MEUSYc1Ch537alLe6CpFrKzgw= github.com/tmc/grpc-websocket-proxy v0.0.0-20190109142713-0ad062ec5ee5/go.mod h1:ncp9v5uamzpCO7NfCPTXjqaC+bZgJeR0sMTm6dMHP7U= diff --git a/rpc/client/rpc_test.go b/rpc/client/rpc_test.go index d325384dd..8d16cbc69 100644 --- a/rpc/client/rpc_test.go +++ b/rpc/client/rpc_test.go @@ -233,9 +233,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 diff --git a/rpc/core/blocks.go b/rpc/core/blocks.go index b2af66ce0..228defb4a 100644 --- a/rpc/core/blocks.go +++ b/rpc/core/blocks.go @@ -124,11 +124,14 @@ func BlockResults(ctx *rpctypes.Context, heightPtr *int64) (*ctypes.ResultBlockR return nil, err } - res := &ctypes.ResultBlockResults{ - Height: height, - Results: results, - } - return res, nil + return &ctypes.ResultBlockResults{ + Height: height, + TxsResults: results.DeliverTxs, + BeginBlockEvents: results.BeginBlock.Events, + EndBlockEvents: results.EndBlock.Events, + ValidatorUpdates: results.EndBlock.ValidatorUpdates, + ConsensusParamUpdates: results.EndBlock.ConsensusParamUpdates, + }, nil } func getHeight(currentHeight int64, heightPtr *int64) (int64, error) { diff --git a/rpc/core/blocks_test.go b/rpc/core/blocks_test.go index da3920c22..b7e40a091 100644 --- a/rpc/core/blocks_test.go +++ b/rpc/core/blocks_test.go @@ -4,11 +4,18 @@ import ( "fmt" "testing" + "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + + abci "github.com/tendermint/tendermint/abci/types" + 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" + dbm "github.com/tendermint/tm-db" ) func TestBlockchainInfo(t *testing.T) { - cases := []struct { min, max int64 height int64 @@ -54,5 +61,61 @@ func TestBlockchainInfo(t *testing.T) { require.Equal(t, 1+max-min, c.resultLength, caseString) } } +} + +func TestBlockResults(t *testing.T) { + results := &sm.ABCIResponses{ + DeliverTxs: []*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.DeliverTxs, + BeginBlockEvents: results.BeginBlock.Events, + EndBlockEvents: results.EndBlock.Events, + 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) { } diff --git a/rpc/core/types/responses.go b/rpc/core/types/responses.go index f8a9476f3..dd6d8e363 100644 --- a/rpc/core/types/responses.go +++ b/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,12 @@ type ResultCommit struct { // ABCI results from a block type ResultBlockResults struct { - Height int64 `json:"height"` - Results *state.ABCIResponses `json:"results"` + Height int64 `json:"height"` + TxsResults []*abci.ResponseDeliverTx `json:"txs_results"` + BeginBlockEvents []abci.Event `json:"begin_block_events"` + EndBlockEvents []abci.Event `json:"end_block_events"` + ValidatorUpdates []abci.ValidatorUpdate `json:"validator_updates"` + ConsensusParamUpdates *abci.ConsensusParams `json:"consensus_param_updates"` } // NewResultCommit is a helper to initialize the ResultCommit with diff --git a/rpc/swagger/swagger.yaml b/rpc/swagger/swagger.yaml index 2ec7f1b40..e2703c64c 100644 --- a/rpc/swagger/swagger.yaml +++ b/rpc/swagger/swagger.yaml @@ -1405,55 +1405,168 @@ definitions: type: "string" example: "" result: + type: "object" required: - "height" - - "results" properties: height: type: "string" example: "12" - results: - required: - - "deliver_tx" - - "end_block" - - "begin_block" - properties: - deliver_tx: - type: "array" - x-nullable: true - items: + txs_results: + type: "array" + x-nullable: true + items: + type: "object" + properties: + code: + type: "string" + example: "0" + data: + type: "string" + example: "" + log: + type: "string" + example: "not enough gas" + info: + type: "string" + example: "" + gasWanted: + type: "string" + example: "100" + gasUsed: + type: "string" + example: "100" + events: + type: "array" + x-nullable: true + items: + type: "object" + properties: + type: + type: "string" + example: "app" + attributes: + type: "array" + x-nullable: false + items: + type: "object" + properties: + key: + type: "string" + example: "Y3JlYXRvcg==" + value: + type: "string" + example: "Q29zbW9zaGkgTmV0b3dva28=" + codespace: + type: "string" + example: "ibc" + begin_block_events: + type: "array" + x-nullable: true + items: + type: "object" + properties: + type: + type: "string" + example: "app" + attributes: + type: "array" + x-nullable: false + items: + type: "object" + properties: + key: + type: "string" + example: "Y3JlYXRvcg==" + value: + type: "string" + example: "Q29zbW9zaGkgTmV0b3dva28=" + end_block: + type: "array" + x-nullable: true + items: + type: "object" + properties: + type: + type: "string" + example: "app" + attributes: + type: "array" + x-nullable: false + items: + type: "object" + properties: + key: + type: "string" + example: "Y3JlYXRvcg==" + value: + type: "string" + example: "Q29zbW9zaGkgTmV0b3dva28=" + validator_updates: + type: "array" + x-nullable: true + items: + type: "object" + properties: + pub_key: type: "object" + required: + - "type" + - "value" properties: - log: - type: "string" - example: '[{"msg_index":"0","success":true,"log":""}]' - gasWanted: + type: type: "string" - example: "25629" - gasUsed: + example: "tendermint/PubKeyEd25519" + value: type: "string" - example: "25629" - tags: - type: "array" - items: - type: "object" - properties: - key: - type: "string" - example: "YWN0aW9u" - value: - type: "string" - example: "c2VuZA==" - end_block: + example: "9tK9IT+FPdf2qm+5c2qaxi10sWP+3erWTKgftn2PaQM=" + power: + type: "string" + example: "300" + consensus_param_updates: + type: "object" + x-nullable: true + required: + - "block" + - "evidence" + - "validator" + properties: + block: + type: "object" required: - - "validator_updates" - properties: {} + - "max_bytes" + - "max_gas" + - "time_iota_ms" + properties: + max_bytes: + type: "string" + example: "22020096" + max_gas: + type: "string" + example: "1000" + time_iota_ms: + type: "string" + example: "1000" + evidence: type: "object" - begin_block: - properties: {} + required: + - "max_age" + properties: + max_age: + type: "string" + example: "100000" + validator: type: "object" - type: "object" - type: "object" + required: + - "pub_key_types" + properties: + pub_key_types: + type: "array" + items: + type: "string" + example: + - "ed25519" + CommitResponse: type: "object" required: @@ -1713,10 +1826,12 @@ definitions: type: "string" example: "" result: + type: "object" required: - "genesis" properties: genesis: + type: "object" required: - "genesis_time" - "chain_id" @@ -1731,12 +1846,14 @@ definitions: type: "string" example: "cosmoshub-2" consensus_params: + type: "object" required: - "block" - "evidence" - "validator" properties: block: + type: "object" required: - "max_bytes" - "max_gas" @@ -1744,23 +1861,23 @@ definitions: properties: max_bytes: type: "string" - example: "200000" + example: "22020096" max_gas: type: "string" - example: "2000000" + example: "1000" time_iota_ms: type: "string" example: "1000" - type: "object" evidence: + type: "object" required: - "max_age" properties: max_age: type: "string" - example: "1000000" - type: "object" + example: "100000" validator: + type: "object" required: - "pub_key_types" properties: @@ -1770,8 +1887,6 @@ definitions: type: "string" example: - "ed25519" - type: "object" - type: "object" validators: type: "array" items: @@ -1804,8 +1919,7 @@ definitions: app_state: properties: {} type: "object" - type: "object" - type: "object" + DumpConsensusResponse: type: object required: @@ -2236,6 +2350,7 @@ definitions: type: "string" example: "" result: + type: "object" required: - "block_height" - "consensus_params" @@ -2244,12 +2359,14 @@ definitions: type: "string" example: "1313448" consensus_params: + type: "object" required: - "block" - "evidence" - "validator" properties: block: + type: "object" required: - "max_bytes" - "max_gas" @@ -2257,23 +2374,23 @@ definitions: properties: max_bytes: type: "string" - example: "200000" + example: "22020096" max_gas: type: "string" - example: "2000000" + example: "1000" time_iota_ms: type: "string" example: "1000" - type: "object" evidence: + type: "object" required: - "max_age" properties: max_age: type: "string" - example: "1000000" - type: "object" + example: "100000" validator: + type: "object" required: - "pub_key_types" properties: @@ -2283,9 +2400,7 @@ definitions: type: "string" example: - "ed25519" - type: "object" - type: "object" - type: "object" + NumUnconfirmedTransactionsResponse: type: object required: diff --git a/state/execution.go b/state/execution.go index 5cb109878..1369ec045 100644 --- a/state/execution.go +++ b/state/execution.go @@ -138,7 +138,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 @@ -163,7 +163,7 @@ func (blockExec *BlockExecutor) ApplyBlock(state State, blockID types.BlockID, b } // Lock mempool, commit app state, update mempoool. - appHash, err := blockExec.Commit(state, block, abciResponses.DeliverTx) + appHash, err := blockExec.Commit(state, block, abciResponses.DeliverTxs) if err != nil { return state, fmt.Errorf("Commit failed for application: %v", err) } @@ -267,7 +267,7 @@ func execBlockOnProxyApp( logger.Debug("Invalid tx", "code", txRes.Code, "log", txRes.Log) invalidTxs++ } - abciResponses.DeliverTx[txIndex] = txRes + abciResponses.DeliverTxs[txIndex] = txRes txIndex++ } } @@ -475,7 +475,7 @@ func fireEvents( Height: block.Height, Index: uint32(i), Tx: tx, - Result: *(abciResponses.DeliverTx[i]), + Result: *(abciResponses.DeliverTxs[i]), }}) } diff --git a/state/export_test.go b/state/export_test.go index 823eb4251..c023e8753 100644 --- a/state/export_test.go +++ b/state/export_test.go @@ -43,12 +43,6 @@ func CalcValidatorsKey(height int64) []byte { return calcValidatorsKey(height) } -// SaveABCIResponses is an alias for the private saveABCIResponses method in -// store.go, exported exclusively and explicitly for testing. -func SaveABCIResponses(db dbm.DB, height int64, abciResponses *ABCIResponses) { - saveABCIResponses(db, height, abciResponses) -} - // SaveConsensusParamsInfo is an alias for the private saveConsensusParamsInfo // method in store.go, exported exclusively and explicitly for testing. func SaveConsensusParamsInfo(db dbm.DB, nextHeight, changeHeight int64, params types.ConsensusParams) { diff --git a/state/state_test.go b/state/state_test.go index 9999fca74..ad0ac8398 100644 --- a/state/state_test.go +++ b/state/state_test.go @@ -89,8 +89,8 @@ func TestABCIResponsesSaveLoad1(t *testing.T) { // Build mock responses. block := makeBlock(state, 2) abciResponses := sm.NewABCIResponses(block) - abciResponses.DeliverTx[0] = &abci.ResponseDeliverTx{Data: []byte("foo"), Events: nil} - abciResponses.DeliverTx[1] = &abci.ResponseDeliverTx{Data: []byte("bar"), Log: "ok", Events: nil} + abciResponses.DeliverTxs[0] = &abci.ResponseDeliverTx{Data: []byte("foo"), Events: nil} + abciResponses.DeliverTxs[1] = &abci.ResponseDeliverTx{Data: []byte("bar"), Log: "ok", Events: nil} abciResponses.EndBlock = &abci.ResponseEndBlock{ValidatorUpdates: []abci.ValidatorUpdate{ types.TM2PB.NewValidatorUpdate(ed25519.GenPrivKey().PubKey(), 10), }} @@ -158,7 +158,7 @@ func TestABCIResponsesSaveLoad2(t *testing.T) { for i, tc := range cases { h := int64(i + 1) // last block height, one below what we save responses := &sm.ABCIResponses{ - DeliverTx: tc.added, + DeliverTxs: tc.added, EndBlock: &abci.ResponseEndBlock{}, } sm.SaveABCIResponses(stateDB, h, responses) diff --git a/state/store.go b/state/store.go index 18476660f..010d7d473 100644 --- a/state/store.go +++ b/state/store.go @@ -115,7 +115,7 @@ func saveState(db dbm.DB, state State, key []byte) { // of the various ABCI calls during block processing. // It is persisted to disk for each height before calling Commit. type ABCIResponses struct { - DeliverTx []*abci.ResponseDeliverTx `json:"deliver_tx"` + DeliverTxs []*abci.ResponseDeliverTx `json:"deliver_txs"` EndBlock *abci.ResponseEndBlock `json:"end_block"` BeginBlock *abci.ResponseBeginBlock `json:"begin_block"` } @@ -128,7 +128,7 @@ func NewABCIResponses(block *types.Block) *ABCIResponses { resDeliverTxs = nil } return &ABCIResponses{ - DeliverTx: resDeliverTxs, + DeliverTxs: resDeliverTxs, } } @@ -138,7 +138,7 @@ func (arz *ABCIResponses) Bytes() []byte { } func (arz *ABCIResponses) ResultsHash() []byte { - results := types.NewResults(arz.DeliverTx) + results := types.NewResults(arz.DeliverTxs) return results.Hash() } @@ -165,8 +165,11 @@ 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) { +// Responses are indexed by height so they can also be loaded later to produce +// Merkle proofs. +// +// Exposed for testing. +func SaveABCIResponses(db dbm.DB, height int64, abciResponses *ABCIResponses) { db.SetSync(calcABCIResponsesKey(height), abciResponses.Bytes()) }