Evidence Age & Tendermint

[alexanderbez]

Summary

Currently, the SDK handles evidence's age via a MaxEvidenceAge param defined in the x/evidence module. It is used to determine if evidence submitted by Tendermint is stale or not. It is used as such:

func (k Keeper) HandleDoubleSign(ctx sdk.Context, evidence types.Equivocation) {
	logger := k.Logger(ctx)
	consAddr := evidence.GetConsensusAddress()
	infractionHeight := evidence.GetHeight()

	// calculate the age of the evidence
	blockTime := ctx.BlockHeader().Time
	age := blockTime.Sub(evidence.GetTime())

	// ...

	// reject evidence if the double-sign is too old
	if age > k.MaxEvidenceAge(ctx) {
		logger.Info(
			fmt.Sprintf(
				"ignored double sign from %s at height %d, age of %d past max age of %d",
				consAddr, infractionHeight, age, k.MaxEvidenceAge(ctx),
			),
		)
		return
	}

	// ...

However, Tendermint has the following consensus parameters:

• consensus_params.evidence.max_age_duration
• consensus_params.evidence.max_age_num_blocks

One is block-based and the other is duration. When should an application use one over the other? Should the SDK even have its own MaxEvidenceAge? Is it redundant to have this parameter? Would these values ever differ (in semantics) and if so, how? What is the relationship between all of these?

/cc @ebuchman @marbar3778 @melekes

---

For Admin Use

• Not duplicate issue
• Appropriate labels applied
• Appropriate contributors tagged
• Contributor assigned/self-assigned

[cwgoes]

Ah yes, this is the same issue as #5529, I'll close the other one.

[cwgoes]

Basically, the unbonding period is now a minimum of (elapsed time, elapsed blocks) and we should update the SDK / evidence handlers / proof-of-stake module to reflect this.

[alexanderbez]

Sorry, didn't mean to duplicate the issue! Can you elaborate a bit more on "elapsed time, elapsed blocks". How do you envision these changes in the SDK? I presume we still have a single parameter/notion of "unbonding time". However, this value is now set via the minimum of two values?

[tac0turtle]

could this issue be included in the 0.39 milestone?
Also solving this issue may need a discussion on how the sdk should handle changing tendermint params. With and without the gov module, as some chains, won't want to use the gov module

[alexanderbez]

With governance, we need to hookup the params store (somehow) to baseapp.

Without governance, chains will need their own live mechanism or simply have a hard fork.

Regardless, I'm still looking for clarification on this issue.

[cwgoes]

I think it would be preferable for this to be updatable with a ParamChangeProposal, if possible, but for that to work Tendermint will need to allow the evidence age to be updated as well.

[tac0turtle]

but for that to work Tendermint will need to allow the evidence age to be updated as well.

this should be possible through ResponseEndBlock.

[melekes]

When should an application use one over the other?

@alexanderbez I think the idea is to use both parameters for now. More info can be found here: tendermint/tendermint#2653 (comment)

[alexanderbez]

Thanks @melekes that was helpful!

So as I understand it, the SDK's MaxEvidenceAge is not needed. Instead, we take the hybrid approach of both Tendermint's max_age_duration and max_age_num_blocks instead. Correct me if I'm wrong.

[cwgoes]

Right, although ideally we could change those dynamically in the state machine & Tendermint would be able to take account of those changes (that way they could be changed with ParamChangeProposals).

[alexanderbez]

Yeah, this ties into another discussion taken elsewhere (can't seem to place the issue). We essentially need the ability for governance to also change consensus (non-app-state) params.

This would mean that BaseApp would need reference/use of the params keeper somehow. Might be trivial, might not. I need to actually examine the LOE.

[cwgoes]

This blocks IBC 1.0 due to safety concerns.