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.

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(blocktime): Relay next_block_delay from application to comet #59

Merged
merged 1 commit into from
Nov 27, 2024

Conversation

teddyding
Copy link

@teddyding teddyding commented Nov 25, 2024

Relay next_block_delay from application to CometBFT. Relaying next_block_delay = 0 means CometBFT will default to using timeout_commit.

Copy link

@teddyding your pull request is missing a changelog!

baseapp/abci.go Outdated Show resolved Hide resolved
baseapp/abci.go Outdated Show resolved Hide resolved
baseapp/abci.go Outdated
Comment on lines 871 to 876
}

events = append(events, endBlock.Events...)

cp := app.GetConsensusParams(app.finalizeBlockState.Context())

return &abci.ResponseFinalizeBlock{

Choose a reason for hiding this comment

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

Change potentially affects state.

Call sequence:

(*github.com/cosmos/cosmos-sdk/baseapp.BaseApp).internalFinalizeBlock (baseapp/abci.go:733)
(*github.com/cosmos/cosmos-sdk/baseapp.BaseApp).FinalizeBlock (baseapp/abci.go:894)

baseapp/abci.go Outdated
Comment on lines 878 to 883
Events: events,
TxResults: txResults,
ValidatorUpdates: endBlock.ValidatorUpdates,
ConsensusParamUpdates: &cp,
// If `NextBlockDelay` is 0, cometbft uses the legacy config `TimeoutCommit` instead.
NextBlockDelay: app.GetNextBlockDelay(app.finalizeBlockState.Context()),
}, nil

Choose a reason for hiding this comment

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

Change potentially affects state.

Call sequence:

(*github.com/cosmos/cosmos-sdk/baseapp.BaseApp).internalFinalizeBlock (baseapp/abci.go:733)
(*github.com/cosmos/cosmos-sdk/baseapp.BaseApp).FinalizeBlock (baseapp/abci.go:894)

baseapp/baseapp.go Outdated Show resolved Hide resolved
@vincentwschau
Copy link

SimApp build is broken, I think you need to update it's dependencies on comet as well in the go.mod.

vincentwschau
vincentwschau previously approved these changes Nov 26, 2024
Comment on lines 878 to 884
TxResults: txResults,
ValidatorUpdates: endBlock.ValidatorUpdates,
ConsensusParamUpdates: &cp,
// If `NextBlockDelay` is 0, cometbft uses the legacy config `TimeoutCommit` instead.
NextBlockDelay: app.GetNextBlockDelay(app.finalizeBlockState.Context()),
}, nil
}

Choose a reason for hiding this comment

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

Change potentially affects state.

Call sequence:

(*github.com/cosmos/cosmos-sdk/baseapp.BaseApp).internalFinalizeBlock (baseapp/abci.go:733)
(*github.com/cosmos/cosmos-sdk/baseapp.BaseApp).FinalizeBlock (baseapp/abci.go:894)

@teddyding teddyding merged commit 4ee5843 into dydx-fork-v0.50.5 Nov 27, 2024
36 of 39 checks passed
@teddyding teddyding deleted the td/next-block-delay branch November 27, 2024 17:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

2 participants