[Bug]: Tracing context removed before the entire block is processed

[facundomedica]

Summary of Bug

This is possibly a bug, but I'm not entirely sure. Opening the issue to confirm/dismiss.

At the start of FinalizeBlock we set the tracing context containing the block height.

cosmos-sdk/baseapp/abci.go

Lines 664 to 668 in 7daf40a

	if app.cms.TracingEnabled() {
	app.cms.SetTracingContext(storetypes.TraceContext(
	map[string]any{"blockHeight": req.Height},
	))
	}

Near the end we remove this tracing context (I suppose to avoid any store interactions made after the block to contain the block height):

cosmos-sdk/baseapp/abci.go

Lines 753 to 760 in 7daf40a

	if app.finalizeBlockState.ms.TracingEnabled() {
	app.finalizeBlockState.ms = app.finalizeBlockState.ms.SetTracingContext(nil).(storetypes.CacheMultiStore)
	}
	endBlock, err := app.endBlock(app.finalizeBlockState.ctx)
	if err != nil {
	return nil, err
	}

What I think is a bug is that we are removing the tracing context before calling app.endBlock meaning that any interactions with the store during the endBlock won't contain the block height.

This isn't something related to v0.50 as it was present already in v0.47:

cosmos-sdk/baseapp/abci.go

Lines 213 to 221 in 3b509c1

	func (app *BaseApp) EndBlock(req abci.RequestEndBlock) (res abci.ResponseEndBlock) {
	if app.deliverState.ms.TracingEnabled() {
	app.deliverState.ms = app.deliverState.ms.SetTracingContext(nil).(sdk.CacheMultiStore)
	}
	if app.endBlocker != nil {
	res = app.endBlocker(app.deliverState.ctx, req)
	res.Events = sdk.MarkEventsToIndex(res.Events, app.indexEvents)
	}

[tac0turtle]

we should look at how it was in 0.47 and if i this is a regression we should fix and backport, @facundomedica want to quickly check?

[alexanderbez]

It has always been like this. Even on v0.45.x

[tac0turtle]

should we capture endblock tracing then too?

[alexanderbez]

If memory serves me correct, the tracing context was introduced to primarily trace ops for txs, thus delivertx. We set it in BeginBlock and reset it in EndBlock.

I think the tracing context should've been added at the end of BeginBlock.

#1599

[tac0turtle]

closing this with the advent of store/v2