From c45367e22c8fad4c46c5cd4026df0ab805887461 Mon Sep 17 00:00:00 2001 From: Sam Kleinman Date: Wed, 13 Apr 2022 14:09:49 -0400 Subject: [PATCH 1/6] rpc: avoid leaking threads (#8328) --- internal/rpc/core/mempool.go | 122 ++++++++++++++++++++--------------- 1 file changed, 70 insertions(+), 52 deletions(-) diff --git a/internal/rpc/core/mempool.go b/internal/rpc/core/mempool.go index c2a9084db..a852aeb1d 100644 --- a/internal/rpc/core/mempool.go +++ b/internal/rpc/core/mempool.go @@ -38,23 +38,32 @@ func (env *Environment) BroadcastTxSync(ctx context.Context, tx types.Tx) (*core err := env.Mempool.CheckTx( ctx, tx, - func(res *abci.ResponseCheckTx) { resCh <- res }, + func(res *abci.ResponseCheckTx) { + select { + case <-ctx.Done(): + case resCh <- res: + } + }, mempool.TxInfo{}, ) if err != nil { return nil, err } - r := <-resCh + select { + case <-ctx.Done(): + return nil, fmt.Errorf("broadcast confirmation not received: %w", ctx.Err()) + case r := <-resCh: + return &coretypes.ResultBroadcastTx{ + Code: r.Code, + Data: r.Data, + Log: r.Log, + Codespace: r.Codespace, + MempoolError: r.MempoolError, + Hash: tx.Hash(), + }, nil + } - return &coretypes.ResultBroadcastTx{ - Code: r.Code, - Data: r.Data, - Log: r.Log, - Codespace: r.Codespace, - MempoolError: r.MempoolError, - Hash: tx.Hash(), - }, nil } // BroadcastTxCommit returns with the responses from CheckTx and DeliverTx. @@ -64,61 +73,70 @@ func (env *Environment) BroadcastTxCommit(ctx context.Context, tx types.Tx) (*co err := env.Mempool.CheckTx( ctx, tx, - func(res *abci.ResponseCheckTx) { resCh <- res }, + func(res *abci.ResponseCheckTx) { + select { + case <-ctx.Done(): + case resCh <- res: + } + }, mempool.TxInfo{}, ) if err != nil { return nil, err } - r := <-resCh - if r.Code != abci.CodeTypeOK { - return &coretypes.ResultBroadcastTxCommit{ - CheckTx: *r, - Hash: tx.Hash(), - }, fmt.Errorf("transaction encountered error (%s)", r.MempoolError) - } - - if !indexer.KVSinkEnabled(env.EventSinks) { - return &coretypes.ResultBroadcastTxCommit{ + select { + case <-ctx.Done(): + return nil, fmt.Errorf("broadcast confirmation not received: %w", ctx.Err()) + case r := <-resCh: + if r.Code != abci.CodeTypeOK { + return &coretypes.ResultBroadcastTxCommit{ CheckTx: *r, Hash: tx.Hash(), - }, - errors.New("cannot confirm transaction because kvEventSink is not enabled") - } + }, fmt.Errorf("transaction encountered error (%s)", r.MempoolError) + } - startAt := time.Now() - timer := time.NewTimer(0) - defer timer.Stop() - - count := 0 - for { - count++ - select { - case <-ctx.Done(): - env.Logger.Error("error on broadcastTxCommit", - "duration", time.Since(startAt), - "err", err) + if !indexer.KVSinkEnabled(env.EventSinks) { return &coretypes.ResultBroadcastTxCommit{ CheckTx: *r, Hash: tx.Hash(), - }, fmt.Errorf("timeout waiting for commit of tx %s (%s)", - tx.Hash(), time.Since(startAt)) - case <-timer.C: - txres, err := env.Tx(ctx, tx.Hash(), false) - if err != nil { - jitter := 100*time.Millisecond + time.Duration(rand.Int63n(int64(time.Second))) // nolint: gosec - backoff := 100 * time.Duration(count) * time.Millisecond - timer.Reset(jitter + backoff) - continue - } + }, + errors.New("cannot confirm transaction because kvEventSink is not enabled") + } - return &coretypes.ResultBroadcastTxCommit{ - CheckTx: *r, - TxResult: txres.TxResult, - Hash: tx.Hash(), - Height: txres.Height, - }, nil + startAt := time.Now() + timer := time.NewTimer(0) + defer timer.Stop() + + count := 0 + for { + count++ + select { + case <-ctx.Done(): + env.Logger.Error("error on broadcastTxCommit", + "duration", time.Since(startAt), + "err", err) + return &coretypes.ResultBroadcastTxCommit{ + CheckTx: *r, + Hash: tx.Hash(), + }, fmt.Errorf("timeout waiting for commit of tx %s (%s)", + tx.Hash(), time.Since(startAt)) + case <-timer.C: + txres, err := env.Tx(ctx, tx.Hash(), false) + if err != nil { + jitter := 100*time.Millisecond + time.Duration(rand.Int63n(int64(time.Second))) // nolint: gosec + backoff := 100 * time.Duration(count) * time.Millisecond + timer.Reset(jitter + backoff) + continue + } + + return &coretypes.ResultBroadcastTxCommit{ + CheckTx: *r, + TxResult: txres.TxResult, + Hash: tx.Hash(), + Height: txres.Height, + }, nil + } } } } From 0e4b18806e91f82acfaa533adcb5c876ce82789d Mon Sep 17 00:00:00 2001 From: Sam Kleinman Date: Wed, 13 Apr 2022 15:20:56 -0400 Subject: [PATCH 2/6] pubsub: [minor] remove unused stub method (#8316) OnReset was removed from the service interface and we missed deleting this. --- internal/pubsub/pubsub.go | 3 --- 1 file changed, 3 deletions(-) diff --git a/internal/pubsub/pubsub.go b/internal/pubsub/pubsub.go index df2dd90e3..3e887226b 100644 --- a/internal/pubsub/pubsub.go +++ b/internal/pubsub/pubsub.go @@ -309,9 +309,6 @@ func (s *Server) Wait() { <-s.exited; s.BaseService.Wait() } // OnStart implements Service.OnStart by starting the server. func (s *Server) OnStart(ctx context.Context) error { s.run(ctx); return nil } -// OnReset implements Service.OnReset. It has no effect for this service. -func (s *Server) OnReset() error { return nil } - func (s *Server) publish(ctx context.Context, data types.EventData, events []abci.Event) error { s.pubs.RLock() defer s.pubs.RUnlock() From 6dc15b2f502189e45ece4c9055d4c8f7a4fd47a1 Mon Sep 17 00:00:00 2001 From: "M. J. Fromberger" Date: Wed, 13 Apr 2022 13:13:59 -0700 Subject: [PATCH 3/6] Only run the markdown linter if markdown was touched. (#8337) --- .github/workflows/markdown-links.yml | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/.github/workflows/markdown-links.yml b/.github/workflows/markdown-links.yml index 503af7b0e..634072404 100644 --- a/.github/workflows/markdown-links.yml +++ b/.github/workflows/markdown-links.yml @@ -12,7 +12,12 @@ jobs: runs-on: ubuntu-latest steps: - uses: actions/checkout@v3 + - uses: technote-space/get-diff-action@v6.0.1 + with: + PATTERNS: | + **/**.md - uses: gaurav-nelson/github-action-markdown-link-check@1.0.14 with: check-modified-files-only: 'yes' config-file: '.md-link-check.json' + if: env.GIT_DIFF From 929671743341200062d633b07e568ad648b6b4b3 Mon Sep 17 00:00:00 2001 From: "M. J. Fromberger" Date: Wed, 13 Apr 2022 13:25:59 -0700 Subject: [PATCH 4/6] Update RFC ToC for RFC-015. (#8338) --- docs/rfc/README.md | 1 + 1 file changed, 1 insertion(+) diff --git a/docs/rfc/README.md b/docs/rfc/README.md index 8905c0160..b5c42ea85 100644 --- a/docs/rfc/README.md +++ b/docs/rfc/README.md @@ -52,5 +52,6 @@ sections. - [RFC-012: Event Indexing Revisited](./rfc-012-custom-indexing.md) - [RFC-013: ABCI++](./rfc-013-abci++.md) - [RFC-014: Semantic Versioning](./rfc-014-semantic-versioning.md) +- [RFC-015: ABCI++ Tx Mutation](./rfc-015-abci++-tx-mutation.md) From 024f6117ee473a34396e70d99a937e6bbca6fa46 Mon Sep 17 00:00:00 2001 From: "M. J. Fromberger" Date: Wed, 13 Apr 2022 13:41:35 -0700 Subject: [PATCH 5/6] confix: remove mempool.version in v0.36 (#8334) --- go.mod | 2 +- go.sum | 4 ++-- scripts/confix/confix.go | 10 +++++++--- 3 files changed, 10 insertions(+), 6 deletions(-) diff --git a/go.mod b/go.mod index 3101f26ad..1232f180b 100644 --- a/go.mod +++ b/go.mod @@ -73,7 +73,7 @@ require ( github.com/charithe/durationcheck v0.0.9 // indirect github.com/chavacava/garif v0.0.0-20210405164556-e8a0a408d6af // indirect github.com/containerd/continuity v0.2.1 // indirect - github.com/creachadair/tomledit v0.0.15 + github.com/creachadair/tomledit v0.0.16 github.com/daixiang0/gci v0.3.3 // indirect github.com/davecgh/go-spew v1.1.1 // indirect github.com/denis-tingaikin/go-header v0.4.3 // indirect diff --git a/go.sum b/go.sum index 0da80e4d6..251be8a4b 100644 --- a/go.sum +++ b/go.sum @@ -225,8 +225,8 @@ github.com/creachadair/atomicfile v0.2.4 h1:GRjpQLmz/78I4+nBQpGMFrRa9yrL157AUTrA github.com/creachadair/atomicfile v0.2.4/go.mod h1:BRq8Une6ckFneYXZQ+kO7p1ZZP3I2fzVzf28JxrIkBc= github.com/creachadair/taskgroup v0.3.2 h1:zlfutDS+5XG40AOxcHDSThxKzns8Tnr9jnr6VqkYlkM= github.com/creachadair/taskgroup v0.3.2/go.mod h1:wieWwecHVzsidg2CsUnFinW1faVN4+kq+TDlRJQ0Wbk= -github.com/creachadair/tomledit v0.0.15 h1:g/qlpwSFIXEngyz8gcTEGZtvdGBLHODwV+Z0BFILVxw= -github.com/creachadair/tomledit v0.0.15/go.mod h1:gvtfnSZLa+YNQD28vaPq0Nk12bRxEhmUdBzAWn+EGF4= +github.com/creachadair/tomledit v0.0.16 h1:PDNxgDjeeiNk1cyFfliIVQmagh1jPbDMabOw9yfSKLk= +github.com/creachadair/tomledit v0.0.16/go.mod h1:gvtfnSZLa+YNQD28vaPq0Nk12bRxEhmUdBzAWn+EGF4= github.com/creack/pty v1.1.7/go.mod h1:lj5s0c3V2DBrqTV7llrYr5NG6My20zk30Fl46Y7DoTY= github.com/creack/pty v1.1.9/go.mod h1:oKZEueFk5CKHvIhNR5MUki03XCEU+Q6VDXinZuGJ33E= github.com/cyphar/filepath-securejoin v0.2.2/go.mod h1:FpkQEhXnPnOthhzymB7CGsFk2G9VLXONKD9G7QGMM+4= diff --git a/scripts/confix/confix.go b/scripts/confix/confix.go index d77950e7f..7a1f22670 100644 --- a/scripts/confix/confix.go +++ b/scripts/confix/confix.go @@ -131,9 +131,13 @@ var plan = transform.Plan{ ErrorOK: true, }, { - // Since https://github.com/tendermint/tendermint/pull/6396. - Desc: "Remove vestigial mempool.wal-dir setting", - T: transform.Remove(parser.Key{"mempool", "wal-dir"}), + // Removed wal-dir: https://github.com/tendermint/tendermint/pull/6396. + // Removed version: https://github.com/tendermint/tendermint/pull/7171. + Desc: "Remove vestigial mempool.wal-dir settings", + T: transform.Remove( + parser.Key{"mempool", "wal-dir"}, + parser.Key{"mempool", "version"}, + ), ErrorOK: true, }, { From 6bbf5b6d0fa90c5fa5574827c7bf34bf20e260e8 Mon Sep 17 00:00:00 2001 From: "M. J. Fromberger" Date: Wed, 13 Apr 2022 16:01:39 -0700 Subject: [PATCH 6/6] Work around markdown-link-check issues. (#8339) Work around two issues causing the markdown link check to fail in CI. 1. https://github.com/actions/checkout/pull/760. A git permissions issue, apparently triggered by a combination of a git change and the behaviour of actions/checkout. 2. https://github.com/gaurav-nelson/github-action-markdown-link-check/pull/129. Merging an updated version of the underlying package that fixes a bug in the handling of local #anchors. The workaround is a temporary patched fork of the link-checker action. This should be removed once the upstream issues are addressed. --- .github/workflows/markdown-links.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/markdown-links.yml b/.github/workflows/markdown-links.yml index 634072404..356eb3f31 100644 --- a/.github/workflows/markdown-links.yml +++ b/.github/workflows/markdown-links.yml @@ -16,7 +16,7 @@ jobs: with: PATTERNS: | **/**.md - - uses: gaurav-nelson/github-action-markdown-link-check@1.0.14 + - uses: creachadair/github-action-markdown-link-check@master with: check-modified-files-only: 'yes' config-file: '.md-link-check.json'