feat: add timeout info to initChain and finalizeBlock resps by damiannolan · Pull Request #432 · celestiaorg/cosmos-sdk

damiannolan · 2025-03-21T14:41:23Z

Description

Depends on #431 to clean diffs.

Pins to celestia-core fork (marko/core_changes_v3 temporarily) to pick up TimeoutInfo type.
Adds TimeoutInfo to EndBlock and propagates in InitChain and FinalizeBlock responses.

Author Checklist

All items are required. Please add a note to the item if the item is not applicable and
please add links to any relevant follow up issues.

I have...

Reviewers Checklist

All items are required. Please add a note if the item is not applicable and please add
your handle next to the items reviewed if you only reviewed selected items.

I have...

confirmed the correct type prefix in the PR title
confirmed ! in the type prefix if API or client breaking change
confirmed all author checklist items have been addressed
reviewed state machine logic
reviewed API design and naming
reviewed documentation is accurate
reviewed tests and test coverage
manually tested (if applicable)

damiannolan · 2025-03-24T14:16:49Z

should pull in latest of celestiaorg/celestia-core#1672 and eventually lock in when merged

rootulp

LGTM. Seems like this PR is:

propagating TimeoutInfo in a few ABCI responses
uses replace statements to pick up changes needed for the TimeoutInfo propagating

and I assume the replaces will look cleaner (i.e. use an official release) after we merge some other prerequisite work

damiannolan · 2025-03-25T16:42:06Z

It looks like the e2e/tx service suite is failing. So will need to look into that.

From what I can see grpc/grpc-gateway tx query with txHash is hanging, and we hit a timeout alarm in CI and it also hangs for me locally.

Also some failures in the TxEvents (off by one errors in expected len of tx results) for both grpc and grpc-gateway too.

Seems comet/celestia-core related I guess.

damiannolan · 2025-03-25T17:27:10Z

possibly something to do with Tx query looking to get a shareProof for inclusion and calling into a custom abci request query endpoint which won't exist in the simapp under test in here.

damiannolan · 2025-03-25T17:35:03Z

I actually think its deadlock because of that ^^

core will lock on the query conn when calling "/cosmos.tx.v1beta1.Service/GetTx" and later attempt to call again when going into "custom/txInclusionProof/{index}" while in Tx query

how was this avoided previously, or am I missing something?

evan-forbes · 2025-03-28T12:31:06Z

x/upgrade/go.mod

-replace github.com/cosmos/cosmos-sdk => ../../.
+replace (
+	// celestia-core
+	github.com/cometbft/cometbft => github.com/celestiaorg/celestia-core v0.38.11-0.20250325150452-89ef6e8a5cb3 // marko/core_changes_v3


random note that all these aws imports reminds me we need to merge celestiaorg/celestia-core#1437

I was going to ask about them on the core PR actually!

rootulp

@damiannolan do you need us to cut celestia-core and/or cosmos-sdk releases so we can fix the replace directives in this PR?

rootulp · 2025-04-01T13:07:02Z

go.mod

 	// use cosmos fork of keyring
 	github.com/99designs/keyring => github.com/cosmos/keyring v1.2.0
+	// celestia-core
+	github.com/cometbft/cometbft => github.com/celestiaorg/celestia-core v0.38.11-0.20250331172655-bbd3b9bdd1ee


Seems like we should create a celestia-core release so this can use an official release. Is everything merged in celestia-core and you just need a release?

If you want to cut a rc/alpha tag on core then that would be great. We could pick that up here.
I'm not 100% sure if everything is merged. There was still some discussion around mempool, right?

I don't have enough context on the v0.38.x branch and what the plan is regarding celestia-core releases moving forward so I don't feel comfortable cutting a release from that branch.

@tac0turtle what's the status of v0.38.x in celestia-core? Is it safe to cut release candidates from?

rootulp · 2025-04-01T13:37:59Z

x/upgrade/go.mod

+	github.com/cometbft/cometbft => github.com/celestiaorg/celestia-core v0.38.11-0.20250331172655-bbd3b9bdd1ee
+
+	github.com/cosmos/cosmos-sdk => ../../.


[blocking]

These replaces should use official releases

Agree, but on the release branch I think it is normally using local replace, but when tagged is locked in to an official version. cc. @julienrbrt

Yes, once you tag the SDK, you should bump x/upgrade go.mod to use that version and tag x/upgrade.
See description from #430

rootulp

Approving to unblock but we need a follow up issue or PR to update the replace directives to use official releases

…org#432) * chore: go1.23 everywhere * chore: use go1.23 * feat: add timeout info to initChain and finalizeBlock resps * chore: use latest celestia-core branch * chore: set prove flag to false for query tx * chore: add changelogs * chore: pin to HEAD of v0.38.x-celestia (core) adjust

damiannolan added 3 commits March 20, 2025 17:13

chore: go1.23 everywhere

df647fe

chore: use go1.23

ebf743e

feat: add timeout info to initChain and finalizeBlock resps

b4bc4ad

github-actions bot added Type: CI Type: Build C:x/upgrade labels Mar 21, 2025

Merge branch 'release/v0.50.x-celestia' into damian/timeout-info

b122cab

rootulp assigned damiannolan Mar 24, 2025

rootulp requested review from evan-forbes and rootulp March 24, 2025 14:47

rootulp reviewed Mar 24, 2025

View reviewed changes

Merge branch 'release/v0.50.x-celestia' into damian/timeout-info

b655e24

github-actions bot removed Type: CI Type: Build labels Mar 25, 2025

chore: use latest celestia-core branch

54f81f1

damiannolan marked this pull request as ready for review March 25, 2025 15:28

damiannolan requested a review from julienrbrt March 25, 2025 15:30

chore: set prove flag to false for query tx

39e16c9

github-actions bot added the C:x/auth label Mar 27, 2025

damiannolan added 2 commits March 27, 2025 14:58

Merge branch 'release/v0.50.x-celestia' into damian/timeout-info

9021f5f

chore: add changelogs

e6f7a9b

evan-forbes reviewed Mar 28, 2025

View reviewed changes

chore: pin to HEAD of v0.38.x-celestia (core)

39fe615

damiannolan requested a review from chatton April 1, 2025 06:26

rootulp requested review from evan-forbes and rootulp April 1, 2025 13:10

rootulp reviewed Apr 1, 2025

View reviewed changes

evan-forbes approved these changes Apr 1, 2025

View reviewed changes

rootulp approved these changes Apr 2, 2025

View reviewed changes

Merge branch 'release/v0.50.x-celestia' into damian/timeout-info

b68b42d

damiannolan merged commit 9d04f7a into celestiaorg:release/v0.50.x-celestia Apr 2, 2025
40 of 45 checks passed

damiannolan deleted the damian/timeout-info branch April 2, 2025 07:27

		github.com/cometbft/cometbft => github.com/celestiaorg/celestia-core v0.38.11-0.20250331172655-bbd3b9bdd1ee

		github.com/cosmos/cosmos-sdk => ../../.

Conversation

damiannolan commented Mar 21, 2025

Description

Author Checklist

Reviewers Checklist

Uh oh!

damiannolan commented Mar 24, 2025

Uh oh!

rootulp left a comment

Choose a reason for hiding this comment

Uh oh!

damiannolan commented Mar 25, 2025

Uh oh!

damiannolan commented Mar 25, 2025

Uh oh!

damiannolan commented Mar 25, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rootulp left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rootulp left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

damiannolan commented Mar 25, 2025 •

edited

Loading