search

celo-org / celo-monorepo

Official repository for core projects comprising the Celo platform
https://celo.org
Apache License 2.0
684 stars 360 forks source link

Governance.getProposalStage returns incorrect data for half-way expired proposals #7111

Closed zviadm closed 3 years ago

zviadm commented 3 years ago

Expected Behavior

Governance.getProposalStage(id) should return Expired for dequeued proposals that return 'True' for: isDequeuedProposalExpired(id).

Current Behavior

For dequeued proposals, getProposalStage(id) just returns stage based on current time in the stages. So it will be incorrect for proposals that may not get approved or that may not pass.

While this doesn't seem to cause issues in Governance contract itself (since everything critical is manually checking isDequeuedProposalExpired), it causes unintentional mistakes in ContractKit, and it is also a potential time bomb waiting for someone to make a bigger mistake assuming getProposalStage returns correct data.

aslawson commented 3 years ago

@yorhodes did you have a draft pr for this fix already?

yorhodes commented 3 years ago

@aslawson it's linked but I still want to add unit tests

yorhodes commented 3 years ago

PR is ready for review

aslawson commented 3 years ago

@yorhodes is this "in review" or "closed"?

yorhodes commented 3 years ago

closed ;)