search

crypto-com / cosmos-sdk-codeql

A query suite for common bug patterns in Cosmos SDK-based applications
Apache License 2.0
20 stars 6 forks source link

feat: update system-time rule to not trigger on `time.Now().Round(0).UTC()` #8

Open julienrbrt opened 10 months ago

julienrbrt commented 10 months ago

ref: https://github.com/cosmos/cosmos-sdk/pull/18239#discussion_r1376005648

I am not too familiar with CodeQL expression, so I have used GitHub Copilot to help me.

calvinaco commented 9 months ago

Hello @julienrbrt , thank you for your PR and sorry the late reply.

I read your Cosmos SDK reference and understand the use of time.Now() are at the genesis generation and test cases, which are legit cases.

However, I believe the existence of this rule is for a wider context that inside any state transition there shouldn't be use of time.Now() anywhere to avoid non-determinism.

As a result, I don't think we should loosen this rule or suggest to use UTC time.

Happy to discuss further here.

julienrbrt commented 9 months ago

That is definitely true. Do you think there is a way to tweak this rule better then?