Merge tag 'v0.38.10' into cometbft-0.38.9

Release v0.38.10

Author: iKapitonau

.changelog/v0.38.10/bug-fixes/3002-invalid-txs-results.md

@@ -0,0 +1,3 @@
+- `[rpc]` Fix an issue where a legacy ABCI response, created on `v0.37` or before, is not returned properly in `v0.38` and up
+  on the `/block_results` RPC endpoint.
+  ([\#3002](https://github.com/cometbft/cometbft/issues/3002))

.changelog/v0.38.10/bug-fixes/3406-blocksync-dont-stall-if-blocking-chain.md

@@ -0,0 +1,3 @@
+- `[blocksync]` Do not stay in blocksync if the node's validator voting power
+  is high enough to block the chain while it is not online
+  ([\#3406](https://github.com/cometbft/cometbft/pull/3406))

.changelog/v0.38.10/bug-fixes/486-p2p-max-outbound.md

@@ -0,0 +1,3 @@
+- `[p2p]` Node respects configured `max_num_outbound_peers` limit when dialing
+  peers provided by a seed node
+  ([\#486](https://github.com/cometbft/cometbft/issues/486))

.changelog/v0.38.10/improvements/3382-single-send-monitor-per-packet.md

@@ -0,0 +1,2 @@
+- `[p2p/conn]` Update send monitor, used for sending rate limiting, once per batch of packets sent
+  ([\#3382](https://github.com/cometbft/cometbft/pull/3382))

.changelog/v0.38.10/improvements/3401-allow-dash-in-event-tags.md

@@ -0,0 +1,2 @@
+- `[libs/pubsub]` Allow dash (`-`) in event tags
+  ([\#3401](https://github.com/cometbft/cometbft/issues/3401))

.changelog/v0.38.10/improvements/3403-remove-pool-buffer-usage-in-secretconn.md

@@ -0,0 +1,2 @@
+- `[p2p/conn]` Remove the usage of a synchronous pool of buffers in secret connection, storing instead the buffer in the connection struct. This reduces the synchronization primitive usage, speeding up the code.
+  ([\#3403](https://github.com/cometbft/cometbft/issues/3403))

.changelog/v0.38.10/summary.md

@@ -0,0 +1,5 @@
+*July 16, 2024*
+
+This release fixes a bug in `v0.38.x` that prevented ABCI responses from being
+correctly read when upgrading from `v0.37.x` or below. It also includes a few other
+bug fixes and performance improvements.

.changelog/v0.38.8/improvements/14-abc.md renamed to .changelog/v0.38.8/bug-fixes/14-abc.md

@@ -41,7 +41,7 @@ jobs:
           platforms: all
 
       - name: Set up Docker Build
-        uses: docker/setup-buildx-action@v3.3.0
+        uses: docker/setup-buildx-action@v3.4.0
 
       - name: Login to DockerHub
         if: ${{ github.event_name != 'pull_request' }}
@@ -51,7 +51,7 @@ jobs:
           password: ${{ secrets.DOCKERHUB_TOKEN }}
 
       - name: Publish to Docker Hub
-        uses: docker/build-push-action@v6.2.0
+        uses: docker/build-push-action@v6.3.0
         with:
           context: .
           file: ./DOCKER/Dockerfile

.github/workflows/cometbft-docker.yml

@@ -41,7 +41,7 @@ jobs:
           platforms: all
 
       - name: Set up Docker Build
-        uses: docker/setup-buildx-action@v3.3.0
+        uses: docker/setup-buildx-action@v3.4.0
 
       - name: Login to DockerHub
         if: ${{ github.event_name != 'pull_request' }}
@@ -51,7 +51,7 @@ jobs:
           password: ${{ secrets.DOCKERHUB_TOKEN }}
 
       - name: Publish to Docker Hub
-        uses: docker/build-push-action@v6.2.0
+        uses: docker/build-push-action@v6.3.0
         with:
           context: .
           file: ./test/e2e/docker/Dockerfile

.github/workflows/testapp-docker.yml

@@ -1,5 +1,34 @@
 # CHANGELOG
 
+## v0.38.10
+
+*July 16, 2024*
+
+This release fixes a bug in `v0.38.x` that prevented ABCI responses from being
+correctly read when upgrading from `v0.37.x` or below. It also includes a few other
+bug fixes and performance improvements.
+
+### BUG FIXES
+
+- `[p2p]` Node respects configured `max_num_outbound_peers` limit when dialing
+  peers provided by a seed node
+  ([\#486](https://github.com/cometbft/cometbft/issues/486))
+- `[rpc]` Fix an issue where a legacy ABCI response, created on `v0.37` or before, is not returned properly in `v0.38` and up
+  on the `/block_results` RPC endpoint.
+  ([\#3002](https://github.com/cometbft/cometbft/issues/3002))
+- `[blocksync]` Do not stay in blocksync if the node's validator voting power
+  is high enough to block the chain while it is not online
+  ([\#3406](https://github.com/cometbft/cometbft/pull/3406))
+
+### IMPROVEMENTS
+
+- `[p2p/conn]` Update send monitor, used for sending rate limiting, once per batch of packets sent
+  ([\#3382](https://github.com/cometbft/cometbft/pull/3382))
+- `[libs/pubsub]` Allow dash (`-`) in event tags
+  ([\#3401](https://github.com/cometbft/cometbft/issues/3401))
+- `[p2p/conn]` Remove the usage of a synchronous pool of buffers in secret connection, storing instead the buffer in the connection struct. This reduces the synchronization primitive usage, speeding up the code.
+  ([\#3403](https://github.com/cometbft/cometbft/issues/3403))
+
 ## v0.38.9
 
 *July 1, 2024*
@@ -35,6 +64,10 @@ This release contains a few bug fixes and performance improvements.
 
 ### BUG FIXES
 
+- `[blockstore]` Added peer banning in blockstore
+  ([\#ABC-0013](https://github.com/cometbft/cometbft/security/advisories/GHSA-hg58-rf2h-6rr7))
+- `[blockstore]` Send correct error message when vote extensions do not align with received packet
+  ([\#ABC-0014](https://github.com/cometbft/cometbft/security/advisories/GHSA-hg58-rf2h-6rr7))
 - [`mempool`] Fix data race when rechecking with async ABCI client
   ([\#1827](https://github.com/cometbft/cometbft/issues/1827))
 - `[consensus]` Fix a race condition in the consensus timeout ticker. Race is caused by two timeouts being scheduled at the same time.
@@ -44,10 +77,6 @@ This release contains a few bug fixes and performance improvements.
 
 ### IMPROVEMENTS
 
-- `[blockstore]` Added peer banning in blockstore
-  ([\#ABC-0013](https://github.com/cometbft/cometbft/security/advisories/GHSA-hg58-rf2h-6rr7))
-- `[blockstore]` Send correct error message when vote extensions do not align with received packet
-  ([\#ABC-0014](https://github.com/cometbft/cometbft/security/advisories/GHSA-hg58-rf2h-6rr7))
 - `[config]` Added `recheck_timeout` mempool parameter to set how much time to wait for recheck
   responses from the app (only applies to non-local ABCI clients).
   ([\#1827](https://github.com/cometbft/cometbft/issues/1827/))

CHANGELOG.md

@@ -6,6 +6,7 @@ import (
 	"sync"
 	"time"
 
+	"github.com/cometbft/cometbft/crypto"
 	"github.com/cometbft/cometbft/libs/log"
 	"github.com/cometbft/cometbft/p2p"
 	bcproto "github.com/cometbft/cometbft/proto/tendermint/blocksync"
@@ -56,6 +57,7 @@ type Reactor struct {
 	store         sm.BlockStore
 	pool          *BlockPool
 	blockSync     bool
+	localAddr     crypto.Address
 	poolRoutineWg sync.WaitGroup
 
 	requestsCh <-chan BlockRequest
@@ -70,6 +72,13 @@ type Reactor struct {
 func NewReactor(state sm.State, blockExec *sm.BlockExecutor, store *store.BlockStore,
 	blockSync bool, metrics *Metrics, offlineStateSyncHeight int64,
 ) *Reactor {
+	return NewReactorWithAddr(state, blockExec, store, blockSync, nil, metrics, offlineStateSyncHeight)
+}
+
+// Function added to keep existing API.
+func NewReactorWithAddr(state sm.State, blockExec *sm.BlockExecutor, store *store.BlockStore,
+	blockSync bool, localAddr crypto.Address, metrics *Metrics, offlineStateSyncHeight int64,
+) *Reactor {
 
 	storeHeight := store.Height()
 	if storeHeight == 0 {
@@ -105,6 +114,7 @@ func NewReactor(state sm.State, blockExec *sm.BlockExecutor, store *store.BlockS
 		store:        store,
 		pool:         pool,
 		blockSync:    blockSync,
+		localAddr:    localAddr,
 		requestsCh:   requestsCh,
 		errorsCh:     errorsCh,
 		metrics:      metrics,
@@ -294,6 +304,15 @@ func (bcR *Reactor) Receive(e p2p.Envelope) { //nolint: dupl // recreated in a t
 	}
 }
 
+func (bcR *Reactor) localNodeBlocksTheChain(state sm.State) bool {
+	_, val := state.Validators.GetByAddress(bcR.localAddr)
+	if val == nil {
+		return false
+	}
+	total := state.Validators.TotalVotingPower()
+	return val.VotingPower >= total/3
+}
+
 // Handle messages from the poolReactor telling the reactor what to do.
 // NOTE: Don't sleep in the FOR_LOOP or otherwise slow it down!
 func (bcR *Reactor) poolRoutine(stateSynced bool) {
@@ -402,7 +421,7 @@ FOR_LOOP:
 				)
 				continue FOR_LOOP
 			}
-			if bcR.pool.IsCaughtUp() {
+			if bcR.pool.IsCaughtUp() || bcR.localNodeBlocksTheChain(state) {
 				bcR.Logger.Info("Time to switch to consensus reactor!", "height", height)
 				if err := bcR.pool.Stop(); err != nil {
 					bcR.Logger.Error("Error stopping pool", "err", err)

blocksync/reactor.go

@@ -111,7 +111,7 @@ func newReactor(
 	// Make the Reactor itself.
 	// NOTE we have to create and commit the blocks first because
 	// pool.height is determined from the store.
-	fastSync := true
+	blockSync := true
 	db := dbm.NewMemDB()
 	stateStore = sm.NewStore(db, sm.StoreOptions{
 		DiscardABCIResponses: false,
@@ -125,6 +125,13 @@ func newReactor(
 	// The commit we are building for the current height.
 	seenExtCommit := &types.ExtendedCommit{}
 
+	pubKey, err := privVals[0].GetPubKey()
+	if err != nil {
+		panic(err)
+	}
+	addr := pubKey.Address()
+	idx, _ := state.Validators.GetByAddress(addr)
+
 	// let's add some blocks in
 	for blockHeight := int64(1); blockHeight <= maxBlockHeight; blockHeight++ {
 		voteExtensionIsEnabled := genDoc.ConsensusParams.ABCI.VoteExtensionsEnabled(blockHeight)
@@ -138,12 +145,6 @@ func newReactor(
 		blockID := types.BlockID{Hash: thisBlock.Hash(), PartSetHeader: thisParts.Header()}
 
 		// Simulate a commit for the current height
-		pubKey, err := privVals[0].GetPubKey()
-		if err != nil {
-			panic(err)
-		}
-		addr := pubKey.Address()
-		idx, _ := state.Validators.GetByAddress(addr)
 		vote, err := types.MakeVote(
 			privVals[0],
 			thisBlock.Header.ChainID,
@@ -177,7 +178,7 @@ func newReactor(
 		}
 	}
 
-	bcReactor := NewByzantineReactor(incorrectBlock, NewReactor(state.Copy(), blockExec, blockStore, fastSync, NopMetrics(), 0))
+	bcReactor := NewByzantineReactor(incorrectBlock, NewReactor(state.Copy(), blockExec, blockStore, blockSync, NopMetrics(), 0))
 	bcReactor.SetLogger(logger.With("module", "blocksync"))
 
 	return ReactorPair{bcReactor, proxyApp}

blocksync/reactor_test.go

@@ -262,26 +262,44 @@ You can query for a paginated set of blocks by their events by calling the
 `/block_search` RPC endpoint:
 
 ```bash
-curl "localhost:26657/block_search?query=\"block.height > 10 AND val_set.num_changed > 0\""
+curl "localhost:26657/block_search?query=\"block.height > 10\""
 ```
 
 
-Storing the event sequence was introduced in CometBFT 0.34.26. Before that, up until Tendermint Core 0.34.26,
-the event sequence was not stored in the kvstore and events were stored only by height. That means that queries
-returned blocks and transactions whose event attributes match within the height but can match across different
-events on that height.
-This behavior was fixed with CometBFT 0.34.26+. However, if the data was indexed with earlier versions of
-Tendermint Core and not re-indexed, that data will be queried as if all the attributes within a height
-occurred within the same event.
+Storing the event sequence was introduced in CometBFT 0.34.26. Before that, up
+until Tendermint Core 0.34.26, the event sequence was not stored in the kvstore
+and events were stored only by height. That means that queries returned blocks
+and transactions whose event attributes match within the height but can match
+across different events on that height.
+
+This behavior was fixed with CometBFT 0.34.26+. However, if the data was
+indexed with earlier versions of Tendermint Core and not re-indexed, that data
+will be queried as if all the attributes within a height occurred within the
+same event.
 
 ## Event attribute value types
 
-Users can use anything as an event value. However, if the event attribute value is a number, the following needs to be taken into account:
+Users can use anything as an event value. However, if the event attribute value
+is a number, the following needs to be taken into account:
 
 - Negative numbers will not be properly retrieved when querying the indexer.
-- Event values are converted to big floats (from the `big/math` package). The precision of the floating point number is set to the bit length
-of the integer it is supposed to represent, so that there is no loss of information due to insufficient precision. This was not present before CometBFT v0.38.x and all float values were ignored.
+- Event values are converted to big floats (from the `big/math` package). The
+  precision of the floating point number is set to the bit length of the
+  integer it is supposed to represent, so that there is no loss of information
+  due to insufficient precision. This was not present before CometBFT v0.38.x
+  and all float values were ignored.
 - As of CometBFT v0.38.x, queries can contain floating point numbers as well.
 - Note that comparing to floats can be imprecise with a high number of decimals.
 
+## Event type and attribute key format
+
+An event type/attribute key is a string that can contain any Unicode letter or
+digit, as well as the following characters: `.` (dot), `-` (dash), `_`
+(underscore). The event type/attribute key must not start with `-` (dash) or
+`.` (dot).
+
+```
+^[\w]+[\.-\w]?$
+```
+
 [abci-events]: ../spec/abci/abci++_basic_concepts.md#events

docs/app-dev/indexing-transactions.md

@@ -16,7 +16,6 @@ require (
 	github.com/gorilla/websocket v1.5.0
 	github.com/informalsystems/tm-load-test v1.3.0
 	github.com/lib/pq v1.10.7
-	github.com/libp2p/go-buffer-pool v0.1.0
 	github.com/minio/highwayhash v1.0.2
 	github.com/ory/dockertest v3.3.5+incompatible
 	github.com/pkg/errors v0.9.1

go.mod

@@ -562,8 +562,6 @@ github.com/leonklingele/grouper v1.1.1 h1:suWXRU57D4/Enn6pXR0QVqqWWrnJ9Osrz+5rjt
 github.com/leonklingele/grouper v1.1.1/go.mod h1:uk3I3uDfi9B6PeUjsCKi6ndcf63Uy7snXgR4yDYQVDY=
 github.com/lib/pq v1.10.7 h1:p7ZhMD+KsSRozJr34udlUrhboJwWAgCg34+/ZZNvZZw=
 github.com/lib/pq v1.10.7/go.mod h1:AlVN5x4E4T544tWzH6hKfbfQvm3HdbOxrmggDNAPY9o=
-github.com/libp2p/go-buffer-pool v0.1.0 h1:oK4mSFcQz7cTQIfqbe4MIj9gLW+mnanjyFtc6cdF0Y8=
-github.com/libp2p/go-buffer-pool v0.1.0/go.mod h1:N+vh8gMqimBzdKkSMVuydVDq+UV5QTWy5HSiZacSbPg=
 github.com/lufeee/execinquery v1.2.1 h1:hf0Ems4SHcUGBxpGN7Jz78z1ppVkP/837ZlETPCEtOM=
 github.com/lufeee/execinquery v1.2.1/go.mod h1:EC7DrEKView09ocscGHC+apXMIaorh4xqSxS/dy8SbM=
 github.com/magiconair/properties v1.8.0/go.mod h1:PppfXfuXeibc/6YijjN8zIbojt8czPbwD3XqdrwzmxQ=

go.sum

@@ -17,7 +17,7 @@
 // The lexical terms are defined here using RE2 regular expression notation:
 //
 //	// The name of an event attribute (type.value)
-//	tag    = #'\w+(\.\w+)*'
+//	 tag   = #`^[\w]+[\.-\w]?$`
 //
 //	// A datestamp (YYYY-MM-DD)
 //	date   = #'DATE \d{4}-\d{2}-\d{2}'

libs/pubsub/query/syntax/doc.go

@@ -99,7 +99,7 @@ func (s *Scanner) Next() error {
 		}
 		if '0' <= ch && ch <= '9' {
 			return s.scanNumber(ch)
-		} else if isTagRune(ch) {
+		} else if isFirstTagRune(ch) {
 			return s.scanTagLike(ch)
 		}
 		switch ch {
@@ -302,7 +302,11 @@ func (s *Scanner) invalid(ch rune) error {
 func isDigit(r rune) bool { return '0' <= r && r <= '9' }
 
 func isTagRune(r rune) bool {
-	return r == '.' || r == '_' || unicode.IsLetter(r) || unicode.IsDigit(r)
+	return r == '.' || r == '_' || r == '-' || unicode.IsLetter(r) || unicode.IsDigit(r)
+}
+
+func isFirstTagRune(r rune) bool {
+	return r == '_' || unicode.IsLetter(r) || unicode.IsDigit(r)
 }
 
 func isTimeRune(r rune) bool {

libs/pubsub/query/syntax/scanner.go

@@ -25,6 +25,8 @@ func TestScanner(t *testing.T) {
 
 		// Tags
 		{`foo foo.bar`, []syntax.Token{syntax.TTag, syntax.TTag}},
+		{`foo foo-foo.bar`, []syntax.Token{syntax.TTag, syntax.TTag}},
+		{`foo foo._bar_bar`, []syntax.Token{syntax.TTag, syntax.TTag}},
 
 		// Strings (values)
 		{` '' x 'x' 'x y'`, []syntax.Token{syntax.TString, syntax.TTag, syntax.TString, syntax.TString}},
@@ -167,6 +169,8 @@ func TestParseValid(t *testing.T) {
 
 		{"hash='136E18F7E4C348B780CF873A0BF43922E5BAFA63'", true},
 		{"hash=136E18F7E4C348B780CF873A0BF43922E5BAFA63", false},
+
+		{"cosm-wasm.transfer_amount=100", true},
 	}
 
 	for _, test := range tests {

libs/pubsub/query/syntax/syntax_test.go

@@ -344,9 +344,10 @@ func NewNodeWithContext(ctx context.Context,
 	if err != nil {
 		return nil, fmt.Errorf("can't get pubkey: %w", err)
 	}
+	localAddr := pubKey.Address()
 
 	// Determine whether we should attempt state sync.
-	stateSync := config.StateSync.Enable && !onlyValidatorIsUs(state, pubKey)
+	stateSync := config.StateSync.Enable && !onlyValidatorIsUs(state, localAddr)
 	if stateSync && state.LastBlockHeight > 0 {
 		logger.Info("Found local state with non-zero height, skipping state sync")
 		stateSync = false
@@ -371,7 +372,7 @@ func NewNodeWithContext(ctx context.Context,
 
 	// Determine whether we should do block sync. This must happen after the handshake, since the
 	// app may modify the validator set, specifying ourself as the only validator.
-	blockSync := !onlyValidatorIsUs(state, pubKey)
+	blockSync := !onlyValidatorIsUs(state, localAddr)
 
 	logNodeStartupInfo(state, pubKey, logger, consensusLogger)
 
@@ -401,7 +402,7 @@ func NewNodeWithContext(ctx context.Context,
 		}
 	}
 	// Don't start block sync if we're doing a state sync first.
-	bcReactor, err := createBlocksyncReactor(config, state, blockExec, blockStore, blockSync && !stateSync, logger, bsMetrics, offlineStateSyncHeight)
+	bcReactor, err := createBlocksyncReactor(config, state, blockExec, blockStore, blockSync && !stateSync, localAddr, logger, bsMetrics, offlineStateSyncHeight)
 	if err != nil {
 		return nil, fmt.Errorf("could not create blocksync reactor: %w", err)
 	}
@@ -591,10 +592,11 @@ func (n *Node) OnStop() {
 	if err := n.eventBus.Stop(); err != nil {
 		n.Logger.Error("Error closing eventBus", "err", err)
 	}
-	if err := n.indexerService.Stop(); err != nil {
-		n.Logger.Error("Error closing indexerService", "err", err)
+	if n.indexerService != nil {
+		if err := n.indexerService.Stop(); err != nil {
+			n.Logger.Error("Error closing indexerService", "err", err)
+		}
 	}
-
 	// now stop the reactors
 	if err := n.sw.Stop(); err != nil {
 		n.Logger.Error("Error closing switch", "err", err)