Skip to content

abci: Add unsynchronized local client#9660

Merged
thanethomson merged 10 commits intomainfrom
thane/7073-abci-unsync-mtx
Nov 7, 2022
Merged

abci: Add unsynchronized local client#9660
thanethomson merged 10 commits intomainfrom
thane/7073-abci-unsync-mtx

Conversation

@thanethomson
Copy link
Contributor

@thanethomson thanethomson commented Nov 3, 2022

Closes #7073.

Adds an unsynchronized local client without changing the existing local client behaviour, so API consumers can switch at their leisure and we can roll this out in a non-breaking update from v0.34 onwards.

Also adds a "synchronized" version of the E2E application that can be used with the unsynchronized client, and modifies the E2E tests to randomly choose either a synchronized or unsynchronized application for full nodes and validators whose ABCI protocol is chosen as "builtin".

A manual full E2E run (like the nightly run) seems to show that this works: https://github.com/tendermint/tendermint/actions/runs/3385473966/jobs/5623656412

The CI E2E tests have also been updated to ensure that one validator (validator04) always uses the sync version of the app with the unsynchronized local client.


PR checklist

  • Tests written/updated, or no tests needed
  • CHANGELOG_PENDING.md updated, or no changelog entry needed
  • Updated relevant documentation (docs/) and code comments, or no
    documentation updates needed

@thanethomson thanethomson added the C:abci Component: Application Blockchain Interface label Nov 3, 2022
@thanethomson thanethomson requested a review from a team November 3, 2022 11:38
@thanethomson thanethomson force-pushed the thane/7073-abci-unsync-mtx branch from 1e2413f to 02dcebc Compare November 3, 2022 11:46
Copy link
Contributor

@sergio-mena sergio-mena left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for this Thane!

tmsync "github.com/tendermint/tendermint/libs/sync"
)

var _ Client = (*localClient)(nil)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This improves readability in my view. Is there a reason to remove it?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The exact same constraint definition occurs just after the localClient struct definition.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point :-)

}

func (app *SyncApplication) ListSnapshots(req abci.RequestListSnapshots) abci.ResponseListSnapshots {
// Calls app.snapshots.List(), which is thread-safe.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is right, but it is making the assumption app will always be test/e2e/app/app.go.
In that case, I would remove the Application parameter in line 19, and instantiate the actual struct that makes the assumption true. This prevents any "mis-usage" of SyncApplication

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm now enforcing that the type of the inner app variable is indeed our E2E app - see L14 and L25.

Signed-off-by: Thane Thomson <connect@thanethomson.com>
Signed-off-by: Thane Thomson <connect@thanethomson.com>
Signed-off-by: Thane Thomson <connect@thanethomson.com>
Signed-off-by: Thane Thomson <connect@thanethomson.com>
Signed-off-by: Thane Thomson <connect@thanethomson.com>
Signed-off-by: Thane Thomson <connect@thanethomson.com>
Signed-off-by: Thane Thomson <connect@thanethomson.com>
Signed-off-by: Thane Thomson <connect@thanethomson.com>
Signed-off-by: Thane Thomson <connect@thanethomson.com>
Signed-off-by: Thane Thomson <connect@thanethomson.com>
@thanethomson thanethomson force-pushed the thane/7073-abci-unsync-mtx branch from 0717a2b to 3c60672 Compare November 7, 2022 11:27
@thanethomson thanethomson merged commit 45071d1 into main Nov 7, 2022
@thanethomson thanethomson deleted the thane/7073-abci-unsync-mtx branch November 7, 2022 11:46
panic(err)
}
return NewLocalClientCreator(app)
case "e2e_sync":
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't this be e2e_unsync? Isn't it a bit confusing this way

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think so. It's a synchronized application that we're connecting to with an unsynchronized client.

The e2e app is an unsynchronized application with a synchronized client, by comparison.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ah I see. I was looking at this from the point of view of the client not the application

thanethomson added a commit that referenced this pull request Dec 3, 2022
This reverts commit 45071d1.

Signed-off-by: Thane Thomson <connect@thanethomson.com>
thanethomson added a commit that referenced this pull request Dec 3, 2022
This reverts commit 45071d1.

Signed-off-by: Thane Thomson <connect@thanethomson.com>
thanethomson added a commit that referenced this pull request Dec 12, 2022
This reverts commit 45071d1.

