Skip to content

rpc/core: do not lock ConsensusState mutex#4844

Merged
mergify[bot] merged 5 commits intomasterfrom
anton/3161-consensus-state-rpc
May 15, 2020
Merged

rpc/core: do not lock ConsensusState mutex#4844
mergify[bot] merged 5 commits intomasterfrom
anton/3161-consensus-state-rpc

Conversation

@melekes
Copy link
Contributor

@melekes melekes commented May 15, 2020

in /validators, /consensus_params and /status

Closes #3161

melekes added 2 commits May 15, 2020 11:50
in /validators, /consensus_params and /status

Closes #3161
@melekes melekes requested a review from tessr as a code owner May 15, 2020 08:05
@auto-comment
Copy link

auto-comment bot commented May 15, 2020

👋 Thanks for creating a PR!

Before we can merge this PR, please make sure that all the following items have been
checked off. If any of the checklist items are not applicable, please leave them but
write a little note why.

  • Wrote tests
  • Updated CHANGELOG_PENDING.md
  • Linked to Github issue with discussion and accepted design OR link to spec that describes this work.
  • Updated relevant documentation (docs/) and code comments
  • Re-reviewed Files changed in the Github PR explorer
  • Applied Appropriate Labels

Thank you for your contribution to Tendermint! 🚀

@melekes melekes self-assigned this May 15, 2020
@melekes melekes added the R:minor Release: Minor label May 15, 2020
rpc/core/env.go Outdated
return 0, fmt.Errorf("height %d must be less than or equal to the current blockchain height %d",
height, currentHeight)
}
currentBase := env.BlockStore.Base()
Copy link
Contributor

Choose a reason for hiding this comment

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

Why are we passing currentHeight via param but getting currentBase from environment? Shouldn't both be passed via the same mechanism?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

because currentHeight differs - it can be latestUncommittedHeight or latest block height. current base stays the same

@melekes melekes added the S:automerge Automatically merge PR when requirements pass label May 15, 2020
@mergify mergify bot merged commit e9897c8 into master May 15, 2020
@mergify mergify bot deleted the anton/3161-consensus-state-rpc branch May 15, 2020 11:30
@tessr tessr removed the R:minor Release: Minor label May 27, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

S:automerge Automatically merge PR when requirements pass

Projects

None yet

Development

Successfully merging this pull request may close these issues.

rpc: don't lock consensus state

3 participants