Merge pull request from GHSA-cpqw-5g6w-h8rr

* Revert "Fixed ordering of match.events in light client RPC (tendermint#9877)" * Revert "state/kvindexer: associate event attributes with events (tendermint#9759)" * Revert "consensus: correctly save reference to previous height precommits (backport tendermint#9760) (tendermint#9776)" * add peer gossip sleep (tendermint#241) (tendermint#245) * add peer gossip sleep * add changelog * Update .changelog/unreleased/bug-fixes/4-busy-loop-send-block-part.md Co-authored-by: Jasmina Malicevic <[email protected]> * Update .changelog/unreleased/bug-fixes/4-busy-loop-send-block-part.md --------- Co-authored-by: jhernandezb <[email protected]> Co-authored-by: Jasmina Malicevic <[email protected]> (cherry picked from commit 5954e75) Co-authored-by: Sergio Mena <[email protected]> * Build changelog for v0.34.25 release Signed-off-by: Thane Thomson <[email protected]> * Bump version --------- Signed-off-by: Thane Thomson <[email protected]> Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com> Co-authored-by: Thane Thomson <[email protected]>
informalsystems · Feb 3, 2023 · 1132e3c · 1132e3c
1 parent 936221e
commit 1132e3c
Show file tree

Hide file tree

Showing 22 changed files with 121 additions and 897 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -2,6 +2,32 @@
 
 Friendly reminder, we have a [bug bounty program](https://hackerone.com/cosmos).
 
+## v0.34.25
+
+*Feb 3, 2023*
+
+This is primarily a security patch from the Informal Systems team's public fork
+of Tendermint Core, but additionally includes some minor improvements that were
+not yet released on the `v0.34.x` branch.
+
+Special thanks to @jhernandezb and the Notional team for picking up on this
+issue!
+
+### SECURITY
+
+- `[consensus]`
+  [informalsystems/tendermint\#4](https://github.com/informalsystems/tendermint/issues/4)
+  Fixed a busy loop that happened when sending of a block part failed by
+  sleeping in case of error (@jhernandezb and @sergio-mena)
+
+### IMPROVEMENTS
+
+- `[crypto]` [\#9250](https://github.com/tendermint/tendermint/issues/9250)
+  Update to use btcec v2 and the latest btcutil. (@wcsiu)
+- `[metrics]` [\#9733](https://github.com/tendermint/tendermint/issues/9733) Add
+  metrics for timing the consensus steps and for the progress of block gossip.
+  (@williambanfield)
+
 ## v0.34.24
 
 *Nov 21, 2022*

diff --git a/CHANGELOG_PENDING.md b/CHANGELOG_PENDING.md
@@ -1,6 +1,6 @@
 # Unreleased Changes
 
-## v0.34.25
+## v0.34.26
 
 ### BREAKING CHANGES
 
@@ -16,14 +16,7 @@
 
 ### FEATURES
 
-- `[rpc]` [\#9759] Added `match_event` query parameter to indicate to Tendermint that the query should match event attributes within events, not only within a height.
-
 ### IMPROVEMENTS
 
-- `[state/kvindexer]` [\#9759] Added `match.event` keyword to support condition evalution based on the event attributes belong to. (@jmalicevic)
-- [crypto] \#9250 Update to use btcec v2 and the latest btcutil. (@wcsiu)
-- [consensus] \#9760 Save peer LastCommit correctly to achieve 50% reduction in gossiped precommits. (@williambanfield)
-- [metrics] \#9733 Add metrics for timing the consensus steps and for the progress of block gossip. (@williambanfield)
-
 ### BUG FIXES
 
diff --git a/abci/example/kvstore/kvstore.go b/abci/example/kvstore/kvstore.go
@@ -68,20 +68,13 @@ type Application struct {
 
 	state        State
 	RetainBlocks int64 // blocks to retain after commit (via ResponseCommit.RetainHeight)
-	// If true, the app will generate block events in BeginBlock. Used to test the event indexer
-	// Should be false by default to avoid generating too much data.
-	genBlockEvents bool
 }
 
 func NewApplication() *Application {
 	state := loadState(dbm.NewMemDB())
 	return &Application{state: state}
 }
 
-func (app *Application) SetGenBlockEvents() {
-	app.genBlockEvents = true
-}
-
 func (app *Application) Info(req types.RequestInfo) (resInfo types.ResponseInfo) {
 	return types.ResponseInfo{
 		Data:             fmt.Sprintf("{\"size\":%v}", app.state.Size),
@@ -118,15 +111,6 @@ func (app *Application) DeliverTx(req types.RequestDeliverTx) types.ResponseDeli
 				{Key: []byte("noindex_key"), Value: []byte("index is working"), Index: false},
 			},
 		},
-		{
-			Type: "app",
-			Attributes: []types.EventAttribute{
-				{Key: []byte("creator"), Value: []byte("Cosmoshi"), Index: true},
-				{Key: []byte("key"), Value: value, Index: true},
-				{Key: []byte("index_key"), Value: []byte("index is working"), Index: true},
-				{Key: []byte("noindex_key"), Value: []byte("index is working"), Index: false},
-			},
-		},
 	}
 
 	return types.ResponseDeliverTx{Code: code.CodeTypeOK, Events: events}
@@ -186,71 +170,3 @@ func (app *Application) Query(reqQuery types.RequestQuery) (resQuery types.Respo
 
 	return resQuery
 }
-
-func (app *Application) BeginBlock(req types.RequestBeginBlock) types.ResponseBeginBlock {
-
-	response := types.ResponseBeginBlock{}
-
-	if !app.genBlockEvents {
-		return response
-	}
-
-	if app.state.Height%2 == 0 {
-		response = types.ResponseBeginBlock{
-			Events: []types.Event{
-				{
-					Type: "begin_event",
-					Attributes: []types.EventAttribute{
-						{
-							Key:   []byte("foo"),
-							Value: []byte("100"),
-							Index: true,
-						},
-						{
-							Key:   []byte("bar"),
-							Value: []byte("200"),
-							Index: true,
-						},
-					},
-				},
-				{
-					Type: "begin_event",
-					Attributes: []types.EventAttribute{
-						{
-							Key:   []byte("foo"),
-							Value: []byte("200"),
-							Index: true,
-						},
-						{
-							Key:   []byte("bar"),
-							Value: []byte("300"),
-							Index: true,
-						},
-					},
-				},
-			},
-		}
-	} else {
-		response = types.ResponseBeginBlock{
-			Events: []types.Event{
-				{
-					Type: "begin_event",
-					Attributes: []types.EventAttribute{
-						{
-							Key:   []byte("foo"),
-							Value: []byte("400"),
-							Index: true,
-						},
-						{
-							Key:   []byte("bar"),
-							Value: []byte("300"),
-							Index: true,
-						},
-					},
-				},
-			},
-		}
-	}
-
-	return response
-}
diff --git a/abci/example/kvstore/persistent_kvstore.go b/abci/example/kvstore/persistent_kvstore.go
@@ -51,9 +51,6 @@ func NewPersistentKVStoreApplication(dbDir string) *PersistentKVStoreApplication
 	}
 }
 
-func (app *PersistentKVStoreApplication) SetGenBlockEvents() {
-	app.app.genBlockEvents = true
-}
 func (app *PersistentKVStoreApplication) SetLogger(l log.Logger) {
 	app.logger = l
 }
@@ -145,7 +142,7 @@ func (app *PersistentKVStoreApplication) BeginBlock(req types.RequestBeginBlock)
 		}
 	}
 
-	return app.app.BeginBlock(req)
+	return types.ResponseBeginBlock{}
 }
 
 // Update the validator set

diff --git a/consensus/reactor.go b/consensus/reactor.go
@@ -706,6 +706,8 @@ func (conR *Reactor) gossipDataForCatchup(logger log.Logger, rs *cstypes.RoundSt
 			ps.SetHasProposalBlockPart(prs.Height, prs.Round, index)
 		} else {
 			logger.Debug("Sending block part for catchup failed")
+			// sleep to avoid retrying too fast
+			time.Sleep(conR.conS.config.PeerGossipSleepDuration)
 		}
 		return
 	}