Signed-off-by: Thane Thomson <connect@thanethomson.com>
thanethomson added a commit that referenced this pull request Dec 16, 2022
…ncurrency (#9830)

* Revert "abci: Add unsynchronized local client (#9660)"

This reverts commit 45071d1.

Signed-off-by: Thane Thomson <connect@thanethomson.com>

* proxy: Add unsync local client creator

Signed-off-by: Thane Thomson <connect@thanethomson.com>

* e2e: Extend tests

Extend the E2E tests to randomly choose between the sync (default) and
unsync (new) local client creator.

Signed-off-by: Thane Thomson <connect@thanethomson.com>

* abci: Remove redundant interface constraint

Signed-off-by: Thane Thomson <connect@thanethomson.com>

* abci: Remove irrelevant doc comment

Signed-off-by: Thane Thomson <connect@thanethomson.com>

* proxy: Remove backticks in doc comments

Signed-off-by: Thane Thomson <connect@thanethomson.com>

* e2e: Remove unnecessary gap between doc comment and struct

Signed-off-by: Thane Thomson <connect@thanethomson.com>

* Add pending changelog entry

Signed-off-by: Thane Thomson <connect@thanethomson.com>

* e2e: Expand on BuiltinProxyMode param docstring

Signed-off-by: Thane Thomson <connect@thanethomson.com>

* Remove builtin proxy mode config option from CI test

Signed-off-by: Thane Thomson <connect@thanethomson.com>

* e2e: Make builtin proxy mode option testnet-wide

Signed-off-by: Thane Thomson <connect@thanethomson.com>

* e2e: Embed sync/unsync notion in node protocol

The approach of randomly generating the proxy mode across testnets
resulted in a totally uneven ratio of sync to unsync modes for all
testnets that happened to have a protocol of "builtin".

This commit adapts the E2E tests to have a new ABCI protocol option:
"builtin_unsync". This results in a better spread of sync/unsync choices
for generated testnets.

Signed-off-by: Thane Thomson <connect@thanethomson.com>

* e2e: Remove unused type

Signed-off-by: Thane Thomson <connect@thanethomson.com>

Signed-off-by: Thane Thomson <connect@thanethomson.com>
lcwik added a commit to dydxprotocol/cometbft that referenced this pull request Apr 25, 2023
This is effectively a copy of the fully unsynchronized local client found in tendermint/tendermint#9660. Note that this was reverted and replaced by a version that has a mutex per instance of the client in tendermint/tendermint#9830. This change can be removed once a fully unsynchronized client is added back.
lcwik added a commit to dydxprotocol/cometbft that referenced this pull request Apr 25, 2023
This is effectively a copy of the fully unsynchronized local client found in tendermint/tendermint#9660. Note that this was reverted and replaced by a version that has a mutex per instance of the client in tendermint/tendermint#9830. This change can be removed once a fully unsynchronized client is added back.
lcwik added a commit to dydxprotocol/cometbft that referenced this pull request Apr 25, 2023
This is effectively a copy of the fully unsynchronized local client found in tendermint/tendermint#9660. Note that this was reverted and replaced by a version that has a mutex per instance of the client in tendermint/tendermint#9830. This change can be removed once a fully unsynchronized client is added back.
lcwik added a commit to dydxprotocol/cometbft that referenced this pull request Apr 26, 2023
This is effectively a copy of the fully unsynchronized local client found in tendermint/tendermint#9660. Note that this was reverted and replaced by a version that has a mutex per instance of the client in tendermint/tendermint#9830. This change can be removed once a fully unsynchronized client is added back.
lcwik added a commit to dydxprotocol/cometbft that referenced this pull request Apr 26, 2023
This is effectively a copy of the fully unsynchronized local client found in tendermint/tendermint#9660. Note that this was reverted and replaced by a version that has a mutex per instance of the client in tendermint/tendermint#9830. This change can be removed once a fully unsynchronized client is added back.
lcwik added a commit to dydxprotocol/cometbft that referenced this pull request Aug 30, 2023
This is effectively a copy of the fully unsynchronized local client found in tendermint/tendermint#9660. Note that this was reverted and replaced by a version that has a mutex per instance of the client in tendermint/tendermint#9830. This change can be removed once a fully unsynchronized client is added back.
lcwik added a commit to dydxprotocol/cometbft that referenced this pull request Aug 30, 2023
This is effectively a copy of the fully unsynchronized local client found in tendermint/tendermint#9660. Note that this was reverted and replaced by a version that has a mutex per instance of the client in tendermint/tendermint#9830. This change can be removed once a fully unsynchronized client is added back.
lcwik added a commit to dydxprotocol/cometbft that referenced this pull request Aug 30, 2023
This is effectively a copy of the fully unsynchronized local client found in tendermint/tendermint#9660. Note that this was reverted and replaced by a version that has a mutex per instance of the client in tendermint/tendermint#9830. This change can be removed once a fully unsynchronized client is added back.
lcwik added a commit to dydxprotocol/cometbft that referenced this pull request Aug 30, 2023
This is effectively a copy of the fully unsynchronized local client found in tendermint/tendermint#9660. Note that this was reverted and replaced by a version that has a mutex per instance of the client in tendermint/tendermint#9830. This change can be removed once a fully unsynchronized client is added back.
lcwik added a commit to dydxprotocol/cometbft that referenced this pull request Aug 31, 2023
This is effectively a copy of the fully unsynchronized local client found in tendermint/tendermint#9660. Note that this was reverted and replaced by a version that has a mutex per instance of the client in tendermint/tendermint#9830. This change can be removed once a fully unsynchronized client is added back.
lcwik added a commit to dydxprotocol/cometbft that referenced this pull request Aug 31, 2023
This is effectively a copy of the fully unsynchronized local client found in tendermint/tendermint#9660. Note that this was reverted and replaced by a version that has a mutex per instance of the client in tendermint/tendermint#9830. This change can be removed once a fully unsynchronized client is added back.
lcwik added a commit to dydxprotocol/cometbft that referenced this pull request Aug 31, 2023
This is effectively a copy of the fully unsynchronized local client found in tendermint/tendermint#9660. Note that this was reverted and replaced by a version that has a mutex per instance of the client in tendermint/tendermint#9830. This change can be removed once a fully unsynchronized client is added back.
lcwik added a commit to dydxprotocol/cometbft that referenced this pull request Aug 31, 2023
This is effectively a copy of the fully unsynchronized local client found in tendermint/tendermint#9660. Note that this was reverted and replaced by a version that has a mutex per instance of the client in tendermint/tendermint#9830. This change can be removed once a fully unsynchronized client is added back.
matthewdowney pushed a commit to matthewdowney/cometbft that referenced this pull request Aug 31, 2023
This is effectively a copy of the fully unsynchronized local client found in tendermint/tendermint#9660. Note that this was reverted and replaced by a version that has a mutex per instance of the client in tendermint/tendermint#9830. This change can be removed once a fully unsynchronized client is added back.
lcwik added a commit to dydxprotocol/cometbft that referenced this pull request Dec 4, 2023
This is effectively a copy of the fully unsynchronized local client found in tendermint/tendermint#9660. Note that this was reverted and replaced by a version that has a mutex per instance of the client in tendermint/tendermint#9830. This change can be removed once a fully unsynchronized client is added back.
lcwik added a commit to dydxprotocol/cometbft that referenced this pull request Dec 4, 2023
This is effectively a copy of the fully unsynchronized local client found in tendermint/tendermint#9660. Note that this was reverted and replaced by a version that has a mutex per instance of the client in tendermint/tendermint#9830. This change can be removed once a fully unsynchronized client is added back.
lcwik added a commit to dydxprotocol/cometbft that referenced this pull request Dec 4, 2023
This is effectively a copy of the fully unsynchronized local client found in tendermint/tendermint#9660. Note that this was reverted and replaced by a version that has a mutex per instance of the client in tendermint/tendermint#9830. This change can be removed once a fully unsynchronized client is added back.
roy-dydx pushed a commit to dydxprotocol/cometbft that referenced this pull request Feb 20, 2024
This is effectively a copy of the fully unsynchronized local client found in tendermint/tendermint#9660. Note that this was reverted and replaced by a version that has a mutex per instance of the client in tendermint/tendermint#9830. This change can be removed once a fully unsynchronized client is added back.
roy-dydx pushed a commit to dydxprotocol/cometbft that referenced this pull request Apr 9, 2024
This is effectively a copy of the fully unsynchronized local client found in tendermint/tendermint#9660. Note that this was reverted and replaced by a version that has a mutex per instance of the client in tendermint/tendermint#9830. This change can be removed once a fully unsynchronized client is added back.
roy-dydx pushed a commit to dydxprotocol/cometbft that referenced this pull request Apr 9, 2024
This is effectively a copy of the fully unsynchronized local client found in tendermint/tendermint#9660. Note that this was reverted and replaced by a version that has a mutex per instance of the client in tendermint/tendermint#9830. This change can be removed once a fully unsynchronized client is added back.
roy-dydx pushed a commit to dydxprotocol/cometbft that referenced this pull request Apr 14, 2024
This is effectively a copy of the fully unsynchronized local client found in tendermint/tendermint#9660. Note that this was reverted and replaced by a version that has a mutex per instance of the client in tendermint/tendermint#9830. This change can be removed once a fully unsynchronized client is added back.
roy-dydx pushed a commit to dydxprotocol/cometbft that referenced this pull request Apr 16, 2024
This is effectively a copy of the fully unsynchronized local client found in tendermint/tendermint#9660. Note that this was reverted and replaced by a version that has a mutex per instance of the client in tendermint/tendermint#9830. This change can be removed once a fully unsynchronized client is added back.
roy-dydx pushed a commit to dydxprotocol/cometbft that referenced this pull request Apr 16, 2024
This is effectively a copy of the fully unsynchronized local client found in tendermint/tendermint#9660. Note that this was reverted and replaced by a version that has a mutex per instance of the client in tendermint/tendermint#9830. This change can be removed once a fully unsynchronized client is added back.
roy-dydx pushed a commit to dydxprotocol/cometbft that referenced this pull request Apr 16, 2024
This is effectively a copy of the fully unsynchronized local client found in tendermint/tendermint#9660. Note that this was reverted and replaced by a version that has a mutex per instance of the client in tendermint/tendermint#9830. This change can be removed once a fully unsynchronized client is added back.
roy-dydx pushed a commit to dydxprotocol/cometbft that referenced this pull request Apr 22, 2024
This is effectively a copy of the fully unsynchronized local client found in tendermint/tendermint#9660. Note that this was reverted and replaced by a version that has a mutex per instance of the client in tendermint/tendermint#9830. This change can be removed once a fully unsynchronized client is added back.
pthmas pushed a commit to 01builders/dydx-cometbft that referenced this pull request Aug 7, 2025
This is effectively a copy of the fully unsynchronized local client found in tendermint/tendermint#9660. Note that this was reverted and replaced by a version that has a mutex per instance of the client in tendermint/tendermint#9830. This change can be removed once a fully unsynchronized client is added back.
pthmas pushed a commit to 01builders/dydx-cometbft that referenced this pull request Sep 5, 2025
This is effectively a copy of the fully unsynchronized local client found in tendermint/tendermint#9660. Note that this was reverted and replaced by a version that has a mutex per instance of the client in tendermint/tendermint#9830. This change can be removed once a fully unsynchronized client is added back.
pthmas pushed a commit to 01builders/dydx-cometbft that referenced this pull request Sep 5, 2025
This is effectively a copy of the fully unsynchronized local client found in tendermint/tendermint#9660. Note that this was reverted and replaced by a version that has a mutex per instance of the client in tendermint/tendermint#9830. This change can be removed once a fully unsynchronized client is added back.
pthmas pushed a commit to 01builders/dydx-cometbft that referenced this pull request Sep 24, 2025
This is effectively a copy of the fully unsynchronized local client found in tendermint/tendermint#9660. Note that this was reverted and replaced by a version that has a mutex per instance of the client in tendermint/tendermint#9830. This change can be removed once a fully unsynchronized client is added back.
pthmas pushed a commit to 01builders/dydx-cometbft that referenced this pull request Sep 29, 2025
This is effectively a copy of the fully unsynchronized local client found in tendermint/tendermint#9660. Note that this was reverted and replaced by a version that has a mutex per instance of the client in tendermint/tendermint#9830. This change can be removed once a fully unsynchronized client is added back.
northstar456 pushed a commit to dydxprotocol/cometbft that referenced this pull request Oct 9, 2025
* update LICENSE (#25)

* Add fork workflow instructions

* Ignore PlaceOrder/CancelOrder in mempool reaping and recheck

* Add locking around processing of a block

* Add dydx-specific changes to CList mempool:
* purge based upon block height and block time
* filter out short term order placements and cancellations

* Set KeepInvalidTxsInCache to true in mempool config

* [STAB-17] Make unsynchronized local client (#3)

This is effectively a copy of the fully unsynchronized local client found in tendermint/tendermint#9660. Note that this was reverted and replaced by a version that has a mutex per instance of the client in tendermint/tendermint#9830. This change can be removed once a fully unsynchronized client is added back.

* [CLOB-547] `IsClobOrderTransaction` only filter out Short-Term orders/cancels. (#7)

* generate protos for dydx

* add the matches and tx protos

* add dydx order id logic and constants

* IsClobOrderTransaction => IsShortTermClobOrderTransaction

* disable lint for dydx order id constants

* more specific linter disables

* rename proto package due to global proto registry name clash

* Update import paths for dydx clob types

* helpful text string function for order id

* minor pr comments, no logic changes

* README updates

* consensus: remove logic to unlock block on +2/3 prevote for nil (cometbft#1175)

* consensus: remove logic to unlock block on +2/3 prevote for nil

* add tests

* add changelog

* fix tests, remove extra logic

* update to match forward port

* Add check for non-`nil` in `enterCommit` (cometbft#1208)

* Minor

* Add check for non-`nil` in `enterCommit`

* Forward-port: update state to prevote `nil` when proposal block does not match locked block (cometbft#1203)

* consensus: update state to prevote nil when proposal block does not match locked block. (#6986)

* add failing test

* tweak comments in failing test

* failing test comment

* initial attempt at removing prevote locked block logic

* comment out broken function

* undo reset on prevotes

* fixing TestProposeValidBlock test

* update test for completed POL update

* comment updates

* further unlock testing

* update comments

* Update internal/consensus/state.go

* spacing nit

* comment cleanup

* nil check in addVote

* update unlock description

* update precommit on relock comment

* add ensure new timeout back

* rename IsZero to IsNil and replace uses of block len check with helper

* add testing.T to new assertions

* begin removing unlock condition

* fix TestStateProposerSelection2 to precommit for nil correctly

* remove erroneous sleep

* update TestStatePOL comment

* update relock test to be more clear

* add _ into test names

* rename slashing

* udpate no relock function to be cleaner

* do not relock on old proposal test cleanup

* con state name update

* remove all references to unlock

* update test comments to include new

* add relock test

* add ensureRelock to common_test

* remove all event unlock

* remove unlock checks

* no lint add space

* lint ++

* add test for nil prevote on different proposal

* fix prevote nil condition

* fix defaultDoPrevote

* state_test.go fixes to accomodate prevoting for nil

* add failing test for POL from previous round case

* update prevote logic to prevote POL from previous round

* state.go comment fixes

* update validatePrevotes to correctly look for nil

* update new test name and comment

* update POLFromPreviousRound test

* fixes post merge

* fix spacing

* make the linter happy

* change prevote log message

* update prevote nil debug line

* update enterPrevote comment

* lint

* Update internal/consensus/state.go

Co-authored-by: Dev Ojha <ValarDragon@users.noreply.github.com>

* Update internal/consensus/state.go

Co-authored-by: Dev Ojha <ValarDragon@users.noreply.github.com>

* add english description of alg rules

* Update internal/consensus/state.go

Co-authored-by: Dev Ojha <ValarDragon@users.noreply.github.com>

* comment fixes from review

* fix comment

* fix comment

Co-authored-by: Dev Ojha <ValarDragon@users.noreply.github.com>

* Fix UTs

* Addressed comments

* Add changelog

* Update consensus/state.go

Co-authored-by: Hernán Vanzetto <15466498+hvanz@users.noreply.github.com>

---------

Co-authored-by: William Banfield <4561443+williambanfield@users.noreply.github.com>
Co-authored-by: Dev Ojha <ValarDragon@users.noreply.github.com>
Co-authored-by: Hernán Vanzetto <15466498+hvanz@users.noreply.github.com>

* consensus: Avoid process proposal when we know correct nodes validated it (cometbft#1231)

* Request ProcessProposal only the first time

* Avoid validation when proposal matches locked block

* comments

* Add changelog

* validate inside LockedRound == -1 block

* Prevote if valid block matches proposal

* Update consensus/state.go

Co-authored-by: Sergio Mena <sergio@informal.systems>

* Update consensus/state.go

Co-authored-by: lasaro <lasaro@informal.systems>

* Typo in spec

---------

Co-authored-by: Sergio Mena <sergio@informal.systems>
Co-authored-by: lasaro <lasaro@informal.systems>

* fix(consensus): prevote nil upon timeout when Proposal is missing (cometbft#2218)

This bug was possibly introduced by
cometbft#1203. This PR refactored the
consensus operation, propose -> prevote transition (`defaultDoPrevote`
method), to match the pseudo-code.

The refactoring assumed that when `cs.ProposalBlock` is unset (nil), so
should be `cs.Proposal` (meaning that the node didn't receive the round
proposal). In this case, the node must prevote nil.

The added test unit describes a scenario where `cs.Proposal` is nil (not
received), but the block ` cs.ProposalBlock` is received because the
node sees a Polka for a block, then receives the full associated block.
The node must prevote nil in this scenario, in line with cometbft#1203. But the
lack of the test on `cs.Proposal` leads to a bug.

This bug does not affect previous releases except for `v1.0`.

---

- [ ] Tests written/updated
- [ ] Changelog entry added in `.changelog` (we use
[unclog](https://github.com/informalsystems/unclog) to manage our
changelog)
- [ ] Updated relevant documentation (`docs/` or `spec/`) and code
comments
- [ ] Title follows the [Conventional
Commits](https://www.conventionalcommits.org/en/v1.0.0/) spec

* exclude MsgBatchCancel from mempool (#28)

* Fix proto/lint/e2e (#33)

* Log out proposer when proposal is rejected (#32)

* Merge commit from fork (#37)

Co-authored-by: Anton Kaliaev <anton.kalyaev@gmail.com>

* Upgrade cosmossdk.io/math to v1.4.0 (#38)

* feat(abci): Backport: move timeout_commit into FinalizeBlockResponse (#34)

* Backport timeout_commit change

* change test

* Fix byzantine test

* Fix more consensus test

* Cherry-pick fix for ASA-2025-001 (#41)

* feat(blocksync): set the max number of (concurrently) downloaded blocks (backport cometbft#2467) (cometbft#2515)

Manual backport of cometbft#2467

* feat(blocksync): sort peers by download rate & multiple requests for closer blocks (backport cometbft#2475) (cometbft#2576)

This is an automatic backport of pull request cometbft#2475 done by
[Mergify](https://mergify.com).
Cherry-pick of f8366fc has failed:
```
On branch mergify/bp/v0.38.x/pr-2475
Your branch is up to date with 'origin/v0.38.x'.

You are currently cherry-picking commit f8366fc.
  (fix conflicts and run "git cherry-pick --continue")
  (use "git cherry-pick --skip" to skip this patch)
  (use "git cherry-pick --abort" to cancel the cherry-pick operation)

Changes to be committed:
	new file:   .changelog/unreleased/improvements/2475-blocksync-2nd-request.md
	new file:   .changelog/unreleased/improvements/2475-blocksync-no-block-response.md
	new file:   .changelog/unreleased/improvements/2475-blocksync-sort-peers.md
	modified:   blocksync/reactor.go

Unmerged paths:
  (use "git add <file>..." to mark resolution)
	both modified:   blocksync/pool.go

```

To fix up this pull request, you can check it out locally. See
documentation:
https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/reviewing-changes-in-pull-requests/checking-out-pull-requests-locally

---

<details>
<summary>Mergify commands and options</summary>

<br />

More conditions and actions can be found in the
[documentation](https://docs.mergify.com/).

You can also trigger Mergify actions by commenting on this pull request:

- `@Mergifyio refresh` will re-evaluate the rules
- `@Mergifyio rebase` will rebase this PR on its base branch
- `@Mergifyio update` will merge the base branch into this PR
- `@Mergifyio backport <destination>` will backport this PR on
`<destination>` branch

Additionally, on Mergify [dashboard](https://dashboard.mergify.com) you
can:

- look at your merge queues
- generate the Mergify configuration with the config editor.

Finally, you can contact us on https://mergify.com
</details>

---------

Co-authored-by: Anton Kaliaev <anton.kalyaev@gmail.com>

* fix(blocksync): use timer instead of time.After (backport cometbft#2584) (cometbft#2587)

This is an automatic backport of pull request cometbft#2584 done by
[Mergify](https://mergify.com).

---

<details>
<summary>Mergify commands and options</summary>

<br />

More conditions and actions can be found in the
[documentation](https://docs.mergify.com/).

You can also trigger Mergify actions by commenting on this pull request:

- `@Mergifyio refresh` will re-evaluate the rules
- `@Mergifyio rebase` will rebase this PR on its base branch
- `@Mergifyio update` will merge the base branch into this PR
- `@Mergifyio backport <destination>` will backport this PR on
`<destination>` branch

Additionally, on Mergify [dashboard](https://dashboard.mergify.com) you
can:

- look at your merge queues
- generate the Mergify configuration with the config editor.

Finally, you can contact us on https://mergify.com
</details>

Co-authored-by: Anton Kaliaev <anton.kalyaev@gmail.com>

* ABC-0013 fix and broken test

* blocksync pool ban test

* Simplified blocksync malicious node test, fix nil pointer error

* Test assertions have more detail

* Update blocksync/pool_test.go

Co-authored-by: Sergio Mena <sergio@informal.systems>

* Update blocksync/pool_test.go

Co-authored-by: Sergio Mena <sergio@informal.systems>

* Update blocksync/pool_test.go

* Remove one thread to make test more compact

* Removed defers from test

* Readded good peer to test

* Update blocksync/pool_test.go

* Release/v0.38.8 (cometbft#3350)

<!--

Please add a reference to the issue that this PR addresses and indicate
which
files are most critical to review. If it fully addresses a particular
issue,
please include "Closes #XXX" (where "XXX" is the issue number).

If this PR is non-trivial/large/complex, please ensure that you have
either
created an issue that the team's had a chance to respond to, or had some
discussion with the team prior to submitting substantial pull requests.
The team
can be reached via GitHub Discussions or the Cosmos Network Discord
server in
the #cometbft channel. GitHub Discussions is preferred over Discord as
it
allows us to keep track of conversations topically.
https://github.com/cometbft/cometbft/discussions

If the work in this PR is not aligned with the team's current
priorities, please
be advised that it may take some time before it is merged - especially
if it has
not yet been discussed with the team.

See the project board for the team's current priorities:
https://github.com/orgs/cometbft/projects/1

-->
Release v0.38.8

[CHANGELOG](https://github.com/cometbft/cometbft/blob/6814b5fff269f5ec6988a4832f24f4804d705ca9/CHANGELOG.md)
---

- [ ] Tests written/updated
- [ ] Changelog entry added in `.changelog` (we use
[unclog](https://github.com/informalsystems/unclog) to manage our
changelog)
- [ ] Updated relevant documentation (`docs/` or `spec/`) and code
comments
- [ ] Title follows the [Conventional
Commits](https://www.conventionalcommits.org/en/v1.0.0/) spec

* fix(test): fix TestBlockPoolMaliciousNode DATA RACE (backport cometbft#4636) (cometbft#4641)

Co-authored-by: Anton Kaliaev <anton.kalyaev@gmail.com>

* fix(test): `TestBlockPoolMaliciousNode` shutdown threads at exit (backport cometbft#4633) (cometbft#4635)

This is a drive-by fix of a test that doesn't shut its threads down
until the whole `go test` execution finishes. I think we have a bunch of
these, but I came across this one during an unrelated troubleshooting.

Is it worth fixing this? It's not really causing any issues, it's just
sloppy coding.

The only way to see any difference is to run the `go test` until it
reaches its time limit and panics. In that case, the trace will contain
references to the threads.

For example:
```
go test github.com/cometbft/cometbft/blocksync -v -run TestBlockPoolMaliciousNode -count 100 -failfast -race -timeout 30s
```

After 30 seconds the test didn't run 100 times yet, hence `go test`
panics. Because the test has been run multiple times already, multiple
sets of threads will be reported in the panic. With the fix, only one
set is reported.

Author: @greg-szabo
<hr>This is an automatic backport of pull request cometbft#4633 done by
[Mergify](https://mergify.com).

---------

Co-authored-by: Anton Kaliaev <anton.kalyaev@gmail.com>
Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>

* Merge commit from fork

lower than what was previously reported
GHSA-22qq-3xwm-r5x4

---------

Co-authored-by: Anton Kaliaev <anton.kalyaev@gmail.com>
Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
Co-authored-by: Greg Szabo <greg@philosobear.com>
Co-authored-by: Greg Szabo <16846635+greg-szabo@users.noreply.github.com>
Co-authored-by: Sergio Mena <sergio@informal.systems>

* replace CanPropose with ProposeDisabled for backward compatibility (#48)

Co-authored-by: Tian <tian@dydx.exchange>
Co-authored-by: Pierrick Thomas <pierrick@binary.builders>

* Update LICENSE 2025 (#50)

* do not include ProposeDisabled in validator hash (#51)

* Add locking to applyverifiedblock

* Fix doUpdate test

* Skip testing build for 32bits ARM Linux

* fix protobuf version

* Remove double triggering of tests

* bump go version for mock generation

---------

Co-authored-by: James Jia <james@dydx.exchange>
Co-authored-by: Bryce Neal <bryce@dydx.exchange>
Co-authored-by: Lukasz Cwik <lcwik@apache.org>
Co-authored-by: Lukasz Cwik <126621805+lcwik@users.noreply.github.com>
Co-authored-by: Brendan Chou <3680392+BrendanChou@users.noreply.github.com>
Co-authored-by: Sergio Mena <sergio@informal.systems>
Co-authored-by: William Banfield <4561443+williambanfield@users.noreply.github.com>
Co-authored-by: Dev Ojha <ValarDragon@users.noreply.github.com>
Co-authored-by: Hernán Vanzetto <15466498+hvanz@users.noreply.github.com>
Co-authored-by: lasaro <lasaro@informal.systems>
Co-authored-by: Daniel <daniel.cason@informal.systems>
Co-authored-by: Jonathan Fung <121899091+jonfung-dydx@users.noreply.github.com>
Co-authored-by: Teddy Ding <teddy@dydx.exchange>
Co-authored-by: Anton Kaliaev <anton.kalyaev@gmail.com>
Co-authored-by: vincentwschau <99756290+vincentwschau@users.noreply.github.com>
Co-authored-by: roy-dydx <133032749+roy-dydx@users.noreply.github.com>
Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
Co-authored-by: Greg Szabo <greg@philosobear.com>
Co-authored-by: Greg Szabo <16846635+greg-szabo@users.noreply.github.com>
Co-authored-by: Tian <tian@dydx.exchange>
Co-authored-by: legalteamlabs <neeta@dydx.exchange>
pthmas pushed a commit to 01builders/dydx-cometbft that referenced this pull request Oct 17, 2025
This is effectively a copy of the fully unsynchronized local client found in tendermint/tendermint#9660. Note that this was reverted and replaced by a version that has a mutex per instance of the client in tendermint/tendermint#9830. This change can be removed once a fully unsynchronized client is added back.
pthmas pushed a commit to 01builders/dydx-cometbft that referenced this pull request Oct 17, 2025
This is effectively a copy of the fully unsynchronized local client found in tendermint/tendermint#9660. Note that this was reverted and replaced by a version that has a mutex per instance of the client in tendermint/tendermint#9830. This change can be removed once a fully unsynchronized client is added back.
pthmas pushed a commit to 01builders/dydx-cometbft that referenced this pull request Oct 21, 2025
This is effectively a copy of the fully unsynchronized local client found in tendermint/tendermint#9660. Note that this was reverted and replaced by a version that has a mutex per instance of the client in tendermint/tendermint#9830. This change can be removed once a fully unsynchronized client is added back.
pthmas pushed a commit to dydxprotocol/cometbft that referenced this pull request Oct 21, 2025
This is effectively a copy of the fully unsynchronized local client found in tendermint/tendermint#9660. Note that this was reverted and replaced by a version that has a mutex per instance of the client in tendermint/tendermint#9830. This change can be removed once a fully unsynchronized client is added back.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

C:abci Component: Application Blockchain Interface

Projects

Status: Done/Merged

Development

Successfully merging this pull request may close these issues.

abci: remove global mutex around local client

3 participants