Skip to content
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.

Sign up for GitHub

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

Jump to bottom

[Consensus] Pacemaker - MinimumBlockTime Configuration #705

Open
9 tasks
Tracked by #890
jessicadaugherty opened this issue Apr 27, 2023 · 4 comments · May be fixed by #924
Open
9 tasks
Tracked by #890

[Consensus] Pacemaker - MinimumBlockTime Configuration #705

jessicadaugherty opened this issue Apr 27, 2023 · 4 comments · May be fixed by #924
Assignees
@red-0ne
Labels
consensus Consensus specific changes core starter task Good for newcomers, but aimed at core team members though still open for everyone
Milestone
M1: Pocket PoS (P...

Comments

@jessicadaugherty
Copy link
Contributor

jessicadaugherty commented Apr 27, 2023 •
edited by Olshansk
Loading

Objective

Implement a block time configuration feature in the Consensus / Pacemaker Module, which allows setting a minimum block time before proceeding to the next height.

Origin Document

Since Tendermint is not an optimistically responsive BFT algorithm, a set of configurations were added to it during implementation. However, HotPOKT follows Hotstuff Consensus, which is optimistically responsive and moves at the speed of the network:

Responsive.mov

Hotstuff's simplicity and scalability is very important for the core of Pocket but does not need to be this fast at the moment of writing.

Goals

  • Develop a configuration setting to define a minimum block time in the Pacemaker's configurations
  • Allow transactions to propagate through the network before proceeding to the next height
  • Reduce the responsiveness of the network

Deliverable

  • Add a new MinimumBlockTime value in PacemakerConfig
  • Update the Pacemaker module to wait at least MinimumBlockTime before proceeding to the next height when a block is successfully committed

Non-goals / Non-deliverables

  • Overhauling the entire pacemaker or consensus modules
  • Developing additional features unrelated to the block time configuration
  • Adopting all of Tendermint's configurations

General issue deliverables

  • Update the appropriate CHANGELOG(s)
  • Update any relevant local/global README(s)
  • Update relevant source code tree explanations
  • Add or update any relevant or supporting mermaid diagrams

Testing Methodology

  • All tests: make test_all
  • LocalNet: verify a LocalNet is still functioning correctly by following the instructions at docs/development/README.md
  • k8s LocalNet: verify a k8s LocalNet is still functioning correctly by following the instructions here

Creator: @jessicadaugherty
Editor: @Olshansk
@jessicadaugherty jessicadaugherty added consensus Consensus specific changes triage It requires some decision-making at team level (it can't be worked on as it stands) labels Apr 27, 2023
@Olshansk Olshansk added core starter task Good for newcomers, but aimed at core team members though still open for everyone and removed triage It requires some decision-making at team level (it can't be worked on as it stands) labels Apr 27, 2023
@Olshansk
Copy link
Member

Triaged and updated as a core starter task

@Olshansk Olshansk changed the title [Consensus] Block time - implement pacemaker configs to set min block time in auto-mode [Consensus] Pacemaker - MinimumBlockTime Configuration Apr 27, 2023
@Olshansk Olshansk mentioned this issue May 20, 2023
Merged
23 tasks
@Olshansk
Copy link
Member

@3scava1i3r I'm not sure how much experience you have, but what do you think of this ticket?

It's a starter issue but directly impacts the protocol.

If this seems a bit overwhelming at first (but hopefully not later), @bryanchriswhite is also going to put together the ticket related to logging.

@3scava1i3r
Copy link

@3scava1i3r I'm not sure how much experience you have, but what do you think of this ticket?

It's a starter issue but directly impacts the protocol.

If this seems a bit overwhelming at first (but hopefully not later), @bryanchriswhite is also going to put together the ticket related to logging.

Just wanted to give you an update—I'm currently going through the codebase and diving deep into it. I'm really excited about working on this project! Do we have any specific deadlines or timeframes to keep in mind?

@Olshansk
Copy link
Member

Just wanted to give you an update—I'm currently going through the codebase and diving deep into it. I'm really excited about working on this project!

Excited to have a new external contributor ready to dive into the important stuff :D

Do we have any specific deadlines or timeframes to keep in mind?

This one is quite important and will be a "show stopper" for our TestNet launch. Ideally we should get it done within the next 4 weeks, and I plan to pick it up myself if no one else does before then.

@Olshansk Olshansk assigned Olshansk and unassigned Olshansk Jul 3, 2023
@Olshansk Olshansk mentioned this issue Jul 10, 2023
Open
14 tasks
@red-0ne red-0ne linked a pull request Jul 21, 2023 that will close this issue
Open
20 tasks
@Olshansk Olshansk mentioned this issue Aug 2, 2023
Open
20 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@red-0ne red-0ne

Labels
consensus Consensus specific changes core starter task Good for newcomers, but aimed at core team members though still open for everyone
Projects
V1 Dashboard
Status: In Review
Milestone
M1: Pocket PoS (Proof of Stake)
Development

Successfully merging a pull request may close this issue.

[Consensus] Add MinBlockTime to delay mempool reaping pokt-network/pocket
5 participants
@red-0ne @Olshansk @3scava1i3r @jessicadaugherty @gokutheengineer