-
Notifications
You must be signed in to change notification settings - Fork 207
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
feat(consensus/cometbft): stable block time #2422
base: main
Are you sure you want to change the base?
Conversation
func SetTargetBlockTime[ | ||
LoggerT log.AdvancedLogger[LoggerT], | ||
](t time.Duration) func(*Service[LoggerT]) { | ||
return func(bs *Service[LoggerT]) { bs.setTargetBlockTime(t) } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
note that we do enforce some floor on timeouts in
func validateConfig(cfg *cmtcfg.Config) error { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Makes sense. I will factor this in.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is unclear to me how to do it from the API perspective. Do you have any suggestions @abi87?
|
||
// ToBytes converts the blockDelay to bytes. | ||
func (d *blockDelay) ToBytes() []byte { | ||
bz, err := json.Marshal(d) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is probably sub-optimal
using FinalizeBlockResponse.NextBlockDelay feature cometbft/cometbft#3089
25c16fb
to
b078dba
Compare
Do we need to enforce a minimum for |
using
FinalizeBlockResponse.NextBlockDelay
featurecometbft/cometbft#3089