search

celo-org / celo-monorepo

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

Invalid assert triggering in `relock` call of LockedGold library #9437

Closed zviadm closed 1 year ago

zviadm commented 2 years ago

It is possible for pending withdrawals to not be sorted by timestamp, and trigger this assert: "Pending withdrawals not sorted by timestamp".

https://github.com/celo-org/celo-monorepo/blob/8e21a5d970c69917deda5f587e07276273cfb19f/packages/sdk/contractkit/src/wrappers/LockedGold.ts#L118

zviadm commented 2 years ago

assigning based on git blame: @asaj

github-actions[bot] commented 1 year ago

This issue is stale and will be closed in 30 days without activity

zviadm commented 1 year ago

It is really sad that this issue is just closed like this. This is a real, very clear and easy to fix bug. PendingWithdrawals aren't guaranteed to be sorted by timestamp at all. Here is code that deletes pending withdrawals:

https://github.com/celo-org/celo-monorepo/blob/master/packages/protocol/contracts/governance/LockedGold.sol#L339

You can easily see that it makes no effort to keep things sorted. So it is very easy for pending withdrawals to end up being not sorted.

vissequ commented 1 year ago

Hi Zviadm. I'm very sorry for such a delay. I have forwarded this on and I have replied on Discord with more details.