Spearbit audit discussion #1526
Replies: 5 comments 3 replies
-
Patrick's List
|
Beta Was this translation helpful? Give feedback.
-
Quentin's List
|
Beta Was this translation helpful? Give feedback.
-
Discussion for issue 34: maxSortedUsers has no upper bounds validation and is not the same in Compound/Aave-2 I don't believe we should do anything regarding this issue. We agree that we should not set a higher limit, and in the same vein having the possibility to set to 0 the max sorted users variable seems interesting. Setting to 0 is prevented in Aave because of an edge case in the implementation that could be fixed in the future, see morpho-org/morpho-data-structures#80 For Compound we don't have that restriction, and we might want to set MSU to 0 to not have any sorting at all. I feel like the benefit of having a sanity check is small enough that it does not outweight the loss of this feature |
Beta Was this translation helpful? Give feedback.
-
Discussion for issue 35: onBehalf argument can be set as the Morpho protocols address It seems to be fine for now, but I'm not confident that this will not bite us later. Two examples where this might be tricky and we might forget about it:
|
Beta Was this translation helpful? Give feedback.
-
Discussion for issue 69: Setting a new rewards manager breaks claiming old rewards The rewards manager has some meaningful storage that should not be wiped. It is behind a proxy, and should be updated, so I think that having the ability to set is useless and dangerous. I think we should remove the rewards manager setter in Compound |
Beta Was this translation helpful? Give feedback.
-
✅ Ok, will do
❌ No (most of the time, it is exlained in the issue)
🚧 Need further work
Merlin's List
- 19 🚧 - 20 ❌ - 21 ❌ - 22 ✅ - 23 ✅ - 24 ✅ - 25 ✅ - 26 🚧 (related to 19) - 27 ✅ (gas opti, not mandatory) - 28 ✅ - 29 🚧 - 30 ✅ - 31 ❌ (for future version) - 32 ❌ - 33 ❌ (not necessary) - 34 ✅ (for the 0 check, not the upper limit) - 35 ❌ - 36 ❌ (Aave is not doing it) - 37 ✅ - 38 ✅ (minor gas opti, not mandatory) - 39 ✅ - 40 ✅ - 41 ❌ - 42 ❌ - 43 ❌ - 44 ❌ - 45 ✅ - 46 ✅ - 47 ✅ - 48 ✅ - 49 ✅ - 50 ✅ - 51 ✅ - 52 ✅ - 53 ✅ - 54 ✅ - 55 ✅ - 56 ✅ - 57 ✅ - 58 ✅ - 59 ✅ - 60 ✅ - 61 ✅ - 62 ✅ - 63 ✅ - 64 ✅ - 65 ✅ - 66 ✅ - 67 ❌ (Acknowledge but need to conduct quantitative research) - 68 ❌ (won't change anything about it) - 69 ✅ - 70 ❌ - 71 ❌ (Acknowledge but need to conduct quantitative research)Beta Was this translation helpful? Give feedback.
All reactions