FilOzone / FilOzone/filecoin-pay
Audit Fix L06: RateChangeQueue clearing behavior
- Dominant language
- Solidity
- Stars
- 8
- Forks
- 13
- PR merge metrics
- No merged PRs in 30d
Description
Audit ref: `[FIL-1132b525-L06]`
PR #86 introduced unreachable code. Upon dequeue of the rate change queue down to zero elements we try to reset the array completely with some fancy evm assembly zeroing out the changes dynamic array slot.
The reason for this as far as I can tell is code cleanliness. Super-theoretically it makes overflowing the head and changes.length slots easier to reason about. But reasonably-theoretically we could never overflow these 256 bit ints if we ran for the age of the universe.
Either removing this case entirely and letting the array's length grow without resetting to zero OR doing head++ first and then checking isEmpty condition are acceptable. I lean towards just letting array grow to remove a tiny bit of code from the expensive settlement operation.
Note that if KAMT behaved like the naive AMT datastructure used in other places in filecoin actors there would be a significant state overhead to operating with larger array sizes. There is a known inefficiency where the AMT adds a greater number of internal nodes eagerly when adding a bigger integer key than a small integer key. This is not the case with the HAMT which symmetrically handles all keys since all keys are hashed. The KAMT is a modification of the core HAMT logic and does not suffer from this inefficiency.
Contributor guide
No contributing guide indexed for this repository
Assessment
This issue has not been assessed yet.