8000 perf(state): Cache the block hash by ValarDragon · Pull Request #2924 · cometbft/cometbft · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

perf(state): Cache the block hash #2924

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Apr 29, 2024

Conversation

ValarDragon
Copy link
Contributor
@ValarDragon ValarDragon commented Apr 29, 2024

Closes #2923

Caches the block hash to ensure we only compute it once in consensus execution.


PR checklist

  • Tests written/updated - not sure what test I should write, any suggestions?
  • Changelog entry added in .changelog (we use unclog to manage our changelog)
  • Updated relevant documentation (docs/ or spec/) and code comments
  • Title follows the Conventional Commits spec

@ValarDragon ValarDragon requested review from a team as code owners April 29, 2024 06:14
Copy link
Contributor
@melekes melekes left a comment

Choose a reason for hiding this comment

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

Thanks @ValarDragon ❤️

@melekes melekes added backport-to-v0.37.x Tell Mergify to backport the PR to v0.37.x backport-to-v0.38.x Tell Mergify to backport the PR to v0.38.x backport-to-v1.x Tell Mergify to backport the PR to v1.x labels Apr 29, 2024
@melekes melekes added this pull request to the merge queue Apr 29, 2024
Merged via the queue into cometbft:main with commit 1b16d91 Apr 29, 2024
38 checks passed
mergify bot pushed a commit that referenced this pull request Apr 29, 2024
<!--

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

-->

Closes #2923

Caches the block hash to ensure we only compute it once in consensus
execution.

---

#### PR checklist