@@ -1375,7 +1377,6 @@ func (ps *PeerState) ApplyNewRoundStepMessage(msg *NewRoundStepMessage) {
 	psRound := ps.PRS.Round
 	psCatchupCommitRound := ps.PRS.CatchupCommitRound
 	psCatchupCommit := ps.PRS.CatchupCommit
-	lastPrecommits := ps.PRS.Precommits
 
 	startTime := tmtime.Now().Add(-1 * time.Duration(msg.SecondsSinceStartTime) * time.Second)
 	ps.PRS.Height = msg.Height
@@ -1403,7 +1404,7 @@ func (ps *PeerState) ApplyNewRoundStepMessage(msg *NewRoundStepMessage) {
 		// Shift Precommits to LastCommit.
 		if psHeight+1 == msg.Height && psRound == msg.LastCommitRound {
 			ps.PRS.LastCommitRound = msg.LastCommitRound
-			ps.PRS.LastCommit = lastPrecommits
+			ps.PRS.LastCommit = ps.PRS.Precommits
 		} else {
 			ps.PRS.LastCommitRound = msg.LastCommitRound
 			ps.PRS.LastCommit = nil

diff --git a/docs/app-dev/indexing-transactions.md b/docs/app-dev/indexing-transactions.md
@@ -34,9 +34,6 @@ would be equal to the composite key of `jack.account.number`.
 By default, Tendermint will index all transactions by their respective hashes
 and height and blocks by their height.
 
-Tendermint allows for different events within the same height to have 
-equal attributes.
-
 ## Configuration
 
 Operators can configure indexing via the `[tx_index]` section. The `indexer`
@@ -70,56 +67,6 @@ for block and transaction events directly against Tendermint's RPC. However, the
 query syntax is limited and so this indexer type might be deprecated or removed
 entirely in the future.
 
-**Implementation and data layout**
-
-The kv indexer stores each attribute of an event individually, by creating a composite key 
-of the *event type*, *attribute key*, *attribute value*, *height* and *event sequence*. 
-
-For example the following events:
-
-```
-Type: "transfer",
-  Attributes: []abci.EventAttribute{
-   {Key: []byte("sender"), Value: []byte("Bob"), Index: true},
-   {Key: []byte("recipient"), Value: []byte("Alice"), Index: true},
-   {Key: []byte("balance"), Value: []byte("100"), Index: true},
-   {Key: []byte("note"), Value: []byte("nothing"), Index: true},
-   },
- 
-```
-
-```
-Type: "transfer",
-  Attributes: []abci.EventAttribute{
-   {Key: []byte("sender"), Value: []byte("Tom"), Index: true},
-   {Key: []byte("recipient"), Value: []byte("Alice"), Index: true},
-   {Key: []byte("balance"), Value: []byte("200"), Index: true},
-   {Key: []byte("note"), Value: []byte("nothing"), Index: true},
-   },
-```
-
-will be represented as follows in the store: 
-
-```
-Key                                 value
-transferSenderBobEndBlock1           1
-transferRecepientAliceEndBlock11     1
-transferBalance100EndBlock11         1
-transferNodeNothingEndblock11        1
----- event2 ------
-transferSenderTomEndBlock12          1
-transferRecepientAliceEndBlock12     1
-transferBalance200EndBlock12         1
-transferNodeNothingEndblock12        1
- 
-```
-The key is thus formed of the event type, the attribute key and value, the event the attribute belongs to (`EndBlock` or `BeginBlock`),
-the height and the event number. The event number is a local variable kept by the indexer and incremented when a new event is processed. 
-
-It is an `int64` variable and has no other semantics besides being used to associate attributes belonging to the same events within a height. 
-This variable is not atomically incremented as event indexing is deterministic. **Should this ever change**, the event id generation
-will be broken. 
-
 #### PostgreSQL
 
 The `psql` indexer type allows an operator to enable block and transaction event
@@ -198,9 +145,6 @@ You can query for a paginated set of transaction by their events by calling the
 ```bash
 curl "localhost:26657/tx_search?query=\"message.sender='cosmos1...'\"&prove=true"
 ```
-If the conditions are related to transaction events and the user wants to make sure the
-conditions are true within the same events, the `match.event` keyword should be used, 
-as described [below](#querying_block_events)
 
 Check out [API docs](https://docs.tendermint.com/v0.34/rpc/#/Info/tx_search)
 for more information on query syntax and other options.
@@ -224,7 +168,7 @@ a query to `/subscribe` RPC endpoint.
 Check out [API docs](https://docs.tendermint.com/v0.34/rpc/#subscribe) for more information
 on query syntax and other options.
 
-## Querying Block Events
+## Querying Blocks Events
 
 You can query for a paginated set of blocks by their events by calling the
 `/block_search` RPC endpoint:
@@ -233,30 +177,5 @@ You can query for a paginated set of blocks by their events by calling the
 curl "localhost:26657/block_search?query=\"block.height > 10 AND val_set.num_changed > 0\""
 ```
 
-## `match_events` keyword 
-
-The query results in the height number(s) (or transaction hashes when querying transactions) which contain events whose attributes match the query conditions. 
-However, there are two options to query the indexers. To demonstrate the two modes, we reuse the two events
-where Bob and Tom send money to Alice and query the block indexer. We issue the following query:
-
-```bash
-curl "localhost:26657/block_search?query=\"sender=Bob AND balance = 200\""
-```
-
-The result will return height 1 even though the attributes matching the conditions in the query 
-occurred in different events. 
-
-If we wish to retrieve only heights where the attributes occurred within the same event,
-the query syntax is as follows:
-
-```bash
-curl "localhost:26657/block_search?query=\"sender=Bob AND balance = 200\"&match_events=true"
-```
-Currently the default behaviour is if `match_events` is set  to false.
-
 Check out [API docs](https://docs.tendermint.com/v0.34/rpc/#/Info/block_search)
 for more information on query syntax and other options.
-
-**Backwards compatibility**
-
-Up until Tendermint 0.34.25, the event sequence was not stored in the kvstore and the `match_events` keyword in the RPC query is not ignored by older versions. Thus, in a network running mixed  Tendermint versions, nodes running older versions will still return blocks (or transactions) whose attributes match within different events on the same height.
diff --git a/light/proxy/routes.go b/light/proxy/routes.go
@@ -29,8 +29,8 @@ func RPCRoutes(c *lrpc.Client) map[string]*rpcserver.RPCFunc {
 		"block_results":        rpcserver.NewRPCFunc(makeBlockResultsFunc(c), "height", rpcserver.Cacheable("height")),
 		"commit":               rpcserver.NewRPCFunc(makeCommitFunc(c), "height", rpcserver.Cacheable("height")),
 		"tx":                   rpcserver.NewRPCFunc(makeTxFunc(c), "hash,prove", rpcserver.Cacheable()),
-		"tx_search":            rpcserver.NewRPCFunc(makeTxSearchFuncMatchEvents(c), "query,prove,page,per_page,order_by,match_events"),
-		"block_search":         rpcserver.NewRPCFunc(makeBlockSearchFuncMatchEvents(c), "query,page,per_page,order_by,match_events"),
+		"tx_search":            rpcserver.NewRPCFunc(makeTxSearchFunc(c), "query,prove,page,per_page,order_by"),
+		"block_search":         rpcserver.NewRPCFunc(makeBlockSearchFunc(c), "query,page,per_page,order_by"),
 		"validators":           rpcserver.NewRPCFunc(makeValidatorsFunc(c), "height,page,per_page", rpcserver.Cacheable("height")),
 		"dump_consensus_state": rpcserver.NewRPCFunc(makeDumpConsensusStateFunc(c), ""),
 		"consensus_state":      rpcserver.NewRPCFunc(makeConsensusStateFunc(c), ""),
@@ -141,52 +141,42 @@ func makeTxFunc(c *lrpc.Client) rpcTxFunc {
 	}
 }
 
-type rpcTxSearchFuncMatchEvents func(
+type rpcTxSearchFunc func(
 	ctx *rpctypes.Context,
 	query string,
 	prove bool,
 	page, perPage *int,
 	orderBy string,
-	matchEvents bool,
 ) (*ctypes.ResultTxSearch, error)
 
-func makeTxSearchFuncMatchEvents(c *lrpc.Client) rpcTxSearchFuncMatchEvents {
+func makeTxSearchFunc(c *lrpc.Client) rpcTxSearchFunc {
 	return func(
 		ctx *rpctypes.Context,
 		query string,
 		prove bool,
 		page, perPage *int,
 		orderBy string,
-		matchEvents bool,
 	) (*ctypes.ResultTxSearch, error) {
-		if matchEvents {
-			query = "match.events = 1 AND " + query
-		}
 		return c.TxSearch(ctx.Context(), query, prove, page, perPage, orderBy)
 	}
 }
 
-type rpcBlockSearchFuncMatchEvents func(
+type rpcBlockSearchFunc func(
 	ctx *rpctypes.Context,
 	query string,
 	prove bool,
 	page, perPage *int,
 	orderBy string,
-	matchEvents bool,
 ) (*ctypes.ResultBlockSearch, error)
 
-func makeBlockSearchFuncMatchEvents(c *lrpc.Client) rpcBlockSearchFuncMatchEvents {
+func makeBlockSearchFunc(c *lrpc.Client) rpcBlockSearchFunc {
 	return func(
 		ctx *rpctypes.Context,
 		query string,
 		prove bool,
 		page, perPage *int,
 		orderBy string,
-		matchEvents bool,
 	) (*ctypes.ResultBlockSearch, error) {
-		if matchEvents {
-			query = "match.events = 1 AND " + query
-		}
 		return c.BlockSearch(ctx.Context(), query, page, perPage, orderBy)
 	}
 }