Skip to content

Log out proposer when proposal block is rejected#32

Merged
teddyding merged 1 commit intodydx-fork-v0.38.5from
td/print-proposer
Oct 1, 2024
Merged

Log out proposer when proposal block is rejected#32
teddyding merged 1 commit intodydx-fork-v0.38.5from
td/print-proposer

Conversation

@teddyding
Copy link

This comes up a few times - it would be nice if we could directly infer the proposer from the log:
prevote step: state machine rejected a proposed block; this should not happen:the proposer may be misbehaving; prevoting nil

vincentwschau
vincentwschau previously approved these changes Sep 30, 2024
@teddyding teddyding merged commit cfee87f into dydx-fork-v0.38.5 Oct 1, 2024
@teddyding teddyding deleted the td/print-proposer branch October 1, 2024 17:20
pthmas pushed a commit to 01builders/dydx-cometbft that referenced this pull request Aug 7, 2025
pthmas pushed a commit to 01builders/dydx-cometbft that referenced this pull request Aug 7, 2025
pthmas pushed a commit to 01builders/dydx-cometbft that referenced this pull request Sep 5, 2025
pthmas pushed a commit to 01builders/dydx-cometbft that referenced this pull request Sep 5, 2025
pthmas pushed a commit to 01builders/dydx-cometbft that referenced this pull request Sep 24, 2025
pthmas pushed a commit to 01builders/dydx-cometbft that referenced this pull request Sep 29, 2025
northstar456 pushed a commit 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
pthmas pushed a commit to 01builders/dydx-cometbft that referenced this pull request Oct 17, 2025
pthmas pushed a commit to 01builders/dydx-cometbft that referenced this pull request Oct 21, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

2 participants