- [x] Tests written/updated - not sure what test I should write, any
suggestions?
- [x] 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
- [x] Title follows the [Conventional
Commits](https://www.conventionalcommits.org/en/v1.0.0/) spec

(cherry picked from commit 1b16d91)
mergify bot pushed a commit that referenced this pull request Apr 29, 2024
<!--

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

-->

Closes #2923

Caches the block hash to ensure we only compute it once in consensus
execution.

---

#### PR checklist

- [x] Tests written/updated - not sure what test I should write, any
suggestions?
- [x] 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
- [x] Title follows the [Conventional
Commits](https://www.conventionalcommits.org/en/v1.0.0/) spec

(cherry picked from commit 1b16d91)

# Conflicts:
#	.changelog/v0.37.5/improvements/2924-consensus-cache-block-hash.md
mergify bot pushed a commit that referenced this pull request Apr 29, 2024
<!--

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

-->

Closes #2923

Caches the block hash to ensure we only compute it once in consensus
execution.

---

#### PR checklist

- [x] Tests written/updated - not sure what test I should write, any
suggestions?
- [x] 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
- [x] Title follows the [Conventional
Commits](https://www.conventionalcommits.org/en/v1.0.0/) spec

(cherry picked from commit 1b16d91)

# Conflicts:
melekes pushed a commit that referenced this pull request Apr 29, 2024
Closes #2923 

Caches the block hash to ensure we only compute it once in consensus
execution.

---

#### PR checklist

- [x] Tests written/updated - not sure what test I should write, any
suggestions?
- [x] 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
- [x] Title follows the [Conventional
Commits](https://www.conventionalcommits.org/en/v1.0.0/) spec
<hr>This is an automatic backport of pull request #2924 done by
[Mergify](https://mergify.com).

Co-authored-by: Dev Ojha <ValarDragon@users.noreply.github.com>
melekes pushed a commit that referenced this pull request Apr 29, 2024
Closes #2923 

Caches the block hash to ensure we only compute it once in consensus
execution.

---

#### PR checklist

- [x] Tests written/updated - not sure what test I should write, any
suggestions?
- [x] 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
- [x] Title follows the [Conventional
Commits](https://www.conventionalcommits.org/en/v1.0.0/) spec
<hr>This is an automatic backport of pull request #2924 done by
[Mergify](https://mergify.com).

Co-authored-by: Dev Ojha <ValarDragon@users.noreply.github.com>
melekes added a commit that referenced this pull request Apr 29, 2024
Closes #2923 

Caches the block hash to ensure we only compute it once in consensus
execution.

---

#### PR checklist

- [x] Tests written/updated - not sure what test I should write, any
suggestions?
- [x] 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
- [x] Title follows the [Conventional
Commits](https://www.conventionalcommits.org/en/v1.0.0/) spec
<hr>This is an automatic backport of pull request #2924 done by
[Mergify](https://mergify.com).

---------

Co-authored-by: Dev Ojha <ValarDragon@users.noreply.github.com>
Co-authored-by: Anton Kaliaev <anton.kalyaev@gmail.com>
czarcas7ic pushed a commit to osmosis-labs/cometbft that referenced this pull request Apr 29, 2024
…2932)

Closes cometbft#2923 

Caches the block hash to ensure we only compute it once in consensus
execution.

---

#### PR checklist

- [x] Tests written/updated - not sure what test I should write, any
suggestions?
- [x] 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
- [x] Title follows the [Conventional
Commits](https://www.conventionalcommits.org/en/v1.0.0/) spec
<hr>This is an automatic backport of pull request cometbft#2924 done by
[Mergify](https://mergify.com).

---------

Co-authored-by: Dev Ojha <ValarDragon@users.noreply.github.com>
Co-authored-by: Anton Kaliaev <anton.kalyaev@gmail.com>
czarcas7ic added a commit to osmosis-labs/cometbft that referenced this pull request Apr 30, 2024
* perf(state): Cache the block hash (backport cometbft#2924) (cometbft#2932)

Closes cometbft#2923 

Caches the block hash to ensure we only compute it once in consensus
execution.

---

#### PR checklist

- [x] Tests written/updated - not sure what test I should write, any
suggestions?
- [x] 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
- [x] Title follows the [Conventional
Commits](https://www.conventionalcommits.org/en/v1.0.0/) spec
<hr>This is an automatic backport of pull request cometbft#2924 done by
[Mergify](https://mergify.com).

---------

Co-authored-by: Dev Ojha <ValarDragon@users.noreply.github.com>
Co-authored-by: Anton Kaliaev <anton.kalyaev@gmail.com>

* delete md

* changelog

* changelog

---------

Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
Co-authored-by: Dev Ojha <ValarDragon@users.noreply.github.com>
Co-authored-by: Anton Kaliaev <anton.kalyaev@gmail.com>
mergify bot added a commit to osmosis-labs/cometbft that referenced this pull request Apr 30, 2024
* perf(state): Cache the block hash (backport cometbft#2924) (cometbft#2932)

Closes cometbft#2923

Caches the block hash to ensure we only compute it once in consensus
execution.

---

#### PR checklist

- [x] Tests written/updated - not sure what test I should write, any
suggestions?
- [x] 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
- [x] Title follows the [Conventional
Commits](https://www.conventionalcommits.org/en/v1.0.0/) spec
<hr>This is an automatic backport of pull request cometbft#2924 done by
[Mergify](https://mergify.com).

---------

Co-authored-by: Dev Ojha <ValarDragon@users.noreply.github.com>
Co-authored-by: Anton Kaliaev <anton.kalyaev@gmail.com>

* delete md

* changelog

* changelog

---------

Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
Co-authored-by: Dev Ojha <ValarDragon@users.noreply.github.com>
Co-authored-by: Anton Kaliaev <anton.kalyaev@gmail.com>
(cherry picked from commit 7cbbf31)
czarcas7ic added a commit to osmosis-labs/cometbft that referenced this pull request Apr 30, 2024
* perf(state): Cache the block hash (backport cometbft#2924) (cometbft#2932)

Closes cometbft#2923

Caches the block hash to ensure we only compute it once in consensus
execution.

---

#### PR checklist

- [x] Tests written/updated - not sure what test I should write, an
8000
y
suggestions?
- [x] 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
- [x] Title follows the [Conventional
Commits](https://www.conventionalcommits.org/en/v1.0.0/) spec
<hr>This is an automatic backport of pull request cometbft#2924 done by
[Mergify](https://mergify.com).

---------

Co-authored-by: Dev Ojha <ValarDragon@users.noreply.github.com>
Co-authored-by: Anton Kaliaev <anton.kalyaev@gmail.com>

* delete md

* changelog

* changelog

---------

Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
Co-authored-by: Dev Ojha <ValarDragon@users.noreply.github.com>
Co-authored-by: Anton Kaliaev <anton.kalyaev@gmail.com>
(cherry picked from commit 7cbbf31)

Co-authored-by: Adam Tucker <adam@osmosis.team>
Copy link
Contributor
@cason cason left a comment

Choose a reason for hiding this comment

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

Do you know why we need a mutex here?

Data `json:"data"`
Evidence EvidenceData `json:"evidence"`
LastCommit *Commit `json:"last_commit"`
verifiedHash cmtbytes.HexBytes // Verified block hash (not included in the struct hash)
Copy link
Contributor

Choose a reason for hiding this comment

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

You could skip a line, to separate private from public field, just a nit.

@ValarDragon
Copy link
Contributor Author

I don't know why the mutex is needed

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport-to-v0.37.x Tell Mergify to backport the PR to v0.37.x backport-to-v0.38.x Tell Mergify to backport the PR to v0.38.x backport-to-v1.x Tell Mergify to backport the PR to v1.x
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Cache computation of the block hash in state
4 participants
0