evidence: fix usage of time field in abci evidence

[cmwaters]

Description

This PR fixes a bug where we were sending the time that evidence was committed to a block not the time that the actual infraction occurred to abci.

NOTE: This PR just addresses the issue of the wrong usage of time. There is a separate issue posted below which is that the evidence time (the time of infraction) itself is unreliably sourced from votes and should be changed to a more reliable source. This aims to be solved in a later PR

Addresses: #4150

[auto-comment]

👋 Thanks for creating a PR!

Before we can merge this PR, please make sure that all the following items have been
checked off. If any of the checklist items are not applicable, please leave them but
write a little note why.

• Wrote tests
• Updated CHANGELOG_PENDING.md
• Linked to Github issue with discussion and accepted design OR link to spec that describes this work.
• Updated relevant documentation (docs/) and code comments
• Re-reviewed Files changed in the Github PR explorer
• Applied Appropriate Labels

Thank you for your contribution to Tendermint! 🚀

[codecov]

Codecov Report

Merging #5170 into master will decrease coverage by 1.00%.
The diff coverage is 60.00%.

@@            Coverage Diff             @@
##           master    #5170      +/-   ##
==========================================
- Coverage   63.49%   62.48%   -1.01%     
==========================================
  Files         141      259     +118     
  Lines       12345    27110   +14765     
==========================================
+ Hits         7838    16941    +9103     
- Misses       3774     8694    +4920     
- Partials      733     1475     +742     

Impacted Files	Coverage Δ
types/protobuf.go	57.14% <50.00%> (ø)
state/execution.go	72.95% <100.00%> (ø)
libs/events/events.go	93.20% <0.00%> (-4.86%)	⬇️
statesync/snapshots.go	92.59% <0.00%> (-1.49%)	⬇️
proxy/multi_app_conn.go	58.02% <0.00%> (ø)
crypto/ed25519/ed25519.go	42.59% <0.00%> (ø)
blockchain/v1/pool.go	95.78% <0.00%> (ø)
behaviour/peer_behaviour.go	75.00% <0.00%> (ø)
abci/types/application.go	0.00% <0.00%> (ø)
blockchain/v2/types.go	55.55% <0.00%> (ø)
... and 114 more

[erikgrinaker]

Is this evidence stuff present in the 0.33.x versions? If so, we need a changelog entry and upgrading note.

[cmwaters]

Is this evidence stuff present in the 0.33.x versions?

Yes but this PR just fixes the time. I'm working on another PR to update the abci with the correct evidence

[erikgrinaker]

Yes but this PR just fixes the time

Sure, but it's still a change in behavior that users need to be aware of when upgrading.

[cmwaters]

Sure, but it's still a change in behavior that users need to be aware of when upgrading.

Ok yeah definitely agree

[melekes]

nice initiative 👍 but see my comment about validating time field

[melekes]

I believe we have to verify time now (since we allow arbitrary values) - "it doesn't solve the problem if we need to verify that time is actually correct, where we'd need to load the old block anyways. So one way or another we need to get access to the old time." #4150 in state#VerifyEvidence

[cmwaters]

hmm are you suggesting that we need to load the block for the height that the infraction occurred and use that time? What about all the other times we use ev.Time() for checking expiration?

[melekes]

hmm are you suggesting that we need to load the block for the height that the infraction occurred and use that time?

yep

What about all the other times we use ev.Time() for checking expiration?

can u point me to them?

[cmwaters]

tendermint/state/validation.go

Line 199 in 3c21c35

ageDuration = state.LastBlockTime.Sub(evidence.Time())

[cmwaters]

tendermint/evidence/pool.go

Line 287 in 3c21c35

return evpool.IsExpired(evidence.Height(), evidence.Time())

[cmwaters]

yep

That's annoying

[melekes]

make sure to read #4150 for more context.

[cmwaters]

@melekes, we've decided to separate the issue such that this PR just addresses the issue of the wrong usage of time (using time it was committed on the block instead of the time of infraction). In another PR we will look to change where evidence sources it's time (i.e. currently with votes) to a more reliable source.